Re: [PATCH 0/2] Fix couple of issues with 'ethtool' get/set API's

[Arun Parameswaran <aparames@broadcom.com>]

On 15-06-01 12:29 PM, Ben Hutchings wrote:
> On Mon, 2015-06-01 at 12:12 -0700, Arun Parameswaran wrote:
>> On 15-06-01 11:07 AM, Ben Hutchings wrote:
>>> On Mon, 2015-06-01 at 10:14 -0700, Arun Parameswaran wrote:
>>>> On 15-05-31 12:59 PM, Ben Hutchings wrote:
>>>>> On Fri, 2015-05-22 at 15:43 -0700, Arun Parameswaran wrote:
>>>>>> Hi,
>>>>>> The patch fixes 2 issues with 'ethtool' getting/setting parametres in
>>>>>> the do_gset() do_sset() API's.
>>>>>>
>>>>>> I have pushed a patch to the Kernel to fix an issue in the handling of
>>>>>> the 'ethtool' commands which got accepted.
>>>>>> This Kernel patch was based on Linux v4.1-rc4 and is available in:
>>>>>> https://github.com/Broadcom/cygnus-linux/tree/net-core-ethtool-fix-v1
>>>>>>
>>>>>> The Kernel was always clearing the command from the 'ethtool' resulting
>>>>>> in all operations to deal with PHY0. This prevents querying/setting
>>>>>> PHY 1's settings.
>>>>> [...]
>>>>>
>>>>> Each net device can be associated with a single PHY at a time, and the
>>>>> ETHTOOL_GSET implementation should fill in the PHY address in the
>>>>> ethtool_cmd::phy_address field.  Where there are multiple PHYs that can
>>>>> be connected to the net device's MAC, an ETHTOOL_SSET operation can be
>>>>> used to change that PHY address.
>>>>>
>>>> The above can be done by the driver when there is one PHY per MAC. In our
>>>> case we have multiple PHYs controlled by the same MAC. I should have
>>>> clarified this earlier, I apologize.
>>>
>>> I understand that you can have multiple PHYs on the same MDIO bus, but
>>> not how the MAC can use them at the same time.  Is this hardware level
>>> bonding?  Or are multiple PHYs needed for a single link?
>>>
>> We have an internal switch which manages the traffic to the PHY's (ports).
>> There is 1 PHY per external port.
>> The MAC is connected to the internal port of the switch.
> 
> Then you should create net devices for those external ports as well as
> the internal port.
We can only have one MAC address as we have only one MAC. So we prefer to
have a single net device which reflects the hardware better.

> 
> If I understand the switchdev API rightly, the external port devices
> should implement the ethtool {get,set}_settings operations and the
> ndo_switch_parent_id_get operation.  The existing net device should
> expose only the internal link to the switch (which presumably isn't
> configurable at all).
I agree, there can be an implementation like the above.
The switch can be handled in multiple ways, in our case we use one net
device as we are limited by the MAC.

Since the 'ethtool' provides the 'phyad' parameter to specifically
address a particular PHY, I think the 'ethtool' should allow for this,
irrespective of the net implementation. Wouldn't this make the 'ethtool'
flexible and detached from the network driver implementation.

> 
> [...]
>> But this prevents the 'ethtool' from being used to get/set data of
>> specific PHY's.
> 
> That is fine because it is meant to manage the net device's own link (in
> this case, the internal port), not other switch ports.
> 
> Ben.
>