linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC for-next 0/6] ofed support to send ib port link event
@ 2020-01-16  4:10 Weihang Li
  2020-01-16  4:10 ` [PATCH RFC for-next 1/6] RDMA/core: support deliver net device event Weihang Li
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Weihang Li @ 2020-01-16  4:10 UTC (permalink / raw)
  To: dledford, jgg
  Cc: leon, shiraz.saleem, aditr, mkalderon, aelior, linux-rdma, linuxarm

Some provider's driver has supported to send port link event to ofed, but
this function is implemented separately by each manufacturer.

This series provides a solution in ib core, and remove the relevant codes
of some manufacturers, supports reporting port active time during device
registration and sending port error events when device is deregistered.

The key point is how to shield the port event of the backup port in the ib
bonding scenario. Since the active-backup control is judged by the vendor
driver, so the ops.query_port of vendor would determine the port role. And
there is no relevant data structure in ib_core, so modify struct
ib_port_cache to store this information.

Lang Cheng (6):
  RDMA/core: support deliver net device event
  RDMA/mlx5: remove deliver net device event
  RDMA/i40iw: remove deliver net device event
  RDMA/qedr: remove deliver net device event
  RDMA/vmw_pvrdma: remove deliver net device event
  qede: remove invalid notify operation

 drivers/infiniband/core/cache.c                |  21 ++++-
 drivers/infiniband/core/device.c               | 123 +++++++++++++++++++++++++
 drivers/infiniband/hw/i40iw/i40iw_main.c       |   6 --
 drivers/infiniband/hw/i40iw/i40iw_utils.c      |  44 ---------
 drivers/infiniband/hw/mlx5/main.c              |  95 ++-----------------
 drivers/infiniband/hw/qedr/main.c              |  19 ----
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c |   5 -
 drivers/net/ethernet/qlogic/qede/qede_rdma.c   |   4 -
 include/rdma/ib_cache.h                        |  13 +++
 include/rdma/ib_verbs.h                        |   8 ++
 10 files changed, 173 insertions(+), 165 deletions(-)

-- 
2.8.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH RFC for-next 1/6] RDMA/core: support deliver net device event
  2020-01-16  4:10 [PATCH RFC for-next 0/6] ofed support to send ib port link event Weihang Li
@ 2020-01-16  4:10 ` Weihang Li
  2020-01-16 11:37   ` Leon Romanovsky
  2020-01-17 14:23   ` Jason Gunthorpe
  2020-01-16  4:10 ` [PATCH RFC for-next 2/6] RDMA/mlx5: remove " Weihang Li
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 13+ messages in thread
From: Weihang Li @ 2020-01-16  4:10 UTC (permalink / raw)
  To: dledford, jgg
  Cc: leon, shiraz.saleem, aditr, mkalderon, aelior, linux-rdma, linuxarm

From: Lang Cheng <chenglang@huawei.com>

For the process of handling the link event of the net device, the driver
of each provider is similar, so it can be integrated into the ib_core for
unified processing.

Signed-off-by: Lang Cheng <chenglang@huawei.com>
Signed-off-by: Weihang Li <liweihang@huawei.com>
---
 drivers/infiniband/core/cache.c  |  21 ++++++-
 drivers/infiniband/core/device.c | 123 +++++++++++++++++++++++++++++++++++++++
 include/rdma/ib_cache.h          |  13 +++++
 include/rdma/ib_verbs.h          |   8 +++
 4 files changed, 164 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
index 17bfedd..791e965 100644
--- a/drivers/infiniband/core/cache.c
+++ b/drivers/infiniband/core/cache.c
@@ -1174,6 +1174,23 @@ int ib_get_cached_port_state(struct ib_device   *device,
 }
 EXPORT_SYMBOL(ib_get_cached_port_state);
 
+int ib_get_cached_port_event_flags(struct ib_device   *device,
+				   u8                  port_num,
+				   enum ib_port_flags *event_flags)
+{
+	unsigned long flags;
+
+	if (!rdma_is_port_valid(device, port_num))
+		return -EINVAL;
+
+	read_lock_irqsave(&device->cache_lock, flags);
+	*event_flags = device->port_data[port_num].cache.port_event_flags;
+	read_unlock_irqrestore(&device->cache_lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL(ib_get_cached_port_event_flags);
+
 /**
  * rdma_get_gid_attr - Returns GID attributes for a port of a device
  * at a requested gid_index, if a valid GID entry exists.
@@ -1391,7 +1408,7 @@ ib_cache_update(struct ib_device *device, u8 port, bool enforce_security)
 	if (!rdma_is_port_valid(device, port))
 		return -EINVAL;
 
-	tprops = kmalloc(sizeof *tprops, GFP_KERNEL);
+	tprops = kzalloc(sizeof(*tprops), GFP_KERNEL);
 	if (!tprops)
 		return -ENOMEM;
 
@@ -1435,6 +1452,8 @@ ib_cache_update(struct ib_device *device, u8 port, bool enforce_security)
 	device->port_data[port].cache.pkey = pkey_cache;
 	device->port_data[port].cache.lmc = tprops->lmc;
 	device->port_data[port].cache.port_state = tprops->state;
+	device->port_data[port].cache.port_event_flags =
+						tprops->port_event_flags;
 
 	device->port_data[port].cache.subnet_prefix = tprops->subnet_prefix;
 	write_unlock_irq(&device->cache_lock);
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index f6c2552..f03d6ce 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -1325,6 +1325,77 @@ static int enable_device_and_get(struct ib_device *device)
 	return ret;
 }
 
+unsigned int ib_query_ndev_port_num(struct ib_device *device,
+				    struct net_device *netdev)
+{
+	unsigned int port_num;
+
+	rdma_for_each_port(device, port_num)
+		if (netdev == device->port_data[port_num].netdev)
+			break;
+
+	return port_num;
+}
+EXPORT_SYMBOL(ib_query_ndev_port_num);
+
+static inline enum ib_port_state get_port_state(struct net_device *netdev)
+{
+	return (netif_running(netdev) && netif_carrier_ok(netdev)) ?
+		IB_PORT_ACTIVE : IB_PORT_DOWN;
+}
+
+static int ib_netdev_event(struct notifier_block *this,
+			   unsigned long event, void *ptr)
+{
+	struct ib_device *device = container_of(this, struct ib_device, nb);
+	struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
+
+	switch (event) {
+	case NETDEV_CHANGE:
+	case NETDEV_UP:
+	case NETDEV_DOWN: {
+		unsigned int port_num = ib_query_ndev_port_num(device, netdev);
+		enum ib_port_state last_state;
+		enum ib_port_state curr_state;
+		struct ib_event ibev;
+		enum ib_port_flags flags;
+
+		if (ib_get_cached_port_event_flags(device, port_num, &flags))
+			return NOTIFY_DONE;
+
+		if (flags & IB_PORT_BONDING_SLAVE)
+			goto done;
+
+		if (ib_get_cached_port_state(device, port_num, &last_state))
+			return NOTIFY_DONE;
+
+		curr_state = get_port_state(netdev);
+
+		if (last_state == curr_state)
+			goto done;
+
+		ibev.device = device;
+		if (curr_state == IB_PORT_DOWN)
+			ibev.event = IB_EVENT_PORT_ERR;
+		else if (curr_state == IB_PORT_ACTIVE)
+			ibev.event = IB_EVENT_PORT_ACTIVE;
+		else
+			goto done;
+
+		ibev.element.port_num = port_num;
+		ib_dispatch_event(&ibev);
+		dev_dbg(&device->dev,
+			"core send %s\n", ib_event_msg(ibev.event));
+		break;
+	}
+
+	default:
+		break;
+	}
+done:
+	return NOTIFY_DONE;
+}
+
 /**
  * ib_register_device - Register an IB device with IB core
  * @device: Device to register
@@ -1342,6 +1413,7 @@ static int enable_device_and_get(struct ib_device *device)
  */
 int ib_register_device(struct ib_device *device, const char *name)
 {
+	unsigned int port;
 	int ret;
 
 	ret = assign_name(device, name);
@@ -1406,6 +1478,34 @@ int ib_register_device(struct ib_device *device, const char *name)
 	}
 	ib_device_put(device);
 
+	device->nb.notifier_call = ib_netdev_event;
+	ret = register_netdevice_notifier(&device->nb);
+	if (ret) {
+		device->nb.notifier_call = NULL;
+		return ret;
+	}
+
+	rdma_for_each_port(device, port) {
+		struct ib_event event;
+		struct net_device *netdev = ib_device_get_netdev(device, port);
+		enum ib_port_flags flags;
+
+		if (ib_get_cached_port_event_flags(device, port, &flags))
+			continue;
+
+		if (flags & IB_PORT_BONDING_SLAVE)
+			continue;
+
+		if (get_port_state(netdev) == IB_PORT_ACTIVE) {
+			event.device = device;
+			event.event = IB_EVENT_PORT_ACTIVE;
+			event.element.port_num = port;
+			ib_dispatch_event(&event);
+			dev_dbg(&device->dev, "init event %s\n",
+				ib_event_msg(event.event));
+		}
+	}
+
 	return 0;
 
 dev_cleanup:
@@ -1470,6 +1570,29 @@ static void __ib_unregister_device(struct ib_device *ib_dev)
  */
 void ib_unregister_device(struct ib_device *ib_dev)
 {
+	unsigned int port;
+
+	unregister_netdevice_notifier(&ib_dev->nb);
+
+	rdma_for_each_port(ib_dev, port) {
+		struct net_device *netdev = ib_device_get_netdev(ib_dev, port);
+		struct ib_event event;
+		u32 flags;
+
+		if (ib_get_cached_port_event_flags(ib_dev, port, &flags))
+			continue;
+
+		if (flags)
+			continue;
+
+		if (get_port_state(netdev) == IB_PORT_ACTIVE) {
+			event.device = ib_dev;
+			event.event = IB_EVENT_PORT_ERR;
+			event.element.port_num = port;
+			ib_dispatch_event(&event);
+		}
+	}
+
 	get_device(&ib_dev->dev);
 	__ib_unregister_device(ib_dev);
 	put_device(&ib_dev->dev);
diff --git a/include/rdma/ib_cache.h b/include/rdma/ib_cache.h
index 870b5e6..f37f667 100644
--- a/include/rdma/ib_cache.h
+++ b/include/rdma/ib_cache.h
@@ -131,6 +131,19 @@ int ib_get_cached_port_state(struct ib_device *device,
 			      u8                port_num,
 			      enum ib_port_state *port_active);
 
+/**
+ * ib_get_cached_port_event_flags - Returns a cached port event flags
+ * @device: The device to query.
+ * @port_num: The port number of the device to query.
+ * @event_flags: port_state for the specified port for that device.
+ *
+ * ib_get_cached_port_event_flags() fetches the specified event flags stored in
+ * the local software cache.
+ */
+int ib_get_cached_port_event_flags(struct ib_device   *device,
+				   u8                  port_num,
+				   enum ib_port_flags *event_flags);
+
 bool rdma_is_zero_gid(const union ib_gid *gid);
 const struct ib_gid_attr *rdma_get_gid_attr(struct ib_device *device,
 					    u8 port_num, int index);
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index d8031f6..ce88d0b 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -670,6 +670,7 @@ struct ib_port_attr {
 	u8			active_speed;
 	u8                      phys_state;
 	u16			port_cap_flags2;
+	u32			port_event_flags;
 };
 
 enum ib_device_modify_flags {
@@ -2149,12 +2150,18 @@ enum ib_mad_result {
 	IB_MAD_RESULT_CONSUMED = 1 << 2  /* Packet consumed: stop processing */
 };
 
+enum ib_port_flags {
+	IB_PORT_NORMAL = 0,		/* normarl port            */
+	IB_PORT_BONDING_SLAVE = 1 << 0, /* rdma bonding slave port */
+};
+
 struct ib_port_cache {
 	u64		      subnet_prefix;
 	struct ib_pkey_cache  *pkey;
 	struct ib_gid_table   *gid;
 	u8                     lmc;
 	enum ib_port_state     port_state;
+	enum ib_port_flags     port_event_flags;
 };
 
 struct ib_port_immutable {
@@ -2706,6 +2713,7 @@ struct ib_device {
 	/* Used by iWarp CM */
 	char iw_ifname[IFNAMSIZ];
 	u32 iw_driver_flags;
+	struct notifier_block nb;
 };
 
 struct ib_client_nl_info;
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RFC for-next 2/6] RDMA/mlx5: remove deliver net device event
  2020-01-16  4:10 [PATCH RFC for-next 0/6] ofed support to send ib port link event Weihang Li
  2020-01-16  4:10 ` [PATCH RFC for-next 1/6] RDMA/core: support deliver net device event Weihang Li
