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] [day] [month] [year] [list]

Message-ID: <20200103023837.GA28099@lunn.ch>
Date:   Fri, 3 Jan 2020 03:38:37 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Andy Duan <fugang.duan@....com>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        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

On Fri, Jan 03, 2020 at 02:02:50AM +0000, Andy Duan wrote:
> 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).

Hi Andy

The function fec_enet_get_regs() does not perform a
netif_running(ndev) check. So it seems like it was intended to work
with the interface down. It could be a difference between variants of
the FEC. I was seeing the data abort on the vf610. Maybe its clocks
are different to other FEC implementations?

I've no strong preference about seeing the registers when the
interface is down, or return maybe -ENETDOWN.  What i don't want is a
data abort, and locks left in a bad state.

    Andrew