Thanks for responding.
-----Original Message-----
From: Florian Fainelli [mailto:f.faine...@gmail.com]
Sent: Friday, September 01, 2017 5:53 AM
To: Bhadram Varka <vbhad...@nvidia.com>; and...@lunn.ch
Cc: linux-netdev <netdev@vger.kernel.org>
Subject: Re: netdev carrier changes is one even after ethernet link up.
On 08/30/2017 10:53 PM, Bhadram Varka wrote:
> Hi,
>
>
>
> I have observed that carrier_changes is one even in case of the
> ethernet link is up.
>
>
>
> After investigating the code below is my observation –
>
>
>
> ethernet_driver_probe()
>
> +--->phy_connect()
>
> | +--->phy_attach_direct()
>
> | +---> netif_carrier_off() : which increments
> carrier_changes to one.
>
> +--->register_netdevice() : will the carrier_changes becomes zero here ?
>
> +--->netif_carrier_off(): not increment the carrier_changes since
> __LINK_STATE_NOCARRIER already set.
>
>
>
> From ethernet driver open will start the PHY and trigger the
> phy_state_machine.
>
> Phy_state_machine workqueue calling netif_carrier_on() once the link is UP.
>
> netif_carrier_on() increments the carrier_changes by one.
If the call trace is correct, then there is at least two problems here:
- phy_connect() does start the PHY machine which means that as soon as it
detects a link state of any kind (up or down) it can call
netif_carrier_off() respectively netif_carrier_on()
- as soon as you call register_netdevice() notifiers run and other parts of the
kernel or user-space programs can see an inconsistent link state
I would suggest doing the following sequence instead:
netif_carrier_off()
register_netdevice()
phy_connect()
Which should result in a consistent link state and carrier value.
Yes, It will address the issue.
If we did the phy_conect in ndo_open it will make the carrier changes as one.
But if we did in probe function then it's not working.
In ethernet driver probe - (below sequence is not working)
phy_connect()
register_netdevice()
netif_carrier_off()
working sequence:
In probe():
register_netdevice()
ndo_open:
phy_connect()
Thanks,
Bhadram.
>
>
>
> After link is UP if we check the carrier_changes sysfs node - it will
> be one only.
>
>
>
> $ cat /sys/class/net/eth0/carrier_changes
>
> 1
>
>
>
> After reverting the change - https://lkml.org/lkml/2016/1/9/173 (net:
> phy: turn carrier off on phy attach) then I could see the carrier
> changes incremented to 2 after Link UP.
>
> $ cat /sys/class/net/eth0/carrier_changes
>
> 2
>
>
>
> Thanks,
>
> Bhadram.
>
> ----------------------------------------------------------------------
> -- This email message is for the sole use of the intended recipient(s)
> and may contain confidential information. Any unauthorized review,
> use, disclosure or distribution is prohibited. If you are not the
> intended recipient, please contact the sender by reply email and
> destroy all copies of the original message.
> ----------------------------------------------------------------------
> --
--
Florian
