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: Mon, 11 Nov 2024 15:42:26 -0800	[thread overview]
Message-ID: <20241111154226.5d7b29bf@kernel.org> (raw)
In-Reply-To: <CABAhCOQPw0zR1Cn7RoabjA0P-Onmpq_4jLgmtkTYpfZbFAOoGQ@mail.gmail.com>

On Mon, 11 Nov 2024 16:15:41 +0800 Xiao Liang wrote:
> > Let's start with annotating the drivers which need the old behavior.
> > It seems like something that was done as a workaround for old drivers,
> > maybe there isn't that many of them and we can convert them all in one
> > series.  
> 
> I'd like to clarify a bit here.
> Link netns is closely coupled with source netns, as it's passed to drivers
> as source netns. Without introducing a flag, after applying the logic in
> this patchset, drivers won't be able to distinguish the two:
>   1) ip -n ns1 link add netns ns2 ...
>   2) ip link add netns ns2 link-netns ns1 ...

True, but the question is how many drivers actually care about the net
parameter. Ideally we would pass both netns to the drivers, refactor
the ->newlink callback to take a parameter struct and add both netns
as members. Passing just one or the other will always be confusing.

> There's no problem for drivers that already handle source netns.
> But it changes the semantics of 1) for ip tunnels silently. The effective
> link-netns is ns2 before, and will be changed to ns1 afterwards, which will
> almost certainly affect some users. Is this acceptable?

No, changing the behavior for the commands you provided is not
acceptable. At the same time we shouldn't be adding technical debt 
of supporting both converted and unconverted drivers upstream.

> On the other hand, do we need to deal with out-of-tree drivers?

Nope, but again, changing the prototype of newlink would also make it
hard to get wrong for OOT modules.

  reply	other threads:[~2024-11-11 23:42 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
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 [this message]
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=20241111154226.5d7b29bf@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).