netdev - Re: [PATCH rdma-next 1/1] RDMA/mana_ib: Set correct device into ib

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]

Date: Thu, 27 Jun 2024 12:57:05 +0300
From: Leon Romanovsky <leon@...nel.org>
To: Haiyang Zhang <haiyangz@...rosoft.com>,
	Konstantin Taranov <kotaranov@...rosoft.com>
Cc: Stephen Hemminger <stephen@...workplumber.org>,
	Konstantin Taranov <kotaranov@...ux.microsoft.com>,
	Wei Hu <weh@...rosoft.com>,
	"sharmaajay@...rosoft.com" <sharmaajay@...rosoft.com>,
	Long Li <longli@...rosoft.com>, "jgg@...pe.ca" <jgg@...pe.ca>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
	linux-netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH rdma-next 1/1] RDMA/mana_ib: Set correct device into ib

On Wed, Jun 26, 2024 at 06:29:02PM +0000, Haiyang Zhang wrote:
> 
> 
> > -----Original Message-----
> > From: Konstantin Taranov <kotaranov@...rosoft.com>
> > Sent: Wednesday, June 26, 2024 2:20 PM
> > To: Stephen Hemminger <stephen@...workplumber.org>; Leon Romanovsky
> > <leon@...nel.org>; Haiyang Zhang <haiyangz@...rosoft.com>
> > Cc: Konstantin Taranov <kotaranov@...ux.microsoft.com>; Wei Hu
> > <weh@...rosoft.com>; sharmaajay@...rosoft.com; Long Li
> > <longli@...rosoft.com>; jgg@...pe.ca; linux-rdma@...r.kernel.org; linux-
> > netdev <netdev@...r.kernel.org>
> > Subject: Re: [PATCH rdma-next 1/1] RDMA/mana_ib: Set correct device into
> > ib
> > 
> > > > > > On Wed, Jun 26, 2024 at 09:05:05AM +0000, Konstantin Taranov
> > wrote:
> > > > > > > > > When mc->ports[0] is not slave, use it in the set_netdev.
> > > > > > > > > When mana is used in netvsc, the stored net devices in mana
> > > > > > > > > are slaves and GIDs should be taken from their master
> > devices.
> > > > > > > > > In the baremetal case, the mc->ports devices will not be
> > slaves.
> > > > > > > >
> > > > > > > > I wonder, why do you have "... | IFF_SLAVE" in
> > > > > > > > __netvsc_vf_setup() in a first place? Isn't IFF_SLAVE is
> > supposed to
> > > be set by bond driver?
> > > > > > > >
> > > > > > >
> > > > > > > I guess it is just a valid use of the IFF_SLAVE bit. In the
> > bond
> > > > > > > case it is also set as a BOND netdev. The IFF_SLAVE helps to
> > show
> > > users that another master
> > > > > > > netdev should be used for networking. But I am not an expert in
> > > netvsc.
> > > > > >
> > > > > > The thing is that netvsc is virtual device like many others, but
> > > > > > it is the only one who uses IFF_SLAVE bit. The comment around
> > that
> > > > > > bit says "slave of a load balancer.", which is not the case
> > > > > > according to the Hyper-V documentation.
> > > > > >
> > > > > > You will need to get Ack from netdev maintainers to rely on
> > > > > > IFF_SLAVE bit in the way you are relying on it now.
> > > > >
> > > > > This is used to tell userspace tools to not interact directly with
> > the device.
> > > > > For example, it is used when VF is connected to netvsc device.
> > > > > It prevents things like IPv6 local address, and Network Manager
> > won't
> > > modify device.
> > > >
> > > > You described how hyper-v uses it, but I'm interested to get
> > > > acknowledgment that it is a valid use case for IFF_SLAVE, despite
> > sentence
> > > written in the comment.
> > >
> > > There is no documented semantics around any of the IF flags, only
> > historical
> > > precedent used by bond, team and bridge drivers. Initially Hyper-V VF
> > used
> > > bonding but it was impossibly difficult to make this work across all
> > versions of
> > > Linux, so transparent VF support was added instead. Ideally, the VF
> > device
> > > could be hidden from userspace but that required more kernel
> > modifications
> > > than would be accepted.
> > 
> > Thanks Stephen for the explanation!
> > 
> > I am also CCing Haiyang, who maintains Hyper-V netvsc.
> > 
> 
> Yes, netvsc sets the IFF_SLAVE on VF for the bonding.
> 
> Acked-by: Haiyang Zhang <haiyangz@...rosoft.com>

Konstantin.

I don't want to be first and only one driver that uses this flag
outside of netdev. So please add new function to netdev part of mana
driver to return right ndev.

Something like that:
struct net_device *mana__get_netdev(struct mana_context *mc)
{
	struct net_device *ndev;

	if (mana_ndev_is_slave(mc->ports[0]))
		ndev = netdev_master_upper_dev_get_rcu(mc->ports[0]);
	else
		ndev = mc->ports[0];

	return ndev;
}

And get Acks from netdev maintainers (Jakub, David, Eric, Paolo).