[RFC,ETHTOOL,2/2] ethtool: allow setting MDI-X state

A bit ago ethtool added support for reading MDI-X state, this
patch finishes the implementation, adding the complementary write
command.

Add support to ethtool for controlling the MDI-X (crossover)
state of a network port.  Most adapters correctly negotiate
MDI-X, but some ill-behaved switches have trouble and end up
picking the wrong MDI setting, which results in complete loss of
link.  Usually this error condition can be observed when multiple
ethtool -r ethX are required before link is achieved.

This patch allows the user to override the normal "auto" setting
and force the crossover state to on or off.

The set will fail if the driver doesn't support the get, as
suggested by Ben Hutchings.

# ./ethtool -s p1p1 mdix off
setting MDI not supported

In addition the do_gset output was changed slightly to report the
value set by the user (when the driver supports the set)

old:
MDI-X: on

new:
MDI-X: on (auto)
or
MDI-X: on (forced)

usage is ethtool -s eth0 mdix [auto|on|off]

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Ben Hutchings <bhutchings@solarflare.com>
---

 ethtool.8.in |    8 ++++++++
 ethtool.c    |   48 +++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 53 insertions(+), 3 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, 2012-07-25 at 10:53 -0700, Jesse Brandeburg wrote:
> A bit ago ethtool added support for reading MDI-X state, this
> patch finishes the implementation, adding the complementary write
> command.
> 
> Add support to ethtool for controlling the MDI-X (crossover)
> state of a network port.  Most adapters correctly negotiate
> MDI-X, but some ill-behaved switches have trouble and end up
> picking the wrong MDI setting, which results in complete loss of
> link.  Usually this error condition can be observed when multiple
> ethtool -r ethX are required before link is achieved.
> 
> This patch allows the user to override the normal "auto" setting
> and force the crossover state to on or off.
> 
> The set will fail if the driver doesn't support the get, as
> suggested by Ben Hutchings.
> 
> # ./ethtool -s p1p1 mdix off
> setting MDI not supported
> 
> In addition the do_gset output was changed slightly to report the
> value set by the user (when the driver supports the set)
> 
> old:
> MDI-X: on
> 
> new:
> MDI-X: on (auto)
> or
> MDI-X: on (forced)
> 
> usage is ethtool -s eth0 mdix [auto|on|off]
[...]
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -18,6 +18,8 @@
>   * Rx Network Flow Control configuration support <santwona.behera@sun.com>
>   * Various features by Ben Hutchings <bhutchings@solarflare.com>;
>   *	Copyright 2009, 2010 Solarflare Communications
> + * MDI-X set support by Jesse Brandeburg <jesse.brandeburg@intel.com>
> + * 	Copyright 2012 Intel Corporation
>   *
>   * TODO:
>   *   * show settings for all devices
> @@ -559,13 +561,29 @@ static int dump_ecmd(struct ethtool_cmd *ep)
>  		fprintf(stdout, "	MDI-X: ");
>  		switch (ep->eth_tp_mdix) {
>  		case ETH_TP_MDI:
> -			fprintf(stdout, "off\n");
> +			fprintf(stdout, "off");
>  			break;
>  		case ETH_TP_MDI_X:
> -			fprintf(stdout, "on\n");
> +			fprintf(stdout, "on");
>  			break;
>  		default:
> -			fprintf(stdout, "Unknown\n");
> +			fprintf(stdout, "Unknown");
> +			break;
> +		}
> +		switch (ep->eth_tp_mdix_ctrl) {
> +		case ETH_TP_MDI:
> +		case ETH_TP_MDI_X:
> +			/* forced via sset */
> +			fprintf(stdout, " (forced)\n");
[...]

How about when you have a forced mode but the driver reports eth_tp_mdix
= ETH_TP_MDI_INVALID because the link is down?  This is going to result
in:

        MDI-X: Unknown (forced)

which makes no sense at all.  So I think that we have to do something
like:

	if (ep->eth_tp_mdix_ctrl == ETH_TP_MDI) {
		fprintf(stdout, "off (forced)\n");
	} else if (ep->eth_tp_mdix_ctrl == ETH_TP_MDI_X) {
		fprintf(stdout, "on (forced)\n");
	} else {
		switch (ep->eth_tp_mdix) {
			...
		}
		if (ep->eth_tp_mdix_ctrl == ETH_TP_MDI_AUTO)
			fprintf(stdout, " (auto)");
		fprintf(stdout, "\n");
	}

Or else we require that when the mode is forced then drivers report that
as the current status even if the link is down.

Ben.

On Thu, 2012-07-26 at 09:30 -0700, Jesse Brandeburg wrote:
> On Wed, Jul 25, 2012 at 11:59:02PM +0100, Ben Hutchings wrote:
> > On Wed, 2012-07-25 at 10:53 -0700, Jesse Brandeburg wrote:
> > How about when you have a forced mode but the driver reports eth_tp_mdix
> > = ETH_TP_MDI_INVALID because the link is down?  This is going to result
> > in:
> > 
> >         MDI-X: Unknown (forced)
> > 
> > which makes no sense at all.  So I think that we have to do something
> > like:
> > 
> > 	if (ep->eth_tp_mdix_ctrl == ETH_TP_MDI) {
> > 		fprintf(stdout, "off (forced)\n");
> > 	} else if (ep->eth_tp_mdix_ctrl == ETH_TP_MDI_X) {
> > 		fprintf(stdout, "on (forced)\n");
> > 	} else {
> > 		switch (ep->eth_tp_mdix) {
> > 			...
> > 		}
> > 		if (ep->eth_tp_mdix_ctrl == ETH_TP_MDI_AUTO)
> > 			fprintf(stdout, " (auto)");
> > 		fprintf(stdout, "\n");
> > 	}
> > 
> > Or else we require that when the mode is forced then drivers report that
> > as the current status even if the link is down.
> 
> I tried that and it works swimmingly.  How does this look?
[...]

That looks good, thanks.

Ben.