netdev - RE: [EXTERNAL] Re: [Patch v2 0/3] IB/core: Fix GID cache for bonded net devices

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] [day] [month] [year] [list]

Message-ID:
 <SA6PR21MB42311935C1955034D9EDFF5ECEFF2@SA6PR21MB4231.namprd21.prod.outlook.com>
Date: Thu, 13 Feb 2025 02:20:23 +0000
From: Long Li <longli@...rosoft.com>
To: Leon Romanovsky <leon@...nel.org>, "longli@...uxonhyperv.com"
	<longli@...uxonhyperv.com>, Stephen Hemminger <stephen@...workplumber.org>
CC: Jason Gunthorpe <jgg@...pe.ca>, Ajay Sharma <sharmaajay@...rosoft.com>,
	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 v2 0/3] IB/core: Fix GID cache for bonded
 net devices

> Subject: [EXTERNAL] Re: [Patch v2 0/3] IB/core: Fix GID cache for bonded net
> devices
> 
> On Fri, Feb 07, 2025 at 01:36:15PM -0800, longli@...uxonhyperv.com wrote:
> > From: Long Li <longli@...rosoft.com>
> >
> > When populating GID cache for net devices in a bonded setup, it should
> > use the master device's address whenever applicable.
> >
> > The current code has some incorrect behaviors when dealing with bonded
> devices:
> > 1. It adds IP of bonded slave to the GID cache when the device is
> > already bonded 2. It adds IP of bonded slave to the GID cache when the
> > device becomes bonded (via NETDEV_CHANGEUPPER notifier) 3. When a
> bonded slave device is unbonded, it doesn't add its IP to the default table in GID
> cache.
> 
> I took a look at the patches and would like to see the reasoning why current
> behaviour is incorrect and need to be changed. In addition, there is a need to add
> examples of what is "broken" now and will start to work after the fixes.
> 
> Thanks

Thanks for looking. I will work on another set of patches based on feedback from:
https://lore.kernel.org/lkml/20250211163735.18d0fd02@hermes.local/

I have some questions on the RDMA GID cache code determining GID cache based on bonded device states. Please see following.

For an IB device, the RDMA GID cache code (rdma_roce_rescan_device() in drivers/infiniband/core/roce_gid_mgmt.c) looks at the following devices for its default GIDs:
1. This IB device if it is not a bonded slave (if this IB device is a slave but not bonded, it will be used for default GIDs)
2. This IB device's bonded master devices
Please see is_ndev_for_default_gid_filter() as its filtering function

And for those devices for this IB device's non-default GIDs:
1. An upper device to this IB device that is not bonded
2. An upper device to this IB device that is bonded to this IB device, and this IB device is the current active bonded slave
3. This IB device if it's not a VLAN type
See is_eth_port_of_netdev_filter() as its filtering function

To summarize, the GID caching behavior for an IB device which is also a slave device, looks like below:
1. It seems all upper devices (bonded or not) to this IB device will be used in the GID cache, but only its bonding master is used for the default GID cache
2. The IB device will not be used in default GID cache if it's bonded
3. The IB device will always be used in non-default GID caches. (assuming it's not VLAN)

My understanding is that the default GID cache will look for bonded master when checking on a slave device and its upper devices. The non-default GID cache just takes this IB device and all its upper devices.

Why the default GID cache needs to check bonded devices?

Thanks,
Long