* [PATCH net 1/2] net: advertise 'netns local' property via netlink [not found] <20250206165132.2898347-1-nicolas.dichtel@6wind.com> @ 2025-02-06 16:50 ` Nicolas Dichtel 2025-02-06 16:59 ` Eric Dumazet 2025-02-06 23:39 ` Jakub Kicinski 0 siblings, 2 replies; 6+ messages in thread From: Nicolas Dichtel @ 2025-02-06 16:50 UTC (permalink / raw) To: David S . Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet, Alexander Lobakin Cc: netdev, Nicolas Dichtel, stable Since the below commit, 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. CC: stable@vger.kernel.org Fixes: 05c1280a2bcf ("netdev_features: convert NETIF_F_NETNS_LOCAL to dev->netns_local") Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> --- include/uapi/linux/if_link.h | 1 + net/core/rtnetlink.c | 3 +++ 2 files changed, 4 insertions(+) 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 d1e559fce918..5032e65b8faa 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -1287,6 +1287,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 */ @@ -2041,6 +2042,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)) || @@ -2229,6 +2231,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_U8 }, }; static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = { -- 2.47.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] net: advertise 'netns local' property via netlink 2025-02-06 16:50 ` [PATCH net 1/2] net: advertise 'netns local' property via netlink Nicolas Dichtel @ 2025-02-06 16:59 ` Eric Dumazet 2025-02-06 17:11 ` Ido Schimmel 2025-02-06 23:39 ` Jakub Kicinski 1 sibling, 1 reply; 6+ messages in thread From: Eric Dumazet @ 2025-02-06 16:59 UTC (permalink / raw) To: Nicolas Dichtel Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Alexander Lobakin, netdev, stable On Thu, Feb 6, 2025 at 5:51 PM Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote: > > Since the below commit, 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. > > CC: stable@vger.kernel.org > Fixes: 05c1280a2bcf ("netdev_features: convert NETIF_F_NETNS_LOCAL to dev->netns_local") > Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> > --- > include/uapi/linux/if_link.h | 1 + > net/core/rtnetlink.c | 3 +++ > 2 files changed, 4 insertions(+) > > 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 d1e559fce918..5032e65b8faa 100644 > --- a/net/core/rtnetlink.c > +++ b/net/core/rtnetlink.c > @@ -1287,6 +1287,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 */ > @@ -2041,6 +2042,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)) || > @@ -2229,6 +2231,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_U8 }, As this is a read-only attribute, I would suggest NLA_REJECT ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] net: advertise 'netns local' property via netlink 2025-02-06 16:59 ` Eric Dumazet @ 2025-02-06 17:11 ` Ido Schimmel 0 siblings, 0 replies; 6+ messages in thread From: Ido Schimmel @ 2025-02-06 17:11 UTC (permalink / raw) To: Eric Dumazet, nicolas.dichtel Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Alexander Lobakin, netdev, stable On Thu, Feb 06, 2025 at 05:59:03PM +0100, Eric Dumazet wrote: > On Thu, Feb 6, 2025 at 5:51 PM Nicolas Dichtel > > @@ -2229,6 +2231,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_U8 }, > > As this is a read-only attribute, I would suggest NLA_REJECT And please update the spec: Documentation/netlink/specs/rt_link.yaml ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] net: advertise 'netns local' property via netlink 2025-02-06 16:50 ` [PATCH net 1/2] net: advertise 'netns local' property via netlink Nicolas Dichtel 2025-02-06 16:59 ` Eric Dumazet @ 2025-02-06 23:39 ` Jakub Kicinski 2025-02-07 9:10 ` Nicolas Dichtel 1 sibling, 1 reply; 6+ messages in thread From: Jakub Kicinski @ 2025-02-06 23:39 UTC (permalink / raw) To: Nicolas Dichtel Cc: David S . Miller, Paolo Abeni, Eric Dumazet, Alexander Lobakin, netdev, stable On Thu, 6 Feb 2025 17:50:26 +0100 Nicolas Dichtel wrote: > Since the below commit, 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. I think the motivation for the change may be worth elaborating on. It's a bit unclear to me what user space would care about this information, a bit of a "story" on how you hit the issue could be useful perhaps? The uAPI is new but the stable tag indicates regression.. > @@ -2041,6 +2042,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) || Maybe nla_put_flag() ? Or do you really care about false being there? The 3 bytes wasted on padding always makes me question when people pick NLA_u8. > 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)) || ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] net: advertise 'netns local' property via netlink 2025-02-06 23:39 ` Jakub Kicinski @ 2025-02-07 9:10 ` Nicolas Dichtel 2025-02-07 19:35 ` Jakub Kicinski 0 siblings, 1 reply; 6+ messages in thread From: Nicolas Dichtel @ 2025-02-07 9:10 UTC (permalink / raw) To: Jakub Kicinski Cc: David S . Miller, Paolo Abeni, Eric Dumazet, Alexander Lobakin, netdev, stable Le 07/02/2025 à 00:39, Jakub Kicinski a écrit : > On Thu, 6 Feb 2025 17:50:26 +0100 Nicolas Dichtel wrote: >> Since the below commit, 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. > > I think the motivation for the change may be worth elaborating on. > It's a bit unclear to me what user space would care about this > information, a bit of a "story" on how you hit the issue could > be useful perhaps? The uAPI is new but the stable tag indicates > regression.. To make it short: we were trying a new NIC with a custom distro provided by a vendor (with out of tree drivers). We were unable to move the interface in another netns. Thanks to ethtool we were able to confirm that the 'netns-local' flag was set. Having this information helps debugging. > >> @@ -2041,6 +2042,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) || > > Maybe nla_put_flag() ? Or do you really care about false being there? It depends if the commit is backported or not. If it won't be backported, having the false value helps to know that the kernel support this attribute (and so that the property is not set). FWIW, I will be off for one week, I will come back to this later. > The 3 bytes wasted on padding always makes me question when people pick > NLA_u8. > >> 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)) || ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] net: advertise 'netns local' property via netlink 2025-02-07 9:10 ` Nicolas Dichtel @ 2025-02-07 19:35 ` Jakub Kicinski 0 siblings, 0 replies; 6+ messages in thread From: Jakub Kicinski @ 2025-02-07 19:35 UTC (permalink / raw) To: Nicolas Dichtel Cc: David S . Miller, Paolo Abeni, Eric Dumazet, Alexander Lobakin, netdev, stable On Fri, 7 Feb 2025 10:10:49 +0100 Nicolas Dichtel wrote: > Le 07/02/2025 à 00:39, Jakub Kicinski a écrit : > > On Thu, 6 Feb 2025 17:50:26 +0100 Nicolas Dichtel wrote: > >> Since the below commit, 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. > > > > I think the motivation for the change may be worth elaborating on. > > It's a bit unclear to me what user space would care about this > > information, a bit of a "story" on how you hit the issue could > > be useful perhaps? The uAPI is new but the stable tag indicates > > regression.. > To make it short: we were trying a new NIC with a custom distro provided by a > vendor (with out of tree drivers). We were unable to move the interface in > another netns. Thanks to ethtool we were able to confirm that the 'netns-local' > flag was set. Having this information helps debugging. Thanks, makes sense. Still a bit unsure if this is a stable candidate, if you don't mind net-next that'd be my preference. If you do mind, I'll live with it :) > >> @@ -2041,6 +2042,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) || > > > > Maybe nla_put_flag() ? Or do you really care about false being there? > It depends if the commit is backported or not. If it won't be backported, having > the false value helps to know that the kernel support this attribute (and so > that the property is not set). Wish we had a good solution for this, it's always the argument against flags :( ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-02-07 19:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20250206165132.2898347-1-nicolas.dichtel@6wind.com>
2025-02-06 16:50 ` [PATCH net 1/2] net: advertise 'netns local' property via netlink Nicolas Dichtel
2025-02-06 16:59 ` Eric Dumazet
2025-02-06 17:11 ` Ido Schimmel
2025-02-06 23:39 ` Jakub Kicinski
2025-02-07 9:10 ` Nicolas Dichtel
2025-02-07 19:35 ` Jakub Kicinski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox