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

[Jiri Pirko <jiri@resnulli.us>]

Thu, Oct 26, 2023 at 07:08:04PM CEST, dsahern@gmail.com wrote:
>On 10/24/23 4:04 AM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.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@nvidia.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);
>>  }
>>  
>
>