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: <355e6f707b57413098af339d0bd6dcba@hygon.cn>
Date: Wed, 24 Dec 2025 00:48:36 +0000
From: Zhud <zhud@...on.cn>
To: Eric Dumazet <edumazet@...gle.com>
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);
>  }

Yes, I also want to do it this way.