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

[Qingfang Deng <qingfang.deng@linux.dev>]

On 2026/11/6 2:53, Alexander Duyck wrote:
> On Thu, Nov 5, 2026 at 5:33 AM Qingfang Deng <qingfang.deng@linux.dev> wrote:
>> Hi,
>>
>> 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?
>> +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.