diff mbox series

[net-next,V2,11/14] ethtool: Add support for low latency RS FEC

Message ID 20200125051039.59165-12-saeedm@mellanox.com
State Not Applicable
Delegated to: David Miller
Headers show
Series [net-next,V2,01/14] devlink: Force enclosing array on binary fmsg data | expand

Commit Message

Saeed Mahameed Jan. 25, 2020, 5:11 a.m. UTC 
From: Aya Levin <ayal@mellanox.com>

Add support for low latency Reed Solomon FEC as LLRS.

Signed-off-by: Aya Levin <ayal@mellanox.com>
Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/phy/phy-core.c   | 2 +-
 include/uapi/linux/ethtool.h | 4 +++-
 net/ethtool/common.c         | 1 +
 net/ethtool/linkmodes.c      | 1 +
 4 files changed, 6 insertions(+), 2 deletions(-)

Comments

Jakub Kicinski Jan. 25, 2020, 7:40 p.m. UTC | #1 
On Sat, 25 Jan 2020 05:11:52 +0000, Saeed Mahameed wrote:
> From: Aya Levin <ayal@mellanox.com>
> 
> Add support for low latency Reed Solomon FEC as LLRS.
> 
> Signed-off-by: Aya Levin <ayal@mellanox.com>
> Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>

This is kind of buried in the midst of the driver patches.
It'd preferably be a small series of its own. 
Let's at least try to CC PHY folk now.

Is this from some standard?

> diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
> index a4d2d59fceca..e083e7a76ada 100644
> --- a/drivers/net/phy/phy-core.c
> +++ b/drivers/net/phy/phy-core.c
> @@ -8,7 +8,7 @@
>  
>  const char *phy_speed_to_str(int speed)
>  {
> -	BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 74,
> +	BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 75,
>  		"Enum ethtool_link_mode_bit_indices and phylib are out of sync. "
>  		"If a speed or mode has been added please update phy_speed_to_str "
>  		"and the PHY settings array.\n");
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 116bcbf09c74..e0c4383ea952 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -1326,6 +1326,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)
> @@ -1333,6 +1334,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.
> @@ -1517,7 +1519,7 @@ enum ethtool_link_mode_bit_indices {
>  	ETHTOOL_LINK_MODE_400000baseLR8_ER8_FR8_Full_BIT = 71,
>  	ETHTOOL_LINK_MODE_400000baseDR8_Full_BIT	 = 72,
>  	ETHTOOL_LINK_MODE_400000baseCR8_Full_BIT	 = 73,
> -
> +	ETHTOOL_LINK_MODE_FEC_LLRS_BIT			 = 74,
>  	/* must be last entry */
>  	__ETHTOOL_LINK_MODE_MASK_NBITS
>  };
> diff --git a/net/ethtool/common.c b/net/ethtool/common.c
> index e621b1694d2f..8e4e809340f0 100644
> --- a/net/ethtool/common.c
> +++ b/net/ethtool/common.c
> @@ -167,6 +167,7 @@ const char link_mode_names[][ETH_GSTRING_LEN] = {
>  	__DEFINE_LINK_MODE_NAME(400000, LR8_ER8_FR8, Full),
>  	__DEFINE_LINK_MODE_NAME(400000, DR8, Full),
>  	__DEFINE_LINK_MODE_NAME(400000, CR8, Full),
> +	__DEFINE_SPECIAL_MODE_NAME(FEC_LLRS, "LLRS"),
>  };
>  static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS);
>  
> diff --git a/net/ethtool/linkmodes.c b/net/ethtool/linkmodes.c
> index 96f20be64553..f049b97072fe 100644
> --- a/net/ethtool/linkmodes.c
> +++ b/net/ethtool/linkmodes.c
> @@ -237,6 +237,7 @@ static const struct link_mode_info link_mode_params[] = {
>  	__DEFINE_LINK_MODE_PARAMS(400000, LR8_ER8_FR8, Full),
>  	__DEFINE_LINK_MODE_PARAMS(400000, DR8, Full),
>  	__DEFINE_LINK_MODE_PARAMS(400000, CR8, Full),
> +	__DEFINE_SPECIAL_MODE_PARAMS(FEC_LLRS),
>  };
>  
>  static const struct nla_policy
Andrew Lunn Jan. 25, 2020, 9:05 p.m. UTC | #2 
On Sat, Jan 25, 2020 at 11:40:37AM -0800, Jakub Kicinski wrote:
> On Sat, 25 Jan 2020 05:11:52 +0000, Saeed Mahameed wrote:
> > From: Aya Levin <ayal@mellanox.com>
> > 
> > Add support for low latency Reed Solomon FEC as LLRS.
> > 
> > Signed-off-by: Aya Levin <ayal@mellanox.com>
> > Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> 
> This is kind of buried in the midst of the driver patches.
> It'd preferably be a small series of its own. 
> Let's at least try to CC PHY folk now.

Thanks Jakuv

> Is this from some standard?

A reference would be good.

I assume the existing ETHTOOL_LINK_MODE_FEC_RS_BIT is for Clause 91.
What clause does this LLRS refer to?

Thanks
	Andrew
