Re: [RFC net] net: force software segmentation of PPPoE packets

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Tue, Oct 20, 2026 at 06:44:50PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Oct 21, 2026 at 12:10:32AM +0800, Qingfang Deng wrote:
> > On 10/20/2026 11:08 PM, Eric Dumazet wrote:
> > > Le mar. 20 oct. 2026 à 16:30, Qingfang Deng <qingfang.deng@linux.dev> a écrit :
> > > > 
> > > > Ordinary TSO does not update the PPPoE payload length. A PPPoE GRO packet
> > > > forwarded by a bridge can retain hardware GSO features and reach the
> > > > driver without software segmentation, if the egress Ethernet interface
> > > > advertises TSO.
> > > > 
> > > > To force software segmentation, clear the GSO feature mask for
> > > > ETH_P_PPP_SES in harmonize_features().
> > > > 
> > > > Fixes: 55a5d8fca836 ("net: pppoe: implement GRO/GSO support")
> > > > Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
> > > > ---
> > > >   net/core/dev.c | 4 ++++
> > > >   1 file changed, 4 insertions(+)
> > > > 
> > > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > > index a6951ea08961..27483a2ad747 100644
> > > > --- a/net/core/dev.c
> > > > +++ b/net/core/dev.c
> > > > @@ -3831,6 +3831,10 @@ static netdev_features_t harmonize_features(struct sk_buff *skb,
> > > >          type = skb_network_protocol(skb, NULL);
> > > >          features = net_mpls_features(skb, features, type);
> > > > 
> > > > +       /* Ordinary TSO does not update the PPPoE payload length. */
> > > > +       if (IS_ENABLED(CONFIG_PPPOE) && type == htons(ETH_P_PPP_SES))
> > > > +               features &= ~NETIF_F_GSO_MASK;
> > > > +
> > > >          if (skb->ip_summed != CHECKSUM_NONE &&
> > > >              !can_checksum_protocol(features, type)) {
> > > >                  features &= ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
> > > > --
> > > > 2.34.1
> > > > 
> > > 
> > > Ouch.
> > > 
> > > harmonize_features() is called for every packet, GSO or not.
> > > 
> > > This is a GSO-only problem, please do not add more protocol-specific
> > > tests in this path. gso_features_check() is only called for GSO
> > > packets, and the cost there is amortized over all segments.
> > > 
> > > Devices without NETIF_F_HW_CSUM already lose GSO for ETH_P_PPP_SES
> > > because of the can_checksum_protocol() check in harmonize_features(),
> > > and fraglist GRO packets (the main use case for PPPoE GRO) are
> > > segmented in software anyway.
> > > 
> > > This leaves bridged, non fraglist GRO PPPoE packets sent to a
> > > NETIF_F_HW_CSUM + TSO device. Have you hit this on real hardware?
> > 
> > I reproduced this on a VM veth interface, which advertises both features.
> > The real hardware I have is an Intel IGC but I have not tested with it.
> 
> IIRC, I could reproduce the issue too with a virtio netdevice.
> 
> I made this inaceptable patch (see oneliner patch with protocol
> specific update below), so the recent pppoe gso support kicks.
> 
> I guess adding NETIF_F_GSO_PPPOE (similar to NETIF_F_GSO_SCTP) is a
> no-go to force software tx path to segment skbs too?

s/NETIF_F_GSO_/SKB_GSO_

> Thanks.
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 9981d637f8b5..07bf265d0295 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -5514,7 +5514,8 @@ static inline bool netif_needs_gso(struct sk_buff *skb,
>  {
>         return skb_is_gso(skb) && (!skb_gso_ok(skb, features) ||
>                 unlikely((skb->ip_summed != CHECKSUM_PARTIAL) &&
> -                        (skb->ip_summed != CHECKSUM_UNNECESSARY)));
> +                        (skb->ip_summed != CHECKSUM_UNNECESSARY)) ||
> +               unlikely(skb->protocol == htons(ETH_P_PPP_SES)));
>  }
>  
>  void netif_set_tso_max_size(struct net_device *dev, unsigned int size);
>