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

[Qingfang Deng]

On 2026/11/6 2:53, Alexander Duyck wrote:
> On Thu, Nov 5, 2026 at 5:33 AM Qingfang Deng <qingfang.deng@xxxxxxxxx> wrote:
> > Hi,
> >
> > On 10/20/2026 11:08 PM, Eric Dumazet wrote:
> > > Le mar. 20 oct. 2026 à 16:30, Qingfang Deng <qingfang.deng@xxxxxxxxx> 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@xxxxxxxxx>
> > > > ---
> > > >    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?
> > +Cc: Alexander Duyck
> >
> > I ran the real hardware test on Intel igc.
> >
> > igc_tso() requires skb_network_header() to point to the IP header, which
> > is not the case for PPPoE packets, so it won't work on an unmodified
> > driver. I therefore used a kernel module to inject PPPoE GSO skbs
> > directly to the driver's TX path, deliberately pointing
> > skb_network_header() to IP.
> >
> > Test results show that the hardware can still perform TCP segmentation,
> > but it cannot update the length field in the PPPoE header. So
> > theoretically, we might be able to make GSO_PARTIAL work in PPPoE.
> >
> > Kind regards,
> > Qingfang
> Is PPPoE being stacked similar to a VLAN or being ran as an L3? To be
> able to make it work for the GSO_PARTIAL on most these parts it would
> need to be inserted as part of the MAC/VLAN headers instead of as an
> L3/network header. The main problem is that the drivers normally need
> access to the L3 header in order to set the payload length and things
> like the IPv4 ID field.

Currently, PPPoE occupies the skb's network-header (L3) position. 
pppoe_xmit() points skb->network_header at the PPPoE header and sets 
skb->protocol to ETH_P_PPP_SES. pppoe_gso_segment() temporarily pulls 
the 8 bytes of PPPoE/PPP headers before calling the upper layer handler, 
then restores network_header to PPPoE on the returned segments. For an 
untagged frame, network_header therefore points at offset 14, while the 
actual IP header starts at offset 22.

> Really if you are going to support doing GSO with a PPPoE protocol in
> there you probably need to look at adding a new gso feature flag bit
> as any driver that will support it will need to know how to move the
> header out of the way.

LLM suggested the same and wrote the code, which I have not tested. It 
could make both hardware segmentation and software fallback work.