[PATCH RFC v2 for-next 0/7] ib core support to send ib port link event

[PATCH RFC v2 for-next 0/7] ib core support to send ib port link event

[Weihang Li]

From: Lang Cheng <chenglang@huawei.com>

Some provider driver has realized this function, but these code are
implemented separately by each manufacturer. This series provides an
solution in ib_core, and remove the relevant codes of some manufacturers.

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.

Supports reporting port active time during device registration and sending
port error events when device is deregistered.

The previous discussion can be found at:
https://patchwork.kernel.org/cover/11335999/

Changes since v1:
- Fix comments from Leon and Jason.

- Move event processing flow into global notifier instead of one notifier
  per device.

Lang Cheng (7):
  RDMA/core: add inactive attribute of ib_port_cache
  RDMA/mlx5: remove deliver net device event
  qede: remove invalid notify operation
  RDMA/qedr: remove deliver net device event
  RDMA/vmw_pvrdma: remove deliver net device event
  RDMA/core: support send port event
  RDMA/core: report link status when register and deregister ib device

 drivers/infiniband/core/cache.c                | 16 ++++-
 drivers/infiniband/core/device.c               | 45 ++++++++++++
 drivers/infiniband/core/roce_gid_mgmt.c        | 45 ++++++++++++
 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                        | 10 +++
 include/rdma/ib_verbs.h                        |  2 +
 9 files changed, 126 insertions(+), 115 deletions(-)

-- 
2.8.1

[PATCH RFC v2 for-next 1/7] RDMA/core: add inactive attribute of ib_port_cache

[Weihang Li]

From: Lang Cheng <chenglang@huawei.com>

Add attribute inactive to mark bonding backup port.

Signed-off-by: Lang Cheng <chenglang@huawei.com>
---
 drivers/infiniband/core/cache.c | 16 +++++++++++++++-
 include/rdma/ib_cache.h         | 10 ++++++++++
 include/rdma/ib_verbs.h         |  2 ++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
index d535995..7a7ef0e 100644
--- a/drivers/infiniband/core/cache.c
+++ b/drivers/infiniband/core/cache.c
@@ -1175,6 +1175,19 @@ int ib_get_cached_port_state(struct ib_device   *device,
 }
 EXPORT_SYMBOL(ib_get_cached_port_state);
 