@ 2020-01-16  4:10 ` Weihang Li
  2020-01-16 11:41   ` Leon Romanovsky
  2020-01-16  4:10 ` [PATCH RFC for-next 3/6] RDMA/i40iw: " Weihang Li
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Weihang Li @ 2020-01-16  4:10 UTC (permalink / raw)
  To: dledford, jgg
  Cc: leon, shiraz.saleem, aditr, mkalderon, aelior, linux-rdma, linuxarm

From: Lang Cheng <chenglang@huawei.com>

The code that handles the link event of the net device has been moved
into the core, and the related processing should been removed from the
provider's driver.

Signed-off-by: Lang Cheng <chenglang@huawei.com>
Signed-off-by: Weihang Li <liweihang@huawei.com>
---
 drivers/infiniband/hw/mlx5/main.c | 95 ++++-----------------------------------
 1 file changed, 9 insertions(+), 86 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 97bcf01..bb0dbfb 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -144,48 +144,6 @@ mlx5_ib_port_link_layer(struct ib_device *device, u8 port_num)
 	return mlx5_port_type_cap_to_rdma_ll(port_type_cap);
 }
 
-static int get_port_state(struct ib_device *ibdev,
-			  u8 port_num,
-			  enum ib_port_state *state)
-{
-	struct ib_port_attr attr;
-	int ret;
-
-	memset(&attr, 0, sizeof(attr));
-	ret = ibdev->ops.query_port(ibdev, port_num, &attr);
-	if (!ret)
-		*state = attr.state;
-	return ret;
-}
-
-static struct mlx5_roce *mlx5_get_rep_roce(struct mlx5_ib_dev *dev,
-					   struct net_device *ndev,
-					   u8 *port_num)
-{
-	struct mlx5_eswitch *esw = dev->mdev->priv.eswitch;
-	struct net_device *rep_ndev;
-	struct mlx5_ib_port *port;
-	int i;
-
-	for (i = 0; i < dev->num_ports; i++) {
-		port  = &dev->port[i];
-		if (!port->rep)
-			continue;
-
-		read_lock(&port->roce.netdev_lock);
-		rep_ndev = mlx5_ib_get_rep_netdev(esw,
-						  port->rep->vport);
-		if (rep_ndev == ndev) {
-			read_unlock(&port->roce.netdev_lock);
-			*port_num = i + 1;
-			return &port->roce;
-		}
-		read_unlock(&port->roce.netdev_lock);
-	}
-
-	return NULL;
-}
-
 static int mlx5_netdev_event(struct notifier_block *this,
 			     unsigned long event, void *ptr)
 {
@@ -219,52 +177,10 @@ static int mlx5_netdev_event(struct notifier_block *this,
 		write_unlock(&roce->netdev_lock);
 		break;
 
-	case NETDEV_CHANGE:
-	case NETDEV_UP:
-	case NETDEV_DOWN: {
-		struct net_device *lag_ndev = mlx5_lag_get_roce_netdev(mdev);
-		struct net_device *upper = NULL;
-
-		if (lag_ndev) {
-			upper = netdev_master_upper_dev_get(lag_ndev);
-			dev_put(lag_ndev);
-		}
-
-		if (ibdev->is_rep)
-			roce = mlx5_get_rep_roce(ibdev, ndev, &port_num);
-		if (!roce)
-			return NOTIFY_DONE;
-		if ((upper == ndev || (!upper && ndev == roce->netdev))
-		    && ibdev->ib_active) {
-			struct ib_event ibev = { };
-			enum ib_port_state port_state;
-
-			if (get_port_state(&ibdev->ib_dev, port_num,
-					   &port_state))
-				goto done;
-
-			if (roce->last_port_state == port_state)
-				goto done;
-
-			roce->last_port_state = port_state;
-			ibev.device = &ibdev->ib_dev;
-			if (port_state == IB_PORT_DOWN)
-				ibev.event = IB_EVENT_PORT_ERR;
-			else if (port_state == IB_PORT_ACTIVE)
-				ibev.event = IB_EVENT_PORT_ACTIVE;
-			else
-				goto done;
-
-			ibev.element.port_num = port_num;
-			ib_dispatch_event(&ibev);
-		}
-		break;
-	}
-
 	default:
 		break;
 	}
-done:
+
 	mlx5_ib_put_native_port_mdev(ibdev, port_num);
 	return NOTIFY_DONE;
 }
@@ -569,7 +485,14 @@ static int mlx5_query_port_roce(struct ib_device *device, u8 port_num,
 
 	dev_put(ndev);
 
-	props->active_mtu	= min(props->max_mtu, ndev_ib_mtu);
+	props->active_mtu = min(props->max_mtu, ndev_ib_mtu);
+
+	if ((dev->lag_active && ndev != mlx5_lag_get_roce_netdev(mdev)) ||
+	    (!dev->lag_active && port_num != mdev_port_num))
+		props->port_event_flags = IB_PORT_BONDING_SLAVE;
+	else
+		props->port_event_flags &= ~IB_PORT_BONDING_SLAVE;
+
 out:
 	if (put_mdev)
 		mlx5_ib_put_native_port_mdev(dev, port_num);
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RFC for-next 3/6] RDMA/i40iw: remove deliver net device event
  2020-01-16  4:10 [PATCH RFC for-next 0/6] ofed support to send ib port link event Weihang Li
  2020-01-16  4:10 ` [PATCH RFC for-next 1/6] RDMA/core: support deliver net device event Weihang Li
  2020-01-16  4:10 ` [PATCH RFC for-next 2/6] RDMA/mlx5: remove " Weihang Li
@ 2020-01-16  4:10 ` Weihang Li
  2020-01-16  4:10 ` [PATCH RFC for-next 4/6] RDMA/qedr: " Weihang Li
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Weihang Li @ 2020-01-16  4:10 UTC (permalink / raw)
  To: dledford, jgg
  Cc: leon, shiraz.saleem, aditr, mkalderon, aelior, linux-rdma, linuxarm

From: Lang Cheng <chenglang@huawei.com>

The code that handles the link event of the net device has been moved
into the core, and the related processing should been removed from the
provider's driver.

Signed-off-by: Lang Cheng <chenglang@huawei.com>
Signed-off-by: Weihang Li <liweihang@huawei.com>
---
 drivers/infiniband/hw/i40iw/i40iw_main.c  |  6 -----
 drivers/infiniband/hw/i40iw/i40iw_utils.c | 44 -------------------------------
 2 files changed, 50 deletions(-)

diff --git a/drivers/infiniband/hw/i40iw/i40iw_main.c b/drivers/infiniband/hw/i40iw/i40iw_main.c
index 2386143..6c41458e 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_main.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_main.c
@@ -99,10 +99,6 @@ static struct notifier_block i40iw_net_notifier = {
 	.notifier_call = i40iw_net_event
 };
 
-static struct notifier_block i40iw_netdevice_notifier = {
-	.notifier_call = i40iw_netdevice_event
-};
-
 /**
  * i40iw_find_i40e_handler - find a handler given a client info
  * @ldev: pointer to a client info
@@ -1401,7 +1397,6 @@ static void i40iw_register_notifiers(void)
 	register_inetaddr_notifier(&i40iw_inetaddr_notifier);
 	register_inet6addr_notifier(&i40iw_inetaddr6_notifier);
 	register_netevent_notifier(&i40iw_net_notifier);
-	register_netdevice_notifier(&i40iw_netdevice_notifier);
 }
 
 /**
@@ -1413,7 +1408,6 @@ static void i40iw_unregister_notifiers(void)
 	unregister_netevent_notifier(&i40iw_net_notifier);
 	unregister_inetaddr_notifier(&i40iw_inetaddr_notifier);
 	unregister_inet6addr_notifier(&i40iw_inetaddr6_notifier);
-	unregister_netdevice_notifier(&i40iw_netdevice_notifier);
 }
 
 /**
diff --git a/drivers/infiniband/hw/i40iw/i40iw_utils.c b/drivers/infiniband/hw/i40iw/i40iw_utils.c
index 0165246..09171e6 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_utils.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_utils.c
@@ -311,50 +311,6 @@ int i40iw_net_event(struct notifier_block *notifier, unsigned long event, void *
 }
 
 /**
- * i40iw_netdevice_event - system notifier for netdev events
- * @notfier: not used
- * @event: event for notifier
- * @ptr: netdev
- */
-int i40iw_netdevice_event(struct notifier_block *notifier,
-			  unsigned long event,
-			  void *ptr)
-{
-	struct net_device *event_netdev;
-	struct net_device *netdev;
-	struct i40iw_device *iwdev;
-	struct i40iw_handler *hdl;
-
-	event_netdev = netdev_notifier_info_to_dev(ptr);
-
-	hdl = i40iw_find_netdev(event_netdev);
-	if (!hdl)
-		return NOTIFY_DONE;
-
-	iwdev = &hdl->device;
-	if (iwdev->init_state < RDMA_DEV_REGISTERED || iwdev->closing)
-		return NOTIFY_DONE;
-
-	netdev = iwdev->ldev->netdev;
-	if (netdev != event_netdev)
-		return NOTIFY_DONE;
-
-	iwdev->iw_status = 1;
-
-	switch (event) {
-	case NETDEV_DOWN:
-		iwdev->iw_status = 0;
-		/* Fall through */
-	case NETDEV_UP:
-		i40iw_port_ibevent(iwdev);
-		break;
-	default:
-		break;
-	}
-	return NOTIFY_DONE;
-}
-
-/**
  * i40iw_get_cqp_request - get cqp struct
  * @cqp: device cqp ptr
  * @wait: cqp to be used in wait mode
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RFC for-next 4/6] RDMA/qedr: remove deliver net device event
  2020-01-16  4:10 [PATCH RFC for-next 0/6] ofed support to send ib port link event Weihang Li
                   ` (2 preceding siblings ...)
  2020-01-16  4:10 ` [PATCH RFC for-next 3/6] RDMA/i40iw: " Weihang Li
@ 2020-01-16  4:10 ` Weihang Li
  2020-01-16  4:10 ` [PATCH RFC for-next 5/6] RDMA/vmw_pvrdma: " Weihang Li
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Weihang Li @ 2020-01-16  4:10 UTC (permalink / raw)
  To: dledford, jgg
  Cc: leon, shiraz.saleem, aditr, mkalderon, aelior, linux-rdma, linuxarm

From: Lang Cheng <chenglang@huawei.com>

The code that handles the link event of the net device has been moved
into the core, and the related processing should been removed from the
provider's driver.

Signed-off-by: Lang Cheng <chenglang@huawei.com>
Signed-off-by: Weihang Li <liweihang@huawei.com>
---
 drivers/infiniband/hw/qedr/main.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/drivers/infiniband/hw/qedr/main.c b/drivers/infiniband/hw/qedr/main.c
index dcdc85a..d85894b 100644
--- a/drivers/infiniband/hw/qedr/main.c
+++ b/drivers/infiniband/hw/qedr/main.c
@@ -957,24 +957,11 @@ static void qedr_remove(struct qedr_dev *dev)
 	ib_dealloc_device(&dev->ibdev);
 }
 
-static void qedr_close(struct qedr_dev *dev)
-{
-	if (test_and_clear_bit(QEDR_ENET_STATE_BIT, &dev->enet_state))
-		qedr_ib_dispatch_event(dev, QEDR_PORT, IB_EVENT_PORT_ERR);
-}
-
 static void qedr_shutdown(struct qedr_dev *dev)
 {
-	qedr_close(dev);
 	qedr_remove(dev);
 }
 
-static void qedr_open(struct qedr_dev *dev)
-{
-	if (!test_and_set_bit(QEDR_ENET_STATE_BIT, &dev->enet_state))
-		qedr_ib_dispatch_event(dev, QEDR_PORT, IB_EVENT_PORT_ACTIVE);
-}
-
 static void qedr_mac_address_change(struct qedr_dev *dev)
 {
 	union ib_gid *sgid = &dev->sgid_tbl[0];
@@ -1014,12 +1001,6 @@ static void qedr_mac_address_change(struct qedr_dev *dev)
 static void qedr_notify(struct qedr_dev *dev, enum qede_rdma_event event)
 {
 	switch (event) {
-	case QEDE_UP:
-		qedr_open(dev);
-		break;
-	case QEDE_DOWN:
-		qedr_close(dev);
-		break;
 	case QEDE_CLOSE:
 		qedr_shutdown(dev);
 		break;
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RFC for-next 5/6] RDMA/vmw_pvrdma: remove deliver net device event
  2020-01-16  4:10 [PATCH RFC for-next 0/6] ofed support to send ib port link event Weihang Li
                   ` (3 preceding siblings ...)
  2020-01-16  4:10 ` [PATCH RFC for-next 4/6] RDMA/qedr: " Weihang Li
@ 2020-01-16  4:10 ` Weihang Li
  2020-01-16  4:10 ` [PATCH RFC for-next 6/6] qede: remove invalid notify operation Weihang Li
  2020-01-16 11:15 ` [PATCH RFC for-next 0/6] ofed support to send ib port link event Leon Romanovsky
  6 siblings, 0 replies; 13+ messages in thread
