linux-kernel - Re: [PATCH NET V5 2/2] net: hns: Use 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: <20170629135648.GD9244@lunn.ch>
Date:   Thu, 29 Jun 2017 15:56:48 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Yunsheng Lin <linyunsheng@...wei.com>
Cc:     davem@...emloft.net, f.fainelli@...il.com,
        huangdaode@...ilicon.com, xuwei5@...ilicon.com,
        liguozhu@...ilicon.com, Yisen.Zhuang@...wei.com,
        gabriele.paoloni@...wei.com, john.garry@...wei.com,
        linuxarm@...wei.com, salil.mehta@...wei.com, lipeng321@...wei.com,
        tremyfr@...il.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH NET V5 2/2] net: hns: Use phy_driver to setup Phy loopback

> > You only call dev_close() if the device is running. What if somebody
> > runs the self test on an interface when it has never been opened? It
> > looks like you will call phy_resume(). But since it has never been
> > suspended, you could be in trouble.
> Here is what I can think of:
> 1. when the mac driver is first loaded, the phy has a default state. suspended?

Nope. The PHY will only be suspended when you actually call
phy_suspend.

> 2. If user runs the self test after using 'ifconfig ethX down', then I suppose
> phy is already suspended.

Assuming the phy has at some point been up, after a down, it should be
suspended.

The key thing here is, phy_resume() can only be called after a
successful phy_suspend(). Those are the power management rules, and
the expectations of the drivers. Doing a resume without first doing an
explicit suspend is asking for bad things to happen.

You are having trouble because you are not using the API for what it
was intended. Maybe you need to take a step back and look at the
bigger picture of how self tests are being performed. Why do you need
the dev_close()/dev_open()? Maybe
netif_device_detach()/netif_device_attach() would be better?
How do other drivers do self test?

	 Andrew