[PATCH ethtool v2 0/5] Extend uAPI with lanes parameter

[PATCH ethtool v2 0/5] Extend uAPI with lanes parameter

[Danielle Ratson]

Currently, there is no way of knowing how many lanes will be use to
achieve a wanted speed.
For example, 100G speed can be achieved using: 2X50 or 4X25.

In order to solve that, extend ethtool uAPI with lanes as a new link
mode setting so the command below, for example, will be supported:
$ ethtool -s swp5 lanes N

Patch #1: Update headers with the new parameter.
Patch #2: Support lanes in netlink.
Patch #3: Expose the number of lanes in use.
Patch #4: Add auto-completion for lanes.
Patch #5: Add lanes to man page.

Danielle Ratson (5):
  ethtool: Extend ethtool link modes settings uAPI with lanes
  netlink: settings: Add netlink support for lanes parameter
  netlink: settings: Expose the number of lanes in use
  shell-completion: Add completion for lanes
  man: Add man page for setting lanes parameter

 ethtool.8.in                  |  4 ++++
 ethtool.c                     |  1 +
 netlink/desc-ethtool.c        |  1 +
 netlink/settings.c            | 14 ++++++++++++++
 shell-completion/bash/ethtool |  4 ++++
 uapi/linux/ethtool_netlink.h  |  1 +
 6 files changed, 25 insertions(+)

-- 
2.26.2

[PATCH ethtool v2 1/5] ethtool: Extend ethtool link modes settings uAPI with lanes

[Danielle Ratson]

Add ETHTOOL_A_LINKMODES_LANES, expand ethtool_link_settings with
lanes attribute and define valid lanes in order to support a new
lanes-selector.

Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---

Notes:
    v2:
    	* Update headers after changes in upstream patches.

 netlink/desc-ethtool.c       | 1 +
 uapi/linux/ethtool_netlink.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/netlink/desc-ethtool.c b/netlink/desc-ethtool.c
