diff mbox series

[ethtool] ethtool: Add support for Low Latency Reed Solomon

Message ID 1584025923-5385-1-git-send-email-tariqt@mellanox.com
State Changes Requested
Delegated to: John Linville
Headers show
Series [ethtool] ethtool: Add support for Low Latency Reed Solomon | expand

Commit Message

Tariq Toukan March 12, 2020, 3:12 p.m. UTC 
From: Aya Levin <ayal@mellanox.com>

Introduce a new FEC mode LL-RS: Low Latency Reed Solomon, update print
and initialization functions accordingly. In addition, update related
man page.

Signed-off-by: Aya Levin <ayal@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
 ethtool-copy.h |  3 +++
 ethtool.8.in   |  5 +++--
 ethtool.c      | 12 +++++++++++-
 3 files changed, 17 insertions(+), 3 deletions(-)

Comments

John W. Linville March 13, 2020, 7:28 p.m. UTC | #1 
On Thu, Mar 12, 2020 at 05:12:03PM +0200, Tariq Toukan wrote:
> From: Aya Levin <ayal@mellanox.com>
> 
> Introduce a new FEC mode LL-RS: Low Latency Reed Solomon, update print
> and initialization functions accordingly. In addition, update related
> man page.
> 
> Signed-off-by: Aya Levin <ayal@mellanox.com>
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> ---
>  ethtool-copy.h |  3 +++
>  ethtool.8.in   |  5 +++--
>  ethtool.c      | 12 +++++++++++-
>  3 files changed, 17 insertions(+), 3 deletions(-)

Hey...

Thanks for the patch! Unfortunately for you, I just merged "[PATCH
ethtool v3 01/25] move UAPI header copies to a separate directory"
from Michal Kubecek <mkubecek@suse.cz>, and that patch did this:

 ethtool-copy.h => uapi/linux/ethtool.h       | 0
 rename ethtool-copy.h => uapi/linux/ethtool.h (100%)

Could you please rework your patch against the current kernel.org tree?

Thanks!

John
 
