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  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, 11 Mar 2017 15:55:39 -0500
From:   Vivien Didelot <vivien.didelot@...oirfairelinux.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel@...oirfairelinux.com,
        "David S. Miller" <davem@...emloft.net>,
        Florian Fainelli <f.fainelli@...il.com>
Subject: Re: [PATCH net-next 04/14] net: dsa: mv88e6xxx: rework ATU Load/Purge

Hi Andrew,

Andrew Lunn <andrew@...n.ch> writes:

> On Thu, Mar 09, 2017 at 06:33:14PM -0500, Vivien Didelot wrote:
>> All Marvell switch chips have an ATU accessed using the same Global (1)
>> register layout. Only the handling of the FID differs as more bits were
>> necessary to support more and more databases.
>> 
>> Add and use a fresh documented implementation of the ATU Load/Purge.
>
> This is not really the Linux way of doing something. You don't throw
> something away and replace it. You incrementally modify what you have
> into something better.
>
> I really wished you had moved the code, unmodified, into
> global1_atu.c. Then made lots of easy to review small changes. I
> cannot just look at this patch and know it is correct. What i need to
> compare against is not in this patch. So it is a lot harder to review.

I've addressed all of your comments in this patchset except this one.

A patch file cannot guarantee that a chunk of code moved around has not
been altered in the process. This will just generate more diff for no
value, that needs to be updated afterwards anyway.

Plus you already complained in the first iteration I sent about
modifying lines that I previously added. I took care of logically
splitting the new ATU Load/Purge, GetNext, Flush and Remove operations
into incremental unmodified chunks in this series.

I have updated the commit messages to detail the addition of the static
helpers and their counterparts, but I will resend this patch as is.

Thanks,

        Vivien

Powered by blists - more mailing lists