Re: [EXTERNAL] Re: [Patch rdma-next] RDMA/mana_ib: handle net event for pointing to the current netdev

[Jason Gunthorpe <jgg@ziepe.ca>]

On Tue, Mar 04, 2025 at 06:26:03PM +0000, Long Li wrote:
> > On 2025-03-01 at 04:11:59, longli@linuxonhyperv.com
> > (longli@linuxonhyperv.com) wrote:
> > > From: Long Li <longli@microsoft.com>
> > >
> > > When running under Hyper-V, the master device to the RDMA device is
> > > always bonded to this RDMA device if it's present in the kernel. 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 relect the
> > > change of master device from those events.
> > >
> > > Signed-off-by: Long Li <longli@microsoft.com>
> > > ---
> > >  drivers/infiniband/hw/mana/device.c  | 35
> > > ++++++++++++++++++++++++++++  drivers/infiniband/hw/mana/mana_ib.h |
> > > 1 +
> > >  2 files changed, 36 insertions(+)
> > >
> > > diff --git a/drivers/infiniband/hw/mana/device.c
> > > b/drivers/infiniband/hw/mana/device.c
> > > index 3416a85f8738..3e4f069c2258 100644
> > > --- a/drivers/infiniband/hw/mana/device.c
> > > +++ b/drivers/infiniband/hw/mana/device.c
> > > @@ -51,6 +51,37 @@ 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;
> > > +
> > > +     if (event_dev != mc->ports[0])
> > > +             return NOTIFY_DONE;
> > > +
> > > +     switch (event) {
> > > +     case NETDEV_CHANGEUPPER:
> > > +             rcu_read_lock();
> > > +             ndev = mana_get_primary_netdev_rcu(mc, 0);
> > > +             rcu_read_unlock();
> > ...
> > > +
> > > +             /*
> > > +              * 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);
> > rcu_read_unlock() should be here, right ?
> 
> It can't.  ib_device_set_netdev() is calling alloc_port_data() and may sleep.
> 
> I think this locking is okay. This event only comes in when:
> 1. the master device has changed to netvsc. In this case ndev is guaranteed to be valid as this notification is triggered by netvsc.
> 2. the master device has changed to itself (the ethernet device parent for the IB device).  In this case, ndev is valid because mana_ib is an auxiliary device to ndev and it can't unload itself at this time.


Why not return with the netdev refcount held so you don't need this
weirdo rcu thing?

Jason