netdev - RE: [EXT] Re: [PATCH net] net: freescale: fec: Fix ethtool -d runtime PM

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]

Date:   Fri, 3 Jan 2020 02:02:50 +0000
From:   Andy Duan <fugang.duan@....com>
To:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>
CC:     David Miller <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Chris Healy <Chris.Healy@....aero>
Subject: RE: [EXT] Re: [PATCH net] net: freescale: fec: Fix ethtool -d runtime
 PM

From: Andrew Lunn <andrew@...n.ch> Sent: Friday, January 3, 2020 9:02 AM
> > This fix will do, but you should consider implementing
> > ethtool_ops::begin and ethtool_ops::end to make sure this condition is
> > resolved for all ethtool operations.
> >
> > For instance the following looks possibly problematic too:
> > fec_enet_set_coalesce -> fec_enet_itr_coal_set
> 
> Hi Florian
> 
> I did a quick test of all the ethtool operations which the driver supports,
> including setting coalescing. I did not exhaustively try all possible coalescing
> settings, but the ones i did try did not provoke a data abort.
The original design is that driver power off clocks when net interface is down,
use ethtool to dump registers are not allowed. Only .get_regs/ .get/set_coalesce
are allowed when the net interface is up by checking netif_running(ndev).

If there have requirement to access registers when net interface is down status,
It is better to change the common code to enhance ethtool_ops callbacks usage
to support ethtool_ops::begin and ethtool_ops::end for all net drivers.

> 
> Still, it would make sense to implement begin and end, but only for net-next.
> 
>         Andrew