netdev - Re: [patch iproute2-next v3 4/6] devlink: introduce support for netns id for nested handle

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZTt05z87/89QXmcC@nanopsycho>
Date: Fri, 27 Oct 2023 10:29:27 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: David Ahern <dsahern@...il.com>
Cc: netdev@...r.kernel.org, stephen@...workplumber.org,
	daniel.machon@...rochip.com
Subject: Re: [patch iproute2-next v3 4/6] devlink: introduce support for
 netns id for nested handle

Thu, Oct 26, 2023 at 07:08:04PM CEST, dsahern@...il.com wrote:
>On 10/24/23 4:04 AM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@...dia.com>
>> 
>> Nested handle may contain DEVLINK_ATTR_NETNS_ID attribute that indicates
>> the network namespace where the nested devlink instance resides. Process
>> this converting to netns name if possible and print to user.
>> 
>> Signed-off-by: Jiri Pirko <jiri@...dia.com>
>> ---
>> v2->v3:
>> - moved netns_name_by_id() into lib/namespace.c
>> - s/netns_name_by_id/netns_name_from_id/
>> - rebased on top of new patch "devlink: extend pr_out_nested_handle() to
>>   print object"
>> v1->v2:
>> - use previously introduced netns_netnsid_from_name() instead of code
>>   duplication for the same function.
>> - s/nesns_name_by_id_func/netns_name_by_id_func/
>> ---
>>  devlink/devlink.c   | 23 ++++++++++++++++++++++-
>>  include/namespace.h |  1 +
>>  lib/namespace.c     | 34 ++++++++++++++++++++++++++++++++++
>>  3 files changed, 57 insertions(+), 1 deletion(-)
>> 
>> diff --git a/devlink/devlink.c b/devlink/devlink.c
>> index f7325477f271..7ba2d0dcac72 100644
>> --- a/devlink/devlink.c
>> +++ b/devlink/devlink.c
>> @@ -24,6 +24,7 @@
>>  #include <linux/genetlink.h>
>>  #include <linux/devlink.h>
>>  #include <linux/netlink.h>
>> +#include <linux/net_namespace.h>
>>  #include <libmnl/libmnl.h>
>>  #include <netinet/ether.h>
>>  #include <sys/select.h>
>> @@ -722,6 +723,7 @@ static const enum mnl_attr_data_type devlink_policy[DEVLINK_ATTR_MAX + 1] = {
>>  	[DEVLINK_ATTR_LINECARD_SUPPORTED_TYPES] = MNL_TYPE_NESTED,
>>  	[DEVLINK_ATTR_NESTED_DEVLINK] = MNL_TYPE_NESTED,
>>  	[DEVLINK_ATTR_SELFTESTS] = MNL_TYPE_NESTED,
>> +	[DEVLINK_ATTR_NETNS_ID] = MNL_TYPE_U32,
>>  };
>>  
>>  static const enum mnl_attr_data_type
>> @@ -2865,7 +2867,26 @@ static void __pr_out_nested_handle(struct dl *dl, struct nlattr *nla_nested_dl,
>>  		return;
>>  	}
>>  
>> -	__pr_out_handle_start(dl, tb, false, false);
>> +	__pr_out_handle_start(dl, tb, tb[DEVLINK_ATTR_NETNS_ID], false);
>> +	if (tb[DEVLINK_ATTR_NETNS_ID]) {
>> +		int32_t id = mnl_attr_get_u32(tb[DEVLINK_ATTR_NETNS_ID]);
>> +
>> +		if (id >= 0) {
>
>why does the kernel side even add DEVLINK_ATTR_NETNS_ID if
>peernet2id_alloc fails?

Yep. So the user knows that this is in a different namespace. This
logic is copied from iplink


>
>
>> +			char *name = netns_name_from_id(id);
>> +
>> +			if (name) {
>> +				print_string(PRINT_ANY, "netns",
>> +					     " netns %s", name);
>> +				free(name);
>> +			} else {
>> +				print_int(PRINT_ANY, "netnsid",
>> +					  " netnsid %d", id);
>> +			}
>> +		} else {
>> +			print_string(PRINT_FP, NULL, " netnsid %s", "unknown");
>> +			print_int(PRINT_JSON, "netnsid", NULL, id);
>
>ie., how is -1 useful to userspace?

The userspace knows that this device is in a different netnamespace, but
it is unknown as peernet2id_alloc failed. Should be really a rare
corner case when system is under memory pressure.


>
>
>> +		}
>> +	}
>>  	pr_out_handle_end(dl);
>>  }
>>  
>
>