Aya Levin Jan. 26, 2020, 9:13 a.m. UTC | #3 
On 1/25/2020 11:05 PM, Andrew Lunn wrote:
> On Sat, Jan 25, 2020 at 11:40:37AM -0800, Jakub Kicinski wrote:
>> On Sat, 25 Jan 2020 05:11:52 +0000, Saeed Mahameed wrote:
>>> From: Aya Levin <ayal@mellanox.com>
>>>
>>> Add support for low latency Reed Solomon FEC as LLRS.
>>>
>>> Signed-off-by: Aya Levin <ayal@mellanox.com>
>>> Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
>>> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
>>
>> This is kind of buried in the midst of the driver patches.
>> It'd preferably be a small series of its own.
>> Let's at least try to CC PHY folk now.
> 
> Thanks Jakuv
> 
>> Is this from some standard?
> 
> A reference would be good.
https://25gethernet.org/
25G consortium
> 
> I assume the existing ETHTOOL_LINK_MODE_FEC_RS_BIT is for Clause 91.
It is for both Clause91 and for Cluase 134 for 50GBASE-R PHYs
> What clause does this LLRS refer to?
The LL-FEC  is defined in the document titled "Low Latency Reed Solomon
Forward Error Correction", in https://25gethernet.org/
> 
> Thanks
> 	Andrew
>
Andrew Lunn Jan. 26, 2020, 5:22 p.m. UTC | #4 
> The LL-FEC  is defined in the document titled "Low Latency Reed Solomon
> Forward Error Correction", in https://25gethernet.org/

Please add this to the commit message.

Thanks
	Andrew
Saeed Mahameed Jan. 27, 2020, 9:56 p.m. UTC | #5 
On Sat, 2020-01-25 at 22:05 +0100, Andrew Lunn wrote:
> On Sat, Jan 25, 2020 at 11:40:37AM -0800, Jakub Kicinski wrote:
> > On Sat, 25 Jan 2020 05:11:52 +0000, Saeed Mahameed wrote:
> > > From: Aya Levin <ayal@mellanox.com>
> > > 
> > > Add support for low latency Reed Solomon FEC as LLRS.
> > > 
> > > Signed-off-by: Aya Levin <ayal@mellanox.com>
> > > Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
> > > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > 
> > This is kind of buried in the midst of the driver patches.
> > It'd preferably be a small series of its own. 
> > Let's at least try to CC PHY folk now.
> 
> Thanks Jakuv
> 

I actually CCed PHY on V1 .. but forgot on V2 :(.

Anyway the BIT is very clear and standard as Aya pointed out.. 

Shall I resubmit with the update commit message ?

I see the series is marked as "Not Applicable" i don't know why though
.. 

> > Is this from some standard?
> 
> A reference would be good.
> 
> I assume the existing ETHTOOL_LINK_MODE_FEC_RS_BIT is for Clause 91.
> What clause does this LLRS refer to?
> 
> Thanks
> 	Andrew
diff mbox series

Patch

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index a4d2d59fceca..e083e7a76ada 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -8,7 +8,7 @@ 
 
 const char *phy_speed_to_str(int speed)
 {
-	BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 74,
+	BUILD_BUG_ON_MSG(__ETHTOOL_LINK_MODE_MASK_NBITS != 75,
 		"Enum ethtool_link_mode_bit_indices and phylib are out of sync. "
 		"If a speed or mode has been added please update phy_speed_to_str "
 		"and the PHY settings array.\n");
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 116bcbf09c74..e0c4383ea952 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1326,6 +1326,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)
@@ -1333,6 +1334,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.
@@ -1517,7 +1519,7 @@  enum ethtool_link_mode_bit_indices {
 	ETHTOOL_LINK_MODE_400000baseLR8_ER8_FR8_Full_BIT = 71,
 	ETHTOOL_LINK_MODE_400000baseDR8_Full_BIT	 = 72,
 	ETHTOOL_LINK_MODE_400000baseCR8_Full_BIT	 = 73,
-
+	ETHTOOL_LINK_MODE_FEC_LLRS_BIT			 = 74,
 	/* must be last entry */
 	__ETHTOOL_LINK_MODE_MASK_NBITS
 };
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index e621b1694d2f..8e4e809340f0 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -167,6 +167,7 @@  const char link_mode_names[][ETH_GSTRING_LEN] = {
 	__DEFINE_LINK_MODE_NAME(400000, LR8_ER8_FR8, Full),
 	__DEFINE_LINK_MODE_NAME(400000, DR8, Full),
 	__DEFINE_LINK_MODE_NAME(400000, CR8, Full),
+	__DEFINE_SPECIAL_MODE_NAME(FEC_LLRS, "LLRS"),
 };
 static_assert(ARRAY_SIZE(link_mode_names) == __ETHTOOL_LINK_MODE_MASK_NBITS);
 
diff --git a/net/ethtool/linkmodes.c b/net/ethtool/linkmodes.c
index 96f20be64553..f049b97072fe 100644
--- a/net/ethtool/linkmodes.c
+++ b/net/ethtool/linkmodes.c
@@ -237,6 +237,7 @@  static const struct link_mode_info link_mode_params[] = {
 	__DEFINE_LINK_MODE_PARAMS(400000, LR8_ER8_FR8, Full),
 	__DEFINE_LINK_MODE_PARAMS(400000, DR8, Full),
 	__DEFINE_LINK_MODE_PARAMS(400000, CR8, Full),
+	__DEFINE_SPECIAL_MODE_PARAMS(FEC_LLRS),
 };
 
 static const struct nla_policy