index 96291b9..fe5d7ba 100644
--- a/netlink/desc-ethtool.c
+++ b/netlink/desc-ethtool.c
@@ -87,6 +87,7 @@ static const struct pretty_nla_desc __linkmodes_desc[] = {
 	NLATTR_DESC_U8(ETHTOOL_A_LINKMODES_DUPLEX),
 	NLATTR_DESC_U8(ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG),
 	NLATTR_DESC_U8(ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE),
+	NLATTR_DESC_U32(ETHTOOL_A_LINKMODES_LANES),
 };
 
 static const struct pretty_nla_desc __linkstate_desc[] = {
diff --git a/uapi/linux/ethtool_netlink.h b/uapi/linux/ethtool_netlink.h
index c022883..0cd6906 100644
--- a/uapi/linux/ethtool_netlink.h
+++ b/uapi/linux/ethtool_netlink.h
@@ -227,6 +227,7 @@ enum {
 	ETHTOOL_A_LINKMODES_DUPLEX,		/* u8 */
 	ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG,	/* u8 */
 	ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE,	/* u8 */
+	ETHTOOL_A_LINKMODES_LANES,		/* u32 */
 
 	/* add new constants above here */
 	__ETHTOOL_A_LINKMODES_CNT,
-- 
2.26.2

Re: [PATCH ethtool v2 1/5] ethtool: Extend ethtool link modes settings uAPI with lanes

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 2105 bytes --]

On Tue, Feb 02, 2021 at 08:25:09PM +0200, Danielle Ratson wrote:
> Add ETHTOOL_A_LINKMODES_LANES, expand ethtool_link_settings with
> lanes attribute and define valid lanes in order to support a new
> lanes-selector.
> 
> Signed-off-by: Danielle Ratson <danieller@nvidia.com>
> ---

When updating the UAPI header copies, please do it in a separate commit
which updates the whole uapi/ subdirectory to the state of a specific
kernel commit. You can use the script at

  https://www.kernel.org/pub/software/network/ethtool/ethtool-import-uapi

It expects the LINUX_GIT environment variable to point to your local git
repository with kernel tree and takes one argument identifying the
commit you want to import the uapi headers from (commit id, tag or
branch name can be used). In your case, net-next would be the most
likely choice.

Michal

> Notes:
>     v2:
>     	* Update headers after changes in upstream patches.
> 
>  netlink/desc-ethtool.c       | 1 +
>  uapi/linux/ethtool_netlink.h | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/netlink/desc-ethtool.c b/netlink/desc-ethtool.c
> index 96291b9..fe5d7ba 100644
> --- a/netlink/desc-ethtool.c
> +++ b/netlink/desc-ethtool.c
> @@ -87,6 +87,7 @@ static const struct pretty_nla_desc __linkmodes_desc[] = {
>  	NLATTR_DESC_U8(ETHTOOL_A_LINKMODES_DUPLEX),
>  	NLATTR_DESC_U8(ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG),
>  	NLATTR_DESC_U8(ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE),
> +	NLATTR_DESC_U32(ETHTOOL_A_LINKMODES_LANES),
>  };
>  
>  static const struct pretty_nla_desc __linkstate_desc[] = {
> diff --git a/uapi/linux/ethtool_netlink.h b/uapi/linux/ethtool_netlink.h
> index c022883..0cd6906 100644
> --- a/uapi/linux/ethtool_netlink.h
> +++ b/uapi/linux/ethtool_netlink.h
> @@ -227,6 +227,7 @@ enum {
>  	ETHTOOL_A_LINKMODES_DUPLEX,		/* u8 */
>  	ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG,	/* u8 */
>  	ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE,	/* u8 */
> +	ETHTOOL_A_LINKMODES_LANES,		/* u32 */
>  
>  	/* add new constants above here */
>  	__ETHTOOL_A_LINKMODES_CNT,
> -- 
> 2.26.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH ethtool v2 1/5] ethtool: Extend ethtool link modes settings uAPI with lanes

[Danielle Ratson]

> -----Original Message-----
> From: Michal Kubecek <mkubecek@suse.cz>
> Sent: Tuesday, February 9, 2021 9:40 PM
> To: Danielle Ratson <danieller@nvidia.com>
> Cc: netdev@vger.kernel.org; f.fainelli@gmail.com; kuba@kernel.org; andrew@lunn.ch; mlxsw <mlxsw@nvidia.com>
> Subject: Re: [PATCH ethtool v2 1/5] ethtool: Extend ethtool link modes settings uAPI with lanes
> 
> On Tue, Feb 02, 2021 at 08:25:09PM +0200, Danielle Ratson wrote:
> > Add ETHTOOL_A_LINKMODES_LANES, expand ethtool_link_settings with lanes
> > attribute and define valid lanes in order to support a new
> > lanes-selector.
> >
> > Signed-off-by: Danielle Ratson <danieller@nvidia.com>
> > ---
> 
> When updating the UAPI header copies, please do it in a separate commit which updates the whole uapi/ subdirectory to the state of
> a specific kernel commit. You can use the script at
> 
>   https://www.kernel.org/pub/software/network/ethtool/ethtool-import-uapi
> 
> It expects the LINUX_GIT environment variable to point to your local git repository with kernel tree and takes one argument
> identifying the commit you want to import the uapi headers from (commit id, tag or branch name can be used). In your case, net-next
> would be the most likely choice.
> 
> Michal

Should I use the commit in net-next that I added it uapi headers, or generally the last commit of net-next?

Thanks,
Danielle

> 
> > Notes:
> >     v2:
> >     	* Update headers after changes in upstream patches.
> >
> >  netlink/desc-ethtool.c       | 1 +
> >  uapi/linux/ethtool_netlink.h | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/netlink/desc-ethtool.c b/netlink/desc-ethtool.c index
> > 96291b9..fe5d7ba 100644
> > --- a/netlink/desc-ethtool.c
> > +++ b/netlink/desc-ethtool.c
> > @@ -87,6 +87,7 @@ static const struct pretty_nla_desc __linkmodes_desc[] = {
> >  	NLATTR_DESC_U8(ETHTOOL_A_LINKMODES_DUPLEX),
> >  	NLATTR_DESC_U8(ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG),
> >  	NLATTR_DESC_U8(ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE),
> > +	NLATTR_DESC_U32(ETHTOOL_A_LINKMODES_LANES),
> >  };
> >
> >  static const struct pretty_nla_desc __linkstate_desc[] = { diff --git
> > a/uapi/linux/ethtool_netlink.h b/uapi/linux/ethtool_netlink.h index
> > c022883..0cd6906 100644
> > --- a/uapi/linux/ethtool_netlink.h
> > +++ b/uapi/linux/ethtool_netlink.h
> > @@ -227,6 +227,7 @@ enum {
> >  	ETHTOOL_A_LINKMODES_DUPLEX,		/* u8 */
> >  	ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG,	/* u8 */
> >  	ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE,	/* u8 */
> > +	ETHTOOL_A_LINKMODES_LANES,		/* u32 */
> >
> >  	/* add new constants above here */
> >  	__ETHTOOL_A_LINKMODES_CNT,
> > --
> > 2.26.2
> >

Re: [PATCH ethtool v2 1/5] ethtool: Extend ethtool link modes settings uAPI with lanes

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 938 bytes --]

On Wed, Feb 10, 2021 at 12:06:04PM +0000, Danielle Ratson wrote:
> > When updating the UAPI header copies, please do it in a separate
> > commit which updates the whole uapi/ subdirectory to the state of a
> > specific kernel commit. You can use the script at
> > 
> >   https://www.kernel.org/pub/software/network/ethtool/ethtool-import-uapi
> > 
> > It expects the LINUX_GIT environment variable to point to your local
> > git repository with kernel tree and takes one argument identifying
> > the commit you want to import the uapi headers from (commit id, tag
> > or branch name can be used). In your case, net-next would be the
> > most likely choice.
> 
> Should I use the commit in net-next that I added it uapi headers, or
> generally the last commit of net-next?

That's up to you, I'm fine with either. The important point is to have
a consistent snapshot of all copied (and sanitized) uapi headers.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH ethtool v2 2/5] netlink: settings: Add netlink support for lanes parameter

[Danielle Ratson]

Add support for "ethtool -s <dev> lanes N ..." for setting a specific
number of lanes.

Signed-off-by: Danielle Ratson <danieller@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
---
 ethtool.c          | 1 +
 netlink/settings.c | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/ethtool.c b/ethtool.c
index 585aafa..fcb09f7 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -5620,6 +5620,7 @@ static const struct option args[] = {
 		.nlfunc	= nl_sset,
 		.help	= "Change generic options",
 		.xhelp	= "		[ speed %d ]\n"
+			  "		[ lanes %d ]\n"
 			  "		[ duplex half|full ]\n"
 			  "		[ port tp|aui|bnc|mii|fibre|da ]\n"
 			  "		[ mdix auto|on|off ]\n"
diff --git a/netlink/settings.c b/netlink/settings.c
index 90c28b1..6cb5d5b 100644
--- a/netlink/settings.c
+++ b/netlink/settings.c
@@ -20,6 +20,7 @@
 struct link_mode_info {
 	enum link_mode_class	class;
 	u32			speed;
+	u32			lanes;
 	u8			duplex;
 };
 
@@ -1067,6 +1068,13 @@ static const struct param_parser sset_params[] = {
 		.handler	= nl_parse_direct_u32,
 		.min_argc	= 1,
 	},
+	{
+		.arg		= "lanes",
+		.group		= ETHTOOL_MSG_LINKMODES_SET,
+		.type		= ETHTOOL_A_LINKMODES_LANES,
+		.handler	= nl_parse_direct_u32,
+		.min_argc	= 1,
+	},
 	{
 		.arg		= "duplex",
 		.group		= ETHTOOL_MSG_LINKMODES_SET,
-- 
2.26.2

Re: [PATCH ethtool v2 2/5] netlink: settings: Add netlink support for lanes parameter

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 1677 bytes --]

On Tue, Feb 02, 2021 at 08:25:10PM +0200, Danielle Ratson wrote:
> Add support for "ethtool -s <dev> lanes N ..." for setting a specific
> number of lanes.
> 
> Signed-off-by: Danielle Ratson <danieller@nvidia.com>
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> ---
>  ethtool.c          | 1 +
>  netlink/settings.c | 8 ++++++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/ethtool.c b/ethtool.c
> index 585aafa..fcb09f7 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -5620,6 +5620,7 @@ static const struct option args[] = {
>  		.nlfunc	= nl_sset,
>  		.help	= "Change generic options",
>  		.xhelp	= "		[ speed %d ]\n"
> +			  "		[ lanes %d ]\n"
>  			  "		[ duplex half|full ]\n"
>  			  "		[ port tp|aui|bnc|mii|fibre|da ]\n"
>  			  "		[ mdix auto|on|off ]\n"
> diff --git a/netlink/settings.c b/netlink/settings.c
> index 90c28b1..6cb5d5b 100644
> --- a/netlink/settings.c
> +++ b/netlink/settings.c
> @@ -20,6 +20,7 @@
>  struct link_mode_info {
>  	enum link_mode_class	class;
>  	u32			speed;
> +	u32			lanes;
>  	u8			duplex;
>  };
>  

This structure member is not used anywhere in this patch and, AFAICS,
not even in the rest of your series. Perhaps a leftover from an older
version?

Michal

> @@ -1067,6 +1068,13 @@ static const struct param_parser sset_params[] = {
>  		.handler	= nl_parse_direct_u32,
>  		.min_argc	= 1,
>  	},
> +	{
> +		.arg		= "lanes",
> +		.group		= ETHTOOL_MSG_LINKMODES_SET,
> +		.type		= ETHTOOL_A_LINKMODES_LANES,
> +		.handler	= nl_parse_direct_u32,
> +		.min_argc	= 1,
> +	},
>  	{
>  		.arg		= "duplex",
>  		.group		= ETHTOOL_MSG_LINKMODES_SET,
> -- 
> 2.26.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH ethtool v2 3/5] netlink: settings: Expose the number of lanes in use

[Danielle Ratson]

Currently, the user does not have the information regarding how many lanes
are used when the link is up.

After adding a possibility to advertise or force a specific number of
lanes this information becomes helpful.

Expose the number of lanes in use if the information is passed from
kernel.

Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---

Notes:
    v2:
    	* Remove possibility for printing unknown lanes, as now unknown
    	  lanes attribute doesn't pass to netlink.

 netlink/settings.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/netlink/settings.c b/netlink/settings.c
index 6cb5d5b..2ebe4c9 100644
--- a/netlink/settings.c
+++ b/netlink/settings.c
@@ -474,6 +474,12 @@ int linkmodes_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 		else
 			printf("\tSpeed: %uMb/s\n", val);
 	}
+	if (tb[ETHTOOL_A_LINKMODES_LANES]) {
+		uint32_t val = mnl_attr_get_u32(tb[ETHTOOL_A_LINKMODES_LANES]);
+
+		print_banner(nlctx);
+		printf("\tLanes: %u\n", val);
+	}
 	if (tb[ETHTOOL_A_LINKMODES_DUPLEX]) {
 		uint8_t val = mnl_attr_get_u8(tb[ETHTOOL_A_LINKMODES_DUPLEX]);
 
-- 
2.26.2

[PATCH ethtool v2 4/5] shell-completion: Add completion for lanes

[Danielle Ratson]

Lanes was added as a new link mode setting in ethtool.

Support completion for lanes when setting parameters.

Signed-off-by: Danielle Ratson <danieller@nvidia.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
---
 shell-completion/bash/ethtool | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/shell-completion/bash/ethtool b/shell-completion/bash/ethtool
index 5305559..4557341 100644
--- a/shell-completion/bash/ethtool
+++ b/shell-completion/bash/ethtool
@@ -97,6 +97,7 @@ _ethtool_change()
 		[speed]=notseen
 		[wol]=notseen
 		[xcvr]=notseen
+		[lanes]=notseen
 	)
 
 	local -A msgtypes=(
@@ -175,6 +176,9 @@ _ethtool_change()
 		xcvr)
 			COMPREPLY=( $( compgen -W 'internal external' -- "$cur" ) )
 			return ;;
+		lanes)
+			# Number
+			return ;;
 	esac
 
 	local -a comp_words=()
-- 
2.26.2

[PATCH ethtool v2 5/5] man: Add man page for setting lanes parameter

[Danielle Ratson]

Lanes parameter was added for setting using ethtool.

Update the man page to include the new parameter.

Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---
 ethtool.8.in | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index ba4e245..fe49b66 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -251,6 +251,7 @@ ethtool \- query or control network driver and hardware settings
 .B ethtool \-s
 .I devname
 .BN speed
+.BN lanes
 .B2 duplex half full
 .B4 port tp aui bnc mii fibre da
 .B3 mdix auto on off
@@ -685,6 +686,9 @@ Set speed in Mb/s.
 .B ethtool
 with just the device name as an argument will show you the supported device speeds.
 .TP
+.BI lanes \ N
+Set number of lanes.
+.TP
 .A2 duplex half full
 Sets full or half duplex mode.
 .TP
-- 
2.26.2

Re: [PATCH ethtool v2 0/5] Extend uAPI with lanes parameter

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]

On Tue, Feb 02, 2021 at 08:25:08PM +0200, Danielle Ratson wrote:
> Currently, there is no way of knowing how many lanes will be use to
> achieve a wanted speed.
> For example, 100G speed can be achieved using: 2X50 or 4X25.
> 
> In order to solve that, extend ethtool uAPI with lanes as a new link
> mode setting so the command below, for example, will be supported:
> $ ethtool -s swp5 lanes N
> 
> Patch #1: Update headers with the new parameter.
> Patch #2: Support lanes in netlink.
> Patch #3: Expose the number of lanes in use.
> Patch #4: Add auto-completion for lanes.
> Patch #5: Add lanes to man page.
> 
> Danielle Ratson (5):
>   ethtool: Extend ethtool link modes settings uAPI with lanes
>   netlink: settings: Add netlink support for lanes parameter
>   netlink: settings: Expose the number of lanes in use
>   shell-completion: Add completion for lanes
>   man: Add man page for setting lanes parameter
> 
>  ethtool.8.in                  |  4 ++++
>  ethtool.c                     |  1 +
>  netlink/desc-ethtool.c        |  1 +
>  netlink/settings.c            | 14 ++++++++++++++
>  shell-completion/bash/ethtool |  4 ++++
>  uapi/linux/ethtool_netlink.h  |  1 +
>  6 files changed, 25 insertions(+)

Sorry for the delay, I was busy with other stuff last week and missed
that with kernel part accepted, I should take care of the userspace
counterpart.

The series looks good to me, except for two minor issues I'll comment to
relevant patches.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]