> diff --git a/ethtool-copy.h b/ethtool-copy.h
> index 9afd2e6c5eea..a5482a91b429 100644
> --- a/ethtool-copy.h
> +++ b/ethtool-copy.h
> @@ -1319,6 +1319,7 @@ enum ethtool_fec_config_bits {
>  	ETHTOOL_FEC_OFF_BIT,
>  	ETHTOOL_FEC_RS_BIT,
>  	ETHTOOL_FEC_BASER_BIT,
> +	ETHTOOL_FEC_LLRS_BIT,
>  };
>  
>  #define ETHTOOL_FEC_NONE		(1 << ETHTOOL_FEC_NONE_BIT)
> @@ -1326,6 +1327,7 @@ enum ethtool_fec_config_bits {
>  #define ETHTOOL_FEC_OFF			(1 << ETHTOOL_FEC_OFF_BIT)
>  #define ETHTOOL_FEC_RS			(1 << ETHTOOL_FEC_RS_BIT)
>  #define ETHTOOL_FEC_BASER		(1 << ETHTOOL_FEC_BASER_BIT)
> +#define ETHTOOL_FEC_LLRS		(1 << ETHTOOL_FEC_LLRS_BIT)
>  
>  /* CMDs currently supported */
>  #define ETHTOOL_GSET		0x00000001 /* DEPRECATED, Get settings.
> @@ -1505,6 +1507,7 @@ enum ethtool_link_mode_bit_indices {
>  	ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT	 = 66,
>  	ETHTOOL_LINK_MODE_100baseT1_Full_BIT		 = 67,
>  	ETHTOOL_LINK_MODE_1000baseT1_Full_BIT		 = 68,
> +	ETHTOOL_LINK_MODE_FEC_LLRS_BIT                   = 74,
>  
>  	/* must be last entry */
>  	__ETHTOOL_LINK_MODE_MASK_NBITS
> diff --git a/ethtool.8.in b/ethtool.8.in
> index 94364c626330..5d16aa27dab1 100644
> --- a/ethtool.8.in
> +++ b/ethtool.8.in
> @@ -404,7 +404,7 @@ ethtool \- query or control network driver and hardware settings
>  .B ethtool \-\-set\-fec
>  .I devname
>  .B encoding
> -.BR auto | off | rs | baser \ [...]
> +.BR auto | off | rs | baser | ll-rs \ [...]
>  .HP
>  .B ethtool \-Q|\-\-per\-queue
>  .I devname
> @@ -1204,7 +1204,7 @@ current FEC mode, the driver or firmware must take the link down
>  administratively and report the problem in the system logs for users to correct.
>  .RS 4
>  .TP
> -.BR encoding\ auto | off | rs | baser \ [...]
> +.BR encoding\ auto | off | rs | baser | ll-rs \ [...]
>  
>  Sets the FEC encoding for the device.  Combinations of options are specified as
>  e.g.
> @@ -1217,6 +1217,7 @@ auto	Use the driver's default encoding
>  off	Turn off FEC
>  RS	Force RS-FEC encoding
>  BaseR	Force BaseR encoding
> +LL-RS	Force LL-RS-FEC encoding
>  .TE
>  .RE
>  .TP
> diff --git a/ethtool.c b/ethtool.c
> index acf183dc5586..7110b269f306 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -562,6 +562,7 @@ static void init_global_link_mode_masks(void)
>  		ETHTOOL_LINK_MODE_FEC_NONE_BIT,
>  		ETHTOOL_LINK_MODE_FEC_RS_BIT,
>  		ETHTOOL_LINK_MODE_FEC_BASER_BIT,
> +		ETHTOOL_LINK_MODE_FEC_LLRS_BIT,
>  	};
>  	unsigned int i;
>  
> @@ -814,6 +815,12 @@ static void dump_link_caps(const char *prefix, const char *an_prefix,
>  			fprintf(stdout, " RS");
>  			fecreported = 1;
>  		}
> +		if (ethtool_link_mode_test_bit(ETHTOOL_LINK_MODE_FEC_LLRS_BIT,
> +					       mask)) {
> +			fprintf(stdout, " LL-RS");
> +			fecreported = 1;
> +		}
> +
>  		if (!fecreported)
>  			fprintf(stdout, " Not reported");
>  		fprintf(stdout, "\n");
> @@ -1696,6 +1703,8 @@ static void dump_fec(u32 fec)
>  		fprintf(stdout, " BaseR");
>  	if (fec & ETHTOOL_FEC_RS)
>  		fprintf(stdout, " RS");
> +	if (fec & ETHTOOL_FEC_LLRS)
> +		fprintf(stdout, " LL-RS");
>  }
>  
>  #define N_SOTS 7
> @@ -5209,7 +5218,8 @@ static int fecmode_str_to_type(const char *str)
>  		return ETHTOOL_FEC_RS;
>  	if (!strcasecmp(str, "baser"))
>  		return ETHTOOL_FEC_BASER;
> -
> +	if (!strcasecmp(str, "ll-rs"))
> +		return ETHTOOL_FEC_LLRS;
>  	return 0;
>  }
>  
> -- 
> 1.8.3.1
> 
>
Tariq Toukan March 17, 2020, 8:06 a.m. UTC | #2 
On 3/13/2020 9:28 PM, John W. Linville wrote:
> On Thu, Mar 12, 2020 at 05:12:03PM +0200, Tariq Toukan wrote:
>> From: Aya Levin <ayal@mellanox.com>
>>
>> Introduce a new FEC mode LL-RS: Low Latency Reed Solomon, update print
>> and initialization functions accordingly. In addition, update related
>> man page.
>>
>> Signed-off-by: Aya Levin <ayal@mellanox.com>
>> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
>> ---
>>   ethtool-copy.h |  3 +++
>>   ethtool.8.in   |  5 +++--
>>   ethtool.c      | 12 +++++++++++-
>>   3 files changed, 17 insertions(+), 3 deletions(-)
> 
> Hey...
> 
> Thanks for the patch! Unfortunately for you, I just merged "[PATCH
> ethtool v3 01/25] move UAPI header copies to a separate directory"
> from Michal Kubecek <mkubecek@suse.cz>, and that patch did this:
> 
>   ethtool-copy.h => uapi/linux/ethtool.h       | 0
>   rename ethtool-copy.h => uapi/linux/ethtool.h (100%)
> 
> Could you please rework your patch against the current kernel.org tree?

Hi John,
Sure, we'll rework and respin.

Thanks,
Tariq

> 
> Thanks!
> 
> John
>
Michal Kubecek March 17, 2020, 8:23 a.m. UTC | #3 
On Tue, Mar 17, 2020 at 10:06:26AM +0200, Tariq Toukan wrote:
> On 3/13/2020 9:28 PM, John W. Linville wrote:
> > On Thu, Mar 12, 2020 at 05:12:03PM +0200, Tariq Toukan wrote:
> > > From: Aya Levin <ayal@mellanox.com>
> > > 
> > > Introduce a new FEC mode LL-RS: Low Latency Reed Solomon, update print
> > > and initialization functions accordingly. In addition, update related
> > > man page.
> > > 
> > > Signed-off-by: Aya Levin <ayal@mellanox.com>
> > > Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> > > ---
> > >   ethtool-copy.h |  3 +++
> > >   ethtool.8.in   |  5 +++--
> > >   ethtool.c      | 12 +++++++++++-
> > >   3 files changed, 17 insertions(+), 3 deletions(-)
> > 
> > Hey...
> > 
> > Thanks for the patch! Unfortunately for you, I just merged "[PATCH
> > ethtool v3 01/25] move UAPI header copies to a separate directory"
> > from Michal Kubecek <mkubecek@suse.cz>, and that patch did this:
> > 
> >   ethtool-copy.h => uapi/linux/ethtool.h       | 0
> >   rename ethtool-copy.h => uapi/linux/ethtool.h (100%)
> > 
> > Could you please rework your patch against the current kernel.org tree?
> 
> Hi John,
> Sure, we'll rework and respin.

