Message-ID: <ZMAMY0MTj7PbJazi@hog>
Date: Tue, 25 Jul 2023 19:54:43 +0200
From: Sabrina Dubroca <sd@...asysnail.net>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Paolo Abeni <pabeni@...hat.com>, davem@...emloft.net,
	netdev@...r.kernel.org, edumazet@...gle.com, mkubecek@...e.cz,
	lorenzo@...nel.org
Subject: Re: [PATCH net-next 1/2] net: store netdevs in an xarray

2023-07-24, 12:07:18 -0700, Jakub Kicinski wrote:
> On Mon, 24 Jul 2023 10:27:41 -0700 Jakub Kicinski wrote:
> > > I still have some minor doubts WRT the 'missed device' scenario you
> > > described in the commit message. What if the user-space is doing
> > > 'create the new one before deleting the old one' with the assumption
> > > that at least one of old/new is always reported in dumps? Is that a too
> > > bold assumption?  
> > 
> > The problem is kinda theoretical in the first place because it assumes
> > ifindexes got wrapped so that the new netdev comes "before" the old in
> > the xarray. Which would require adding and removing 2B netdevs, assuming
> > one add+remove takes 10 usec (impossibly fast), wrapping ifindex would
> > take 68 years.
> 
> I guess the user space can shoot itself in the foot by selecting 
> the lower index for the new device explicitly.

Or just moving devices between netns?

Scenario 1:
- create lots of devices in netns A, last ifindex is 100 (call it dummy100)
- netns B isn't creating many devices, last ifindex is 8
- move dummy100 from netns A to netns B, it keeps ifindex=100
- create a device in netns B, it gets ifindex=9 even though we already
  have a device with ifindex=100

Scenario 2:
- create lots of devices in netns A, last ifindex is 100
- delete devices with ifindex 10..80 from A
- move a device with ifindex=20 into netns A, it keeps ifindex=20
- we have a new device in netns A with a smaller ifindex than the max


I think with this patch we could still see states that never existed,
if both a low ifindex and then a high ifindex are created during the
dump. We could see high ifindex without low ifindex being listed, if
the dump was already past the low id.  Or if we delete one at a low
ifindex then create one at a high ifindex, we could see both devices
listed in the dump when they never both existed at the same time.


> > And if that's not enough we can make the iteration index ulong 
> > (i.e. something separate from ifindex as ifindex is hardwired to 31b
> > by uAPI).
> 
> We can get the create, delete ordering with this or the list, but the
> inverse theoretical case of delete, create ordering can't be covered.
> A case where user wants to make sure at most one device is visible.
> 
> I'm not sure how much we should care about this. The basic hash table
> had the very real problem of hiding devices which were there *before
> and after* the dump.
> 
> Inconsistent info on devices which were created / deleted *during* the
> dump seems to me like something that's best handled with notifications.
> 
> I'm not sure whether we should set the inconsistency mark on the dump
> when del/add operation happened in the meantime either, as 
> the probability that the user space will care is minuscule.

The inconsistent dump mark may be more relevant for changes in device
properties than link creation/removal. If the MTU on 2 devices changes
while the dump is running (one low ifindex, one high ifindex), we'll
see the old MTU for the first device and the new MTU for the 2nd. Or
by adding/removing bridge ports while the dump runs, I can make it
look like bridge0 has mulitple ports with the same port_no.

I don't know how likely those cases are, but if they happen I think
they'd be more confusing than a missing/extra device.

-- 
Sabrina

Powered by blists - more mailing lists