* [RFC net] net: force software segmentation of PPPoE packets
@ 2026-10-20 14:30 Qingfang Deng
2026-10-20 14:34 ` netdev-bot+sinfo
2026-10-20 15:08 ` Eric Dumazet
0 siblings, 2 replies; 6+ messages in thread
From: Qingfang Deng @ 2026-10-20 14:30 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Kuniyuki Iwashima, Stanislav Fomichev, Hangbin Liu,
Martin Karsten, Felix Fietkau, netdev, linux-kernel
Cc: Pablo Neira Ayuso
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC net] net: force software segmentation of PPPoE packets
2026-10-20 14:30 [RFC net] net: force software segmentation of PPPoE packets Qingfang Deng
@ 2026-10-20 14:34 ` netdev-bot+sinfo
2026-10-20 15:08 ` Eric Dumazet
1 sibling, 0 replies; 6+ messages in thread
From: netdev-bot+sinfo @ 2026-10-20 14:34 UTC (permalink / raw)
To: Qingfang Deng
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Kuniyuki Iwashima, Stanislav Fomichev, Hangbin Liu,
Martin Karsten, Felix Fietkau, netdev, linux-kernel,
Pablo Neira Ayuso
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC net] net: force software segmentation of PPPoE packets
2026-10-20 14:30 [RFC net] net: force software segmentation of PPPoE packets Qingfang Deng
2026-10-20 14:34 ` netdev-bot+sinfo
@ 2026-10-20 15:08 ` Eric Dumazet
2026-10-20 16:10 ` Qingfang Deng
1 sibling, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2026-10-20 15:08 UTC (permalink / raw)
To: Qingfang Deng
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
Kuniyuki Iwashima, Stanislav Fomichev, Hangbin Liu,
Martin Karsten, Felix Fietkau, netdev, linux-kernel,
Pablo Neira Ayuso
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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC net] net: force software segmentation of PPPoE packets
2026-10-20 15:08 ` Eric Dumazet
@ 2026-10-20 16:10 ` Qingfang Deng
2026-10-20 16:44 ` Pablo Neira Ayuso
0 siblings, 1 reply; 6+ messages in thread
From: Qingfang Deng @ 2026-10-20 16:10 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
Kuniyuki Iwashima, Stanislav Fomichev, Hangbin Liu,
Martin Karsten, Felix Fietkau, netdev, linux-kernel,
Pablo Neira Ayuso
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC net] net: force software segmentation of PPPoE packets
2026-10-20 16:10 ` Qingfang Deng
@ 2026-10-20 16:44 ` Pablo Neira Ayuso
2026-10-20 16:53 ` Pablo Neira Ayuso
0 siblings, 1 reply; 6+ messages in thread
From: Pablo Neira Ayuso @ 2026-10-20 16:44 UTC (permalink / raw)
To: Qingfang Deng
Cc: Eric Dumazet, David S. Miller, Jakub Kicinski, Paolo Abeni,
Simon Horman, Kuniyuki Iwashima, Stanislav Fomichev, Hangbin Liu,
Martin Karsten, Felix Fietkau, netdev, linux-kernel
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);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC net] net: force software segmentation of PPPoE packets
2026-10-20 16:44 ` Pablo Neira Ayuso
@ 2026-10-20 16:53 ` Pablo Neira Ayuso
0 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2026-10-20 16:53 UTC (permalink / raw)
To: Qingfang Deng
Cc: Eric Dumazet, David S. Miller, Jakub Kicinski, Paolo Abeni,
Simon Horman, Kuniyuki Iwashima, Stanislav Fomichev, Hangbin Liu,
Martin Karsten, Felix Fietkau, netdev, linux-kernel
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);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-20 16:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-20 14:30 [RFC net] net: force software segmentation of PPPoE packets Qingfang Deng
2026-10-20 14:34 ` netdev-bot+sinfo
2026-10-20 15:08 ` Eric Dumazet
2026-10-20 16:10 ` Qingfang Deng
2026-10-20 16:44 ` Pablo Neira Ayuso
2026-10-20 16:53 ` Pablo Neira Ayuso
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.