Message-ID: <2187db98-dc5a-7a3c-7965-7ccbeffc0fa1@gmail.com>
Date:   Mon, 14 Nov 2016 10:20:24 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Sebastian Frias <sf84@...oste.net>, Mason <slash.tmp@...e.fr>,
        Andrew Lunn <andrew@...n.ch>
Cc:     netdev <netdev@...r.kernel.org>, Mans Rullgard <mans@...sr.com>,
        Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
        Tom Lendacky <thomas.lendacky@....com>,
        Zach Brown <zach.brown@...com>,
        Shaohui Xie <shaohui.xie@....com>,
        Tim Beale <tim.beale@...iedtelesis.co.nz>,
        Brian Hill <brian@...ston-radar.com>,
        Vince Bridgers <vbridgers2013@...il.com>,
        Balakumaran Kannan <kumaran.4353@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Kirill Kapranov <kapranoff@...ox.ru>
Subject: Re: Debugging Ethernet issues

On 11/14/2016 09:59 AM, Sebastian Frias wrote:
> On 11/14/2016 06:32 PM, Florian Fainelli wrote:
>> On 11/14/2016 07:33 AM, Mason wrote:
>>> On 14/11/2016 15:58, Mason wrote:
>>>
>>>> nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
>>>> vs
>>>> nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off
>>>>
>>>> I'm not sure whether "flow control" is relevant...
>>>
>>> Based on phy_print_status()
>>> phydev->pause ? "rx/tx" : "off"
>>> I added the following patch.
>>>
>>> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
>>> index defc22a15f67..4e758c1cfa4e 100644
>>> --- a/drivers/net/ethernet/aurora/nb8800.c
>>> +++ b/drivers/net/ethernet/aurora/nb8800.c
>>> @@ -667,6 +667,8 @@ static void nb8800_link_reconfigure(struct net_device *dev)
>>>         struct phy_device *phydev = priv->phydev;
>>>         int change = 0;
>>>  
>>> +       printk("%s from %pf\n", __func__, __builtin_return_address(0));
>>> +
>>>         if (phydev->link) {
>>>                 if (phydev->speed != priv->speed) {
>>>                         priv->speed = phydev->speed;
>>> @@ -1274,9 +1276,9 @@ static int nb8800_hw_init(struct net_device *dev)
>>>         nb8800_writeb(priv, NB8800_PQ2, val & 0xff);
>>>  
>>>         /* Auto-negotiate by default */
>>> -       priv->pause_aneg = true;
>>> -       priv->pause_rx = true;
>>> -       priv->pause_tx = true;
>>> +       priv->pause_aneg = false;
>>> +       priv->pause_rx = false;
>>> +       priv->pause_tx = false;
>>>  
>>>         nb8800_mc_init(dev, 0);
>>>  
>>>
>>> Connected to 1000 Mbps switch:
>>>
>>> # time udhcpc | while read LINE; do date; echo $LINE; done
>>> Thu Jan  1 00:00:22 UTC 1970
>>> udhcpc (v1.22.1) started
>>> Thu Jan  1 00:00:22 UTC 1970
>>> Sending discover...
>>> [   24.565346] nb8800_link_reconfigure from phy_state_machine
>>> Thu Jan  1 00:00:25 UTC 1970
>>> Sending discover...
>>> [   26.575402] nb8800_link_reconfigure from phy_state_machine
>>> [   26.580972] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
>>> Thu Jan  1 00:00:28 UTC 1970
>>> Sending discover...
>>> Thu Jan  1 00:00:29 UTC 1970
>>> Sending select for 172.27.64.58...
>>> Thu Jan  1 00:00:29 UTC 1970
>>> Lease of 172.27.64.58 obtained, lease time 604800
>>> Thu Jan  1 00:00:29 UTC 1970
>>> deleting routers
>>> Thu Jan  1 00:00:29 UTC 1970
>>> adding dns 172.27.0.17
>>>
>>> real    0m7.388s
>>> user    0m0.040s
>>> sys     0m0.090s
>>>
>>>
>>>
>>> Connected to 100 Mbps switch:
>>>
>>> # time udhcpc | while read LINE; do date; echo $LINE; done
>>> Thu Jan  1 00:00:14 UTC 1970
>>> udhcpc (v1.22.1) started
>>> Thu Jan  1 00:00:15 UTC 1970
>>> Sending discover...
>>> [   16.968621] nb8800_link_reconfigure from phy_state_machine
>>> [   17.975359] nb8800_link_reconfigure from phy_state_machine
>>> [   17.980923] nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
>>> Thu Jan  1 00:00:18 UTC 1970
>>> Sending discover...
>>> Thu Jan  1 00:00:19 UTC 1970
>>> Sending select for 172.27.64.58...
>>> Thu Jan  1 00:00:19 UTC 1970
>>> Lease of 172.27.64.58 obtained, lease time 604800
>>> Thu Jan  1 00:00:19 UTC 1970
>>> deleting routers
>>> Thu Jan  1 00:00:19 UTC 1970
>>> adding dns 172.27.0.17
>>>
>>> real    0m4.355s
>>> user    0m0.043s
>>> sys     0m0.083s
>>>
>>
>> And the time difference is clearly accounted for auto-negotiation time
>> here, as you can see it takes about 3 seconds for Gigabit Ethernet to
>> auto-negotiate and that seems completely acceptable and normal to me
>> since it is a more involved process than lower speeds.
>>
>>>
>>>
>>> OK, so now it works (by accident?) even on 100 Mbps switch, but it still
>>> prints "flow control rx/tx"...
>>
>> Because your link partner advertises flow control, and that's what
>> phydev->pause and phydev->asym_pause report (I know it's confusing, but
>> that's what it is at the moment).
> 
> Thanks.
> Could you confirm that Mason's patch is correct and/or that it does not
> has negative side-effects?

The patch is not correct nor incorrect per-se, it changes the default
policy of having pause frames advertised by default to not having them
advertised by default. This influences both your Ethernet MAC and the
link partner in that the result is either flow control is enabled
(before) or it is not (with the patch). There must be something amiss if
you see packet loss or some kind of problem like that with an early
exchange such as DHCP. Flow control tend to kick in under higher packet
rates (at least, that's what you expect).


> 
> Right now we know that Mason's patch makes this work, but we do not understand
> why nor its implications.

You need to understand why, right now, the way this problem is
presented, you came up with a workaround, not with the root cause or the
solution. What does your link partner (switch?) reports, that is, what
is the ethtool output when you have a link up from  your nb8800 adapter?
-- 
Florian

Powered by blists - more mailing lists