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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <f052190e-f81e-491b-aaed-60eaa01fd968@6wind.com>
Date: Fri, 7 Feb 2025 10:10:49 +0100
From: Nicolas Dichtel <nicolas.dichtel@...nd.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: "David S . Miller" <davem@...emloft.net>, Paolo Abeni
 <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
 Alexander Lobakin <aleksander.lobakin@...el.com>, netdev@...r.kernel.org,
 stable@...r.kernel.org
Subject: Re: [PATCH net 1/2] net: advertise 'netns local' property via netlink

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)) ||