Message-ID: <262dfc0e-c248-23e7-cd34-d13e104afe91@gmail.com>
Date:   Tue, 4 Jun 2019 20:06:12 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>,
        Russell King - ARM Linux admin <linux@...linux.org.uk>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Ioana Ciornei <ioana.ciornei@....com>
Subject: Re: Cutting the link on ndo_stop - phy_stop or phy_disconnect?



On 6/4/2019 4:46 PM, Vladimir Oltean wrote:
> On Wed, 5 Jun 2019 at 02:24, Russell King - ARM Linux admin
> <linux@...linux.org.uk> wrote:
>>
>> On Wed, Jun 05, 2019 at 02:03:19AM +0300, Vladimir Oltean wrote:
>>> On Wed, 5 Jun 2019 at 01:59, Russell King - ARM Linux admin
>>> <linux@...linux.org.uk> wrote:
>>>>
>>>> On Wed, Jun 05, 2019 at 01:44:08AM +0300, Vladimir Oltean wrote:
>>>>> You caught me.
>>>>>
>>>>> But even ignoring the NIC case, isn't the PHY state machine
>>>>> inconsistent with itself? It is ok with callink phy_suspend upon
>>>>> ndo_stop, but it won't call phy_suspend after phy_connect, when the
>>>>> netdev is implicitly stopped?
>>>>
>>>> The PHY state machine isn't inconsistent with itself, but it does
>>>> have strange behaviour.
>>>>
>>>> When the PHY is attached, the PHY is resumed and the state machine
>>>> is in PHY_READY state.  If it goes through a start/stop cycle, the
>>>> state machine transitions to PHY_HALTED and attempts to place the
>>>> PHY into a low power state.  So the PHY state is consistent with
>>>> the state machine state (we don't end up in the same state but with
>>>> the PHY in a different state.)
>>>>
>>>> What we do have is a difference between the PHY state (and state
>>>> machine state) between the boot scenario, and the interface up/down
>>>> scenario, the latter behaviour having been introduced by a commit
>>>> back in 2013:
>>>>
>>>>     net: phy: suspend phydev when going to HALTED
>>>>
>>>>     When phydev is going to HALTED state, we can try to suspend it to
>>>>     safe more power. phy_suspend helper will check if PHY can be suspended,
>>>>     so just call it when entering HALTED state.
>>>>
>>>> --
>>>> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
>>>> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
>>>> According to speedtest.net: 11.9Mbps down 500kbps up
>>>
>>> I am really not into the PHYLIB internals, but basically what you're
>>> telling me is that running "ip link set dev eth0 down" is a
>>> stronger/more imperative condition than not running "ip link set dev
>>> eth0 up"... Does it also suspend the PHY if I put the interface down
>>> while it was already down?
>>
>> No - but that has nothing to do with phylib internals, more to do with
>> the higher levels of networking.  ndo_stop() will not be called unless
>> ndo_open() has already been called.  In other words, setting an already
>> down device down via "ip link set dev eth0 down" is a no-op.
>>
>> So, let's a common scenario.  You power up a board.  The PHY comes up
>> and establishes a link.  The boot loader runs, loads the kernel, which
> 
> This may or may not be the case. As you pointed out a few emails back,
> this is a system-level issue that requires a system-level solution -
> so cutting the link in U-boot is not out of the question.
> 
>> then boots.  Your network driver is a module, and hasn't been loaded
>> yet.  The link is still up.
>>
>> The modular network driver gets loaded, and initialises.  Userspace
>> does not bring the network device up, and the network driver does not
>> attach or connect to the PHY (which is actually quite common).  So,
>> the link is still up.
>>
>> The modular PHY driver gets loaded, and binds to the PHY.  The link
>> is still up.
> 
> I would rather say, 'even if the link is not up, Linux brings it up
> (possibly prematurely) via phy_resume'.
> But let's consider the case where the link *was* up. The general idea
> is 'implement your workarounds in whatever other way, that link is
> welcome!'.

With the systems that I work with, we have enforced the following
behavior to happen: the boot loader and kernel only turn on what they
needs, at the time they need it, and nothing more, once done, they put
the blocks back into lowest power mode (clock and power gated if
available). So yes, there are multiple link re-negotiations throughput
the boot process, but when there is no device bound to a driver the
system conserves power by default which is deemed a higher goal than
speed. Your mileage may vary of course.

There is not exactly a simple way of enforcing that kind (or another
kind for that matter) of policy kernel wide, so it's unfortunately up to
the driver writer to propose something that is deemed sensible.

We could however, extend existing tools like iproute2 to offer the
ability to control whether the PHY should be completely powered off, in
a low power state allowing WoL, or remain UP when the network device is
brought down. That would not cover the case that Russell explained, but
it would be another monkey wrench you can throw at the system.
-- 
Florian

Powered by blists - more mailing lists