Skip to contentSkip to conversation

net: mana: Change the function signature of mana_get_primary_netdev_rcu

PATCH Superseded [email protected] 3 participants 4 messages Mar 5, 2025

Subsystems (5)

v4 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

PATCH v4 1/2 net: mana: Change the function signature of mana_get_primary_netdev_rcu

From: Long Li <[email protected]>

Change mana_get_primary_netdev_rcu() to mana_get_primary_netdev(), and return the ndev with refcount held. The caller is responsible for dropping the refcount.

Also drop the check for IFF_SLAVE as it is not necessary if the upper device is present.

---
Changes
v4: use netdev_hold()/netdev_put() and remove the check for IFF_SLAVE

 drivers/infiniband/hw/mana/device.c           |  7 +++----
 drivers/net/ethernet/microsoft/mana/mana_en.c | 20 +++++++++++--------
 include/net/mana/mana.h                       |  2 +-
 3 files changed, 16 insertions(+), 13 deletions(-)
drivers/infiniband/hw/mana/device.c +3 -4
index 3416a85f8738..2b4dd34a41ba 100644
@@ -84,10 +84,8 @@ static int mana_ib_probe(struct auxiliary_device *adev,
 	dev->ib_dev.num_comp_vectors = mdev->gdma_context->max_num_queues;
 	dev->ib_dev.dev.parent = mdev->gdma_context->dev;
 
-	rcu_read_lock(); /* required to get primary netdev */
-	ndev = mana_get_primary_netdev_rcu(mc, 0);
+	ndev = mana_get_primary_netdev(mc, 0);
 	if (!ndev) {
-		rcu_read_unlock();
 		ret = -ENODEV;
 		ibdev_err(&dev->ib_dev, "Failed to get netdev for IB port 1");
 		goto free_ib_device;
@@ -95,7 +93,8 @@ static int mana_ib_probe(struct auxiliary_device *adev,
 	ether_addr_copy(mac_addr, ndev->dev_addr);
 	addrconf_addr_eui48((u8 *)&dev->ib_dev.node_guid, ndev->dev_addr);
 	ret = ib_device_set_netdev(&dev->ib_dev, ndev, 1);
-	rcu_read_unlock();
+	/* mana_get_primary_netdev() returns ndev with refcount held */
+	netdev_put(ndev, NULL);
 	if (ret) {
 		ibdev_err(&dev->ib_dev, "Failed to set ib netdev, ret %d", ret);
 		goto free_ib_device;
drivers/net/ethernet/microsoft/mana/mana_en.c +12 -8
index aa1e47233fe5..a8e7f030c11b 100644
@@ -3131,21 +3131,25 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
 	kfree(ac);
 }
 
-struct net_device *mana_get_primary_netdev_rcu(struct mana_context *ac, u32 port_index)
+struct net_device *mana_get_primary_netdev(struct mana_context *ac, u32 port_index)
 {
 	struct net_device *ndev;
 
-	RCU_LOCKDEP_WARN(!rcu_read_lock_held(),
-			 "Taking primary netdev without holding the RCU read lock");
 	if (port_index >= ac->num_ports)
 		return NULL;
 
-	/* When mana is used in netvsc, the upper netdevice should be returned. */
-	if (ac->ports[port_index]->flags & IFF_SLAVE)
-		ndev = netdev_master_upper_dev_get_rcu(ac->ports[port_index]);
-	else
+	rcu_read_lock();
+
+	/* If mana is used in netvsc, the upper netdevice should be returned. */
+	ndev = netdev_master_upper_dev_get_rcu(ac->ports[port_index]);
+
+	/* If there is no upper device, use the parent Ethernet device */
+	if (!ndev)
 		ndev = ac->ports[port_index];
 
+	netdev_hold(ndev, NULL, GFP_ATOMIC);
+	rcu_read_unlock();
+
 	return ndev;
 }
-EXPORT_SYMBOL_NS(mana_get_primary_netdev_rcu, "NET_MANA");
+EXPORT_SYMBOL_NS(mana_get_primary_netdev, "NET_MANA");
include/net/mana/mana.h +1 -1
index 0d00b24eacaf..770359ce7d3b 100644
@@ -827,5 +827,5 @@ int mana_cfg_vport(struct mana_port_context *apc, u32 protection_dom_id,
 		   u32 doorbell_pg_id);
 void mana_uncfg_vport(struct mana_port_context *apc);
 
-struct net_device *mana_get_primary_netdev_rcu(struct mana_context *ac, u32 port_index);
+struct net_device *mana_get_primary_netdev(struct mana_context *ac, u32 port_index);
 #endif /* _MANA_H */
Signature · 1 line
2.34.1
#1 lore raw

PATCH v4 2/2 RDMA/mana_ib: Handle net event for pointing to the current netdev

From: Long Li <[email protected]>

When running under Hyper-V, the master device to the RDMA device is always bonded to this RDMA device. This is not user-configurable.

The master device can be unbind/bind from the kernel. During those events, the RDMA device should set to the current netdev to reflect the change of master device from those events.

---
Changes
v2: Add missing error handling when register_netdevice_notifier() fails.
v3: Change mana_get_primary_netdev() to return with netdev refcount held.
v4: use netdev_put().

 drivers/infiniband/hw/mana/device.c  | 47 ++++++++++++++++++++++++++--
 drivers/infiniband/hw/mana/mana_ib.h |  1 +
 2 files changed, 46 insertions(+), 2 deletions(-)
drivers/infiniband/hw/mana/device.c +45 -2
index 2b4dd34a41ba..d71e1715aea8 100644
@@ -51,6 +51,38 @@ static const struct ib_device_ops mana_ib_dev_ops = {
 			   ib_ind_table),
 };
 
+static int mana_ib_netdev_event(struct notifier_block *this,
+				unsigned long event, void *ptr)
+{
+	struct mana_ib_dev *dev = container_of(this, struct mana_ib_dev, nb);
+	struct net_device *event_dev = netdev_notifier_info_to_dev(ptr);
+	struct gdma_context *gc = dev->gdma_dev->gdma_context;
+	struct mana_context *mc = gc->mana.driver_data;
+	struct net_device *ndev;
+
+	/* Only process events from our parent device */
+	if (event_dev != mc->ports[0])
+		return NOTIFY_DONE;
+
+	switch (event) {
+	case NETDEV_CHANGEUPPER:
+		ndev = mana_get_primary_netdev(mc, 0);
+		/*
+		 * RDMA core will setup GID based on updated netdev.
+		 * It's not possible to race with the core as rtnl lock is being
+		 * held.
+		 */
+		ib_device_set_netdev(&dev->ib_dev, ndev, 1);
+
+		/* mana_get_primary_netdev() returns ndev with refcount held */
+		netdev_put(ndev, NULL);
+
+		return NOTIFY_OK;
+	default:
+		return NOTIFY_DONE;
+	}
+}
+
 static int mana_ib_probe(struct auxiliary_device *adev,
 			 const struct auxiliary_device_id *id)
 {
@@ -108,17 +140,25 @@ static int mana_ib_probe(struct auxiliary_device *adev,
 	}
 	dev->gdma_dev = &mdev->gdma_context->mana_ib;
 
+	dev->nb.notifier_call = mana_ib_netdev_event;
+	ret = register_netdevice_notifier(&dev->nb);
+	if (ret) {
+		ibdev_err(&dev->ib_dev, "Failed to register net notifier, %d",
+			  ret);
+		goto deregister_device;
+	}
+
 	ret = mana_ib_gd_query_adapter_caps(dev);
 	if (ret) {
 		ibdev_err(&dev->ib_dev, "Failed to query device caps, ret %d",
 			  ret);
-		goto deregister_device;
+		goto deregister_net_notifier;
 	}
 
 	ret = mana_ib_create_eqs(dev);
 	if (ret) {
 		ibdev_err(&dev->ib_dev, "Failed to create EQs, ret %d", ret);
-		goto deregister_device;
+		goto deregister_net_notifier;
 	}
 
 	ret = mana_ib_gd_create_rnic_adapter(dev);
@@ -147,6 +187,8 @@ static int mana_ib_probe(struct auxiliary_device *adev,
 	mana_ib_gd_destroy_rnic_adapter(dev);
 destroy_eqs:
 	mana_ib_destroy_eqs(dev);
+deregister_net_notifier:
+	unregister_netdevice_notifier(&dev->nb);
 deregister_device:
 	mana_gd_deregister_device(dev->gdma_dev);
 free_ib_device:
@@ -162,6 +204,7 @@ static void mana_ib_remove(struct auxiliary_device *adev)
 	xa_destroy(&dev->qp_table_wq);
 	mana_ib_gd_destroy_rnic_adapter(dev);
 	mana_ib_destroy_eqs(dev);
+	unregister_netdevice_notifier(&dev->nb);
 	mana_gd_deregister_device(dev->gdma_dev);
 	ib_dealloc_device(&dev->ib_dev);
 }
drivers/infiniband/hw/mana/mana_ib.h +1 -0
index b53a5b4de908..d88187072899 100644
@@ -64,6 +64,7 @@ struct mana_ib_dev {
 	struct gdma_queue **eqs;
 	struct xarray qp_table_wq;
 	struct mana_ib_adapter_caps adapter_caps;
+	struct notifier_block nb;
 };
 
 struct mana_ib_wq {
Signature · 1 line
2.34.1
maintainer
#2 lore raw
In reply to [email protected]

drivers/net/ethernet/microsoft/mana/mana_en.c void mana_remove(struct gdma_dev *gd, bool suspending) · line 3150

Quoted 1 line · On Wed, 5 Mar 2025 14:22:39 -0800 [email protected] wrote:
drivers/net/ethernet/microsoft/mana/mana_en.c +1 -0
+	netdev_hold(ndev, NULL, GFP_ATOMIC);

😅️

I asked you to use this API for the tracker functionality, obviously. Please don't pass NULL in.

#3 lore raw
Quoted 7 lines · On Wed, 5 Mar 2025 14:22:39 -0800 [email protected] wrote:

drivers/net/ethernet/microsoft/mana/mana_en.c void mana_remove(struct gdma_dev *gd, bool suspending) · line 3150

Quoted 1 line
drivers/net/ethernet/microsoft/mana/mana_en.c +1 -0
+	netdev_hold(ndev, NULL, GFP_ATOMIC);

😅️

I asked you to use this API for the tracker functionality, obviously. Please don't pass NULL in.

Is it okay to put the tracker in another device? (it's not the same device as the event netdev where netdev_hold() is used)

Sorry I'm not clear about where to put the tracker.