netdev - Re: [PATCH net] sfc: ef10: don't overwrite offload features at NIC reset

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]

Message-ID: <CACT4oudb2Jf09w62k65PLFpJyrCPnPAXc1FcVZLeB0To6OtORA@mail.gmail.com>
Date:   Wed, 8 Mar 2023 17:25:43 +0100
From:   Íñigo Huguet <ihuguet@...hat.com>
To:     Edward Cree <ecree.xilinx@...il.com>
Cc:     habetsm.xilinx@...il.com, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org,
        Tianhao Zhao <tizhao@...hat.com>,
        Jonathan Cooper <jonathan.s.cooper@....com>
Subject: Re: [PATCH net] sfc: ef10: don't overwrite offload features at NIC reset

On Wed, Mar 8, 2023 at 4:53 PM Edward Cree <ecree.xilinx@...il.com> wrote:
>
> On 08/03/2023 11:32, Íñigo Huguet wrote:
> > At NIC reset, some offload features related to encapsulated traffic
> > might have changed (this mainly happens if the firmware-variant is
> > changed with the sfboot userspace tool). Because of this, features are
> > checked and set again at reset time.
> >
> > However, this was not done right, and some features were improperly
> > overwritten at NIC reset:
> > - Tunneled IPv6 segmentation was always disabled
> > - Features disabled with ethtool were reenabled
> > - Features that becomes unsupported after the reset were not disabled
> >
> > Also, cleanup a bit the setting of other features not related to
> > encapsulation. Now that Siena devices are unsupported, some checks are
> > unnecessary because they're always supported in all ef10 models.
>
> Could you clarify what checks were removed?  All I can see is the
>  'NETIF_F_TSO6 requires NETIF_F_IPV6_CSUM' check, and Siena already
>  supported NETIF_F_IPV6_CSUM (it's only Falcon that didn't).

Yes, those are the removed checks. It's only one check, actually,
sorry for the inaccuracy.

I didn't know since what device family was this supported. Then this
check was unnecessary since the sfc_falcon split.

> Or are you also referring to some items moving from efx.c to the
>  definition of EF10_OFFLOAD_FEATURES?  That's fine and matches more
>  closely to what we do for ef100, but again the commit message could
>  explain this better.

Yes, a very poor explanation. Moving those items is part of the "cleanup", yes.

> In any case this should really be two separate patches, with the
>  cleanup part going to net-next.

I thought about doing that, but I'm always reluctant to increase
reviewers' work by sending small cleanups, so being small and very
related to this patch, I included it here.

> That said, the above is all nit-picky, and the fix looks good, so:
> Acked-by: Edward Cree <ecree.xilinx@...il.com>
>


-- 
Íñigo Huguet