netdev - Re: [patch iproute2-next v2 3/5] 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]

Date: Wed, 20 Sep 2023 09:30:01 +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 v2 3/5] devlink: introduce support for
 netns id for nested handle

Tue, Sep 19, 2023 at 08:48:29PM CEST, dsahern@...il.com wrote:
>On 9/19/23 11:19 AM, Jiri Pirko wrote:
>>>
>>>>  static void pr_out_nested_handle(struct nlattr *nla_nested_dl)
>>>>  {
>>>>  	struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
>>>> @@ -2740,6 +2776,30 @@ static void pr_out_nested_handle(struct nlattr *nla_nested_dl)
>>>>  	sprintf(buf, "%s/%s", mnl_attr_get_str(tb[DEVLINK_ATTR_BUS_NAME]),
>>>>  		mnl_attr_get_str(tb[DEVLINK_ATTR_DEV_NAME]));
>>>>  	print_string(PRINT_ANY, "nested_devlink", " nested_devlink %s", buf);
>>>> +
>>>> +	if (tb[DEVLINK_ATTR_NETNS_ID]) {
>>>> +		int32_t id = mnl_attr_get_u32(tb[DEVLINK_ATTR_NETNS_ID]);
>>>> +
>>>> +		if (id >= 0) {
>>>> +			char *name = netns_name_by_id(id);
>>>> +
>>>> +			if (name) {
>>>> +				print_string(PRINT_ANY,
>>>> +					     "nested_devlink_netns",
>>>> +					     " nested_devlink_netns %s", name);
>>>> +				free(name);
>>>> +			} else {
>>>> +				print_int(PRINT_ANY,
>>>> +					  "nested_devlink_netnsid",
>>>> +					  " nested_devlink_netnsid %d", id);
>>>> +			}
>>>> +		} else {
>>>> +			print_string(PRINT_FP, NULL,
>>>> +				     " nested_devlink_netnsid %s", "unknown");
>>>> +			print_int(PRINT_JSON,
>>>> +				  "nested_devlink_netnsid", NULL, id);
>>>> +		}
>>> Also, devlink in the name here provides no addititional value (devlink
>>> is the command name) and why add 'nested'? The attribute is just
>>> NETNS_ID, so why not just 'netnsid' here.
>> Well, it is a netnsid of the nested devlink instance, not the object
>> (e.g. port) itself. Omitting that would be misleading. Any idea how to
>> do this differently?
>> 
>> 
>
>The attribute is a namespace id, and the value is a namespace id. Given
>that, the name here should be netnsid (or nsid - we did a horrible job
>with consistency across iproute2 commands). I have not followed the
>kernel patches to understand what you mean by nested devlink instance.

Please do that. Again, the netnsid is related to the nested instance.
Therefore I put the "nested_devlink" in the name. Putting just "netnsid"
as you suggest is wrong. Another possibility would be do nest this into
object, but:
1) I didn't find nice way to do that
2) We would break linecards as they expose nested_devlink already

IDK :/