[PATCH 1/1] ethtool: Expose MDI-X status

[PATCH 1/1] ethtool: Expose MDI-X status

[Chaitanya Lala]

The MDI-X status is a useful tool for diagnosing network
connectivity issues. We expose MDI-X status as a tri-state value
status which drivers can optionally implement.

Signed-off-by: Chaitanya Lala <clala@riverbed.com>
Signed-off-by: Arthur Jones <ajones@riverbed.com>
---
 ethtool-copy.h |    6 ++++++
 ethtool.c      |   13 +++++++++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 3ca4e2c..37bbeaf 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -23,6 +23,7 @@ struct ethtool_cmd {
 	__u8	phy_address;
 	__u8	transceiver;	/* Which transceiver to use */
 	__u8	autoneg;	/* Enable or disable autonegotiation */
+	__u8	is_mdix;
 	__u32	maxtxpkt;	/* Tx pkts before generating tx int */
 	__u32	maxrxpkt;	/* Rx pkts before generating rx int */
 	__u16	speed_hi;
@@ -416,6 +417,11 @@ struct ethtool_rxnfc {
 #define AUTONEG_DISABLE		0x00
 #define AUTONEG_ENABLE		0x01
 
+/* Mode MDI or MDI-X */
+#define MDI_INVALID 0x00
+#define MDI         0x01
+#define MDI_X       0x02
+
 /* Wake-On-Lan options. */
 #define WAKE_PHY		(1 << 0)
 #define WAKE_UCAST		(1 << 1)
diff --git a/ethtool.c b/ethtool.c
index 0110682..bf12168 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -964,6 +964,19 @@ static int dump_ecmd(struct ethtool_cmd *ep)
 	fprintf(stdout, "	Auto-negotiation: %s\n",
 		(ep->autoneg == AUTONEG_DISABLE) ?
 		"off" : "on");
+
+	switch (ep->is_mdix) {
+	case MDI:
+		fprintf(stdout, "	MDI-X: off\n");
+		break;
+	case MDI_X:
+		fprintf(stdout, "	MDI-X: on\n");
+		break;
+	default:
+		fprintf(stdout, "	MDI-X: Unknown\n");
+		break;
+	}
+
 	return 0;
 }
 
-- 
1.6.0.4

Re: [PATCH 1/1] ethtool: Expose MDI-X status

[Stephen Hemminger]

On Fri, 22 May 2009 07:54:26 -0700
Chaitanya Lala <clala@riverbed.com> wrote:

> The MDI-X status is a useful tool for diagnosing network
> connectivity issues. We expose MDI-X status as a tri-state value
> status which drivers can optionally implement.
> 
> Signed-off-by: Chaitanya Lala <clala@riverbed.com>
> Signed-off-by: Arthur Jones <ajones@riverbed.com>
> ---
>  ethtool-copy.h |    6 ++++++
>  ethtool.c      |   13 +++++++++++++
>  2 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/ethtool-copy.h b/ethtool-copy.h
> index 3ca4e2c..37bbeaf 100644
> --- a/ethtool-copy.h
> +++ b/ethtool-copy.h
> @@ -23,6 +23,7 @@ struct ethtool_cmd {
>  	__u8	phy_address;
>  	__u8	transceiver;	/* Which transceiver to use */
>  	__u8	autoneg;	/* Enable or disable autonegotiation */
> +	__u8	is_mdix;
>  	__u32	maxtxpkt;	/* Tx pkts before generating tx int */
>  	__u32	maxrxpkt;	/* Rx pkts before generating rx int */
>  	__u16	speed_hi;
>

Is this an ABI change (or are you just reusing a pad hole)?

-- 

Re: [PATCH 1/1] ethtool: Expose MDI-X status

[Ben Hutchings]

On Fri, 2009-05-22 at 07:54 -0700, Chaitanya Lala wrote:
> The MDI-X status is a useful tool for diagnosing network
> connectivity issues. We expose MDI-X status as a tri-state value
> status which drivers can optionally implement.
> 
> Signed-off-by: Chaitanya Lala <clala@riverbed.com>
> Signed-off-by: Arthur Jones <ajones@riverbed.com>
> ---
>  ethtool-copy.h |    6 ++++++
>  ethtool.c      |   13 +++++++++++++
>  2 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/ethtool-copy.h b/ethtool-copy.h
> index 3ca4e2c..37bbeaf 100644
> --- a/ethtool-copy.h
> +++ b/ethtool-copy.h
> @@ -23,6 +23,7 @@ struct ethtool_cmd {
>  	__u8	phy_address;
>  	__u8	transceiver;	/* Which transceiver to use */
>  	__u8	autoneg;	/* Enable or disable autonegotiation */
> +	__u8	is_mdix;
[...]

I have already filled this hole in the structure.  See Dave's
net-next-2.6 repository and my previously submitted changes to ethtool
<http://thread.gmane.org/gmane.linux.network/127025>.

Ben.

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

[net-next-2.6 PATCH 1/1] e1000e: Expose MDI-X status via ethtool change

[Chaitanya Lala]

Ethtool is a standard way of getting information about ethernet
interfaces. We enhance ethtool kernel interface & e1000e to make
the MDI-X status readable via ethtool in userspace.

Signed-off-by: Chaitanya Lala <clala@riverbed.com>
Signed-off-by: Arthur Jones <ajones@riverbed.com>
---
 drivers/net/e1000e/ethtool.c |    8 ++++++++
 include/linux/ethtool.h      |    6 ++++++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
index 4d25ede..8bbade1 100644
--- a/drivers/net/e1000e/ethtool.c
+++ b/drivers/net/e1000e/ethtool.c
@@ -167,6 +167,14 @@ static int e1000_get_settings(struct net_device *netdev,
 
 	ecmd->autoneg = ((hw->phy.media_type == e1000_media_type_fiber) ||
 			 hw->mac.autoneg) ? AUTONEG_ENABLE : AUTONEG_DISABLE;
+
+	/* MDI-X => 2; MDI =>1; Invalid =>0 */
+	if ((hw->phy.media_type == e1000_media_type_copper) &&
+			!hw->mac.get_link_status)
+		ecmd->is_mdix = hw->phy.is_mdix ? MDI_X : MDI;
+	else
+		ecmd->is_mdix = MDI_INVALID;
+
 	return 0;
 }
 
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 380b042..0533d70 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -26,6 +26,7 @@ struct ethtool_cmd {
 	__u8	phy_address;
 	__u8	transceiver;	/* Which transceiver to use */
 	__u8	autoneg;	/* Enable or disable autonegotiation */
+	__u8	is_mdix;	/* MDI-X status tri-state value */
 	__u8	mdio_support;
 	__u32	maxtxpkt;	/* Tx pkts before generating tx int */
 	__u32	maxrxpkt;	/* Rx pkts before generating rx int */
@@ -632,6 +633,11 @@ struct ethtool_ops {
 #define AUTONEG_DISABLE		0x00
 #define AUTONEG_ENABLE		0x01
 
+/* Mode MDI or MDI-X */
+#define MDI_INVALID	0x00
+#define MDI		0x01
+#define MDI_X		0x02
+
 /* Wake-On-Lan options. */
 #define WAKE_PHY		(1 << 0)
 #define WAKE_UCAST		(1 << 1)
-- 
1.6.0.4

Re: [net-next-2.6 PATCH 1/1] e1000e: Expose MDI-X status via ethtool change

[Ben Hutchings]

On Fri, 2009-05-22 at 07:55 -0700, Chaitanya Lala wrote:
> Ethtool is a standard way of getting information about ethernet
> interfaces. We enhance ethtool kernel interface & e1000e to make
> the MDI-X status readable via ethtool in userspace.
[...] 
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 380b042..0533d70 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -26,6 +26,7 @@ struct ethtool_cmd {
>  	__u8	phy_address;
>  	__u8	transceiver;	/* Which transceiver to use */
>  	__u8	autoneg;	/* Enable or disable autonegotiation */
> +	__u8	is_mdix;	/* MDI-X status tri-state value */
>  	__u8	mdio_support;
[...]

There's no gap here, so this would move all the following fields and
break all older ethtool clients.

Ben.

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

Re: [net-next-2.6 PATCH 1/1] e1000e: Expose MDI-X status via ethtool change

[Chaitanya Lala]

Ben Hutchings wrote:
> On Fri, 2009-05-22 at 07:55 -0700, Chaitanya Lala wrote:
>   
>> Ethtool is a standard way of getting information about ethernet
>> interfaces. We enhance ethtool kernel interface & e1000e to make
>> the MDI-X status readable via ethtool in userspace.
>>     
> [...] 
>   
>> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
>> index 380b042..0533d70 100644
>> --- a/include/linux/ethtool.h
>> +++ b/include/linux/ethtool.h
>> @@ -26,6 +26,7 @@ struct ethtool_cmd {
>>  	__u8	phy_address;
>>  	__u8	transceiver;	/* Which transceiver to use */
>>  	__u8	autoneg;	/* Enable or disable autonegotiation */
>> +	__u8	is_mdix;	/* MDI-X status tri-state value */
>>  	__u8	mdio_support;
>>     
> [...]
>
> There's no gap here, so this would move all the following fields and
> break all older ethtool clients.
>
> Ben.
>
>   
Then should I move it to the bottom ?

Chaitanya

[PATCH 1/1] ethtool: Expose MDI-X status

[Chaitanya Lala]

The MDI-X status is a useful tool for diagnosing network
connectivity issues. We expose MDI-X status as a tri-state value
status which drivers can optionally implement.

Signed-off-by: Chaitanya Lala <clala@riverbed.com>
Signed-off-by: Arthur Jones <ajones@riverbed.com>
---
 ethtool-copy.h |    6 ++++++
 ethtool.c      |   13 +++++++++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 3ca4e2c..d3e1524 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -28,6 +28,7 @@ struct ethtool_cmd {
 	__u16	speed_hi;
 	__u16	reserved2;
 	__u32	reserved[3];
+	__u8	is_mdix;	/* Tri-state value to expose MDI-X */
 };
 
 static inline void ethtool_cmd_speed_set(struct ethtool_cmd *ep,
@@ -416,6 +417,11 @@ struct ethtool_rxnfc {
 #define AUTONEG_DISABLE		0x00
 #define AUTONEG_ENABLE		0x01
 
+/* Mode MDI or MDI-X */
+#define MDI_INVALID 0x00
+#define MDI         0x01
+#define MDI_X       0x02
+
 /* Wake-On-Lan options. */
 #define WAKE_PHY		(1 << 0)
 #define WAKE_UCAST		(1 << 1)
diff --git a/ethtool.c b/ethtool.c
index 0110682..bf12168 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -964,6 +964,19 @@ static int dump_ecmd(struct ethtool_cmd *ep)
 	fprintf(stdout, "	Auto-negotiation: %s\n",
 		(ep->autoneg == AUTONEG_DISABLE) ?
 		"off" : "on");
+
+	switch (ep->is_mdix) {
+	case MDI:
+		fprintf(stdout, "	MDI-X: off\n");
+		break;
+	case MDI_X:
+		fprintf(stdout, "	MDI-X: on\n");
+		break;
+	default:
+		fprintf(stdout, "	MDI-X: Unknown\n");
+		break;
+	}
+
 	return 0;
 }
 
-- 
1.6.0.4

Re: [PATCH 1/1] ethtool: Expose MDI-X status

[Ben Hutchings]

On Fri, 2009-05-22 at 10:18 -0700, Chaitanya Lala wrote:
> The MDI-X status is a useful tool for diagnosing network
> connectivity issues. We expose MDI-X status as a tri-state value
> status which drivers can optionally implement.
> 
> Signed-off-by: Chaitanya Lala <clala@riverbed.com>
> Signed-off-by: Arthur Jones <ajones@riverbed.com>
> ---
>  ethtool-copy.h |    6 ++++++
>  ethtool.c      |   13 +++++++++++++
>  2 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/ethtool-copy.h b/ethtool-copy.h
> index 3ca4e2c..d3e1524 100644
> --- a/ethtool-copy.h
> +++ b/ethtool-copy.h
> @@ -28,6 +28,7 @@ struct ethtool_cmd {
>  	__u16	speed_hi;
>  	__u16	reserved2;
>  	__u32	reserved[3];
> +	__u8	is_mdix;	/* Tri-state value to expose MDI-X */
>  };
[...]

No, you cannot change the size of this structure either.

Ben.

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

Re: [PATCH 1/1] ethtool: Expose MDI-X status

[Chaitanya Lala]

Ben Hutchings wrote:
> On Fri, 2009-05-22 at 10:18 -0700, Chaitanya Lala wrote:
>   
>> The MDI-X status is a useful tool for diagnosing network
>> connectivity issues. We expose MDI-X status as a tri-state value
>> status which drivers can optionally implement.
>>
>> Signed-off-by: Chaitanya Lala <clala@riverbed.com>
>> Signed-off-by: Arthur Jones <ajones@riverbed.com>
>> ---
>>  ethtool-copy.h |    6 ++++++
>>  ethtool.c      |   13 +++++++++++++
>>  2 files changed, 19 insertions(+), 0 deletions(-)
>>
>> diff --git a/ethtool-copy.h b/ethtool-copy.h
>> index 3ca4e2c..d3e1524 100644
>> --- a/ethtool-copy.h
>> +++ b/ethtool-copy.h
>> @@ -28,6 +28,7 @@ struct ethtool_cmd {
>>  	__u16	speed_hi;
>>  	__u16	reserved2;
>>  	__u32	reserved[3];
>> +	__u8	is_mdix;	/* Tri-state value to expose MDI-X */
>>  };
>>     
> [...]
>
> No, you cannot change the size of this structure either.
>   
Hi,

In that case please suggest a way to do this.

Thanks,
Chaitanya
> Ben.
>
>   

Re: [PATCH 1/1] ethtool: Expose MDI-X status

[Ben Hutchings]

On Tue, 2009-05-26 at 08:34 -0700, Chaitanya Lala wrote:
> Ben Hutchings wrote:
> > On Fri, 2009-05-22 at 10:18 -0700, Chaitanya Lala wrote:
> >   
> >> The MDI-X status is a useful tool for diagnosing network
> >> connectivity issues. We expose MDI-X status as a tri-state value
> >> status which drivers can optionally implement.
> >>
> >> Signed-off-by: Chaitanya Lala <clala@riverbed.com>
> >> Signed-off-by: Arthur Jones <ajones@riverbed.com>
> >> ---
> >>  ethtool-copy.h |    6 ++++++
> >>  ethtool.c      |   13 +++++++++++++
> >>  2 files changed, 19 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/ethtool-copy.h b/ethtool-copy.h
> >> index 3ca4e2c..d3e1524 100644
> >> --- a/ethtool-copy.h
> >> +++ b/ethtool-copy.h
> >> @@ -28,6 +28,7 @@ struct ethtool_cmd {
> >>  	__u16	speed_hi;
> >>  	__u16	reserved2;
> >>  	__u32	reserved[3];
> >> +	__u8	is_mdix;	/* Tri-state value to expose MDI-X */
> >>  };
> >>     
> > [...]
> >
> > No, you cannot change the size of this structure either.
> >   
> Hi,
> 
> In that case please suggest a way to do this.

There are 14 bytes at the end of the structure reserved for future
expansion.  You could take one of them.

Ben.

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

[PATCH 1/1] ethtool: Expose MDI-X status

[Chaitanya Lala]

The MDI-X status is a useful tool for diagnosing network
connectivity issues. We expose MDI-X status as a tri-state value
status which drivers can optionally implement.

Signed-off-by: Chaitanya Lala <clala@riverbed.com>
Signed-off-by: Arthur Jones <ajones@riverbed.com>
---
 ethtool-copy.h |    8 +++++++-
 ethtool.c      |   13 +++++++++++++
 2 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 3ca4e2c..48fb1d3 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -26,7 +26,8 @@ struct ethtool_cmd {
 	__u32	maxtxpkt;	/* Tx pkts before generating tx int */
 	__u32	maxrxpkt;	/* Rx pkts before generating rx int */
 	__u16	speed_hi;
-	__u16	reserved2;
+	__u8	is_mdix;
+	__u8	reserved2;
 	__u32	reserved[3];
 };
 
@@ -416,6 +417,11 @@ struct ethtool_rxnfc {
 #define AUTONEG_DISABLE		0x00
 #define AUTONEG_ENABLE		0x01
 
+/* Mode MDI or MDI-X */
+#define MDI_INVALID 0x00
+#define MDI         0x01
+#define MDI_X       0x02
+
 /* Wake-On-Lan options. */
 #define WAKE_PHY		(1 << 0)
 #define WAKE_UCAST		(1 << 1)
diff --git a/ethtool.c b/ethtool.c
index 0110682..bf12168 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -964,6 +964,19 @@ static int dump_ecmd(struct ethtool_cmd *ep)
 	fprintf(stdout, "	Auto-negotiation: %s\n",
 		(ep->autoneg == AUTONEG_DISABLE) ?
 		"off" : "on");
+
+	switch (ep->is_mdix) {
+	case MDI:
+		fprintf(stdout, "	MDI-X: off\n");
+		break;
+	case MDI_X:
+		fprintf(stdout, "	MDI-X: on\n");
+		break;
+	default:
+		fprintf(stdout, "	MDI-X: Unknown\n");
+		break;
+	}
+
 	return 0;
 }
 
-- 
1.6.0.4

[PATCH net-next-2.6 1/1] e1000e: Expose MDI-X status via ethtool change

[Chaitanya Lala]

Ethtool is a standard way of getting information about ethernet
interfaces. We enhance ethtool kernel interface & e1000e to make
the MDI-X status readable via ethtool in userspace.

Signed-off-by: Chaitanya Lala <clala@riverbed.com>
Signed-off-by: Arthur Jones <ajones@riverbed.com>
---
 drivers/net/e1000e/ethtool.c |    8 ++++++++
 include/linux/ethtool.h      |    8 +++++++-
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
index 4d25ede..b6aea6b 100644
--- a/drivers/net/e1000e/ethtool.c
+++ b/drivers/net/e1000e/ethtool.c
@@ -167,6 +167,14 @@ static int e1000_get_settings(struct net_device *netdev,
 
 	ecmd->autoneg = ((hw->phy.media_type == e1000_media_type_fiber) ||
 			 hw->mac.autoneg) ? AUTONEG_ENABLE : AUTONEG_DISABLE;
+
+	/* MDI-X => 2; MDI =>1; Invalid =>0 */
+	if ((hw->phy.media_type == e1000_media_type_copper) &&
+		!hw->mac.get_link_status)
+		ecmd->is_mdix = hw->phy.is_mdix ? MDI_X : MDI;
+	else
+		ecmd->is_mdix = MDI_INVALID;
+
 	return 0;
 }
 
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 380b042..b553bdb 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -30,7 +30,8 @@ struct ethtool_cmd {
 	__u32	maxtxpkt;	/* Tx pkts before generating tx int */
 	__u32	maxrxpkt;	/* Rx pkts before generating rx int */
 	__u16	speed_hi;
-	__u16	reserved2;
+	__u8	is_mdix;
+	__u8	reserved2;
 	__u32	lp_advertising;	/* Features the link partner advertises */
 	__u32	reserved[2];
 };
@@ -632,6 +633,11 @@ struct ethtool_ops {
 #define AUTONEG_DISABLE		0x00
 #define AUTONEG_ENABLE		0x01
 
+/* Mode MDI or MDI-X */
+#define MDI_INVALID 0x00
+#define MDI         0x01
+#define MDI_X       0x02
+
 /* Wake-On-Lan options. */
 #define WAKE_PHY		(1 << 0)
 #define WAKE_UCAST		(1 << 1)
-- 
1.6.0.4

Re: [PATCH net-next-2.6 1/1] e1000e: Expose MDI-X status via ethtool change

[Jeff Kirsher]

On Wed, May 27, 2009 at 4:15 PM, Chaitanya Lala <clala@riverbed.com> wrote:
> Ethtool is a standard way of getting information about ethernet
> interfaces. We enhance ethtool kernel interface & e1000e to make
> the MDI-X status readable via ethtool in userspace.
>
> Signed-off-by: Chaitanya Lala <clala@riverbed.com>
> Signed-off-by: Arthur Jones <ajones@riverbed.com>
> ---
>  drivers/net/e1000e/ethtool.c |    8 ++++++++
>  include/linux/ethtool.h      |    8 +++++++-
>  2 files changed, 15 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
> index 4d25ede..b6aea6b 100644
> --- a/drivers/net/e1000e/ethtool.c
> +++ b/drivers/net/e1000e/ethtool.c
> @@ -167,6 +167,14 @@ static int e1000_get_settings(struct net_device *netdev,
>
>        ecmd->autoneg = ((hw->phy.media_type == e1000_media_type_fiber) ||
>                         hw->mac.autoneg) ? AUTONEG_ENABLE : AUTONEG_DISABLE;
> +
> +       /* MDI-X => 2; MDI =>1; Invalid =>0 */
> +       if ((hw->phy.media_type == e1000_media_type_copper) &&
> +               !hw->mac.get_link_status)
> +               ecmd->is_mdix = hw->phy.is_mdix ? MDI_X : MDI;
> +       else
> +               ecmd->is_mdix = MDI_INVALID;
> +
>        return 0;
>  }
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 380b042..b553bdb 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -30,7 +30,8 @@ struct ethtool_cmd {
>        __u32   maxtxpkt;       /* Tx pkts before generating tx int */
>        __u32   maxrxpkt;       /* Rx pkts before generating rx int */
>        __u16   speed_hi;
> -       __u16   reserved2;
> +       __u8    is_mdix;
> +       __u8    reserved2;
>        __u32   lp_advertising; /* Features the link partner advertises */
>        __u32   reserved[2];
>  };
> @@ -632,6 +633,11 @@ struct ethtool_ops {
>  #define AUTONEG_DISABLE                0x00
>  #define AUTONEG_ENABLE         0x01
>
> +/* Mode MDI or MDI-X */
> +#define MDI_INVALID 0x00
> +#define MDI         0x01
> +#define MDI_X       0x02
> +
>  /* Wake-On-Lan options. */
>  #define WAKE_PHY               (1 << 0)
>  #define WAKE_UCAST             (1 << 1)
> --
> 1.6.0.4
>

I have pulled this into my queue for testing and submission, thanks.

-- 
Cheers,
Jeff

Re: [PATCH net-next-2.6 1/1] e1000e: Expose MDI-X status via ethtool change

[Ben Hutchings]

On Wed, 2009-05-27 at 16:15 -0700, Chaitanya Lala wrote:
[...]
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 380b042..b553bdb 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -30,7 +30,8 @@ struct ethtool_cmd {
>  	__u32	maxtxpkt;	/* Tx pkts before generating tx int */
>  	__u32	maxrxpkt;	/* Rx pkts before generating rx int */
>  	__u16	speed_hi;
> -	__u16	reserved2;
> +	__u8	is_mdix;

Since this is specific to Ethernet over twisted-pair cable, could you
please rename this to "eth_tp_mdix".

> +	__u8	reserved2;
>  	__u32	lp_advertising;	/* Features the link partner advertises */
>  	__u32	reserved[2];
>  };
> @@ -632,6 +633,11 @@ struct ethtool_ops {
>  #define AUTONEG_DISABLE		0x00
>  #define AUTONEG_ENABLE		0x01
>  
> +/* Mode MDI or MDI-X */
> +#define MDI_INVALID 0x00
> +#define MDI         0x01
> +#define MDI_X       0x02
[...]

Similarly, please add the prefix "ETH_TP_" to these.

Ben.

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

Re: [PATCH net-next-2.6 1/1] e1000e: Expose MDI-X status via ethtool change

[Chaitanya Lala]

Ben Hutchings wrote:
> On Wed, 2009-05-27 at 16:15 -0700, Chaitanya Lala wrote:
> [...]
>   
>> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
>> index 380b042..b553bdb 100644
>> --- a/include/linux/ethtool.h
>> +++ b/include/linux/ethtool.h
>> @@ -30,7 +30,8 @@ struct ethtool_cmd {
>>  	__u32	maxtxpkt;	/* Tx pkts before generating tx int */
>>  	__u32	maxrxpkt;	/* Rx pkts before generating rx int */
>>  	__u16	speed_hi;
>> -	__u16	reserved2;
>> +	__u8	is_mdix;
>>     
>
> Since this is specific to Ethernet over twisted-pair cable, could you
> please rename this to "eth_tp_mdix".
>   
Will do for sure.
>   
>> +	__u8	reserved2;
>>  	__u32	lp_advertising;	/* Features the link partner advertises */
>>  	__u32	reserved[2];
>>  };
>> @@ -632,6 +633,11 @@ struct ethtool_ops {
>>  #define AUTONEG_DISABLE		0x00
>>  #define AUTONEG_ENABLE		0x01
>>  
>> +/* Mode MDI or MDI-X */
>> +#define MDI_INVALID 0x00
>> +#define MDI         0x01
>> +#define MDI_X       0x02
>>     
> [...]
>
> Similarly, please add the prefix "ETH_TP_" to these.
>   
Sure.
> Ben.
>
>   

Re: [PATCH net-next-2.6 1/1] e1000e: Expose MDI-X status via ethtool change

[Jeff Kirsher]

On Wed, Jun 3, 2009 at 10:45 AM, Chaitanya Lala <clala@riverbed.com> wrote:
> Ben Hutchings wrote:
>>
>> On Wed, 2009-05-27 at 16:15 -0700, Chaitanya Lala wrote:
>> [...]
>>
>>>
>>> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
>>> index 380b042..b553bdb 100644
>>> --- a/include/linux/ethtool.h
>>> +++ b/include/linux/ethtool.h
>>> @@ -30,7 +30,8 @@ struct ethtool_cmd {
>>>        __u32   maxtxpkt;       /* Tx pkts before generating tx int */
>>>        __u32   maxrxpkt;       /* Rx pkts before generating rx int */
>>>        __u16   speed_hi;
>>> -       __u16   reserved2;
>>> +       __u8    is_mdix;
>>>
>>
>> Since this is specific to Ethernet over twisted-pair cable, could you
>> please rename this to "eth_tp_mdix".
>>
>
> Will do for sure.
>>
>>
>>>
>>> +       __u8    reserved2;
>>>        __u32   lp_advertising; /* Features the link partner advertises */
>>>        __u32   reserved[2];
>>>  };
>>> @@ -632,6 +633,11 @@ struct ethtool_ops {
>>>  #define AUTONEG_DISABLE                0x00
>>>  #define AUTONEG_ENABLE         0x01
>>>  +/* Mode MDI or MDI-X */
>>> +#define MDI_INVALID 0x00
>>> +#define MDI         0x01
>>> +#define MDI_X       0x02
>>>
>>
>> [...]
>>
>> Similarly, please add the prefix "ETH_TP_" to these.
>>
>
> Sure.

Chaitanya, I have made the requested changes to the patch in my tree.

-- 
Cheers,
Jeff

Re: [PATCH net-next-2.6 1/1] e1000e: Expose MDI-X status via ethtool change

[Chaitanya Lala]

Jeff Kirsher wrote:
> On Wed, Jun 3, 2009 at 10:45 AM, Chaitanya Lala <clala@riverbed.com> wrote:
>   
>> Ben Hutchings wrote:
>>     
>>> On Wed, 2009-05-27 at 16:15 -0700, Chaitanya Lala wrote:
>>> [...]
>>>
>>>       
>>>> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
>>>> index 380b042..b553bdb 100644
>>>> --- a/include/linux/ethtool.h
>>>> +++ b/include/linux/ethtool.h
>>>> @@ -30,7 +30,8 @@ struct ethtool_cmd {
>>>>        __u32   maxtxpkt;       /* Tx pkts before generating tx int */
>>>>        __u32   maxrxpkt;       /* Rx pkts before generating rx int */
>>>>        __u16   speed_hi;
>>>> -       __u16   reserved2;
>>>> +       __u8    is_mdix;
>>>>
>>>>         
>>> Since this is specific to Ethernet over twisted-pair cable, could you
>>> please rename this to "eth_tp_mdix".
>>>
>>>       
>> Will do for sure.
>>     
>>>       
>>>> +       __u8    reserved2;
>>>>        __u32   lp_advertising; /* Features the link partner advertises */
>>>>        __u32   reserved[2];
>>>>  };
>>>> @@ -632,6 +633,11 @@ struct ethtool_ops {
>>>>  #define AUTONEG_DISABLE                0x00
>>>>  #define AUTONEG_ENABLE         0x01
>>>>  +/* Mode MDI or MDI-X */
>>>> +#define MDI_INVALID 0x00
>>>> +#define MDI         0x01
>>>> +#define MDI_X       0x02
>>>>
>>>>         
>>> [...]
>>>
>>> Similarly, please add the prefix "ETH_TP_" to these.
>>>
>>>       
>> Sure.
>>     
>
> Chaitanya, I have made the requested changes to the patch in my tree.
>
>   
Thanks. I am very new to procedure of submitting kernel patches. Just 
want to make sure if this means that I not need to send another patch to 
netdev ?

Thanks,
Chaitanya

Re: [PATCH net-next-2.6 1/1] e1000e: Expose MDI-X status via ethtool change

[Jeff Kirsher]

On Wed, Jun 3, 2009 at 2:19 PM, Chaitanya Lala <clala@riverbed.com> wrote:
> Jeff Kirsher wrote:
>>
>> On Wed, Jun 3, 2009 at 10:45 AM, Chaitanya Lala <clala@riverbed.com>
>> wrote:
>>
>>>
>>> Ben Hutchings wrote:
>>>
>>>>
>>>> On Wed, 2009-05-27 at 16:15 -0700, Chaitanya Lala wrote:
>>>> [...]
>>>>
>>>>
>>>>>
>>>>> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
>>>>> index 380b042..b553bdb 100644
>>>>> --- a/include/linux/ethtool.h
>>>>> +++ b/include/linux/ethtool.h
>>>>> @@ -30,7 +30,8 @@ struct ethtool_cmd {
>>>>>       __u32   maxtxpkt;       /* Tx pkts before generating tx int */
>>>>>       __u32   maxrxpkt;       /* Rx pkts before generating rx int */
>>>>>       __u16   speed_hi;
>>>>> -       __u16   reserved2;
>>>>> +       __u8    is_mdix;
>>>>>
>>>>>
>>>>
>>>> Since this is specific to Ethernet over twisted-pair cable, could you
>>>> please rename this to "eth_tp_mdix".
>>>>
>>>>
>>>
>>> Will do for sure.
>>>
>>>>
>>>>
>>>>>
>>>>> +       __u8    reserved2;
>>>>>       __u32   lp_advertising; /* Features the link partner advertises
>>>>> */
>>>>>       __u32   reserved[2];
>>>>>  };
>>>>> @@ -632,6 +633,11 @@ struct ethtool_ops {
>>>>>  #define AUTONEG_DISABLE                0x00
>>>>>  #define AUTONEG_ENABLE         0x01
>>>>>  +/* Mode MDI or MDI-X */
>>>>> +#define MDI_INVALID 0x00
>>>>> +#define MDI         0x01
>>>>> +#define MDI_X       0x02
>>>>>
>>>>>
>>>>
>>>> [...]
>>>>
>>>> Similarly, please add the prefix "ETH_TP_" to these.
>>>>
>>>>
>>>
>>> Sure.
>>>
>>
>> Chaitanya, I have made the requested changes to the patch in my tree.
>>
>>
>
> Thanks. I am very new to procedure of submitting kernel patches. Just want
> to make sure if this means that I not need to send another patch to netdev ?
>
> Thanks,
> Chaitanya
> --

When there are changes requested on a patch, yes the patch needs to be
re-submitted to netdev.  Since I will be pushing this patch along with
other patches for Intel drivers to Dave once testing has been
completed, there is no need for you to re-submit this patch.  I have
already sucked in the changes that Ben has requested into the patch
that is in my tree.

-- 
Cheers,
Jeff

Re: [PATCH net-next-2.6 1/1] e1000e: Expose MDI-X status via ethtool change

[Chaitanya Lala]

Jeff Kirsher wrote:
> On Wed, Jun 3, 2009 at 2:19 PM, Chaitanya Lala <clala@riverbed.com> wrote:
>   
>> Jeff Kirsher wrote:
>>     
>>> On Wed, Jun 3, 2009 at 10:45 AM, Chaitanya Lala <clala@riverbed.com>
>>> wrote:
>>>
>>>       
>>>> Ben Hutchings wrote:
>>>>
>>>>         
>>>>> On Wed, 2009-05-27 at 16:15 -0700, Chaitanya Lala wrote:
>>>>> [...]
>>>>>
>>>>>
>>>>>           
>>>>>> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
>>>>>> index 380b042..b553bdb 100644
>>>>>> --- a/include/linux/ethtool.h
>>>>>> +++ b/include/linux/ethtool.h
>>>>>> @@ -30,7 +30,8 @@ struct ethtool_cmd {
>>>>>>       __u32   maxtxpkt;       /* Tx pkts before generating tx int */
>>>>>>       __u32   maxrxpkt;       /* Rx pkts before generating rx int */
>>>>>>       __u16   speed_hi;
>>>>>> -       __u16   reserved2;
>>>>>> +       __u8    is_mdix;
>>>>>>
>>>>>>
>>>>>>             
>>>>> Since this is specific to Ethernet over twisted-pair cable, could you
>>>>> please rename this to "eth_tp_mdix".
>>>>>
>>>>>
>>>>>           
>>>> Will do for sure.
>>>>
>>>>         
>>>>>           
>>>>>> +       __u8    reserved2;
>>>>>>       __u32   lp_advertising; /* Features the link partner advertises
>>>>>> */
>>>>>>       __u32   reserved[2];
>>>>>>  };
>>>>>> @@ -632,6 +633,11 @@ struct ethtool_ops {
>>>>>>  #define AUTONEG_DISABLE                0x00
>>>>>>  #define AUTONEG_ENABLE         0x01
>>>>>>  +/* Mode MDI or MDI-X */
>>>>>> +#define MDI_INVALID 0x00
>>>>>> +#define MDI         0x01
>>>>>> +#define MDI_X       0x02
>>>>>>
>>>>>>
>>>>>>             
>>>>> [...]
>>>>>
>>>>> Similarly, please add the prefix "ETH_TP_" to these.
>>>>>
>>>>>
>>>>>           
>>>> Sure.
>>>>
>>>>         
>>> Chaitanya, I have made the requested changes to the patch in my tree.
>>>
>>>
>>>       
>> Thanks. I am very new to procedure of submitting kernel patches. Just want
>> to make sure if this means that I not need to send another patch to netdev ?
>>
>> Thanks,
>> Chaitanya
>> --
>>     
>
> When there are changes requested on a patch, yes the patch needs to be
> re-submitted to netdev.  Since I will be pushing this patch along with
> other patches for Intel drivers to Dave once testing has been
> completed, there is no need for you to re-submit this patch.  I have
> already sucked in the changes that Ben has requested into the patch
> that is in my tree.
>
>   
Thanks for the clarification.

Chaitanya

Re-submit [PATCH 1/1] ethtool: Expose MDI-X status

[Chaitanya Lala]

The MDI-X status is a useful tool for diagnosing network
connectivity issues. We expose MDI-X status as a tri-state value
status which drivers can optionally implement.

Signed-off-by: Chaitanya Lala <clala@riverbed.com>
Signed-off-by: Arthur Jones <ajones@riverbed.com>
---
 ethtool-copy.h |    8 +++++++-
 ethtool.c      |   13 +++++++++++++
 2 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 3ca4e2c..48fb1d3 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -26,7 +26,8 @@ struct ethtool_cmd {
 	__u32	maxtxpkt;	/* Tx pkts before generating tx int */
 	__u32	maxrxpkt;	/* Rx pkts before generating rx int */
 	__u16	speed_hi;
-	__u16	reserved2;
+	__u8	is_mdix;
+	__u8	reserved2;
 	__u32	reserved[3];
 };
 
@@ -416,6 +417,11 @@ struct ethtool_rxnfc {
 #define AUTONEG_DISABLE		0x00
 #define AUTONEG_ENABLE		0x01
 
+/* Mode MDI or MDI-X */
+#define MDI_INVALID 0x00
+#define MDI         0x01
+#define MDI_X       0x02
+
 /* Wake-On-Lan options. */
 #define WAKE_PHY		(1 << 0)
 #define WAKE_UCAST		(1 << 1)
diff --git a/ethtool.c b/ethtool.c
index 0110682..bf12168 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -964,6 +964,19 @@ static int dump_ecmd(struct ethtool_cmd *ep)
 	fprintf(stdout, "	Auto-negotiation: %s\n",
 		(ep->autoneg == AUTONEG_DISABLE) ?
 		"off" : "on");
+
+	switch (ep->is_mdix) {
+	case MDI:
+		fprintf(stdout, "	MDI-X: off\n");
+		break;
+	case MDI_X:
+		fprintf(stdout, "	MDI-X: on\n");
+		break;
+	default:
+		fprintf(stdout, "	MDI-X: Unknown\n");
+		break;
+	}
+
 	return 0;
 }
 
-- 
1.6.0.4

Re: Re-submit [PATCH 1/1] ethtool: Expose MDI-X status

[Ben Hutchings]

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

On Wed, 2009-06-10 at 12:11 -0700, Chaitanya Lala wrote:
> The MDI-X status is a useful tool for diagnosing network
> connectivity issues. We expose MDI-X status as a tri-state value
> status which drivers can optionally implement.
> 
> Signed-off-by: Chaitanya Lala <clala@riverbed.com>
> Signed-off-by: Arthur Jones <ajones@riverbed.com>
> ---
>  ethtool-copy.h |    8 +++++++-
>  ethtool.c      |   13 +++++++++++++
>  2 files changed, 20 insertions(+), 1 deletions(-)
> 
> diff --git a/ethtool-copy.h b/ethtool-copy.h
> index 3ca4e2c..48fb1d3 100644
> --- a/ethtool-copy.h
> +++ b/ethtool-copy.h
> @@ -26,7 +26,8 @@ struct ethtool_cmd {
>  	__u32	maxtxpkt;	/* Tx pkts before generating tx int */
>  	__u32	maxrxpkt;	/* Rx pkts before generating rx int */
>  	__u16	speed_hi;
> -	__u16	reserved2;
> +	__u8	is_mdix;
> +	__u8	reserved2;
>  	__u32	reserved[3];
>  };
>  
> @@ -416,6 +417,11 @@ struct ethtool_rxnfc {
>  #define AUTONEG_DISABLE		0x00
>  #define AUTONEG_ENABLE		0x01
>  
> +/* Mode MDI or MDI-X */
> +#define MDI_INVALID 0x00
> +#define MDI         0x01
> +#define MDI_X       0x02
> +
>  /* Wake-On-Lan options. */
>  #define WAKE_PHY		(1 << 0)
>  #define WAKE_UCAST		(1 << 1)
[...]

This doesn't match the changes in <linux/ethtool.h>.  I'm attaching the
patch I applied locally for testing MDI-X reporting in sfc.

Ben.

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

[-- Attachment #2: 0001-ethtool-Expose-MDI-X-status.patch --]
[-- Type: application/mbox, Size: 1883 bytes --]

[net-next-2.6 PATCH 1/1] e1000e: Expose MDI-X status via ethtool change

[Chaitanya Lala]

Ethtool is a standard way of getting information about ethernet
interfaces. We enhance ethtool kernel interface & e1000e to make
the MDI-X status readable via ethtool in userspace.

Signed-off-by: Chaitanya Lala <clala@riverbed.com>
Signed-off-by: Arthur Jones <ajones@riverbed.com>
---
 drivers/net/e1000e/ethtool.c |    8 ++++++++
 include/linux/ethtool.h      |    6 ++++++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
index 4d25ede..8bbade1 100644
--- a/drivers/net/e1000e/ethtool.c
+++ b/drivers/net/e1000e/ethtool.c
@@ -167,6 +167,14 @@ static int e1000_get_settings(struct net_device *netdev,
 
 	ecmd->autoneg = ((hw->phy.media_type == e1000_media_type_fiber) ||
 			 hw->mac.autoneg) ? AUTONEG_ENABLE : AUTONEG_DISABLE;
+
+	/* MDI-X => 2; MDI =>1; Invalid =>0 */
+	if ((hw->phy.media_type == e1000_media_type_copper) &&
+			!hw->mac.get_link_status)
+		ecmd->is_mdix = hw->phy.is_mdix ? MDI_X : MDI;
+	else
+		ecmd->is_mdix = MDI_INVALID;
+
 	return 0;
 }
 
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 380b042..5211f59 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -33,6 +33,7 @@ struct ethtool_cmd {
 	__u16	reserved2;
 	__u32	lp_advertising;	/* Features the link partner advertises */
 	__u32	reserved[2];
+	__u8	is_mdix;	/* MDI-X status tri-state value */
 };
 
 static inline void ethtool_cmd_speed_set(struct ethtool_cmd *ep,
@@ -632,6 +633,11 @@ struct ethtool_ops {
 #define AUTONEG_DISABLE		0x00
 #define AUTONEG_ENABLE		0x01
 
+/* Mode MDI or MDI-X */
+#define MDI_INVALID	0x00
+#define MDI		0x01
+#define MDI_X		0x02
+
 /* Wake-On-Lan options. */
 #define WAKE_PHY		(1 << 0)
 #define WAKE_UCAST		(1 << 1)
-- 
1.6.0.4