netdev - RE: [EXT] Re: [PATCH v2 net-next 01/12] net: atlantic: changes for multi-TC support

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, 22 May 2020 19:47:37 +0000
From:   Mark Starovoytov <mstarovoitov@...vell.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Dmitry Bezrukov <dbezrukov@...vell.com>,
        Igor Russkikh <irusskikh@...vell.com>
Subject: RE: [EXT] Re: [PATCH v2 net-next 01/12] net: atlantic: changes for
 multi-TC support

Hi Jakub,

Thank you for the review.

> > This patch contains the following changes:
> > * access cfg via aq_nic_get_cfg() in aq_nic_start() and
> > aq_nic_map_skb();
...
> > * use AQ_HW_*_TC instead of hardcoded magic numbers;
> > * actually use the 'ret' value in aq_mdo_add_secy();
> 
> Whenever you do an enumeration like this - it's a strong indication that those
> should all be separate patches. Please keep that in mind going forward.

Understood, but I've also seen a recommendation that a single patchset shouldn't have more than 15 patches (if my memory doesn't fail me). And unfortunately it would have been impossible to meet the 15 patches limit, if all these small changes were in separate patches. What's the best/recommended approach in this case?

> >  static int aq_mdo_upd_secy(struct macsec_context *ctx)
> 
> This should have been a separate fix for sure.

My bad, note taken.

Best regards,
Mark.