+u8 ib_get_cached_port_inactive_status(struct ib_device *device, u8 port_num)
+{
+	unsigned long flags;
+	u8 inactive;
+
+	read_lock_irqsave(&device->cache.lock, flags);
+	inactive = device->port_data[port_num].cache.inactive;
+	read_unlock_irqrestore(&device->cache.lock, flags);
+
+	return inactive;
+}
+EXPORT_SYMBOL(ib_get_cached_port_inactive_status);
+
 /**
  * rdma_get_gid_attr - Returns GID attributes for a port of a device
  * at a requested gid_index, if a valid GID entry exists.
@@ -1393,7 +1406,7 @@ static void ib_cache_update(struct ib_device *device,
 	if (!rdma_is_port_valid(device, port))
 		return;
 
-	tprops = kmalloc(sizeof *tprops, GFP_KERNEL);
+	tprops = kzalloc(sizeof *tprops, GFP_KERNEL);
 	if (!tprops)
 		return;
 
@@ -1435,6 +1448,7 @@ static void ib_cache_update(struct ib_device *device,
 	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.inactive = tprops->inactive;
 
 	device->port_data[port].cache.subnet_prefix = tprops->subnet_prefix;
 	write_unlock_irq(&device->cache.lock);
diff --git a/include/rdma/ib_cache.h b/include/rdma/ib_cache.h
index 870b5e6..63b2dd6 100644
--- a/include/rdma/ib_cache.h
+++ b/include/rdma/ib_cache.h
@@ -131,6 +131,16 @@ int ib_get_cached_port_state(struct ib_device *device,
 			      u8                port_num,
 			      enum ib_port_state *port_active);
 
+/**
+ * ib_get_cached_port_inactive_status - Returns a cached port inactive status
+ * @device: The device to query.
+ * @port_num: The port number of the device to query.
+ *
+ * ib_get_cached_port_inactive_status() fetches the specified event inactive
+ * status stored in the local software cache.
+ */
+u8 ib_get_cached_port_inactive_status(struct ib_device *device, u8 port_num);
+
 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 5608e14..e17d846 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -665,6 +665,7 @@ struct ib_port_attr {
 	u8			active_speed;
 	u8                      phys_state;
 	u16			port_cap_flags2;
+	u8 			inactive;
 };
 
 enum ib_device_modify_flags {
@@ -2145,6 +2146,7 @@ struct ib_port_cache {
 	struct ib_gid_table   *gid;
 	u8                     lmc;
 	enum ib_port_state     port_state;
+	u8                     inactive;
 };
 
 struct ib_cache {
-- 
2.8.1

[PATCH RFC v2 for-next 2/7] RDMA/mlx5: remove deliver net device event

[Weihang Li]

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
driver.

Signed-off-by: Lang Cheng <chenglang@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 997cbfe..f202cbc 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->inactive = 1;
+	else
+		props->inactive = 0;
+
 out:
 	if (put_mdev)
 		mlx5_ib_put_native_port_mdev(dev, port_num);
-- 
2.8.1

[PATCH RFC v2 for-next 3/7] qede: remove invalid notify operation

[Weihang Li]

From: Lang Cheng <chenglang@huawei.com>

The qedr notify() will remove the processing of QEDE_UP and QEDE_DOWN,
so qede no more needs to notify rdma of these two events.

Signed-off-by: Lang Cheng <chenglang@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

[PATCH RFC v2 for-next 4/7] RDMA/qedr: remove deliver net device event

[Weihang Li]

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
driver.

Signed-off-by: Lang Cheng <chenglang@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

[PATCH RFC v2 for-next 5/7] RDMA/vmw_pvrdma: remove deliver net device event

[Weihang Li]

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
driver.

Signed-off-by: Lang Cheng <chenglang@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

[PATCH RFC v2 for-next 6/7] RDMA/core: support send port event

[Weihang Li]

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>
---
 drivers/infiniband/core/device.c        |  1 +
 drivers/infiniband/core/roce_gid_mgmt.c | 45 +++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 84dd74f..0427a4d 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -2225,6 +2225,7 @@ struct net_device *ib_device_get_netdev(struct ib_device *ib_dev,
 
 	return res;
 }
+EXPORT_SYMBOL(ib_device_get_netdev);
 
 /**
  * ib_device_get_by_netdev - Find an IB device associated with a netdev
diff --git a/drivers/infiniband/core/roce_gid_mgmt.c b/drivers/infiniband/core/roce_gid_mgmt.c
index 2860def..4170ba3 100644
--- a/drivers/infiniband/core/roce_gid_mgmt.c
+++ b/drivers/infiniband/core/roce_gid_mgmt.c
@@ -751,6 +751,12 @@ static int netdevice_event(struct notifier_block *this, unsigned long event,
 	struct net_device *ndev = netdev_notifier_info_to_dev(ptr);
 	struct netdev_event_work_cmd cmds[ROCE_NETDEV_CALLBACK_SZ] = { {NULL} };
 
+	enum ib_port_state last_state;
+	enum ib_port_state curr_state;
+	struct ib_device *device;
+	struct ib_event ibev;
+	unsigned int port;
+
 	if (ndev->type != ARPHRD_ETHER)
 		return NOTIFY_DONE;
 
@@ -762,6 +768,45 @@ static int netdevice_event(struct notifier_block *this, unsigned long event,
 		cmds[2] = add_cmd;
 		break;
 
+	case NETDEV_CHANGE:
+	case NETDEV_DOWN:
+		device = ib_device_get_by_netdev(ndev, RDMA_DRIVER_UNKNOWN);
+		if (!device)
+			break;
+
+		rdma_for_each_port (device, port) {
+			if (ib_device_get_netdev(device, port) != ndev)
+				continue;
+
+			if (ib_get_cached_port_inactive_status(device, port))
+				break;
+
+			ib_get_cached_port_state(device, port, &last_state);
+			curr_state =
+				netif_running(ndev) && netif_carrier_ok(ndev) ?
+					IB_PORT_ACTIVE :
+					IB_PORT_DOWN;
+
+			if (last_state == curr_state)
+				break;
+
+			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
+				break;
+
+			ibev.device = device;
+			ibev.element.port_num = port;
+			ib_dispatch_event(&ibev);
+			ibdev_dbg(ibev.device, "core send %s\n",
+				  ib_event_msg(ibev.event));
+		}
+
+		ib_device_put(device);
+		break;
+
 	case NETDEV_UNREGISTER:
 		if (ndev->reg_state < NETREG_UNREGISTERED)
 			cmds[0] = del_cmd;
-- 
2.8.1

[PATCH RFC v2 for-next 7/7] RDMA/core: report link status when register and deregister ib device

[Weihang Li]

From: Lang Cheng <chenglang@huawei.com>

Support send link active event for every port of ib device except bonding
backup port when ib device is being registered or deregistered.

Signed-off-by: Lang Cheng <chenglang@huawei.com>
---
 drivers/infiniband/core/device.c | 44 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 0427a4d..6cdfffa 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -1341,6 +1341,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);
@@ -1405,6 +1406,28 @@ int ib_register_device(struct ib_device *device, const char *name)
 	}
 	ib_device_put(device);
 
+	rdma_for_each_port(device, port) {
+		struct net_device *netdev = ib_device_get_netdev(device, port);
+		enum ib_port_state port_state;
+		struct ib_event event;
+
+		/* not every port has a netdevice */
+		if (!netdev ||
+		    ib_get_cached_port_inactive_status(device, port))
+			continue;
+
+		ib_get_cached_port_state(device, port, &port_state);
+		if (port_state != IB_PORT_ACTIVE)
+			continue;
+
+		event.device = device;
+		event.event = IB_EVENT_PORT_ACTIVE;
+		event.element.port_num = port;
+		ib_dispatch_event(&event);
+		ibdev_dbg(device, "init event %s\n",
+			  ib_event_msg(event.event));
+	}
+
 	return 0;
 
 dev_cleanup:
@@ -1469,6 +1492,27 @@ static void __ib_unregister_device(struct ib_device *ib_dev)
  */
 void ib_unregister_device(struct ib_device *ib_dev)
 {
+	unsigned int port;
+
+	rdma_for_each_port(ib_dev, port) {
+		enum ib_port_state port_state;
+		struct ib_event event;
+
+		if (ib_get_cached_port_inactive_status(ib_dev, port))
+			continue;
+
+		ib_get_cached_port_state(ib_dev, port, &port_state);
+		if (port_state != IB_PORT_ACTIVE)
+			continue;
+
+		event.device = ib_dev;
+		event.event = IB_EVENT_PORT_ERR;
+		event.element.port_num = port;
+		ib_dispatch_event(&event);
+		ibdev_dbg(ib_dev, "init event %s\n",
+			  ib_event_msg(event.event));
+	}
+
 	get_device(&ib_dev->dev);
 	__ib_unregister_device(ib_dev);
 	put_device(&ib_dev->dev);
-- 
2.8.1

Re: [PATCH RFC v2 for-next 1/7] RDMA/core: add inactive attribute of ib_port_cache

[Jason Gunthorpe]

On Tue, Feb 04, 2020 at 04:24:02PM +0800, Weihang Li wrote:
> From: Lang Cheng <chenglang@huawei.com>
> 
> Add attribute inactive to mark bonding backup port.
> 
> Signed-off-by: Lang Cheng <chenglang@huawei.com>
>  drivers/infiniband/core/cache.c | 16 +++++++++++++++-
>  include/rdma/ib_cache.h         | 10 ++++++++++
>  include/rdma/ib_verbs.h         |  2 ++
>  3 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
> index d535995..7a7ef0e 100644
> +++ b/drivers/infiniband/core/cache.c
> @@ -1175,6 +1175,19 @@ int ib_get_cached_port_state(struct ib_device   *device,
>  }
>  EXPORT_SYMBOL(ib_get_cached_port_state);
>  
> +u8 ib_get_cached_port_inactive_status(struct ib_device *device, u8 port_num)
> +{
> +	unsigned long flags;
> +	u8 inactive;
> +
> +	read_lock_irqsave(&device->cache.lock, flags);
> +	inactive = device->port_data[port_num].cache.inactive;
> +	read_unlock_irqrestore(&device->cache.lock, flags);
> +
> +	return inactive;
> +}
> +EXPORT_SYMBOL(ib_get_cached_port_inactive_status);
> +
>  /**
>   * rdma_get_gid_attr - Returns GID attributes for a port of a device
>   * at a requested gid_index, if a valid GID entry exists.
> @@ -1393,7 +1406,7 @@ static void ib_cache_update(struct ib_device *device,
>  	if (!rdma_is_port_valid(device, port))
>  		return;
>  
> -	tprops = kmalloc(sizeof *tprops, GFP_KERNEL);
> +	tprops = kzalloc(sizeof *tprops, GFP_KERNEL);
>  	if (!tprops)
>  		return;
>  
> @@ -1435,6 +1448,7 @@ static void ib_cache_update(struct ib_device *device,
>  	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.inactive = tprops->inactive;
>  
>  	device->port_data[port].cache.subnet_prefix = tprops->subnet_prefix;
>  	write_unlock_irq(&device->cache.lock);
> diff --git a/include/rdma/ib_cache.h b/include/rdma/ib_cache.h
> index 870b5e6..63b2dd6 100644
> +++ b/include/rdma/ib_cache.h
> @@ -131,6 +131,16 @@ int ib_get_cached_port_state(struct ib_device *device,
>  			      u8                port_num,
>  			      enum ib_port_state *port_active);
>  
> +/**
> + * ib_get_cached_port_inactive_status - Returns a cached port inactive status
> + * @device: The device to query.
> + * @port_num: The port number of the device to query.
> + *
> + * ib_get_cached_port_inactive_status() fetches the specified event inactive
> + * status stored in the local software cache.
> + */
> +u8 ib_get_cached_port_inactive_status(struct ib_device *device, u8 port_num);
> +

kdocs belong with the implementation, not in the header file

>  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 5608e14..e17d846 100644
> +++ b/include/rdma/ib_verbs.h
> @@ -665,6 +665,7 @@ struct ib_port_attr {
>  	u8			active_speed;
>  	u8                      phys_state;
>  	u16			port_cap_flags2;
> +	u8 			inactive;
>  };

Why is a major structure being changed for this? 

If LAG is to be brought up to the core code then the core should know
what leg is active or not, not the driver.

>  enum ib_device_modify_flags {
> @@ -2145,6 +2146,7 @@ struct ib_port_cache {
>  	struct ib_gid_table   *gid;
>  	u8                     lmc;
>  	enum ib_port_state     port_state;
> +	u8                     inactive;
>  };

Please think carefully about structure patcking here, both placements
of u8 seem poor

Jason

Re: [PATCH RFC v2 for-next 2/7] RDMA/mlx5: remove deliver net device event

[Jason Gunthorpe]

On Tue, Feb 04, 2020 at 04:24:03PM +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
> driver.

It has? How?

> @@ -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);

Where does the core code do this?

Jason

Re: [PATCH RFC v2 for-next 3/7] qede: remove invalid notify operation

[Jason Gunthorpe]

On Tue, Feb 04, 2020 at 04:24:04PM +0800, Weihang Li wrote:
> From: Lang Cheng <chenglang@huawei.com>
> 
> The qedr notify() will remove the processing of QEDE_UP and QEDE_DOWN,
> so qede no more needs to notify rdma of these two events.
> 
> Signed-off-by: Lang Cheng <chenglang@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
> +++ 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);
>  }

