netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Xiao Liang <shaw.leon@gmail.com>
Cc: Eric Dumazet <edumazet@google.com>,
	netdev@vger.kernel.org, linux-kselftest@vger.kernel.org,
	Kuniyuki Iwashima <kuniyu@amazon.com>,
	"David S. Miller" <davem@davemloft.net>,
	David Ahern <dsahern@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Ido Schimmel <idosch@nvidia.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	Simon Horman <horms@kernel.org>,
	Donald Hunter <donald.hunter@gmail.com>,
	Shuah Khan <shuah@kernel.org>, Jiri Pirko <jiri@resnulli.us>,
	Hangbin Liu <liuhangbin@gmail.com>
Subject: Re: [PATCH net-next v2 5/8] net: ip_gre: Add netns_atomic module parameter
Date: Thu, 7 Nov 2024 20:04:10 -0800	[thread overview]
Message-ID: <20241107200410.4126cf52@kernel.org> (raw)
In-Reply-To: <CABAhCOSvhUZE_FE4xFsOimzVBQpQYLNk51uYNLw+46fibzfM2Q@mail.gmail.com>

On Fri, 8 Nov 2024 00:53:55 +0800 Xiao Liang wrote:
> > > It is to control driver behavior at rtnl_ops registration time. I
> > > think rtnetlink
> > > attributes are too late for that, maybe? Can't think of a way other than
> > > module parameters or register separate ops. Any suggestions?  
> >
> > Step back from the implementation you have a little, forget that there
> > is a boolean in rtnl_link_ops. User makes a request to spawn an
> > interface, surely a flag inside that request can dictate how the netns
> > attrs are interpreted.  
> 
> IMO, this is about driver capability, not about user requests.

The bit is a driver capability, that's fine. But the question was how
to achieve backward compatibility. A flag in user request shifts the
responsibility of ensuring all services are compatible to whoever
spawns the interfaces. Which will probably be some network management
daemon.

> As you've pointed out earlier, probably no one would actually want
> the old behavior whenever the driver supports the new one.
> I added the module parameter just for compatibility, because ip_tunnels
> was not implemented to support src_net properly.

And I maintain that it's very unlikely anyone cares about old behavior.
So maybe as a starting point we can have neither the flag nor the
module param? We can add them later if someone screams.

> Yes it's possible to add an extra flag in user request, but I don't
> think it's a good approach.

There are two maintainers with opposing intuition so more data may be
needed to convince..

> BTW, I didn't find what's going on with module parameters, is there
> any documentation?

Not sure if there is documentation, but module params are quite painful
to work with. Main reason is that they are global and not namespace
aware. Plus developers usually default to making them read only, which
means they practically speaking have to be configured at boot.

  reply	other threads:[~2024-11-08  4:04 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-07 13:29 [PATCH net-next v2 0/8] net: Improve netns handling in RTNL and ip_tunnel Xiao Liang
2024-11-07 13:29 ` [PATCH net-next v2 1/8] rtnetlink: Lookup device in target netns when creating link Xiao Liang
2024-11-07 13:29 ` [PATCH net-next v2 2/8] rtnetlink: Add netns_atomic flag in rtnl_link_ops Xiao Liang
2024-11-07 13:29 ` [PATCH net-next v2 3/8] net: ip_tunnel: Build flow in underlay net namespace Xiao Liang
2024-11-07 13:29 ` [PATCH net-next v2 4/8] net: ip_tunnel: Add source netns support for newlink Xiao Liang
2024-11-07 13:30 ` [PATCH net-next v2 5/8] net: ip_gre: Add netns_atomic module parameter Xiao Liang
2024-11-07 13:37   ` Eric Dumazet
2024-11-07 14:11     ` Xiao Liang
2024-11-07 15:59       ` Jakub Kicinski
2024-11-07 16:53         ` Xiao Liang
2024-11-08  4:04           ` Jakub Kicinski [this message]
2024-11-08  6:44             ` Xiao Liang
2024-11-10  0:59               ` Jakub Kicinski
2024-11-11  8:15                 ` Xiao Liang
2024-11-11 23:42                   ` Jakub Kicinski
2024-11-12  6:05                     ` Xiao Liang
2024-11-07 13:30 ` [PATCH net-next v2 6/8] net: dummy: Set netns_atomic in rtnl ops for testing Xiao Liang
2024-11-07 13:30 ` [PATCH net-next v2 7/8] tools/net/ynl: Add retry limit for async notification Xiao Liang
2024-11-07 16:04   ` Jakub Kicinski
2024-11-07 17:16     ` Donald Hunter
2024-11-08  8:45       ` Xiao Liang
2024-11-08 10:04         ` Donald Hunter
2024-11-08 12:00           ` Donald Hunter
2024-11-07 13:30 ` [PATCH net-next v2 8/8] selftests: net: Add two test cases for link netns Xiao Liang
2024-11-10  1:01   ` Jakub Kicinski
2024-11-11  8:16     ` Xiao Liang

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=20241107200410.4126cf52@kernel.org \
    --to=kuba@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=donald.hunter@gmail.com \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=idosch@nvidia.com \
    --cc=jiri@resnulli.us \
    --cc=kuniyu@amazon.com \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=liuhangbin@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=shaw.leon@gmail.com \
    --cc=shuah@kernel.org \
    /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).