netdev - Re: [PATCH net] netdev: increment TSO only if TSO is not enabled on any slave device

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: <CANn89i+p0UX1VW9Pm6_B5tJ-_b_iwJP5Dkk_Agnf+46FD2jY-g@mail.gmail.com>
Date: Tue, 23 Dec 2025 15:47:43 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Zhud <zhud@...on.cn>
Cc: Paolo Abeni <pabeni@...hat.com>, "davem@...emloft.net" <davem@...emloft.net>, 
	"kuba@...nel.org" <kuba@...nel.org>, "horms@...nel.org" <horms@...nel.org>, 
	"andrew+netdev@...n.ch" <andrew+netdev@...n.ch>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, 
	Jing Li <lijing@...on.cn>, Zhiwei Ying <yingzhiwei@...on.cn>
Subject: Re: [PATCH net] netdev: increment TSO only if TSO is not enabled on
 any slave device

On Tue, Dec 23, 2025 at 1:20 PM Zhud <zhud@...on.cn> wrote:
>
>
> > On 12/16/25 9:52 AM, Di Zhu wrote:
> > > Unconditionally increment the TSO flag has a side effect: it will also
> >
> > This changelog is IMHO quite confusing. The code does not 'increment TSO'. Instead
> > it increments the features set to include ALL_TSO.
> >
> > Please reword the changelog accordingly.
> >
> > > directly clear the flags in NETIF_F_ALL_FOR_ALL on the master device,
> > > which can cause issues such as the inability to enable the nocache
> > > copy feature on the bonding network card.
> >
> > bonding network card -> bonding driver.
> >
> > > So, when at least one slave device's TSO is enabled, there is no need
> > > to explicitly increment the TSO flag to the master device.
> > >
> > > Fixes: b0ce3508b25e ("bonding: allow TSO being set on bonding master")
> > > Signed-off-by: Di Zhu <zhud@...on.cn>
> > > ---
> > >  include/linux/netdevice.h | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > index bf99fe8622da..2aca39f7f9e1 100644
> > > --- a/include/linux/netdevice.h
> > > +++ b/include/linux/netdevice.h
> > > @@ -5322,7 +5322,8 @@ netdev_features_t
> > > netdev_increment_features(netdev_features_t all,  static inline
> > netdev_features_t netdev_add_tso_features(netdev_features_t features,
> > >                                                     netdev_features_t mask)
> > >  {
> > > -   return netdev_increment_features(features, NETIF_F_ALL_TSO, mask);
> > > +   return (features & NETIF_F_ALL_TSO) ? features :
> > > +           netdev_increment_features(features, NETIF_F_ALL_TSO, mask);
> >
> > NETIF_F_ALL_TSO is not a single bit, but a (later large) bit mask; the above will yield
> > incorrect result when:
> >
> >       features & NETIF_F_ALL_TSO != NETIF_F_ALL_TSO
>
> Yes, it is indeed necessary to set all tso flags to avoid GSO at the bonding layer.
> I will revise the code and its related changlong, thanks.

What about this instead ?

 static inline netdev_features_t
netdev_add_tso_features(netdev_features_t features,
                                                        netdev_features_t mask)
 {
-       return netdev_increment_features(features, NETIF_F_ALL_TSO, mask);
+       return netdev_increment_features(features, NETIF_F_ALL_TSO |
+                                        NETIF_F_ALL_FOR_ALL, mask);
 }