Leaving empty functions behind? Why?

I'm getting the feeling that this series is inside out or
backwards something. This change should not happen until the rdma
driver stops consuming these events

Jason

Re: [PATCH RFC v2 for-next 6/7] RDMA/core: support send port event

[Jason Gunthorpe]

On Tue, Feb 04, 2020 at 04:24:07PM +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>
>  drivers/infiniband/core/device.c        |  1 +
>  drivers/infiniband/core/roce_gid_mgmt.c | 45 +++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 84dd74f..0427a4d 100644
> +++ b/drivers/infiniband/core/device.c
> @@ -2225,6 +2225,7 @@ struct net_device *ib_device_get_netdev(struct ib_device *ib_dev,
>  
>  	return res;
>  }
> +EXPORT_SYMBOL(ib_device_get_netdev);
>  
>  /**
>   * ib_device_get_by_netdev - Find an IB device associated with a netdev
> diff --git a/drivers/infiniband/core/roce_gid_mgmt.c b/drivers/infiniband/core/roce_gid_mgmt.c
> index 2860def..4170ba3 100644
> +++ b/drivers/infiniband/core/roce_gid_mgmt.c
> @@ -751,6 +751,12 @@ static int netdevice_event(struct notifier_block *this, unsigned long event,
>  	struct net_device *ndev = netdev_notifier_info_to_dev(ptr);
>  	struct netdev_event_work_cmd cmds[ROCE_NETDEV_CALLBACK_SZ] = { {NULL} };
>  
> +	enum ib_port_state last_state;
> +	enum ib_port_state curr_state;
> +	struct ib_device *device;
> +	struct ib_event ibev;
> +	unsigned int port;
> +
>  	if (ndev->type != ARPHRD_ETHER)
>  		return NOTIFY_DONE;
>  
> @@ -762,6 +768,45 @@ static int netdevice_event(struct notifier_block *this, unsigned long event,
>  		cmds[2] = add_cmd;
>  		break;
>  
> +	case NETDEV_CHANGE:
> +	case NETDEV_DOWN:
> +		device = ib_device_get_by_netdev(ndev, RDMA_DRIVER_UNKNOWN);
> +		if (!device)
> +			break;
> +
> +		rdma_for_each_port (device, port) {
> +			if (ib_device_get_netdev(device, port) != ndev)
> +				continue;

This feels strange, maybe we need to fix ib_device_get_by_netdev to
return the port too?

> +
> +			if (ib_get_cached_port_inactive_status(device, port))
> +				break;
> +
> +			ib_get_cached_port_state(device, port, &last_state);
> +			curr_state =
> +				netif_running(ndev) && netif_carrier_ok(ndev) ?
> +					IB_PORT_ACTIVE :
> +					IB_PORT_DOWN;
> +
> +			if (last_state == curr_state)
> +				break;
> +
> +			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
> +				break;

Other states are ignored?

> +
> +			ibev.device = device;
> +			ibev.element.port_num = port;
> +			ib_dispatch_event(&ibev);
> +			ibdev_dbg(ibev.device, "core send %s\n",
> +				  ib_event_msg(ibev.event));
> +		}
> +
> +		ib_device_put(device);
> +		break;

Ah the series is backwards. 

You need to organize your series so that every patch works
properly. This has to be before any drivers are removed, and you'll
need some temporary capability to disable it for drivers that have not
been migrated yet.

Jason

Re: [PATCH RFC v2 for-next 1/7] RDMA/core: add inactive attribute of ib_port_cache

[Lang Cheng]

On 2020/2/20 5:01, Jason Gunthorpe wrote:
> On Tue, Feb 04, 2020 at 04:24:02PM +0800, Weihang Li wrote:
>> From: Lang Cheng <chenglang@huawei.com>
>>
>> Add attribute inactive to mark bonding backup port.
>>
>> Signed-off-by: Lang Cheng <chenglang@huawei.com>
>>   drivers/infiniband/core/cache.c | 16 +++++++++++++++-
>>   include/rdma/ib_cache.h         | 10 ++++++++++
>>   include/rdma/ib_verbs.h         |  2 ++
>>   3 files changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
>> index d535995..7a7ef0e 100644
>> +++ b/drivers/infiniband/core/cache.c
>> @@ -1175,6 +1175,19 @@ int ib_get_cached_port_state(struct ib_device   *device,
>>   }
>>   EXPORT_SYMBOL(ib_get_cached_port_state);
>>   
>> +u8 ib_get_cached_port_inactive_status(struct ib_device *device, u8 port_num)
>> +{
>> +	unsigned long flags;
>> +	u8 inactive;
>> +
>> +	read_lock_irqsave(&device->cache.lock, flags);
>> +	inactive = device->port_data[port_num].cache.inactive;
>> +	read_unlock_irqrestore(&device->cache.lock, flags);
>> +
>> +	return inactive;
>> +}
>> +EXPORT_SYMBOL(ib_get_cached_port_inactive_status);
>> +
>>   /**
>>    * rdma_get_gid_attr - Returns GID attributes for a port of a device
>>    * at a requested gid_index, if a valid GID entry exists.
>> @@ -1393,7 +1406,7 @@ static void ib_cache_update(struct ib_device *device,
>>   	if (!rdma_is_port_valid(device, port))
>>   		return;
>>   
>> -	tprops = kmalloc(sizeof *tprops, GFP_KERNEL);
>> +	tprops = kzalloc(sizeof *tprops, GFP_KERNEL);
>>   	if (!tprops)
>>   		return;
>>   
>> @@ -1435,6 +1448,7 @@ static void ib_cache_update(struct ib_device *device,
>>   	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.inactive = tprops->inactive;
>>   
>>   	device->port_data[port].cache.subnet_prefix = tprops->subnet_prefix;
>>   	write_unlock_irq(&device->cache.lock);
>> diff --git a/include/rdma/ib_cache.h b/include/rdma/ib_cache.h
>> index 870b5e6..63b2dd6 100644
>> +++ b/include/rdma/ib_cache.h
>> @@ -131,6 +131,16 @@ int ib_get_cached_port_state(struct ib_device *device,
>>   			      u8                port_num,
>>   			      enum ib_port_state *port_active);
>>   
>> +/**
>> + * ib_get_cached_port_inactive_status - Returns a cached port inactive status
>> + * @device: The device to query.
>> + * @port_num: The port number of the device to query.
>> + *
>> + * ib_get_cached_port_inactive_status() fetches the specified event inactive
>> + * status stored in the local software cache.
>> + */
>> +u8 ib_get_cached_port_inactive_status(struct ib_device *device, u8 port_num);
>> +
> 
> kdocs belong with the implementation, not in the header file

