| Message ID | 20250206165132.2898347-3-nicolas.dichtel@6wind.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: notify users when an iface cannot change its netns | expand |
On Thu, Feb 06, 2025 at 05:50:27PM +0100, Nicolas Dichtel wrote: > The current message is "Invalid argument". Let's help the user by > explaining the error. > > Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> > --- > net/core/rtnetlink.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c > index 5032e65b8faa..91b358bdfe5c 100644 > --- a/net/core/rtnetlink.c > +++ b/net/core/rtnetlink.c > @@ -3024,8 +3024,12 @@ 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); > - if (err) > + if (err) { > + if (dev->netns_local) > + NL_SET_ERR_MSG(extack, > + "The interface has the 'netns local' property"); This seems to have the wrong order. Why even try calling __dev_change_net_namespace() if you know it is going to fail? Maybe this NL_SET_ERR_MSG() should be pushed into __dev_change_net_namespace()? You could then return useful messages if the altnames conflict, the ifindex is already in use, etc. Andrew
Le 07/02/2025 à 00:02, Andrew Lunn a écrit : > On Thu, Feb 06, 2025 at 05:50:27PM +0100, Nicolas Dichtel wrote: >> The current message is "Invalid argument". Let's help the user by >> explaining the error. >> >> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> >> --- >> net/core/rtnetlink.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c >> index 5032e65b8faa..91b358bdfe5c 100644 >> --- a/net/core/rtnetlink.c >> +++ b/net/core/rtnetlink.c >> @@ -3024,8 +3024,12 @@ 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); >> - if (err) >> + if (err) { >> + if (dev->netns_local) >> + NL_SET_ERR_MSG(extack, >> + "The interface has the 'netns local' property"); > > This seems to have the wrong order. Why even try calling > __dev_change_net_namespace() if you know it is going to fail? > > Maybe this NL_SET_ERR_MSG() should be pushed into > __dev_change_net_namespace()? You could then return useful messages if > the altnames conflict, the ifindex is already in use, etc. Users of dev_change_net_namespace() are not netlink users, so I kept the same API. I will plumb the extack into __dev_change_net_namespace(). Thanks, Nicolas
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 5032e65b8faa..91b358bdfe5c 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -3024,8 +3024,12 @@ 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); - if (err) + if (err) { + if (dev->netns_local) + NL_SET_ERR_MSG(extack, + "The interface has the 'netns local' property"); goto errout; + } status |= DO_SETLINK_MODIFIED; }
The current message is "Invalid argument". Let's help the user by explaining the error. Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> --- net/core/rtnetlink.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)