Thread (11 messages) 11 messages, 3 authors, 2015-06-02

Re: [PATCH 1/1] net: core: 'ethtool' issue with querying phy settings

From: David Miller <davem@davemloft.net>
Date: 2015-06-01 00:19:48
Also in: lkml

From: Ben Hutchings <redacted>
Date: Sun, 31 May 2015 20:54:06 +0100
On Fri, 2015-05-22 at 16:15 -0400, David Miller wrote:
quoted
From: Arun Parameswaran <redacted>
Date: Wed, 20 May 2015 14:35:30 -0700
quoted
When trying to configure the settings for PHY1, using commands
like 'ethtool -s eth0 phyad 1 speed 100', the 'ethtool' seems to
modify other settings apart from the speed of the PHY1, in the
above case.

The ethtool seems to query the settings for PHY0, and use this
as the base to apply the new settings to the PHY1. This is
causing the other settings of the PHY 1 to be wrongly
configured.

The issue is caused by the '_ethtool_get_settings()' API, which
gets called because of the 'ETHTOOL_GSET' command, is clearing
the 'cmd' pointer (of type 'struct ethtool_cmd') by calling
memset. This clears all the parameters (if any) passed for the
'ETHTOOL_GSET' cmd. So the driver's callback is always invoked
with 'cmd->phy_address' as '0'.

The '_ethtool_get_settings()' is called from other files in the
'net/core'. So the fix is applied to the 'ethtool_get_settings()'
which is only called in the context of the 'ethtool'.

Signed-off-by: Arun Parameswaran <redacted>
Reviewed-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Scott Branden <sbranden@broadcom.com>
Applied and queued up for -stable, thanks.
Please revert this.  This is an incompatible API change, not a bug fix.
The established semantics are that 'phyad' is filled in by the driver;
it is not a parameter to the ETHTOOL_GSET command.
But then how in the world can the user specify specific PHY ADs for
a device that will respond to more than one?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help