All of ib_get_cached_XXX() should remove rdma_is_port_valid() and move 
kdocs from ib_cache.h to cache.c.
I can send this first, independently.

> 
>>   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 5608e14..e17d846 100644
>> +++ b/include/rdma/ib_verbs.h
>> @@ -665,6 +665,7 @@ struct ib_port_attr {
>>   	u8			active_speed;
>>   	u8                      phys_state;
>>   	u16			port_cap_flags2;
>> +	u8 			inactive;
>>   };
> 
> Why is a major structure being changed for this?
> 
> If LAG is to be brought up to the core code then the core should know
> what leg is active or not, not the driver.

"LAG is to be brought up to the core", is this under planning?
The link event report could be after this . Then, there may be a better 
way to ignore inactive port.

> 
>>   enum ib_device_modify_flags {
>> @@ -2145,6 +2146,7 @@ struct ib_port_cache {
>>   	struct ib_gid_table   *gid;
>>   	u8                     lmc;
>>   	enum ib_port_state     port_state;
>> +	u8                     inactive;
>>   };
> 
> Please think carefully about structure patcking here, both placements
> of u8 seem poor
> 
> Jason
> 
> .
> 

Re: [PATCH RFC v2 for-next 3/7] qede: remove invalid notify operation

[Lang Cheng]

On 2020/2/20 5:04, Jason Gunthorpe wrote:
> On Tue, Feb 04, 2020 at 04:24:04PM +0800, Weihang Li wrote:
>> From: Lang Cheng <chenglang@huawei.com>
>>
>> The qedr notify() will remove the processing of QEDE_UP and QEDE_DOWN,
>> so qede no more needs to notify rdma of these two events.
>>
>> Signed-off-by: Lang Cheng <chenglang@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
>> +++ 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);
>>   }
> 
> Leaving empty functions behind? Why?
Remove these empty static functions.
> 
> I'm getting the feeling that this series is inside out or
> backwards something. This change should not happen until the rdma
> driver stops consuming these events
> 
"RDMA/qedr: remove deliver net device event" should be in front of 
"qede: remove invalid notify operation".

