netdev - Re: [RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo

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:   Sat, 29 Jul 2017 08:18:57 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Mason <slash.tmp@...e.fr>, Mans Rullgard <mans@...sr.com>
Cc:     Marc Gonzalez <marc_gonzalez@...madesigns.com>,
        "David S. Miller" <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open

On 07/29/2017 05:02 AM, Mason wrote:
> On 29/07/2017 13:24, Måns Rullgård wrote:
> 
>> Until you figure out why it's getting stuck, we can't be sure
>> it isn't caused by something that could trigger at any time.
> Would you take a look at it, if I can reproduce on tango4?
> 
> I have identified a 100% reproducible flaw.
> I have proposed a work-around that brings this down to 0
> (tested 1000 cycles of link up / ping / link down).

Can you also try to get help from your HW resources to eventually help
you find out what is going on here? If anybody has access to that it
would be you.

> 
> In my opinion, upstream should consider this work-around
> for inclusion. I'd like to hear David's and Florian's
> opinion on the topic. It's always a pain to maintain
> out-of-tree patches.

I have to agree with Mans here that the commit message explanation is
not good enough to understand how the RX path is hosed after a call to
ndo_stop() it would be good, both for you and for the people maintaining
this driver to understand what happens exactly so the fix is correct,
understood and maintainable. The patch itself looks reasonable with the
limited description given, but it's the description itself that needs
changing. BTW, this should probably come with a Fixes: tag to identify
which commit (presumably the one introducing the driver) is being fixed.
-- 
Florian