[PATCH net-next v3 0/2] net: notify users when an iface cannot change its netns

[PATCH net-next v3 0/2] net: notify users when an iface cannot change its netns

[Nicolas Dichtel]

This series adds a way to see if an interface cannot be moved to another netns.

v2 -> v3:
 - don't copy patch #1 to stable@vger.kernel.org
 - remove the 'Fixes:' tag

v1 -> v2:
 - target the series to net-next
 - patch #1:
   + use NLA_REJECT for the nl policy
   + update the rt link spec
 - patch #2: plumb extack in __dev_change_net_namespace()

 Documentation/netlink/specs/rt_link.yaml |  3 +++
 include/linux/netdevice.h                |  5 ++--
 include/uapi/linux/if_link.h             |  1 +
 net/core/dev.c                           | 41 +++++++++++++++++++++++++-------
 net/core/rtnetlink.c                     |  5 +++-
 5 files changed, 44 insertions(+), 11 deletions(-)

Comments are welcome.

Regards,
Nicolas

[PATCH net-next v3 1/2] net: advertise 'netns local' property via netlink

[Nicolas Dichtel]

Since commit 05c1280a2bcf ("netdev_features: convert NETIF_F_NETNS_LOCAL to
dev->netns_local"), there is no way to see if the netns_local property is
set on a device. Let's add a netlink attribute to advertise it.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 Documentation/netlink/specs/rt_link.yaml | 3 +++
 include/uapi/linux/if_link.h             | 1 +
 net/core/rtnetlink.c                     | 3 +++
 3 files changed, 7 insertions(+)

diff --git a/Documentation/netlink/specs/rt_link.yaml b/Documentation/netlink/specs/rt_link.yaml
index 0d492500c7e5..a646d8a6bf9d 100644
--- a/Documentation/netlink/specs/rt_link.yaml
+++ b/Documentation/netlink/specs/rt_link.yaml
@@ -1148,6 +1148,9 @@ attribute-sets:
         name: max-pacing-offload-horizon
         type: uint
         doc: EDT offload horizon supported by the device (in nsec).
+      -
+        name: netns-local
+        type: u8
   -
     name: af-spec-attrs
     attributes:
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index bfe880fbbb24..ed4a64e1c8f1 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -378,6 +378,7 @@ enum {
 	IFLA_GRO_IPV4_MAX_SIZE,
 	IFLA_DPLL_PIN,
 	IFLA_MAX_PACING_OFFLOAD_HORIZON,
+	IFLA_NETNS_LOCAL,
 	__IFLA_MAX
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index abe1a461ea67..acf787e4d22d 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1292,6 +1292,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
 	       + nla_total_size(4) /* IFLA_TSO_MAX_SEGS */
 	       + nla_total_size(1) /* IFLA_OPERSTATE */
 	       + nla_total_size(1) /* IFLA_LINKMODE */
+	       + nla_total_size(1) /* IFLA_NETNS_LOCAL */
 	       + nla_total_size(4) /* IFLA_CARRIER_CHANGES */
 	       + nla_total_size(4) /* IFLA_LINK_NETNSID */
 	       + nla_total_size(4) /* IFLA_GROUP */
@@ -2046,6 +2047,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
 		       netif_running(dev) ? READ_ONCE(dev->operstate) :
 					    IF_OPER_DOWN) ||
 	    nla_put_u8(skb, IFLA_LINKMODE, READ_ONCE(dev->link_mode)) ||
+	    nla_put_u8(skb, IFLA_NETNS_LOCAL, dev->netns_local) ||
 	    nla_put_u32(skb, IFLA_MTU, READ_ONCE(dev->mtu)) ||
 	    nla_put_u32(skb, IFLA_MIN_MTU, READ_ONCE(dev->min_mtu)) ||
 	    nla_put_u32(skb, IFLA_MAX_MTU, READ_ONCE(dev->max_mtu)) ||
@@ -2234,6 +2236,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
 	[IFLA_ALLMULTI]		= { .type = NLA_REJECT },
 	[IFLA_GSO_IPV4_MAX_SIZE]	= NLA_POLICY_MIN(NLA_U32, MAX_TCP_HEADER + 1),
 	[IFLA_GRO_IPV4_MAX_SIZE]	= { .type = NLA_U32 },
+	[IFLA_NETNS_LOCAL]	= { .type = NLA_REJECT },
 };
 
 static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
