[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aLVrZu6b_-mpt1Fg@fedora>
Date: Mon, 1 Sep 2025 09:46:14 +0000
From: Hangbin Liu <liuhangbin@...il.com>
To: Ido Schimmel <idosch@...dia.com>
Cc: netdev@...r.kernel.org, Jay Vosburgh <jv@...sburgh.net>,
Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Sabrina Dubroca <sdubroca@...hat.com>,
Jiri Pirko <jiri@...nulli.us>, Simon Horman <horms@...nel.org>,
Nikolay Aleksandrov <razor@...ckwall.org>,
Shuah Khan <shuah@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>,
Kuniyuki Iwashima <kuniyu@...gle.com>,
Ahmed Zaki <ahmed.zaki@...el.com>,
Alexander Lobakin <aleksander.lobakin@...el.com>,
bridge@...ts.linux.dev, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next 1/5] net: add a common function to compute
features from lowers devices
On Sun, Aug 31, 2025 at 06:35:49PM +0300, Ido Schimmel wrote:
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 1d1650d9ecff..fcad2a9f6b65 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -12577,6 +12577,85 @@ netdev_features_t netdev_increment_features(netdev_features_t all,
> > }
> > EXPORT_SYMBOL(netdev_increment_features);
> >
> > +/**
> > + * netdev_compute_features_from_lowers - compute feature from lowers
> > + * @dev: the upper device
> > + *
> > + * Recompute the upper device's feature based on all lower devices.
> > + */
> > +void netdev_compute_features_from_lowers(struct net_device *dev)
> > +{
> > + unsigned int dst_release_flag = IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM;
> > + netdev_features_t gso_partial_features = VIRTUAL_DEV_GSO_PARTIAL_FEATURES;
> > +#ifdef CONFIG_XFRM_OFFLOAD
> > + netdev_features_t xfrm_features = VIRTUAL_DEV_XFRM_FEATURES;
> ^ double space (in other places as well)
>
> > +#endif
> > + netdev_features_t mpls_features = VIRTUAL_DEV_MPLS_FEATURES;
> > + netdev_features_t vlan_features = VIRTUAL_DEV_VLAN_FEATURES;
> > + netdev_features_t enc_features = VIRTUAL_DEV_ENC_FEATURES;
> > + unsigned short max_hard_header_len = ETH_HLEN;
>
> hard_header_len is not really a feature, so does not sound like it
> belongs here. I'm pretty sure it's not needed at all.
>
> It was added to the bond driver in 2006 by commit 54ef31371407 ("[PATCH]
> bonding: Handle large hard_header_len") citing panics with gianfar on
> xmit. In 2009 commit 93c1285c5d92 ("gianfar: reallocate skb when
> headroom is not enough for fcb") fixed the gianfar driver to stop
> assuming that it has enough room to push its custom header. Further,
> commit bee9e58c9e98 ("gianfar:don't add FCB length to hard_header_len")
> from 2012 fixed this driver to use needed_headroom instead of
> hard_header_len.
Wow, thanks a lot for the long history changes..
>
> The team driver is also adjusting hard_header_len according to the lower
> devices, but it most likely copied it from the bond driver. On the other
> hand, the bridge driver does not mess with hard_header_len and no
> problems were reported there (that I know of).
>
> Might be a good idea to remove this hard_header_len logic from bond and
> team and instead set their needed_headroom according to the lower device
> with the highest needed_headroom. Paolo added similar logic in bridge
> and ovs but the use case is a bit different there.
OK, I will get the highest needed_headroom for each slave and set
the minimal one to upper device.
Regards
Hangbin
Powered by blists - more mailing lists