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

[Alexander Duyck]

On Thu, Nov 5, 2026 at 6:08 PM Qingfang Deng <qingfang.deng@xxxxxxxxx> wrote:
>
> 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.

Yeah, it makes the most sense. Basically it will then treat the same
way we handle vXvX tunnels since you are essentially doubling up the
outer L3 headers on the interface.

Basically just means you have to add it to the gso_type on the GRO
side, and add logic to process it in the GSO side. The GSO_PARTIAL
setups gets a bit more complicated as you will have to add logic to
pop the header out of the way before your process the IPv4/6 header on
the inside.