-- 
2.47.1

Re: [PATCH net-next v3 1/2] net: advertise 'netns local' property via netlink

[Eric Dumazet]

On Thu, Feb 20, 2025 at 2:03 PM Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
>
> Since commit 05c1280a2bcf ("netdev_features: convert NETIF_F_NETNS_LOCAL to
> dev->netns_local"), there is no way to see if the netns_local property is
> set on a device. Let's add a netlink attribute to advertise it.
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH net-next v3 1/2] net: advertise 'netns local' property via netlink

[Alexander Lobakin]

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Thu, 20 Feb 2025 14:02:35 +0100

> Since commit 05c1280a2bcf ("netdev_features: convert NETIF_F_NETNS_LOCAL to
> dev->netns_local"), there is no way to see if the netns_local property is
> set on a device. Let's add a netlink attribute to advertise it.
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Meh, I was sure nobody looks at those read-only Ethtool "features"...

Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>

> ---
>  Documentation/netlink/specs/rt_link.yaml | 3 +++
>  include/uapi/linux/if_link.h             | 1 +
>  net/core/rtnetlink.c                     | 3 +++
>  3 files changed, 7 insertions(+)

Thanks,
Olek

Re: [PATCH net-next v3 1/2] net: advertise 'netns local' property via netlink

[Kuniyuki Iwashima]

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Thu, 20 Feb 2025 14:02:35 +0100
> Since commit 05c1280a2bcf ("netdev_features: convert NETIF_F_NETNS_LOCAL to
> dev->netns_local"), there is no way to see if the netns_local property is
> set on a device. Let's add a netlink attribute to advertise it.
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>

[PATCH net-next v3 2/2] net: plumb extack in __dev_change_net_namespace()

[Nicolas Dichtel]

It could be hard to understand why the netlink command fails. For example,
if dev->netns_local is set, the error is "Invalid argument".

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/linux/netdevice.h |  5 +++--
 net/core/dev.c            | 41 +++++++++++++++++++++++++++++++--------
 net/core/rtnetlink.c      |  2 +-
 3 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index fccc03cd2164..58d9f052f154 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4155,12 +4155,13 @@ int dev_change_flags(struct net_device *dev, unsigned int flags,
 int dev_set_alias(struct net_device *, const char *, size_t);
 int dev_get_alias(const struct net_device *, char *, size_t);
 int __dev_change_net_namespace(struct net_device *dev, struct net *net,
-			       const char *pat, int new_ifindex);
+			       const char *pat, int new_ifindex,
+			       struct netlink_ext_ack *extack);
 static inline
 int dev_change_net_namespace(struct net_device *dev, struct net *net,
 			     const char *pat)
 {
-	return __dev_change_net_namespace(dev, net, pat, 0);
+	return __dev_change_net_namespace(dev, net, pat, 0, NULL);
 }
 int __dev_set_mtu(struct net_device *, int);
 int dev_set_mtu(struct net_device *, int);
diff --git a/net/core/dev.c b/net/core/dev.c
index ebc000b56828..9605fa2e7415 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -11917,6 +11917,7 @@ EXPORT_SYMBOL(unregister_netdev);
  *	      is already taken in the destination network namespace.
  *	@new_ifindex: If not zero, specifies device index in the target
  *	              namespace.
+ *	@extack: netlink extended ack
  *
  *	This function shuts down a device interface and moves it
  *	to a new network namespace. On success 0 is returned, on
@@ -11926,7 +11927,8 @@ EXPORT_SYMBOL(unregister_netdev);
  */
 
 int __dev_change_net_namespace(struct net_device *dev, struct net *net,
-			       const char *pat, int new_ifindex)
+			       const char *pat, int new_ifindex,
+			       struct netlink_ext_ack *extack)
 {
 	struct netdev_name_node *name_node;
 	struct net *net_old = dev_net(dev);
@@ -11937,12 +11939,17 @@ int __dev_change_net_namespace(struct net_device *dev, struct net *net,
 
 	/* Don't allow namespace local devices to be moved. */
 	err = -EINVAL;
-	if (dev->netns_local)
+	if (dev->netns_local) {
+		NL_SET_ERR_MSG(extack,
+			       "The interface has the 'netns local' property");
 		goto out;
+	}
 
 	/* Ensure the device has been registered */
-	if (dev->reg_state != NETREG_REGISTERED)
+	if (dev->reg_state != NETREG_REGISTERED) {
+		NL_SET_ERR_MSG(extack, "The interface isn't registered");
 		goto out;
+	}
 
 	/* Get out if there is nothing todo */
 	err = 0;
@@ -11955,30 +11962,48 @@ int __dev_change_net_namespace(struct net_device *dev, struct net *net,
 	err = -EEXIST;
 	if (netdev_name_in_use(net, dev->name)) {
 		/* We get here if we can't use the current device name */
-		if (!pat)
+		if (!pat) {
+			NL_SET_ERR_MSG(extack,
+				       "An interface with the same name exists in the target netns");
 			goto out;
+		}
 		err = dev_prep_valid_name(net, dev, pat, new_name, EEXIST);
-		if (err < 0)
+		if (err < 0) {
+			NL_SET_ERR_MSG_FMT(extack,
+					   "Unable to use '%s' for the new interface name",
+					   pat);
 			goto out;
+		}
 	}
 	/* Check that none of the altnames conflicts. */
 	err = -EEXIST;
 	netdev_for_each_altname(dev, name_node)
-		if (netdev_name_in_use(net, name_node->name))
+		if (netdev_name_in_use(net, name_node->name)) {
+			NL_SET_ERR_MSG_FMT(extack,
+					   "An interface with the altname %s exists in the target netns",
+					   name_node->name);
 			goto out;
+		}
 
 	/* Check that new_ifindex isn't used yet. */
 	if (new_ifindex) {
 		err = dev_index_reserve(net, new_ifindex);
-		if (err < 0)
+		if (err < 0) {
+			NL_SET_ERR_MSG_FMT(extack,
+					   "The ifindex %d is not available in the target netns",
+					   new_ifindex);
 			goto out;
+		}
 	} else {
 		/* If there is an ifindex conflict assign a new one */
 		err = dev_index_reserve(net, dev->ifindex);
 		if (err == -EBUSY)
 			err = dev_index_reserve(net, 0);
-		if (err < 0)
+		if (err < 0) {
+			NL_SET_ERR_MSG(extack,
+				       "Unable to allocate a new ifindex in the target netns");
 			goto out;
+		}
 		new_ifindex = err;
 	}
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index acf787e4d22d..717f2e3e333e 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3028,7 +3028,7 @@ static int do_setlink(const struct sk_buff *skb, struct net_device *dev,
 
 		new_ifindex = nla_get_s32_default(tb[IFLA_NEW_IFINDEX], 0);
 
-		err = __dev_change_net_namespace(dev, tgt_net, pat, new_ifindex);
+		err = __dev_change_net_namespace(dev, tgt_net, pat, new_ifindex, extack);
 		if (err)
 			goto errout;
 
-- 
2.47.1

Re: [PATCH net-next v3 2/2] net: plumb extack in __dev_change_net_namespace()

[Eric Dumazet]

On Thu, Feb 20, 2025 at 2:03 PM Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
>
> It could be hard to understand why the netlink command fails. For example,
> if dev->netns_local is set, the error is "Invalid argument".
>

After your patch, a new message is : "  "The interface has the 'netns
local' property""

Honestly, I am not sure we export to user space the concept of 'netns local'

"This interface netns is not allowed to be changed" or something like that ?

Re: [PATCH net-next v3 2/2] net: plumb extack in __dev_change_net_namespace()

[Nicolas Dichtel]

Le 20/02/2025 à 14:17, Eric Dumazet a écrit :
> On Thu, Feb 20, 2025 at 2:03 PM Nicolas Dichtel
> <nicolas.dichtel@6wind.com> wrote:
>>
>> It could be hard to understand why the netlink command fails. For example,
>> if dev->netns_local is set, the error is "Invalid argument".
>>
> 
> After your patch, a new message is : "  "The interface has the 'netns
> local' property""
> 
> Honestly, I am not sure we export to user space the concept of 'netns local'
> 
> "This interface netns is not allowed to be changed" or something like that ?
Frankly, I was hesitating. I used 'netns local' to ease the link with the new
netlink attribute, and with what was displayed by ethtool for a long time.
I don't have a strong opinion about this.

Re: [PATCH net-next v3 2/2] net: plumb extack in __dev_change_net_namespace()

[Eric Dumazet]

On Thu, Feb 20, 2025 at 2:22 PM Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
>
> Le 20/02/2025 à 14:17, Eric Dumazet a écrit :
> > On Thu, Feb 20, 2025 at 2:03 PM Nicolas Dichtel
> > <nicolas.dichtel@6wind.com> wrote:
> >>
> >> It could be hard to understand why the netlink command fails. For example,
> >> if dev->netns_local is set, the error is "Invalid argument".
> >>
> >
> > After your patch, a new message is : "  "The interface has the 'netns
> > local' property""
> >
> > Honestly, I am not sure we export to user space the concept of 'netns local'
> >
> > "This interface netns is not allowed to be changed" or something like that ?
> Frankly, I was hesitating. I used 'netns local' to ease the link with the new
> netlink attribute, and with what was displayed by ethtool for a long time.
> I don't have a strong opinion about this.

No strong opinion either, I always have been confused by NETNS_LOCAL choice.

Reviewed-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH net-next v3 2/2] net: plumb extack in __dev_change_net_namespace()

[Nicolas Dichtel]

Le 20/02/2025 à 14:24, Eric Dumazet a écrit :
> On Thu, Feb 20, 2025 at 2:22 PM Nicolas Dichtel
> <nicolas.dichtel@6wind.com> wrote:
>>
>> Le 20/02/2025 à 14:17, Eric Dumazet a écrit :
>>> On Thu, Feb 20, 2025 at 2:03 PM Nicolas Dichtel
>>> <nicolas.dichtel@6wind.com> wrote:
>>>>
>>>> It could be hard to understand why the netlink command fails. For example,
>>>> if dev->netns_local is set, the error is "Invalid argument".
>>>>
>>>
>>> After your patch, a new message is : "  "The interface has the 'netns
>>> local' property""
>>>
>>> Honestly, I am not sure we export to user space the concept of 'netns local'
>>>
>>> "This interface netns is not allowed to be changed" or something like that ?
>> Frankly, I was hesitating. I used 'netns local' to ease the link with the new
>> netlink attribute, and with what was displayed by ethtool for a long time.
>> I don't have a strong opinion about this.
> 
> No strong opinion either, I always have been confused by NETNS_LOCAL choice.
Yes, it's not obvious. Maybe it could be renamed before exposing it to userspace
via netlink.
What about 'netns-locked'? Does someone have a better proposal?

Re: [PATCH net-next v3 2/2] net: plumb extack in __dev_change_net_namespace()

[Kuniyuki Iwashima]

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Thu, 20 Feb 2025 16:11:43 +0100
> Le 20/02/2025 à 14:24, Eric Dumazet a écrit :
> > On Thu, Feb 20, 2025 at 2:22 PM Nicolas Dichtel
> > <nicolas.dichtel@6wind.com> wrote:
> >>
> >> Le 20/02/2025 à 14:17, Eric Dumazet a écrit :
> >>> On Thu, Feb 20, 2025 at 2:03 PM Nicolas Dichtel
> >>> <nicolas.dichtel@6wind.com> wrote:
> >>>>
> >>>> It could be hard to understand why the netlink command fails. For example,
> >>>> if dev->netns_local is set, the error is "Invalid argument".
> >>>>
> >>>
> >>> After your patch, a new message is : "  "The interface has the 'netns
> >>> local' property""
> >>>
> >>> Honestly, I am not sure we export to user space the concept of 'netns local'
> >>>
> >>> "This interface netns is not allowed to be changed" or something like that ?
> >> Frankly, I was hesitating. I used 'netns local' to ease the link with the new
> >> netlink attribute, and with what was displayed by ethtool for a long time.
> >> I don't have a strong opinion about this.
> > 
> > No strong opinion either, I always have been confused by NETNS_LOCAL choice.
> Yes, it's not obvious. Maybe it could be renamed before exposing it to userspace
> via netlink.
> What about 'netns-locked'? Does someone have a better proposal?

Maybe NETNS_IMMUTABLE and netns-immutable ?
Then we can say "The interface netns is immutable" in extack.