Exchange 3/7 and 4/7.

thanks.

> Jason
> 

Re: [PATCH RFC v2 for-next 1/7] RDMA/core: add inactive attribute of ib_port_cache

[Leon Romanovsky]

On Thu, Feb 20, 2020 at 11:19:25AM +0800, Lang Cheng wrote:
>
>
> On 2020/2/20 5:01, Jason Gunthorpe wrote:
> > On Tue, Feb 04, 2020 at 04:24:02PM +0800, Weihang Li wrote:
> > > From: Lang Cheng <chenglang@huawei.com>
> > >
> > > Add attribute inactive to mark bonding backup port.
> > >
> > > Signed-off-by: Lang Cheng <chenglang@huawei.com>
> > >   drivers/infiniband/core/cache.c | 16 +++++++++++++++-
> > >   include/rdma/ib_cache.h         | 10 ++++++++++
> > >   include/rdma/ib_verbs.h         |  2 ++
> > >   3 files changed, 27 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
> > > index d535995..7a7ef0e 100644
> > > +++ b/drivers/infiniband/core/cache.c
> > > @@ -1175,6 +1175,19 @@ int ib_get_cached_port_state(struct ib_device   *device,
> > >   }
> > >   EXPORT_SYMBOL(ib_get_cached_port_state);
> > > +u8 ib_get_cached_port_inactive_status(struct ib_device *device, u8 port_num)
> > > +{
> > > +	unsigned long flags;
> > > +	u8 inactive;
> > > +
> > > +	read_lock_irqsave(&device->cache.lock, flags);
> > > +	inactive = device->port_data[port_num].cache.inactive;
> > > +	read_unlock_irqrestore(&device->cache.lock, flags);
> > > +
> > > +	return inactive;
> > > +}
> > > +EXPORT_SYMBOL(ib_get_cached_port_inactive_status);
> > > +
> > >   /**
> > >    * rdma_get_gid_attr - Returns GID attributes for a port of a device
> > >    * at a requested gid_index, if a valid GID entry exists.
> > > @@ -1393,7 +1406,7 @@ static void ib_cache_update(struct ib_device *device,
> > >   	if (!rdma_is_port_valid(device, port))
> > >   		return;
> > > -	tprops = kmalloc(sizeof *tprops, GFP_KERNEL);
> > > +	tprops = kzalloc(sizeof *tprops, GFP_KERNEL);
> > >   	if (!tprops)
> > >   		return;
> > > @@ -1435,6 +1448,7 @@ static void ib_cache_update(struct ib_device *device,
> > >   	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.inactive = tprops->inactive;
> > >   	device->port_data[port].cache.subnet_prefix = tprops->subnet_prefix;
> > >   	write_unlock_irq(&device->cache.lock);
> > > diff --git a/include/rdma/ib_cache.h b/include/rdma/ib_cache.h
> > > index 870b5e6..63b2dd6 100644
> > > +++ b/include/rdma/ib_cache.h
> > > @@ -131,6 +131,16 @@ int ib_get_cached_port_state(struct ib_device *device,
> > >   			      u8                port_num,
> > >   			      enum ib_port_state *port_active);
> > > +/**
> > > + * ib_get_cached_port_inactive_status - Returns a cached port inactive status
> > > + * @device: The device to query.
> > > + * @port_num: The port number of the device to query.
> > > + *
> > > + * ib_get_cached_port_inactive_status() fetches the specified event inactive
> > > + * status stored in the local software cache.
> > > + */
> > > +u8 ib_get_cached_port_inactive_status(struct ib_device *device, u8 port_num);
> > > +
> >
> > kdocs belong with the implementation, not in the header file
>
> All of ib_get_cached_XXX() should remove rdma_is_port_valid() and move kdocs
> from ib_cache.h to cache.c.
> I can send this first, independently.
>
> >
> > >   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 5608e14..e17d846 100644
> > > +++ b/include/rdma/ib_verbs.h
> > > @@ -665,6 +665,7 @@ struct ib_port_attr {
> > >   	u8			active_speed;
> > >   	u8                      phys_state;
> > >   	u16			port_cap_flags2;
> > > +	u8 			inactive;
> > >   };
> >
> > Why is a major structure being changed for this?
> >
> > If LAG is to be brought up to the core code then the core should know
> > what leg is active or not, not the driver.
>
> "LAG is to be brought up to the core", is this under planning?