When you do, please update also link_modes[] array in netlink/settings.c
I'll have to think of some long term solution (like kernel providing
this information) but that is something for the future development.

Michal
diff mbox series

Patch

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 9afd2e6c5eea..a5482a91b429 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -1319,6 +1319,7 @@  enum ethtool_fec_config_bits {
 	ETHTOOL_FEC_OFF_BIT,
 	ETHTOOL_FEC_RS_BIT,
 	ETHTOOL_FEC_BASER_BIT,
+	ETHTOOL_FEC_LLRS_BIT,
 };
 
 #define ETHTOOL_FEC_NONE		(1 << ETHTOOL_FEC_NONE_BIT)
@@ -1326,6 +1327,7 @@  enum ethtool_fec_config_bits {
 #define ETHTOOL_FEC_OFF			(1 << ETHTOOL_FEC_OFF_BIT)
 #define ETHTOOL_FEC_RS			(1 << ETHTOOL_FEC_RS_BIT)
 #define ETHTOOL_FEC_BASER		(1 << ETHTOOL_FEC_BASER_BIT)
+#define ETHTOOL_FEC_LLRS		(1 << ETHTOOL_FEC_LLRS_BIT)
 
 /* CMDs currently supported */
 #define ETHTOOL_GSET		0x00000001 /* DEPRECATED, Get settings.
@@ -1505,6 +1507,7 @@  enum ethtool_link_mode_bit_indices {
 	ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT	 = 66,
 	ETHTOOL_LINK_MODE_100baseT1_Full_BIT		 = 67,
 	ETHTOOL_LINK_MODE_1000baseT1_Full_BIT		 = 68,
+	ETHTOOL_LINK_MODE_FEC_LLRS_BIT                   = 74,
 
 	/* must be last entry */
 	__ETHTOOL_LINK_MODE_MASK_NBITS
diff --git a/ethtool.8.in b/ethtool.8.in
index 94364c626330..5d16aa27dab1 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -404,7 +404,7 @@  ethtool \- query or control network driver and hardware settings
 .B ethtool \-\-set\-fec
 .I devname
 .B encoding
-.BR auto | off | rs | baser \ [...]
+.BR auto | off | rs | baser | ll-rs \ [...]
 .HP
 .B ethtool \-Q|\-\-per\-queue
 .I devname
@@ -1204,7 +1204,7 @@  current FEC mode, the driver or firmware must take the link down
 administratively and report the problem in the system logs for users to correct.
 .RS 4
 .TP
-.BR encoding\ auto | off | rs | baser \ [...]
+.BR encoding\ auto | off | rs | baser | ll-rs \ [...]
 
 Sets the FEC encoding for the device.  Combinations of options are specified as
 e.g.
@@ -1217,6 +1217,7 @@  auto	Use the driver's default encoding
 off	Turn off FEC
 RS	Force RS-FEC encoding
 BaseR	Force BaseR encoding
+LL-RS	Force LL-RS-FEC encoding
 .TE
 .RE
 .TP
diff --git a/ethtool.c b/ethtool.c
index acf183dc5586..7110b269f306 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -562,6 +562,7 @@  static void init_global_link_mode_masks(void)
 		ETHTOOL_LINK_MODE_FEC_NONE_BIT,
 		ETHTOOL_LINK_MODE_FEC_RS_BIT,
 		ETHTOOL_LINK_MODE_FEC_BASER_BIT,
+		ETHTOOL_LINK_MODE_FEC_LLRS_BIT,
 	};
 	unsigned int i;
 
@@ -814,6 +815,12 @@  static void dump_link_caps(const char *prefix, const char *an_prefix,
 			fprintf(stdout, " RS");
 			fecreported = 1;
 		}
+		if (ethtool_link_mode_test_bit(ETHTOOL_LINK_MODE_FEC_LLRS_BIT,
+					       mask)) {
+			fprintf(stdout, " LL-RS");
+			fecreported = 1;
+		}
+
 		if (!fecreported)
 			fprintf(stdout, " Not reported");
 		fprintf(stdout, "\n");
@@ -1696,6 +1703,8 @@  static void dump_fec(u32 fec)
 		fprintf(stdout, " BaseR");
 	if (fec & ETHTOOL_FEC_RS)
 		fprintf(stdout, " RS");
+	if (fec & ETHTOOL_FEC_LLRS)
+		fprintf(stdout, " LL-RS");
 }
 
 #define N_SOTS 7
@@ -5209,7 +5218,8 @@  static int fecmode_str_to_type(const char *str)
 		return ETHTOOL_FEC_RS;
 	if (!strcasecmp(str, "baser"))
 		return ETHTOOL_FEC_BASER;
-
+	if (!strcasecmp(str, "ll-rs"))
+		return ETHTOOL_FEC_LLRS;
 	return 0;
 }