linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Roman Gushchin <roman.gushchin@linux.dev>
Cc: Andrew Lunn <andrew@lunn.ch>, Andy Ren <andy.ren@getcruise.com>,
	netdev@vger.kernel.org, richardbgobert@gmail.com,
	davem@davemloft.net, wsa+renesas@sang-engineering.com,
	edumazet@google.com, petrm@nvidia.com, pabeni@redhat.com,
	corbet@lwn.net, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, David Ahern <dsahern@gmail.com>,
	Stephen Hemminger <sthemmin@microsoft.com>,
	Ido Schimmel <idosch@idosch.org>
Subject: Re: [PATCH net-next v2] netconsole: Enable live renaming for network interfaces used by netconsole
Date: Wed, 2 Nov 2022 12:54:18 -0700	[thread overview]
Message-ID: <20221102125418.272c4381@kernel.org> (raw)
In-Reply-To: <Y2KlfhfijyNl8yxT@P9FQF9L96D.corp.robot.car>

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.

  reply	other threads:[~2022-11-02 19:55 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-02  0:24 [PATCH net-next v2] netconsole: Enable live renaming for network interfaces used by netconsole Andy Ren
2022-11-02  0:48 ` Andrew Lunn
2022-11-02  3:40   ` Jakub Kicinski
2022-11-02 17:14     ` Roman Gushchin
2022-11-02 19:54       ` Jakub Kicinski [this message]
2022-11-03  0:08         ` Roman Gushchin
2022-11-03 17:52         ` Ido Schimmel
2022-11-03 22:38           ` David Ahern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221102125418.272c4381@kernel.org \
    --to=kuba@kernel.org \
    --cc=andrew@lunn.ch \
    --cc=andy.ren@getcruise.com \
    --cc=corbet@lwn.net \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=edumazet@google.com \
    --cc=idosch@idosch.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=petrm@nvidia.com \
    --cc=richardbgobert@gmail.com \
    --cc=roman.gushchin@linux.dev \
    --cc=sthemmin@microsoft.com \
    --cc=wsa+renesas@sang-engineering.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).