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 -0700quoted
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?