On Wed, Nov 02, 2022 at 12:54:18PM -0700, Jakub Kicinski wrote: > On Wed, 2 Nov 2022 10:14:38 -0700 Roman Gushchin wrote: > > > Agreed. BTW I wonder if we really want to introduce a netconsole > > > specific uAPI for this or go ahead with something more general. > > > > Netconsole is a bit special because it brings an interface up very early. > > E.g. in our case without the netconsole the renaming is happening before > > the interface is brought up. > > > > I wonder if the netconsole-specific flag should allow renaming only once. > > > > > A sysctl for global "allow UP rename"? > > > > This will work for us, but I've no idea what it will break for other users > > and how to check it without actually trying to break :) And likely we won't > > learn about it for quite some time, asssuming they don't run net-next. > > Then again IFF_LIVE_RENAME_OK was added in 5.2 so quite a while back. > > > > We added the live renaming for failover a while back and there were > > > no reports of user space breaking as far as I know. So perhaps nobody > > > actually cares and we should allow renaming all interfaces while UP? > > > For backwards compat we can add a sysctl as mentioned or a rtnetlink > > > "I know what I'm doing" flag? > > > > > > Maybe print an info message into the logs for a few releases to aid > > > debug? > > > > > > IOW either there is a reason we don't allow rename while up, and > > > netconsole being bound to an interface is immaterial. Or there is > > > no reason and we should allow all. > > > > My understanding is that it's not an issue for the kernel, but might be > > an issue for some userspace apps which do not expect it. > > There are in-kernel notifier users which could cache the name on up / > down. But yes, the user space is the real worry. > > > If you prefer to go with the 'global sysctl' approach, how the path forward > > should look like? > > That's the question. The sysctl would really just be to cover our back > sides, and be able to tell the users "you opted in by setting that > sysctl, we didn't break backward compat". But practically speaking, > its a different entity that'd be flipping the sysctl (e.g. management > daemon) and different entity that'd be suffering (e.g. routing daemon). > So the sysctl doesn't actually help anyone :/ > > So maybe we should just risk it and wonder about workarounds once > complains surface, if they do. Like generate fake down/up events. > Or create some form of "don't allow live renames now" lock-like > thing a process could take. > > Adding a couple more CCs and if nobody screams at us I vote we just > remove the restriction instead of special casing. Tried looking at history.git to understand the reasoning behind this restriction. I guess it's because back then it was only possible via IOCTL and user space wouldn't be notified about such a change. Nowadays user space gets a notification regardless of the administrative state of the netdev (see rtnetlink_event()). At least in-kernel listeners to NETDEV_CHANGENAME do not seem to care if the netdev is administratively up or not. So, FWIW, the suggested approach sounds sane to me.
