netdev - Re: [PATCH] net phy: Check for aneg completion before setting state to PHY

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]

Message-ID: <CAGVrzcaVEWTm+wJa-G4O8ZTXGLA2WjCj2sfJAzfmKa+AhGrpoQ@mail.gmail.com>
Date:	Wed, 30 Apr 2014 19:11:08 -0700
From:	Florian Fainelli <f.fainelli@...il.com>
To:	Balakumaran Kannan <kumaran.4353@...il.com>
Cc:	netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH] net phy: Check for aneg completion before setting state
 to PHY_RUNNING

2014-04-24 22:53 GMT-07:00 Balakumaran Kannan <kumaran.4353@...il.com>:
> On Fri, Apr 25, 2014 at 12:23 AM, Florian Fainelli <f.fainelli@...il.com> wrote:
>> 2014-04-23 19:52 GMT-07:00 Balakumaran Kannan <kumaran.4353@...il.com>:
>>> phy_state_machine should check whether auto-negotiatin is completed
>>> before changing phydev->state from PHY_NOLINK to PHY_RUNNING. If
>>> auto-negotiation is not completed phydev->state should be set to
>>> PHY_AN.
>>
>> I need some time to review this. Just out of curiosity, is this fixing
>> a bug you have encountered or was this found by state machine
>> inspection?
>
> Actually I encountered a problem that Linux-3.4 sends Neighbor Solicitation (NS)
> before hardware completes auto-negotiation. The driver being used for
> my hardware
> is smsc911x.
>
> While investigation on this, I came to know that state of phydev
> follows this sequence
> PHY_READY --> PHY_UP (in phy_start)
> PHY_UP --> PHY_AN (in phy_start_aneg called by phy_state_machine)
> PHY_AN --> PHY_NOLINK (in phy_state_machine. Link is down)
> PHY_NOLINK --> PHY_RUNNING (in phy_state_machine. Link becomes ready)
>
> So I felt that the sequence of PHY_AN --> PHY_RUNNING while link is up and
> PHY_NOLINK --> PHY_RUNNING are different. In the earlier case aneg completion
> is taken into account but not in later.

I see, we should indeed go back to PHY_AN

>
> But this patch doesn't fix my problem. I made the smsc driver's open function to
> poll on phy_aneg_done until auto-negotiation completes.
>
> Here I have a doubt.
>  * Linux network initialization and phy_state_machine runs in separate threads.
>  * And driver doesn't bother about state of phydev.
> Is it correct?

That is correct, once your driver calls phy_start(), the PHY state
machine executes in the background within the context of a separate
workqueue.

> Doesn't driver need to confirm phydev is ready(PHY_RUNNING) before
> returning to kernel?

phy_start() really starts the PHY state machine, but does not block
until the PHY is in the PHY_RUNNING state, which is something that
might never happen if e.g: the cable is disconnected.

What really tells the network stack whether the link is ready, is when
the PHY state machine calls netif_carrier_on().
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html