netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: Bhadram Varka <vbhadram@nvidia.com>, "andrew@lunn.ch" <andrew@lunn.ch>
Cc: linux-netdev <netdev@vger.kernel.org>
Subject: Re: netdev carrier changes is one even after ethernet link up.
Date: Thu, 31 Aug 2017 17:23:00 -0700	[thread overview]
Message-ID: <25c3b4ca-02d7-e217-fe27-ca3b7d2cbf07@gmail.com> (raw)
In-Reply-To: <974898714b3e4e59b933983ded977ce2@bgmail102.nvidia.com>

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.

> 
>  
> 
> 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

  parent reply	other threads:[~2017-09-01  0:23 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <974898714b3e4e59b933983ded977ce2@bgmail102.nvidia.com>
2017-08-31  6:03 ` netdev carrier changes is one even after ethernet link up Bhadram Varka
2017-09-01  0:23 ` Florian Fainelli [this message]
2017-09-01  5:41   ` Bhadram Varka
2017-09-01  5:49   ` Bhadram Varka
2017-09-02  1:55     ` Florian Fainelli
2017-09-04  5:07       ` Bhadram Varka
2017-09-04  5:10       ` Bhadram Varka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=25c3b4ca-02d7-e217-fe27-ca3b7d2cbf07@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=netdev@vger.kernel.org \
    --cc=vbhadram@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).