Re: [PATCH v3 rdma-next 6/7] RDMA/nldev: Add support for RDMA monitoring

[Leon Romanovsky <leonro@nvidia.com>]

On Mon, Sep 09, 2024 at 08:30:24PM +0300, Michael Guralnik wrote:
> From: Chiara Meiohas <cmeiohas@nvidia.com>
> 
> Introduce a new netlink command to allow rdma event monitoring.
> The rdma events supported now are IB device
> registration/unregistration and net device attachment/detachment.
> 
> Example output of rdma monitor and the commands which trigger
> the events:
> 
> $ rdma monitor
> $ rmmod mlx5_ib
> [UNREGISTER]	ibdev_idx 1 ibdev rocep8s0f1
> [UNREGISTER]	ibdev_idx 0 ibdev rocep8s0f0
> 
> $ modprobe mlx5_ib
> [REGISTER]	ibdev_idx 2 ibdev mlx5_0
> [NETDEV_ATTACH]	ibdev_idx 2 ibdev mlx5_0 port 1 netdev_idx 4 netdev eth2
> [REGISTER]	ibdev_idx 3 ibdev mlx5_1
> [NETDEV_ATTACH]	ibdev_idx 3 ibdev mlx5_1 port 1 netdev_idx 5 netdev eth3
> 
> $ devlink dev eswitch set pci/0000:08:00.0 mode switchdev
> [UNREGISTER]	ibdev_idx 2 ibdev rocep8s0f0
> [REGISTER]	ibdev_idx 4 ibdev mlx5_0
> [NETDEV_ATTACH]	ibdev_idx 4 ibdev mlx5_0 port 30 netdev_idx 4 netdev eth2
> 
> $ echo 4 > /sys/class/net/eth2/device/sriov_numvfs
> [NETDEV_ATTACH]	ibdev_idx 4 ibdev rdmap8s0f0 port 2 netdev_idx 7 netdev eth4
> [NETDEV_ATTACH]	ibdev_idx 4 ibdev rdmap8s0f0 port 3 netdev_idx 8 netdev eth5
> [NETDEV_ATTACH]	ibdev_idx 4 ibdev rdmap8s0f0 port 4 netdev_idx 9 netdev eth6
> [NETDEV_ATTACH]	ibdev_idx 4 ibdev rdmap8s0f0 port 5 netdev_idx 10 netdev eth7
> [REGISTER]	ibdev_idx 5 ibdev mlx5_0
> [NETDEV_ATTACH]	ibdev_idx 5 ibdev mlx5_0 port 1 netdev_idx 11 netdev eth8
> [REGISTER]	ibdev_idx 6 ibdev mlx5_0
> [NETDEV_ATTACH]	ibdev_idx 6 ibdev mlx5_0 port 1 netdev_idx 12 netdev eth9
> [REGISTER]	ibdev_idx 7 ibdev mlx5_0
> [NETDEV_ATTACH]	ibdev_idx 7 ibdev mlx5_0 port 1 netdev_idx 13 netdev eth10
> [REGISTER]	ibdev_idx 8 ibdev mlx5_0
> [NETDEV_ATTACH]	ibdev_idx 8 ibdev mlx5_0 port 1 netdev_idx 14 netdev eth11
> 
> $ echo 0 > /sys/class/net/eth2/device/sriov_numvfs
> [UNREGISTER]	ibdev_idx 5 ibdev rocep8s0f0v0
> [UNREGISTER]	ibdev_idx 6 ibdev rocep8s0f0v1
> [UNREGISTER]	ibdev_idx 7 ibdev rocep8s0f0v2
> [UNREGISTER]	ibdev_idx 8 ibdev rocep8s0f0v3
> [NETDEV_DETACH]	ibdev_idx 4 ibdev rdmap8s0f0 port 2
> [NETDEV_DETACH]	ibdev_idx 4 ibdev rdmap8s0f0 port 3
> [NETDEV_DETACH]	ibdev_idx 4 ibdev rdmap8s0f0 port 4
> [NETDEV_DETACH]	ibdev_idx 4 ibdev rdmap8s0f0 port 5
> 
> Signed-off-by: Chiara Meiohas <cmeiohas@nvidia.com>
> Signed-off-by: Michael Guralnik <michaelgur@nvidia.com>
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
> ---
>  drivers/infiniband/core/device.c  |  38 +++++++++
>  drivers/infiniband/core/netlink.c |   1 +
>  drivers/infiniband/core/nldev.c   | 124 ++++++++++++++++++++++++++++++
>  include/rdma/rdma_netlink.h       |  12 +++
>  include/uapi/rdma/rdma_netlink.h  |  15 ++++
>  5 files changed, 190 insertions(+)

<...>

>  	/* Expedite removing unregistered pointers from the hash table */
>  	free_netdevs(ib_dev);
> @@ -2159,6 +2186,7 @@ static void add_ndev_hash(struct ib_port_data *pdata)
>  int ib_device_set_netdev(struct ib_device *ib_dev, struct net_device *ndev,
>  			 u32 port)
>  {
> +	enum rdma_nl_notify_event_type etype;
>  	struct net_device *old_ndev;
>  	struct ib_port_data *pdata;
>  	unsigned long flags;
> @@ -2190,6 +2218,16 @@ int ib_device_set_netdev(struct ib_device *ib_dev, struct net_device *ndev,
>  	spin_unlock_irqrestore(&pdata->netdev_lock, flags);
>  
>  	add_ndev_hash(pdata);
> +
> +	down_read(&devices_rwsem);
> +	if (xa_get_mark(&devices, ib_dev->index, DEVICE_REGISTERED) &&
> +	    xa_load(&devices, ib_dev->index) == ib_dev) {
> +		etype = ndev ?
> +			RDMA_NETDEV_ATTACH_EVENT : RDMA_NETDEV_DETACH_EVENT;
> +		rdma_nl_notify_event(ib_dev, port, etype);
> +	}
> +	up_read(&devices_rwsem);

There is no need in this locking, let's rewrite the following code
without it. We are in -rc7, I'll add this hunk when applying.

Thanks

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index d571b78d1bcc..3be66dd7b226 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -2219,14 +2219,12 @@ int ib_device_set_netdev(struct ib_device *ib_dev, struct net_device *ndev,
 
 	add_ndev_hash(pdata);
 
-	down_read(&devices_rwsem);
-	if (xa_get_mark(&devices, ib_dev->index, DEVICE_REGISTERED) &&
-	    xa_load(&devices, ib_dev->index) == ib_dev) {
-		etype = ndev ?
-			RDMA_NETDEV_ATTACH_EVENT : RDMA_NETDEV_DETACH_EVENT;
-		rdma_nl_notify_event(ib_dev, port, etype);
-	}
-	up_read(&devices_rwsem);
+	/* Make sure that the device is registered before we send events */
+	if (xa_load(&devices, ib_dev->index) != ib_dev)
+		return 0;
+
+	etype = ndev ? RDMA_NETDEV_ATTACH_EVENT : RDMA_NETDEV_DETACH_EVENT;
+	rdma_nl_notify_event(ib_dev, port, etype);
 
 	return 0;
 }