| Message ID | 20241023023146.372653-1-shaw.leon@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | net: Improve netns handling in RTNL and ip_tunnel | expand |
On Wed, 23 Oct 2024 10:31:41 +0800 Xiao Liang wrote: > This patch series includes some netns-related improvements and fixes for > RTNL and ip_tunnel, to make link creation more intuitive: > > - Creating link in another net namespace doesn't conflict with link names > in current one. > - Add a flag in rtnl_ops, to avoid netns change when link-netns is present > if possible. > - When creating ip tunnel (e.g. GRE) in another netns, use current as > link-netns if not specified explicitly. > > So that > > # modprobe ip_gre netns_atomic=1 > # ip link add netns ns1 link-netns ns2 tun0 type gre ... Do you think the netns_atomic module param is really necessary? I doubt anyone cares about the event popping up in the wrong name space first. BTW would be good to have tests for this. At least the behavior around name / ifindex collisions in different namespaces. You can possibly extend/re-purpose netns-name.sh for this. For notifications you could use python and subscribe to the events using a YNL socket. May be easier than dealing with ip monitor as a background process. But either way is fine.
On Wed, Oct 30, 2024 at 7:17 AM Jakub Kicinski <kuba@kernel.org> wrote: > > On Wed, 23 Oct 2024 10:31:41 +0800 Xiao Liang wrote: > > This patch series includes some netns-related improvements and fixes for > > RTNL and ip_tunnel, to make link creation more intuitive: > > > > - Creating link in another net namespace doesn't conflict with link names > > in current one. > > - Add a flag in rtnl_ops, to avoid netns change when link-netns is present > > if possible. > > - When creating ip tunnel (e.g. GRE) in another netns, use current as > > link-netns if not specified explicitly. > > > > So that > > > > # modprobe ip_gre netns_atomic=1 > > # ip link add netns ns1 link-netns ns2 tun0 type gre ... > > Do you think the netns_atomic module param is really necessary? > I doubt anyone cares about the event popping up in the wrong > name space first. We used FRRouting in our solution which listens to link notifications to set up corresponding objects in userspace. Since the events are sent in different namespaces (thus via different RTNL sockets), we can't guarantee that the events are received in the correct order, and have trouble processing them. The way to solve this problem I can think of is to have a multi-netns RTNL socket where all events are synchronized, or to eliminate the redundant events in the first place. The latter seems easier to implement. > > BTW would be good to have tests for this. At least the behavior > around name / ifindex collisions in different namespaces. > You can possibly extend/re-purpose netns-name.sh for this. > > For notifications you could use python and subscribe to the events > using a YNL socket. May be easier than dealing with ip monitor > as a background process. But either way is fine. I will try to add some tests. Thanks!
On Wed, 30 Oct 2024 10:10:32 +0800 Xiao Liang wrote: > > Do you think the netns_atomic module param is really necessary? > > I doubt anyone cares about the event popping up in the wrong > > name space first. > > We used FRRouting in our solution which listens to link notifications to > set up corresponding objects in userspace. Since the events are sent > in different namespaces (thus via different RTNL sockets), we can't > guarantee that the events are received in the correct order, and have > trouble processing them. The way to solve this problem I can think of is > to have a multi-netns RTNL socket where all events are synchronized, > or to eliminate the redundant events in the first place. The latter seems > easier to implement. I think we're on the same page. I'm saying that any potential user will run into a similar problem, and I don't see a clear use case for notifications in both namespaces. So we can try to make the behavior of netns_atomic=1 the default one and get rid of the module param.
On Thu, Oct 31, 2024 at 7:35 AM Jakub Kicinski <kuba@kernel.org> wrote: > > On Wed, 30 Oct 2024 10:10:32 +0800 Xiao Liang wrote: > > > Do you think the netns_atomic module param is really necessary? > > > I doubt anyone cares about the event popping up in the wrong > > > name space first. > > > > We used FRRouting in our solution which listens to link notifications to > > set up corresponding objects in userspace. Since the events are sent > > in different namespaces (thus via different RTNL sockets), we can't > > guarantee that the events are received in the correct order, and have > > trouble processing them. The way to solve this problem I can think of is > > to have a multi-netns RTNL socket where all events are synchronized, > > or to eliminate the redundant events in the first place. The latter seems > > easier to implement. > > I think we're on the same page. I'm saying that any potential user > will run into a similar problem, and I don't see a clear use case > for notifications in both namespaces. So we can try to make the > behavior of netns_atomic=1 the default one and get rid of the module > param. Ah.. I misunderstood your question... Besides notifications there's another behavior change. In this patchset, RTNL core passes link-netns as src_net to drivers. But ip tunnel driver doesn't support source netns currently, and that's why we have Patch 4. As a side effect, source netns will be used if link-netns is not specified.For example: ip -n ns1 link add netns ns2 gre1 type gre ... In current implementation, the link-netns is ns2. While with netns_atomic=1, link-netns will be ns1 (source netns). The module param serves as a way to keep the current behavior. I think we can make it default for drivers that already have source netns support.