netdev - Re: No traffic with Marvell switch and latest linux-next

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 24 Feb 2019 08:32:56 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Heiner Kallweit <hkallweit1@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: No traffic with Marvell switch and latest linux-next

Le 2/24/19 à 7:49 AM, Russell King - ARM Linux admin a écrit :
> On Sun, Feb 24, 2019 at 04:39:30PM +0100, Heiner Kallweit wrote:
>> On 24.02.2019 16:34, Russell King - ARM Linux admin wrote:
>>> On Sun, Feb 24, 2019 at 04:28:32PM +0100, Heiner Kallweit wrote:
>>>> On 24.02.2019 16:15, Russell King - ARM Linux admin wrote:
>>>>> On Sun, Feb 24, 2019 at 04:04:03PM +0100, Andrew Lunn wrote:
>>>>>>> I think what's not correct is that phydev->autoneg is set
>>>>>>> (by phy_device_create) for a fixed link.
>>>>>>
>>>>>> Fixed-link tries to emulate auto-neg:
>>>>>>
>>>>>>                 bmsr |= BMSR_LSTATUS | BMSR_ANEGCOMPLETE;
>>>>>>
>>>>>> Maybe it needs better emulation of auto-neg?
>>>>>
>>>>> Or maybe it needs to represent a fixed-speed PHY by clearing bit 1.3
>>>>> (BMSR_ANEGCAPABLE).  In any case, 0.12 (BMCR_ANENABLE) is not set,
>>>>> so according to 802.3-2015, we should not be setting 1.5
>>>>> (BMSR_ANEGCOMPLETE).
>>>>>
>>>>> However, swphy does try to emulate autonegotiation - we do have cases
>>>>> where swphy is used in situations where the speed and duplex are not
>>>>> fixed.  It returns an emulated link partner advertisement for the
>>>>> current speed, which would suggest that we should set BMCR_ANENABLE.
>>>>>
>>>> If we emulate auto-neg, then it's not needed to set the speed bits
>>>> in BMCR. Also what just comes to my mind, certain speeds like 1000BaseT
>>>> don't support forced mode. So we may have to go with auto-neg.
>>>
>>> Sure.
>>>
>>>> To avoid the original issue it should be sufficient to copy
>>>> supported -> advertising at a suited place.
>>>
>> Sorry, seems this wasn't clear enough. I don't mean to change
>> swphy but the user side.
>>
>>> Why bother - the software PHY emulation is an emulation to allow
>>> existing userspace that pre-dates the ethtool API to get some link
>>> parameters.  If we augment the PHY emulation in non-standard ways,
>>> userspace will need to be updated to handle those non-standard
>>> ways.  If userspace needs to be updated, why not just bite the
>>> bullet and update to ethtool APIs rather than adding more
>>> complication through an emulation layer?
>>>
>> It's not only userspace. Based on my limited knowledge of DSA this
>> code also uses e.g. genphy_read_status() with a fixed link.
> 
> DSA has support for phylink, which is perfectly capable of supporting
> fixed links without using fixed-phy.c, although we have no way to
> create that without DT.  Support could be added for non-DT though.

I had a branch at some point that provided feature parity with what DT
registration is capable of doing, including representing links between
switches etc [1]. I am not entirely sure it makes to support such a
configuration for platform devices, since they are not so many these
days that would not use DT (except x86 maybe?). It would make sense to
support ACPI-based registration for PHYLINK, if there was a standard for
describing PHYs, SFP/SFFs which is not apparently the case.

[1]: https://github.com/ffainelli/linux/commits/dsa-lbk-pdata2

> 
> That would avoid using phylib functions to read back from a fixed-link
> PHY.  Since phylink presents to the MAC a fixed link in the same
> abstract manner as a real PHY, it should result in a more elegant
> implementation.
> 
> DSA already has phylink support to support SFPs.
> 

The added difficulty here and the reason why Andrew went with the
approach that is used by the code currently is because neither do the
CPU or DSA ports are backed by a net_device. It is somewhere on my TODO
to permit the use of PHYLINK without the need of a net_device to cover
those specific DSA cases unless we just brute force the whole thing and
allocate a net_device structure but not register that net_device? Yes in
fact, why don't we do that?
-- 
Florian