[patch net-next] net: devlink: expose the info about version representing a component

[patch net-next] net: devlink: expose the info about version representing a component

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

If certain version exposed by a driver is marked to be representing a
component, expose this info to the user.

Example:
$ devlink dev info
netdevsim/netdevsim10:
  driver netdevsim
  versions:
      running:
        fw.mgmt 10.20.30
      flash_components:
        fw.mgmt

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 include/uapi/linux/devlink.h | 2 ++
 net/core/devlink.c           | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 2f24b53a87a5..7f2874189188 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -607,6 +607,8 @@ enum devlink_attr {
 
 	DEVLINK_ATTR_SELFTESTS,			/* nested */
 
+	DEVLINK_ATTR_INFO_VERSION_IS_COMPONENT,	/* u8 0 or 1 */
+
 	/* add new attributes above here, update the policy in devlink.c */
 
 	__DEVLINK_ATTR_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 2dcf2bcc3527..31bca879f9cf 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6621,6 +6621,11 @@ static int devlink_info_version_put(struct devlink_info_req *req, int attr,
 	if (err)
 		goto nla_put_failure;
 
+	err = nla_put_u8(req->msg, DEVLINK_ATTR_INFO_VERSION_IS_COMPONENT,
+			 version_type == DEVLINK_INFO_VERSION_TYPE_COMPONENT);
+	if (err)
+		goto nla_put_failure;
+
 	nla_nest_end(req->msg, nest);
 
 	return 0;
-- 
2.37.3

Re: [patch net-next] net: devlink: expose the info about version representing a component

[Jakub Kicinski]

On Fri,  4 Nov 2022 16:24:25 +0100 Jiri Pirko wrote:
> If certain version exposed by a driver is marked to be representing a
> component, expose this info to the user.
> 
> Example:
> $ devlink dev info
> netdevsim/netdevsim10:
>   driver netdevsim
>   versions:
>       running:
>         fw.mgmt 10.20.30
>       flash_components:
>         fw.mgmt

Didn't I complain that this makes no practical sense because
user needs to know what file to flash, to which component?
Or was that a different flag that I was complaining about?

> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
> index 2f24b53a87a5..7f2874189188 100644
> --- a/include/uapi/linux/devlink.h
> +++ b/include/uapi/linux/devlink.h
> @@ -607,6 +607,8 @@ enum devlink_attr {
>  
>  	DEVLINK_ATTR_SELFTESTS,			/* nested */
>  
> +	DEVLINK_ATTR_INFO_VERSION_IS_COMPONENT,	/* u8 0 or 1 */

In the interest of fairness I should complain about the use of u8/u16

devlink is genetlink so user will know kernel supports the attribute 
(by looking at family->maxattr). So this can be a flag.

Re: [patch net-next] net: devlink: expose the info about version representing a component

[Jiri Pirko]

Sat, Nov 05, 2022 at 03:25:10AM CET, kuba@kernel.org wrote:
>On Fri,  4 Nov 2022 16:24:25 +0100 Jiri Pirko wrote:
>> If certain version exposed by a driver is marked to be representing a
>> component, expose this info to the user.
>> 
>> Example:
>> $ devlink dev info
>> netdevsim/netdevsim10:
>>   driver netdevsim
>>   versions:
>>       running:
>>         fw.mgmt 10.20.30
>>       flash_components:
>>         fw.mgmt
>
>Didn't I complain that this makes no practical sense because
>user needs to know what file to flash, to which component?
>Or was that a different flag that I was complaining about?

Different. That was about exposing a default component.


>
>> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>> index 2f24b53a87a5..7f2874189188 100644
>> --- a/include/uapi/linux/devlink.h
>> +++ b/include/uapi/linux/devlink.h
>> @@ -607,6 +607,8 @@ enum devlink_attr {
>>  
>>  	DEVLINK_ATTR_SELFTESTS,			/* nested */
>>  
>> +	DEVLINK_ATTR_INFO_VERSION_IS_COMPONENT,	/* u8 0 or 1 */
>
>In the interest of fairness I should complain about the use of u8/u16
>
>devlink is genetlink so user will know kernel supports the attribute 
>(by looking at family->maxattr). So this can be a flag.

Okay, makes sense.

Re: [patch net-next] net: devlink: expose the info about version representing a component

[Jakub Kicinski]

On Sat, 5 Nov 2022 10:26:33 +0100 Jiri Pirko wrote:
>> Didn't I complain that this makes no practical sense because
>> user needs to know what file to flash, to which component?
>> Or was that a different flag that I was complaining about?  
> 
> Different. That was about exposing a default component.

Oh, my bad. But I think the same justification applies here.
Overloading the API with information with no clear use seems
counter-productive to me.

Re: [patch net-next] net: devlink: expose the info about version representing a component

[Jiri Pirko]

Mon, Nov 07, 2022 at 05:52:18PM CET, kuba@kernel.org wrote:
>On Sat, 5 Nov 2022 10:26:33 +0100 Jiri Pirko wrote:
>>> Didn't I complain that this makes no practical sense because
>>> user needs to know what file to flash, to which component?
>>> Or was that a different flag that I was complaining about?  
>> 
>> Different. That was about exposing a default component.
>
>Oh, my bad. But I think the same justification applies here.
>Overloading the API with information with no clear use seems
>counter-productive to me.

Well, it gives the user hint about what he can pass as a "component
name" on the cmdline. Otherwise, the user has no clue.

Re: [patch net-next] net: devlink: expose the info about version representing a component

[Jakub Kicinski]

On Mon, 7 Nov 2022 18:03:06 +0100 Jiri Pirko wrote:
>> Oh, my bad. But I think the same justification applies here.
>> Overloading the API with information with no clear use seems
>> counter-productive to me.  
> 
> Well, it gives the user hint about what he can pass as a "component
> name" on the cmdline. Otherwise, the user has no clue.

The command line contains:
 - device
 - component (optional)
 - fw file to flash

What scenario are you thinking of where the user has the file they want
to flash, intent to flash a particular component only but does not know
whether that component can be flashed? 

Re: [patch net-next] net: devlink: expose the info about version representing a component

[Jiri Pirko]

Mon, Nov 07, 2022 at 07:02:06PM CET, kuba@kernel.org wrote:
>On Mon, 7 Nov 2022 18:03:06 +0100 Jiri Pirko wrote:
>>> Oh, my bad. But I think the same justification applies here.
>>> Overloading the API with information with no clear use seems
>>> counter-productive to me.  
>> 
>> Well, it gives the user hint about what he can pass as a "component
>> name" on the cmdline. Otherwise, the user has no clue.
>
>The command line contains:
> - device
> - component (optional)
> - fw file to flash
>
>What scenario are you thinking of where the user has the file they want
>to flash, intent to flash a particular component only but does not know
>whether that component can be flashed? 

No scenario. Lets drop this.