It is a development direction, sooner or later that will happen.

Thanks

Re: [PATCH RFC v2 for-next 6/7] RDMA/core: support send port event

[Lang Cheng]

On 2020/2/20 5:07, Jason Gunthorpe wrote:
> On Tue, Feb 04, 2020 at 04:24:07PM +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>
>>   drivers/infiniband/core/device.c        |  1 +
>>   drivers/infiniband/core/roce_gid_mgmt.c | 45 +++++++++++++++++++++++++++++++++
>>   2 files changed, 46 insertions(+)
>>
>> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
>> index 84dd74f..0427a4d 100644
>> +++ b/drivers/infiniband/core/device.c
>> @@ -2225,6 +2225,7 @@ struct net_device *ib_device_get_netdev(struct ib_device *ib_dev,
>>   
>>   	return res;
>>   }
>> +EXPORT_SYMBOL(ib_device_get_netdev);
>>   
>>   /**
>>    * ib_device_get_by_netdev - Find an IB device associated with a netdev
>> diff --git a/drivers/infiniband/core/roce_gid_mgmt.c b/drivers/infiniband/core/roce_gid_mgmt.c
>> index 2860def..4170ba3 100644
>> +++ b/drivers/infiniband/core/roce_gid_mgmt.c
>> @@ -751,6 +751,12 @@ static int netdevice_event(struct notifier_block *this, unsigned long event,
>>   	struct net_device *ndev = netdev_notifier_info_to_dev(ptr);
>>   	struct netdev_event_work_cmd cmds[ROCE_NETDEV_CALLBACK_SZ] = { {NULL} };
>>   
>> +	enum ib_port_state last_state;
>> +	enum ib_port_state curr_state;
>> +	struct ib_device *device;
>> +	struct ib_event ibev;
>> +	unsigned int port;
>> +
>>   	if (ndev->type != ARPHRD_ETHER)
>>   		return NOTIFY_DONE;
>>   
>> @@ -762,6 +768,45 @@ static int netdevice_event(struct notifier_block *this, unsigned long event,
>>   		cmds[2] = add_cmd;
>>   		break;
>>   
>> +	case NETDEV_CHANGE:
>> +	case NETDEV_DOWN:
>> +		device = ib_device_get_by_netdev(ndev, RDMA_DRIVER_UNKNOWN);
>> +		if (!device)
>> +			break;
>> +
>> +		rdma_for_each_port (device, port) {
>> +			if (ib_device_get_netdev(device, port) != ndev)
>> +				continue;
> 
> This feels strange, maybe we need to fix ib_device_get_by_netdev to
> return the port too?

Seems ib_device_get_by_netdev is used infrequently, so the port 
information may only benefit siw and here.

> 
>> +
>> +			if (ib_get_cached_port_inactive_status(device, port))
>> +				break;
>> +
>> +			ib_get_cached_port_state(device, port, &last_state);
>> +			curr_state =
>> +				netif_running(ndev) && netif_carrier_ok(ndev) ?
>> +					IB_PORT_ACTIVE :
>> +					IB_PORT_DOWN;
>> +
>> +			if (last_state == curr_state)
>> +				break;
>> +
>> +			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
>> +				break;
> 
> Other states are ignored?

I think the "curr_state" has only two port states,
maybe the "last_state" has the 3rd state(0) to represent INIT.

> 
>> +
>> +			ibev.device = device;
>> +			ibev.element.port_num = port;
>> +			ib_dispatch_event(&ibev);
>> +			ibdev_dbg(ibev.device, "core send %s\n",
>> +				  ib_event_msg(ibev.event));
>> +		}
>> +
>> +		ib_device_put(device);
>> +		break;
> 
> Ah the series is backwards.
> 
> You need to organize your series so that every patch works
> properly. This has to be before any drivers are removed, and you'll
> need some temporary capability to disable it for drivers that have not
> been migrated yet.

Yes.I will add "some temporary capability" in next to make every patch 
works properly.

Thanks.

> 
> Jason
> 
> .
>