From: Weihang Li @ 2020-01-16  4:10 UTC (permalink / raw)
  To: dledford, jgg
  Cc: leon, shiraz.saleem, aditr, mkalderon, aelior, linux-rdma, linuxarm

From: Lang Cheng <chenglang@huawei.com>

The code that handles the link event of the net device has been moved
into the core, and the related processing should been removed from the
provider's driver.

Signed-off-by: Lang Cheng <chenglang@huawei.com>
Signed-off-by: Weihang Li <liweihang@huawei.com>
---
 drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
index e580ae9..b3877c5 100644
--- a/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
+++ b/drivers/infiniband/hw/vmw_pvrdma/pvrdma_main.c
@@ -694,9 +694,6 @@ static void pvrdma_netdevice_event_handle(struct pvrdma_dev *dev,
 
 	switch (event) {
 	case NETDEV_REBOOT:
-	case NETDEV_DOWN:
-		pvrdma_dispatch_event(dev, 1, IB_EVENT_PORT_ERR);
-		break;
 	case NETDEV_UP:
 		pvrdma_write_reg(dev, PVRDMA_REG_CTL,
 				 PVRDMA_DEVICE_CTL_UNQUIESCE);
@@ -706,8 +703,6 @@ static void pvrdma_netdevice_event_handle(struct pvrdma_dev *dev,
 		if (pvrdma_read_reg(dev, PVRDMA_REG_ERR))
 			dev_err(&dev->pdev->dev,
 				"failed to activate device during link up\n");
-		else
-			pvrdma_dispatch_event(dev, 1, IB_EVENT_PORT_ACTIVE);
 		break;
 	case NETDEV_UNREGISTER:
 		ib_device_set_netdev(&dev->ib_dev, NULL, 1);
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RFC for-next 6/6] qede: remove invalid notify operation
  2020-01-16  4:10 [PATCH RFC for-next 0/6] ofed support to send ib port link event Weihang Li
                   ` (4 preceding siblings ...)
  2020-01-16  4:10 ` [PATCH RFC for-next 5/6] RDMA/vmw_pvrdma: " Weihang Li
@ 2020-01-16  4:10 ` Weihang Li
  2020-01-16 11:15 ` [PATCH RFC for-next 0/6] ofed support to send ib port link event Leon Romanovsky
  6 siblings, 0 replies; 13+ messages in thread
From: Weihang Li @ 2020-01-16  4:10 UTC (permalink / raw)
  To: dledford, jgg
  Cc: leon, shiraz.saleem, aditr, mkalderon, aelior, linux-rdma, linuxarm

From: Lang Cheng <chenglang@huawei.com>

The qedr notify() has removed the processing of QEDE_UP and QEDE_DOWN,
so qede no longer needs to notify rdma of these two events.

Signed-off-by: Lang Cheng <chenglang@huawei.com>
Signed-off-by: Weihang Li <liweihang@huawei.com>
---
 drivers/net/ethernet/qlogic/qede/qede_rdma.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qede/qede_rdma.c b/drivers/net/ethernet/qlogic/qede/qede_rdma.c
index ffabc2d..0493279 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_rdma.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_rdma.c
@@ -145,8 +145,6 @@ void qede_rdma_dev_remove(struct qede_dev *edev, bool recovery)
 
 static void _qede_rdma_dev_open(struct qede_dev *edev)
 {
-	if (qedr_drv && edev->rdma_info.qedr_dev && qedr_drv->notify)
-		qedr_drv->notify(edev->rdma_info.qedr_dev, QEDE_UP);
 }
 
 static void qede_rdma_dev_open(struct qede_dev *edev)
@@ -161,8 +159,6 @@ static void qede_rdma_dev_open(struct qede_dev *edev)
 
 static void _qede_rdma_dev_close(struct qede_dev *edev)
 {
-	if (qedr_drv && edev->rdma_info.qedr_dev && qedr_drv->notify)
-		qedr_drv->notify(edev->rdma_info.qedr_dev, QEDE_DOWN);
 }
 
 static void qede_rdma_dev_close(struct qede_dev *edev)
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH RFC for-next 0/6] ofed support to send ib port link event
  2020-01-16  4:10 [PATCH RFC for-next 0/6] ofed support to send ib port link event Weihang Li
                   ` (5 preceding siblings ...)
  2020-01-16  4:10 ` [PATCH RFC for-next 6/6] qede: remove invalid notify operation Weihang Li
@ 2020-01-16 11:15 ` Leon Romanovsky
  6 siblings, 0 replies; 13+ messages in thread
From: Leon Romanovsky @ 2020-01-16 11:15 UTC (permalink / raw)
  To: Weihang Li
  Cc: dledford, jgg, shiraz.saleem, aditr, mkalderon, aelior,
	linux-rdma, linuxarm

On Thu, Jan 16, 2020 at 12:10:41PM +0800, Weihang Li wrote:
> Some provider's driver has supported to send port link event to ofed, but

How is "ofed" related here?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH RFC for-next 1/6] RDMA/core: support deliver net device event
  2020-01-16  4:10 ` [PATCH RFC for-next 1/6] RDMA/core: support deliver net device event Weihang Li
@ 2020-01-16 11:37   ` Leon Romanovsky
  2020-01-17 14:23   ` Jason Gunthorpe
  1 sibling, 0 replies; 13+ messages in thread
From: Leon Romanovsky @ 2020-01-16 11:37 UTC (permalink / raw)
  To: Weihang Li
  Cc: dledford, jgg, shiraz.saleem, aditr, mkalderon, aelior,
	linux-rdma, linuxarm

On Thu, Jan 16, 2020 at 12:10:42PM +0800, Weihang Li wrote:
> From: Lang Cheng <chenglang@huawei.com>
>
> For the process of handling the link event of the net device, the driver
> of each provider is similar, so it can be integrated into the ib_core for
> unified processing.
>
> Signed-off-by: Lang Cheng <chenglang@huawei.com>
> Signed-off-by: Weihang Li <liweihang@huawei.com>
> ---
>  drivers/infiniband/core/cache.c  |  21 ++++++-
>  drivers/infiniband/core/device.c | 123 +++++++++++++++++++++++++++++++++++++++
>  include/rdma/ib_cache.h          |  13 +++++
>  include/rdma/ib_verbs.h          |   8 +++
>  4 files changed, 164 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
> index 17bfedd..791e965 100644
> --- a/drivers/infiniband/core/cache.c
> +++ b/drivers/infiniband/core/cache.c
> @@ -1174,6 +1174,23 @@ int ib_get_cached_port_state(struct ib_device   *device,
>  }
>  EXPORT_SYMBOL(ib_get_cached_port_state);
>
> +int ib_get_cached_port_event_flags(struct ib_device   *device,
> +				   u8                  port_num,
> +				   enum ib_port_flags *event_flags)
> +{
> +	unsigned long flags;
> +
> +	if (!rdma_is_port_valid(device, port_num))
> +		return -EINVAL;

I know that ib_get_cached_port_state() has the same check, but it is
dumb, we don't need to call this function with wrong port_num.

> +
> +	read_lock_irqsave(&device->cache_lock, flags);
> +	*event_flags = device->port_data[port_num].cache.port_event_flags;
> +	read_unlock_irqrestore(&device->cache_lock, flags);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(ib_get_cached_port_event_flags);

This function is limited to ib_core.ko, no need in EXPORT_SYMBOL.

> +
>  /**
>   * rdma_get_gid_attr - Returns GID attributes for a port of a device
>   * at a requested gid_index, if a valid GID entry exists.
> @@ -1391,7 +1408,7 @@ ib_cache_update(struct ib_device *device, u8 port, bool enforce_security)
>  	if (!rdma_is_port_valid(device, port))
>  		return -EINVAL;
>
> -	tprops = kmalloc(sizeof *tprops, GFP_KERNEL);
> +	tprops = kzalloc(sizeof(*tprops), GFP_KERNEL);

Unrelated change but OK.

>  	if (!tprops)
>  		return -ENOMEM;
>
> @@ -1435,6 +1452,8 @@ ib_cache_update(struct ib_device *device, u8 port, bool enforce_security)
>  	device->port_data[port].cache.pkey = pkey_cache;
>  	device->port_data[port].cache.lmc = tprops->lmc;
>  	device->port_data[port].cache.port_state = tprops->state;
> +	device->port_data[port].cache.port_event_flags =
> +						tprops->port_event_flags;
>
>  	device->port_data[port].cache.subnet_prefix = tprops->subnet_prefix;
>  	write_unlock_irq(&device->cache_lock);
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index f6c2552..f03d6ce 100644
> --- a/drivers/infiniband/core/device.c
> +++ b/drivers/infiniband/core/device.c
> @@ -1325,6 +1325,77 @@ static int enable_device_and_get(struct ib_device *device)
>  	return ret;
>  }
>
> +unsigned int ib_query_ndev_port_num(struct ib_device *device,
> +				    struct net_device *netdev)
> +{
> +	unsigned int port_num;
> +
> +	rdma_for_each_port(device, port_num)
> +		if (netdev == device->port_data[port_num].netdev)
> +			break;

Each event will do this function, while mlx5 has different logic separated
to RoCE, IB and representors.

> +
> +	return port_num;
> +}
> +EXPORT_SYMBOL(ib_query_ndev_port_num);

No need in EXPORT_SYMBOL.

> +
> +static inline enum ib_port_state get_port_state(struct net_device *netdev)

No inline function in C files.
> +{
> +	return (netif_running(netdev) && netif_carrier_ok(netdev)) ?
> +		IB_PORT_ACTIVE : IB_PORT_DOWN;
> +}
> +
> +static int ib_netdev_event(struct notifier_block *this,
> +			   unsigned long event, void *ptr)
> +{
> +	struct ib_device *device = container_of(this, struct ib_device, nb);
> +	struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
> +
> +	switch (event) {
> +	case NETDEV_CHANGE:
> +	case NETDEV_UP:
> +	case NETDEV_DOWN: {
> +		unsigned int port_num = ib_query_ndev_port_num(device, netdev);
> +		enum ib_port_state last_state;
> +		enum ib_port_state curr_state;
> +		struct ib_event ibev;
> +		enum ib_port_flags flags;
> +
> +		if (ib_get_cached_port_event_flags(device, port_num, &flags))
> +			return NOTIFY_DONE;
> +
> +		if (flags & IB_PORT_BONDING_SLAVE)
> +			goto done;

I afraid that this "goto ..." breaks RoCE bonding.

> +
> +		if (ib_get_cached_port_state(device, port_num, &last_state))
> +			return NOTIFY_DONE;
> +
> +		curr_state = get_port_state(netdev);
> +
> +		if (last_state == curr_state)
> +			goto done;
> +
> +		ibev.device = device;
> +		if (curr_state == IB_PORT_DOWN)
> +			ibev.event = IB_EVENT_PORT_ERR;
> +		else if (curr_state == IB_PORT_ACTIVE)
> +			ibev.event = IB_EVENT_PORT_ACTIVE;
> +		else
> +			goto done;
> +
> +		ibev.element.port_num = port_num;
> +		ib_dispatch_event(&ibev);
> +		dev_dbg(&device->dev,
> +			"core send %s\n", ib_event_msg(ibev.event));
> +		break;

You put break in wrong place.

> +	}
> +
> +	default:
> +		break;
> +	}
> +done:
> +	return NOTIFY_DONE;
> +}
> +
>  /**
>   * ib_register_device - Register an IB device with IB core
>   * @device: Device to register
> @@ -1342,6 +1413,7 @@ static int enable_device_and_get(struct ib_device *device)
>   */
>  int ib_register_device(struct ib_device *device, const char *name)
>  {
> +	unsigned int port;
>  	int ret;
>
>  	ret = assign_name(device, name);
> @@ -1406,6 +1478,34 @@ int ib_register_device(struct ib_device *device, const char *name)
>  	}
>  	ib_device_put(device);
>
> +	device->nb.notifier_call = ib_netdev_event;
> +	ret = register_netdevice_notifier(&device->nb);

It is not relevant for IB and OPA devices.

> +	if (ret) {
> +		device->nb.notifier_call = NULL;
> +		return ret;
> +	}
> +
> +	rdma_for_each_port(device, port) {
> +		struct ib_event event;
> +		struct net_device *netdev = ib_device_get_netdev(device, port);
> +		enum ib_port_flags flags;
> +
> +		if (ib_get_cached_port_event_flags(device, port, &flags))
> +			continue;
> +
> +		if (flags & IB_PORT_BONDING_SLAVE)
> +			continue;
> +
> +		if (get_port_state(netdev) == IB_PORT_ACTIVE) {
> +			event.device = device;
> +			event.event = IB_EVENT_PORT_ACTIVE;
> +			event.element.port_num = port;
> +			ib_dispatch_event(&event);
> +			dev_dbg(&device->dev, "init event %s\n",
> +				ib_event_msg(event.event));

You already added print to newly ib_netdev_event().

> +		}
> +	}
> +
>  	return 0;
>
>  dev_cleanup:
> @@ -1470,6 +1570,29 @@ static void __ib_unregister_device(struct ib_device *ib_dev)
>   */
>  void ib_unregister_device(struct ib_device *ib_dev)
>  {
> +	unsigned int port;
> +
> +	unregister_netdevice_notifier(&ib_dev->nb);

Not relevant for IB and OPA.

> +
> +	rdma_for_each_port(ib_dev, port) {
> +		struct net_device *netdev = ib_device_get_netdev(ib_dev, port);
> +		struct ib_event event;
> +		u32 flags;
> +
> +		if (ib_get_cached_port_event_flags(ib_dev, port, &flags))
> +			continue;
> +
> +		if (flags)
> +			continue;
> +
> +		if (get_port_state(netdev) == IB_PORT_ACTIVE) {
> +			event.device = ib_dev;
> +			event.event = IB_EVENT_PORT_ERR;
> +			event.element.port_num = port;
> +			ib_dispatch_event(&event);
> +		}
> +	}
> +
>  	get_device(&ib_dev->dev);
>  	__ib_unregister_device(ib_dev);
>  	put_device(&ib_dev->dev);
> diff --git a/include/rdma/ib_cache.h b/include/rdma/ib_cache.h
> index 870b5e6..f37f667 100644
> --- a/include/rdma/ib_cache.h
> +++ b/include/rdma/ib_cache.h
> @@ -131,6 +131,19 @@ int ib_get_cached_port_state(struct ib_device *device,
>  			      u8                port_num,
>  			      enum ib_port_state *port_active);
>
> +/**
> + * ib_get_cached_port_event_flags - Returns a cached port event flags
> + * @device: The device to query.
> + * @port_num: The port number of the device to query.
> + * @event_flags: port_state for the specified port for that device.
> + *
> + * ib_get_cached_port_event_flags() fetches the specified event flags stored in
> + * the local software cache.
> + */
> +int ib_get_cached_port_event_flags(struct ib_device   *device,
> +				   u8                  port_num,
> +				   enum ib_port_flags *event_flags);
> +
>  bool rdma_is_zero_gid(const union ib_gid *gid);
>  const struct ib_gid_attr *rdma_get_gid_attr(struct ib_device *device,
>  					    u8 port_num, int index);
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index d8031f6..ce88d0b 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -670,6 +670,7 @@ struct ib_port_attr {
>  	u8			active_speed;
>  	u8                      phys_state;
>  	u16			port_cap_flags2;
> +	u32			port_event_flags;
>  };
>
>  enum ib_device_modify_flags {
> @@ -2149,12 +2150,18 @@ enum ib_mad_result {
>  	IB_MAD_RESULT_CONSUMED = 1 << 2  /* Packet consumed: stop processing */
>  };
>
> +enum ib_port_flags {

Please don't do named enums for bitwise declarations.

> +	IB_PORT_NORMAL = 0,		/* normarl port            */

normarl -> normal
Better to describe what do you mean by "normal port".

> +	IB_PORT_BONDING_SLAVE = 1 << 0, /* rdma bonding slave port */
> +};
> +
>  struct ib_port_cache {
>  	u64		      subnet_prefix;
>  	struct ib_pkey_cache  *pkey;
>  	struct ib_gid_table   *gid;
>  	u8                     lmc;
>  	enum ib_port_state     port_state;
> +	enum ib_port_flags     port_event_flags;

Please no, u32.

>  };
>
>  struct ib_port_immutable {
> @@ -2706,6 +2713,7 @@ struct ib_device {
>  	/* Used by iWarp CM */
>  	char iw_ifname[IFNAMSIZ];
>  	u32 iw_driver_flags;
> +	struct notifier_block nb;
>  };
>
>  struct ib_client_nl_info;
> --
> 2.8.1
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH RFC for-next 2/6] RDMA/mlx5: remove deliver net device event
  2020-01-16  4:10 ` [PATCH RFC for-next 2/6] RDMA/mlx5: remove " Weihang Li
@ 2020-01-16 11:41   ` Leon Romanovsky
       [not found]     ` <7f3f8190-6b62-f3c6-e4db-2425411fa639@huawei.com>
  0 siblings, 1 reply; 13+ messages in thread
From: Leon Romanovsky @ 2020-01-16 11:41 UTC (permalink / raw)
  To: Weihang Li
  Cc: dledford, jgg, shiraz.saleem, aditr, mkalderon, aelior,
	linux-rdma, linuxarm

On Thu, Jan 16, 2020 at 12:10:43PM +0800, Weihang Li wrote:
> From: Lang Cheng <chenglang@huawei.com>
>
> The code that handles the link event of the net device has been moved
> into the core, and the related processing should been removed from the
> provider's driver.

I have serious doubts that this patch broke mlx5 LAG functionality.

Thanks

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH RFC for-next 1/6] RDMA/core: support deliver net device event
  2020-01-16  4:10 ` [PATCH RFC for-next 1/6] RDMA/core: support deliver net device event Weihang Li
  2020-01-16 11:37   ` Leon Romanovsky
@ 2020-01-17 14:23   ` Jason Gunthorpe
       [not found]     ` <19bd56ac-5df5-f5bb-e024-54ef3cd0d0ad@huawei.com>
  1 sibling, 1 reply; 13+ messages in thread
From: Jason Gunthorpe @ 2020-01-17 14:23 UTC (permalink / raw)
  To: Weihang Li
  Cc: dledford, leon, shiraz.saleem, aditr, mkalderon, aelior,
	linux-rdma, linuxarm

On Thu, Jan 16, 2020 at 12:10:42PM +0800, Weihang Li wrote:
> From: Lang Cheng <chenglang@huawei.com>
> 
> For the process of handling the link event of the net device, the driver
> of each provider is similar, so it can be integrated into the ib_core for
> unified processing.
> 
> Signed-off-by: Lang Cheng <chenglang@huawei.com>
> Signed-off-by: Weihang Li <liweihang@huawei.com>
>  drivers/infiniband/core/cache.c  |  21 ++++++-
>  drivers/infiniband/core/device.c | 123 +++++++++++++++++++++++++++++++++++++++
>  include/rdma/ib_cache.h          |  13 +++++
>  include/rdma/ib_verbs.h          |   8 +++
>  4 files changed, 164 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
> index 17bfedd..791e965 100644
> +++ b/drivers/infiniband/core/cache.c
> @@ -1174,6 +1174,23 @@ int ib_get_cached_port_state(struct ib_device   *device,
>  }
>  EXPORT_SYMBOL(ib_get_cached_port_state);
>  
> +int ib_get_cached_port_event_flags(struct ib_device   *device,
> +				   u8                  port_num,
> +				   enum ib_port_flags *event_flags)
> +{
> +	unsigned long flags;
> +
> +	if (!rdma_is_port_valid(device, port_num))
> +		return -EINVAL;
> +
> +	read_lock_irqsave(&device->cache_lock, flags);
> +	*event_flags = device->port_data[port_num].cache.port_event_flags;
> +	read_unlock_irqrestore(&device->cache_lock, flags);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(ib_get_cached_port_event_flags);
> +
>  /**
>   * rdma_get_gid_attr - Returns GID attributes for a port of a device
>   * at a requested gid_index, if a valid GID entry exists.
> @@ -1391,7 +1408,7 @@ ib_cache_update(struct ib_device *device, u8 port, bool enforce_security)
>  	if (!rdma_is_port_valid(device, port))
>  		return -EINVAL;
>  
> -	tprops = kmalloc(sizeof *tprops, GFP_KERNEL);
> +	tprops = kzalloc(sizeof(*tprops), GFP_KERNEL);
>  	if (!tprops)
>  		return -ENOMEM;
>  
> @@ -1435,6 +1452,8 @@ ib_cache_update(struct ib_device *device, u8 port, bool enforce_security)
>  	device->port_data[port].cache.pkey = pkey_cache;
>  	device->port_data[port].cache.lmc = tprops->lmc;
>  	device->port_data[port].cache.port_state = tprops->state;
> +	device->port_data[port].cache.port_event_flags =
> +						tprops->port_event_flags;
>  
>  	device->port_data[port].cache.subnet_prefix = tprops->subnet_prefix;
>  	write_unlock_irq(&device->cache_lock);
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index f6c2552..f03d6ce 100644
> +++ b/drivers/infiniband/core/device.c
> @@ -1325,6 +1325,77 @@ static int enable_device_and_get(struct ib_device *device)
>  	return ret;
>  }
>  
> +unsigned int ib_query_ndev_port_num(struct ib_device *device,
> +				    struct net_device *netdev)
> +{
> +	unsigned int port_num;
> +
> +	rdma_for_each_port(device, port_num)
> +		if (netdev == device->port_data[port_num].netdev)
> +			break;
> +
> +	return port_num;
> +}
> +EXPORT_SYMBOL(ib_query_ndev_port_num);

This returns garbage if the netdev isn't found

> +
> +static inline enum ib_port_state get_port_state(struct net_device *netdev)
> +{
> +	return (netif_running(netdev) && netif_carrier_ok(netdev)) ?
> +		IB_PORT_ACTIVE : IB_PORT_DOWN;
> +}
> +
> +static int ib_netdev_event(struct notifier_block *this,
> +			   unsigned long event, void *ptr)
> +{
> +	struct ib_device *device = container_of(this, struct ib_device, nb);
> +	struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
> +
> +	switch (event) {
> +	case NETDEV_CHANGE:
> +	case NETDEV_UP:
> +	case NETDEV_DOWN: {
> +		unsigned int port_num = ib_query_ndev_port_num(device, netdev);
> +		enum ib_port_state last_state;
> +		enum ib_port_state curr_state;
> +		struct ib_event ibev;
> +		enum ib_port_flags flags;
> +
> +		if (ib_get_cached_port_event_flags(device, port_num, &flags))
> +			return NOTIFY_DONE;
> +
> +		if (flags & IB_PORT_BONDING_SLAVE)
> +			goto done;
> +
> +		if (ib_get_cached_port_state(device, port_num, &last_state))
> +			return NOTIFY_DONE;
> +
> +		curr_state = get_port_state(netdev);
> +
> +		if (last_state == curr_state)
> +			goto done;
> +
> +		ibev.device = device;
> +		if (curr_state == IB_PORT_DOWN)
> +			ibev.event = IB_EVENT_PORT_ERR;
> +		else if (curr_state == IB_PORT_ACTIVE)
> +			ibev.event = IB_EVENT_PORT_ACTIVE;
> +		else
> +			goto done;
> +
> +		ibev.element.port_num = port_num;
> +		ib_dispatch_event(&ibev);
> +		dev_dbg(&device->dev,
> +			"core send %s\n", ib_event_msg(ibev.event));
> +		break;
> +	}
> +
> +	default:
> +		break;
> +	}
> +done:
> +	return NOTIFY_DONE;
> +}
> +
>  /**
>   * ib_register_device - Register an IB device with IB core
>   * @device: Device to register
> @@ -1342,6 +1413,7 @@ static int enable_device_and_get(struct ib_device *device)
>   */
>  int ib_register_device(struct ib_device *device, const char *name)
>  {
> +	unsigned int port;
>  	int ret;
>  
>  	ret = assign_name(device, name);
> @@ -1406,6 +1478,34 @@ int ib_register_device(struct ib_device *device, const char *name)
>  	}
>  	ib_device_put(device);
>  
> +	device->nb.notifier_call = ib_netdev_event;
> +	ret = register_netdevice_notifier(&device->nb);

Lets not register a notifer for every device please, we already have
ib_device_get_by_netdev() for this purpose, and we already have global
notifiers in this module.

Jason

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH RFC for-next 1/6] RDMA/core: support deliver net device event
       [not found]     ` <19bd56ac-5df5-f5bb-e024-54ef3cd0d0ad@huawei.com>
@ 2020-01-19  9:32       ` Leon Romanovsky
  0 siblings, 0 replies; 13+ messages in thread
From: Leon Romanovsky @ 2020-01-19  9:32 UTC (permalink / raw)
  To: Lang Cheng
  Cc: Jason Gunthorpe, Weihang Li, aelior, mkalderon, linuxarm, aditr,
	linux-rdma, dledford, shiraz.saleem

On Sun, Jan 19, 2020 at 03:02:11PM +0800, Lang Cheng wrote:
>
> 在 2020/1/17 22:23, Jason Gunthorpe 写道:
> > On Thu, Jan 16, 2020 at 12:10:42PM +0800, Weihang Li wrote:
> > > From: Lang Cheng <chenglang@huawei.com>
> > >
> > > For the process of handling the link event of the net device, the driver
> > > of each provider is similar, so it can be integrated into the ib_core for
> > > unified processing.
> > >
> > > Signed-off-by: Lang Cheng <chenglang@huawei.com>
> > > Signed-off-by: Weihang Li <liweihang@huawei.com>
> > >   drivers/infiniband/core/cache.c  |  21 ++++++-
> > >   drivers/infiniband/core/device.c | 123 +++++++++++++++++++++++++++++++++++++++
> > >   include/rdma/ib_cache.h          |  13 +++++
> > >   include/rdma/ib_verbs.h          |   8 +++
> > >   4 files changed, 164 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
> > > index 17bfedd..791e965 100644
> > > +++ b/drivers/infiniband/core/cache.c
> > > @@ -1174,6 +1174,23 @@ int ib_get_cached_port_state(struct ib_device   *device,
> > >   }
> > >   EXPORT_SYMBOL(ib_get_cached_port_state);
> > > +int ib_get_cached_port_event_flags(struct ib_device   *device,
> > > +				   u8                  port_num,
> > > +				   enum ib_port_flags *event_flags)
> > > +{
> > > +	unsigned long flags;
> > > +
> > > +	if (!rdma_is_port_valid(device, port_num))
> > > +		return -EINVAL;
> > > +
> > > +	read_lock_irqsave(&device->cache_lock, flags);
> > > +	*event_flags = device->port_data[port_num].cache.port_event_flags;
> > > +	read_unlock_irqrestore(&device->cache_lock, flags);
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL(ib_get_cached_port_event_flags);
> > > +
> > >   /**
> > >    * rdma_get_gid_attr - Returns GID attributes for a port of a device
> > >    * at a requested gid_index, if a valid GID entry exists.
> > > @@ -1391,7 +1408,7 @@ ib_cache_update(struct ib_device *device, u8 port, bool enforce_security)
> > >   	if (!rdma_is_port_valid(device, port))
> > >   		return -EINVAL;
> > > -	tprops = kmalloc(sizeof *tprops, GFP_KERNEL);
> > > +	tprops = kzalloc(sizeof(*tprops), GFP_KERNEL);
> > >   	if (!tprops)
> > >   		return -ENOMEM;
> > > @@ -1435,6 +1452,8 @@ ib_cache_update(struct ib_device *device, u8 port, bool enforce_security)
> > >   	device->port_data[port].cache.pkey = pkey_cache;
> > >   	device->port_data[port].cache.lmc = tprops->lmc;
> > >   	device->port_data[port].cache.port_state = tprops->state;
> > > +	device->port_data[port].cache.port_event_flags =
> > > +						tprops->port_event_flags;
> > >   	device->port_data[port].cache.subnet_prefix = tprops->subnet_prefix;
> > >   	write_unlock_irq(&device->cache_lock);
> > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > > index f6c2552..f03d6ce 100644
> > > +++ b/drivers/infiniband/core/device.c
> > > @@ -1325,6 +1325,77 @@ static int enable_device_and_get(struct ib_device *device)
> > >   	return ret;
> > >   }
> > > +unsigned int ib_query_ndev_port_num(struct ib_device *device,
> > > +				    struct net_device *netdev)
> > > +{
> > > +	unsigned int port_num;
> > > +
> > > +	rdma_for_each_port(device, port_num)
> > > +		if (netdev == device->port_data[port_num].netdev)
> > > +			break;
> > > +
> > > +	return port_num;
> > > +}
> > > +EXPORT_SYMBOL(ib_query_ndev_port_num);
> > This returns garbage if the netdev isn't found
>
> ib_get_cache_event_flags() will check port_num by calling
> rdma_is_port_valid() like ib_get_cache_port_state ().
>
> Now, following Leon's opinion, ib_get_cache_xxxx() should remove
> rdma_is_port_valid(),
>
> so it needs to be modified here, too.


rdma_is_port_valid() doesn't check for netdev existence, but only
ensures that port number is in valid range and it is supposed to
be guaranteed in this stage.

Thanks

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH RFC for-next 2/6] RDMA/mlx5: remove deliver net device event
       [not found]     ` <7f3f8190-6b62-f3c6-e4db-2425411fa639@huawei.com>
@ 2020-01-20  7:45       ` Leon Romanovsky
  0 siblings, 0 replies; 13+ messages in thread
From: Leon Romanovsky @ 2020-01-20  7:45 UTC (permalink / raw)
  To: Lang Cheng
  Cc: Weihang Li, aelior, mkalderon, linuxarm, aditr, jgg, dledford,
	shiraz.saleem, linux-rdma

On Mon, Jan 20, 2020 at 03:31:14PM +0800, Lang Cheng wrote:
>
> On 2020/1/16 19:41, Leon Romanovsky wrote:
> > On Thu, Jan 16, 2020 at 12:10:43PM +0800, Weihang Li wrote:
> > > From: Lang Cheng <chenglang@huawei.com>
> > >
> > > The code that handles the link event of the net device has been moved
> > > into the core, and the related processing should been removed from the
> > > provider's driver.
> > I have serious doubts that this patch broke mlx5 LAG functionality.
>
> All vendor drivers need to remove port link event code,
> and query slave info(only if support bonding) in ops.query_port callback.
> Here is about 4 function:
>
> mlx5_netdev_event(): remove all port link event code after ib core supports
> sending them,
>
> mlx5_get_rep_roce(): Only mlx5_netdev_event() ever called it, and now no
> one, so remove it.
>
> get_port_state():just move public operation to ib core.
>
> mlx5_query_port_roce():	query more info, no impact on existing code.
>
>
> Is there any hidden relationship that I didn't notice?

You didn't missed the functions which are relevant to bond, but from
what I saw you implemented wrongly events handling related to mlx5 bond.

I didn't look very deeply yet because the series is far from
completion and maybe I'm mistaken and bond works perfectly.

Thanks

>
> Thanks.
>
> >
> > Thanks
> > _______________________________________________
> > Linuxarm mailing list
> > Linuxarm@huawei.com
> > http://hulk.huawei.com/mailman/listinfo/linuxarm
> >

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-01-20  7:45 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-16  4:10 [PATCH RFC for-next 0/6] ofed support to send ib port link event Weihang Li
2020-01-16  4:10 ` [PATCH RFC for-next 1/6] RDMA/core: support deliver net device event Weihang Li
2020-01-16 11:37   ` Leon Romanovsky
2020-01-17 14:23   ` Jason Gunthorpe
     [not found]     ` <19bd56ac-5df5-f5bb-e024-54ef3cd0d0ad@huawei.com>
2020-01-19  9:32       ` Leon Romanovsky
2020-01-16  4:10 ` [PATCH RFC for-next 2/6] RDMA/mlx5: remove " Weihang Li
2020-01-16 11:41   ` Leon Romanovsky
     [not found]     ` <7f3f8190-6b62-f3c6-e4db-2425411fa639@huawei.com>
2020-01-20  7:45       ` Leon Romanovsky
2020-01-16  4:10 ` [PATCH RFC for-next 3/6] RDMA/i40iw: " Weihang Li
2020-01-16  4:10 ` [PATCH RFC for-next 4/6] RDMA/qedr: " Weihang Li
2020-01-16  4:10 ` [PATCH RFC for-next 5/6] RDMA/vmw_pvrdma: " Weihang Li
2020-01-16  4:10 ` [PATCH RFC for-next 6/6] qede: remove invalid notify operation Weihang Li
2020-01-16 11:15 ` [PATCH RFC for-next 0/6] ofed support to send ib port link event Leon Romanovsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).