[PATCH] net: added support for 40GbE link.

1. link speed of 40GbE and #4 KR4, CR4, SR4, LR4 modes defined.
2. removed code replication for tov calculation for 1G, 10G and
made is common for 1G, 10G, 40G.

Port cost calculation changes for bridging for 40G will be done once have more clarify from 802.1d spec in coming days.

Signed-off-by: Parav Pandit <redacted>
---
 include/linux/ethtool.h |   11 ++++++++++-
 net/packet/af_packet.c  |    8 +++-----
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 297370a..1ebfa6e 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h

@@ -1153,6 +1153,10 @@ struct ethtool_ops {
 #define SUPPORTED_10000baseR_FEC	(1 << 20)
 #define SUPPORTED_20000baseMLD2_Full	(1 << 21)
 #define SUPPORTED_20000baseKR2_Full	(1 << 22)
+#define SUPPORTED_40000baseKR4_Full	(1 << 23)
+#define SUPPORTED_40000baseCR4_Full	(1 << 24)
+#define SUPPORTED_40000baseSR4_Full	(1 << 25)
+#define SUPPORTED_40000baseLR4_Full	(1 << 26)
 
 /* Indicates what features are advertised by the interface. */
 #define ADVERTISED_10baseT_Half		(1 << 0)

@@ -1178,6 +1182,10 @@ struct ethtool_ops {
 #define ADVERTISED_10000baseR_FEC	(1 << 20)
 #define ADVERTISED_20000baseMLD2_Full	(1 << 21)
 #define ADVERTISED_20000baseKR2_Full	(1 << 22)
+#define ADVERTISED_40000baseKR4_Full	(1 << 23)
+#define ADVERTISED_40000baseCR4_Full	(1 << 24)
+#define ADVERTISED_40000baseSR4_Full	(1 << 25)
+#define ADVERTISED_40000baseLR4_Full	(1 << 26)
 
 /* The following are all involved in forcing a particular link
  * mode for the device for setting things.  When getting the

@@ -1185,12 +1193,13 @@ struct ethtool_ops {
  * it was forced up into this mode or autonegotiated.
  */
 
-/* The forced speed, 10Mb, 100Mb, gigabit, 2.5Gb, 10GbE. */
+/* The forced speed, 10Mb, 100Mb, gigabit, 2.5Gb, 10GbE, 40GbE. */
 #define SPEED_10		10
 #define SPEED_100		100
 #define SPEED_1000		1000
 #define SPEED_2500		2500
 #define SPEED_10000		10000
+#define SPEED_40000		40000
 #define SPEED_UNKNOWN		-1
 
 /* Duplex, half or full. */

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 8a10d5b..dd0e503 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c

@@ -542,13 +542,11 @@ static int prb_calc_retire_blk_tmo(struct packet_sock *po,
 	rtnl_unlock();
 	if (!err) {
 		switch (ecmd.speed) {
-		case SPEED_10000:
-			msec = 1;
-			div = 10000/1000;
-			break;
 		case SPEED_1000:
+		case SPEED_10000:
+		case SPEED_40000:
 			msec = 1;
-			div = 1000/1000;
+			div = ecmd.speed / 1000;
 			break;
 		/*
 		 * If the link speed is so slow you don't really

-- 
1.6.0.2

On 06/18/2012 05:44 AM, Parav Pandit wrote:

Any idea how many defines will be wanted for 100 Gbit Ethernet? 
Supported and advertising in ethtool_cmd are __u32s...

rick jones

On Mon, 2012-06-18 at 09:27 -0700, Rick Jones wrote:

On 06/18/2012 05:44 AM, Parav Pandit wrote:

quoted

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 297370a..1ebfa6e 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h

@@ -1153,6 +1153,10 @@ struct ethtool_ops {
  #define SUPPORTED_10000baseR_FEC	(1<<  20)
  #define SUPPORTED_20000baseMLD2_Full	(1<<  21)
  #define SUPPORTED_20000baseKR2_Full	(1<<  22)
+#define SUPPORTED_40000baseKR4_Full	(1<<  23)
+#define SUPPORTED_40000baseCR4_Full	(1<<  24)
+#define SUPPORTED_40000baseSR4_Full	(1<<  25)
+#define SUPPORTED_40000baseLR4_Full	(1<<  26)

  /* Indicates what features are advertised by the interface. */
  #define ADVERTISED_10baseT_Half		(1<<  0)

@@ -1178,6 +1182,10 @@ struct ethtool_ops {
  #define ADVERTISED_10000baseR_FEC	(1<<  20)
  #define ADVERTISED_20000baseMLD2_Full	(1<<  21)
  #define ADVERTISED_20000baseKR2_Full	(1<<  22)
+#define ADVERTISED_40000baseKR4_Full	(1<<  23)
+#define ADVERTISED_40000baseCR4_Full	(1<<  24)
+#define ADVERTISED_40000baseSR4_Full	(1<<  25)
+#define ADVERTISED_40000baseLR4_Full	(1<<  26)

Any idea how many defines will be wanted for 100 Gbit Ethernet? 
Supported and advertising in ethtool_cmd are __u32s...

There are 9 bytes reserved in struct ethtool_cmd, so we can potentially
add extend each of supported, advertising and lp_advertising by 24 bits.
But it might be better to define a new, cleaner struct ethtool_cmd.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

On Mon, 2012-06-18 at 18:14 +0530, Parav Pandit wrote:

1. link speed of 40GbE and #4 KR4, CR4, SR4, LR4 modes defined.
2. removed code replication for tov calculation for 1G, 10G and
made is common for 1G, 10G, 40G.

[...]

I don't think there's any need to name all possible link speeds, and it
just encourages the bad practice of ethtool API users checking for
specific values.  You may notice there is no SPEED_20000.

This function should be fixed properly.  Firstly, it must use
ethtool_cmd_speed() rather than directly accessing ecmd.speed.
Secondly, it should allow any speed value rather than checking for
specific values.  Then there will be no need to make further changes for
100G or any other new speed.

Ben.

 		/*
 		 * If the link speed is so slow you don't really

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

-----Original Message-----
From: Rick Jones [mailto:rick.jones2@hp.com]
Sent: Monday, June 18, 2012 9:58 PM
To: Pandit, Parav
Cc: netdev@vger.kernel.org; bhutchings@solarflare.com
Subject: Re: [PATCH] net: added support for 40GbE link.

On 06/18/2012 05:44 AM, Parav Pandit wrote:

quoted

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index

297370a..1ebfa6e 100644

--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h

@@ -1153,6 +1153,10 @@ struct ethtool_ops {
  #define SUPPORTED_10000baseR_FEC	(1<<  20)
  #define SUPPORTED_20000baseMLD2_Full	(1<<  21)
  #define SUPPORTED_20000baseKR2_Full	(1<<  22)
+#define SUPPORTED_40000baseKR4_Full	(1<<  23)
+#define SUPPORTED_40000baseCR4_Full	(1<<  24)
+#define SUPPORTED_40000baseSR4_Full	(1<<  25)
+#define SUPPORTED_40000baseLR4_Full	(1<<  26)

  /* Indicates what features are advertised by the interface. */
  #define ADVERTISED_10baseT_Half		(1<<  0)

@@ -1178,6 +1182,10 @@ struct ethtool_ops {
  #define ADVERTISED_10000baseR_FEC	(1<<  20)
  #define ADVERTISED_20000baseMLD2_Full	(1<<  21)
  #define ADVERTISED_20000baseKR2_Full	(1<<  22)
+#define ADVERTISED_40000baseKR4_Full	(1<<  23)
+#define ADVERTISED_40000baseCR4_Full	(1<<  24)
+#define ADVERTISED_40000baseSR4_Full	(1<<  25)
+#define ADVERTISED_40000baseLR4_Full	(1<<  26)

Any idea how many defines will be wanted for 100 Gbit Ethernet?
Supported and advertising in ethtool_cmd are __u32s...

100G supports CR10, ER4, LR4, Base-R and SR10 interfaces. So 5 bits. We have space from 27 to 31 bits for 100G as well.

rick jones

From: Ben Hutchings <redacted>
Date: Mon, 18 Jun 2012 18:09:36 +0100

On Mon, 2012-06-18 at 18:14 +0530, Parav Pandit wrote:

...

quoted

-/* The forced speed, 10Mb, 100Mb, gigabit, 2.5Gb, 10GbE. */
+/* The forced speed, 10Mb, 100Mb, gigabit, 2.5Gb, 10GbE, 40GbE. */

I don't think there's any need to name all possible link speeds, and it
just encourages the bad practice of ethtool API users checking for
specific values.  You may notice there is no SPEED_20000.

Agreed.

quoted

@@ -542,13 +542,11 @@ static int prb_calc_retire_blk_tmo(struct packet_sock *po,

 ...

This function should be fixed properly.  Firstly, it must use
ethtool_cmd_speed() rather than directly accessing ecmd.speed.
Secondly, it should allow any speed value rather than checking for
specific values.  Then there will be no need to make further changes for
100G or any other new speed.

Agreed.

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Tuesday, June 19, 2012 12:59 PM
To: bhutchings@solarflare.com
Cc: Pandit, Parav; netdev@vger.kernel.org
Subject: Re: [PATCH] net: added support for 40GbE link.

From: Ben Hutchings <redacted>
Date: Mon, 18 Jun 2012 18:09:36 +0100

quoted

On Mon, 2012-06-18 at 18:14 +0530, Parav Pandit wrote:

...

quoted

quoted

-/* The forced speed, 10Mb, 100Mb, gigabit, 2.5Gb, 10GbE. */
+/* The forced speed, 10Mb, 100Mb, gigabit, 2.5Gb, 10GbE, 40GbE. */

I don't think there's any need to name all possible link speeds, and
it just encourages the bad practice of ethtool API users checking for
specific values.  You may notice there is no SPEED_20000.

Agreed.

Should eventually all net driver should remove using SPEED_xxxxxx and start using hard coded value of 10, 100, 1000, 20000?

quoted

quoted

@@ -542,13 +542,11 @@ static int prb_calc_retire_blk_tmo(struct

packet_sock *po,

 ...

quoted

This function should be fixed properly.  Firstly, it must use
ethtool_cmd_speed() rather than directly accessing ecmd.speed.
Secondly, it should allow any speed value rather than checking for
specific values.  Then there will be no need to make further changes
for 100G or any other new speed.

Agreed.

That means ethtool_cmd_speed() should not be called in this function?

If I understand correctly, it should return the value of 8ms (for 10Mb,s 100Mbps, 2.5 Gbps, 20Gbps) as today and remaining it should return calculated value?
Or
Function needs a fix for all these speeds (10Mbps, 100Mbs, 20Gbps too)?

From: <redacted>
Date: Tue, 19 Jun 2012 07:33:12 +0000

Should eventually all net driver should remove using SPEED_xxxxxx and start using hard coded value of 10, 100, 1000, 20000?

No, the ones that exist can stay, just no new ones.

That means ethtool_cmd_speed() should not be called in this function?

Ben said that it must be called, what are you talking about?

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Tuesday, June 19, 2012 1:05 PM
To: Pandit, Parav
Cc: bhutchings@solarflare.com; netdev@vger.kernel.org
Subject: Re: [PATCH] net: added support for 40GbE link.

From: <redacted>
Date: Tue, 19 Jun 2012 07:33:12 +0000

quoted

Should eventually all net driver should remove using SPEED_xxxxxx and

start using hard coded value of 10, 100, 1000, 20000?

No, the ones that exist can stay, just no new ones.

So driver which supports 40Gpbs, 100Gbps should hardcode to 40000, 100000 respectively?

quoted

That means ethtool_cmd_speed() should not be called in this function?

Ben said that it must be called, what are you talking about?

Sorry, I wanted to ask - Do you need switch case for speed like below new code or its should be speed independent code?
                switch (ethtool_cmd_speed()) {
                case SPEED_100:
                case SPEED_10:
                        return DEFAULT_PRB_RETIRE_TOV;
                default:
                        msec = 1;
                        div = ethtool_cmd_speed() / 1000;
                        break;
                /*
                }

On Tue, 2012-06-19 at 07:42 +0000, Parav.Pandit@Emulex.Com wrote:

quoted

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Tuesday, June 19, 2012 1:05 PM
To: Pandit, Parav
Cc: bhutchings@solarflare.com; netdev@vger.kernel.org
Subject: Re: [PATCH] net: added support for 40GbE link.

From: <redacted>
Date: Tue, 19 Jun 2012 07:33:12 +0000

quoted

Should eventually all net driver should remove using SPEED_xxxxxx and

start using hard coded value of 10, 100, 1000, 20000?

No, the ones that exist can stay, just no new ones.

So driver which supports 40Gpbs, 100Gbps should hardcode to 40000, 100000 respectively?

Right.

quoted

quoted

That means ethtool_cmd_speed() should not be called in this function?

Ben said that it must be called, what are you talking about?

Sorry, I wanted to ask - Do you need switch case for speed like below new code or its should be speed independent code?
                switch (ethtool_cmd_speed()) {
                case SPEED_100:
                case SPEED_10:
                        return DEFAULT_PRB_RETIRE_TOV;
                default:
                        msec = 1;
                        div = ethtool_cmd_speed() / 1000;
                        break;
                /*
                }

I was thinking of something like:

		u64 speed = ethtool_cmd_speed(&ecmd);
		if (speed < 1000 || speed == SPEED_UNKNOWN)
			return DEFAULT_PRB_RETIRE_TOV;
		msec = 1;
		div = speed / 1000;

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

o.k. I am sending PATCH v1 with suggested fixes in short while for ethtool and kernel both.

Parav

-----Original Message-----
From: Ben Hutchings [mailto:bhutchings@solarflare.com]
Sent: Tuesday, June 19, 2012 7:42 PM
To: Pandit, Parav
Cc: davem@davemloft.net; netdev@vger.kernel.org
Subject: RE: [PATCH] net: added support for 40GbE link.

On Tue, 2012-06-19 at 07:42 +0000, Parav.Pandit@Emulex.Com wrote:

quoted

quoted

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Tuesday, June 19, 2012 1:05 PM
To: Pandit, Parav
Cc: bhutchings@solarflare.com; netdev@vger.kernel.org
Subject: Re: [PATCH] net: added support for 40GbE link.

From: <redacted>
Date: Tue, 19 Jun 2012 07:33:12 +0000

quoted

Should eventually all net driver should remove using SPEED_xxxxxx
and

start using hard coded value of 10, 100, 1000, 20000?

No, the ones that exist can stay, just no new ones.

So driver which supports 40Gpbs, 100Gbps should hardcode to 40000,

100000 respectively?

Right.

quoted

quoted

quoted

That means ethtool_cmd_speed() should not be called in this function?

Ben said that it must be called, what are you talking about?

Sorry, I wanted to ask - Do you need switch case for speed like below new

code or its should be speed independent code?

quoted

                switch (ethtool_cmd_speed()) {
                case SPEED_100:
                case SPEED_10:
                        return DEFAULT_PRB_RETIRE_TOV;
                default:
                        msec = 1;
                        div = ethtool_cmd_speed() / 1000;
                        break;
                /*
                }

I was thinking of something like:

		u64 speed = ethtool_cmd_speed(&ecmd);
		if (speed < 1000 || speed == SPEED_UNKNOWN)
			return DEFAULT_PRB_RETIRE_TOV;
		msec = 1;
		div = speed / 1000;

Ben.

--
Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer;
that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.