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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <SA6PR21MB423174BA15D8A909CB2A6950CEC82@SA6PR21MB4231.namprd21.prod.outlook.com>
Date: Tue, 4 Mar 2025 18:26:03 +0000
From: Long Li <longli@...rosoft.com>
To: Ratheesh Kannoth <rkannoth@...vell.com>, "longli@...uxonhyperv.com"
	<longli@...uxonhyperv.com>
CC: Jason Gunthorpe <jgg@...pe.ca>, Leon Romanovsky <leon@...nel.org>,
	Konstantin Taranov <kotaranov@...rosoft.com>, "David S. Miller"
	<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
	<kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>
Subject: RE: [EXTERNAL] Re: [Patch rdma-next] RDMA/mana_ib: handle net event
 for pointing to the current netdev

> On 2025-03-01 at 04:11:59, longli@...uxonhyperv.com
> (longli@...uxonhyperv.com) wrote:
> > From: Long Li <longli@...rosoft.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@...rosoft.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.