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

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

[Qingfang Deng]

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

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

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

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

[Eric Dumazet]

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?

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

[Qingfang Deng]

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.

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

[Pablo Neira Ayuso]

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?

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);

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

[Pablo Neira Ayuso]

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);
>