[PATCH net-next v2 0/4] net: resegment oversized TCP GSO skbs

[PATCH net-next v2 0/4] net: resegment oversized TCP GSO skbs

[Wang Zhan]

BIG TCP is negotiated per netdevice, so BIG TCP and non-BIG TCP ports can
coexist in one path. When a BIG TCP packet goes to a port without BIG TCP
support, it loses its GSO state and the stack segments it into individual
MSS sized packets, so the port TSO is left unused.

This series cuts an oversized unencapsulated TCP GSO skb into GSO skbs
which fit the device limits instead of single segments, so the per-segment
work stays on the device TSO. On a veth -> bridge -> TAP -> guest
virtio-net path, with BIG TCP enabled on the veth endpoints and left off
in the guest, a single iperf3 TCP flow, six alternating runs per state
(-t 15 -O 5, fixed CPU affinity and port tuple):

  protocol  no BIG TCP   mixed, no reseg  mixed, resegmented
  TCP/IPv4  51.550 Gbps  15.850 Gbps      52.617 Gbps
  TCP/IPv6  52.050 Gbps  15.783 Gbps      51.933 Gbps

The middle column comes from the same kernel with the bounded path
disabled for the comparison. A BIG TCP hop which feeds a 64 KiB hop loses
69% of the throughput of a path which never enables BIG TCP, and bounded
resegmentation recovers it.

The new path is taken only when the skb is a plain TCP GSO skb which
exceeds gso_max_size or gso_max_segs, the device offloads that GSO type
and has scatter-gather and checksum offload for the protocol, and the
bound leaves room for at least two MSS segments per output skb.
Encapsulated skbs, frag-list skbs, GSO types the device cannot offload and
bounds below two segments keep the existing full segmentation path
unchanged.

The output obeys the GSO feature and limit contract the device already
advertises - gso_size stays at the MSS, gso_segs stays within the bound,
and the frame length stays below gso_max_size - so this needs no new UAPI,
no device state and no driver change, and it is applied automatically.
I considered a per-device switch and decided against it: it would spend
netlink ABI and net_device state on a decision the stack can make from
capabilities the device already advertises, and it would have to be
configured on every device created later.

Patch layout:

  [1/4] factor the device GSO limit check out of gso_features_check()
  [2/4] let the GSO engine bound the MSS segments per output skb
  [3/4] apply that bound to oversized TCP GSO skbs in the TX path
  [4/4] KUnit coverage for the bound, the device limits and the TCP path

1/4 is a preparation patch with no functional change.

v2:
- patch 2 and patch 4: fix the lines over 80 columns reported by checkpatch
- patch 4: use KUNIT_ASSERT_TRUE() for the __be16 check, EQ warns in sparse
- Cc the full get_maintainer list (patch 2 was missing dev@openvswitch.org)
v1: https://lore.kernel.org/20260917063854.2011613-1-wang.zhan@smartx.com/

Wang Zhan (4):
  net: core: factor out the GSO device limit check
  net: gso: support bounded TCP segmentation
  net: core: resegment oversized TCP GSO skbs
  net: net_test: add tests for bounded GSO segmentation

 drivers/net/tap.c          |   3 +-
 include/linux/netdevice.h  |   4 +-
 include/net/gso.h          |   6 +-
 include/net/udp.h          |   2 +-
 net/core/dev.c             | 123 +++++++++++++++++--
 net/core/gso.c             |   5 +-
 net/core/net_test.c        | 243 +++++++++++++++++++++++++++++++++++++
 net/core/skbuff.c          |  14 ++-
 net/ipv4/tcp_offload.c     |   3 +-
 net/openvswitch/datapath.c |   2 +-
 10 files changed, 383 insertions(+), 22 deletions(-)


base-commit: 4bb9710c6a68d35207f123aef55dcd50e7195ec5
-- 
2.47.3

[PATCH net-next v2 1/4] net: core: factor out the GSO device limit check

[Wang Zhan]

gso_features_check() decides whether an egress device can offload a GSO
skb as a single TSO frame by comparing the segment count and the frame
length against the device limits. Move that test into a helper so that
the bounded resegmentation path added by a later patch can ask the same
question without repeating the two expressions. Make the size limit
lookup take the protocol as an argument, because that path has to ask
for the limit of a protocol other than the one in skb->protocol.

No functional changes.

Assisted-by: LLM
Signed-off-by: Wang Zhan <wang.zhan@smartx.com>
---
 include/linux/netdevice.h |  4 ++--
 net/core/dev.c            | 14 ++++++++------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 1f0710eef185b..427d0d5b94e49 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -5570,10 +5570,10 @@ netif_get_gro_max_size(const struct net_device *dev, const struct sk_buff *skb)
 }
 
 static inline unsigned int
-netif_get_gso_max_size(const struct net_device *dev, const struct sk_buff *skb)
+netif_get_gso_max_size(const struct net_device *dev, __be16 protocol)
 {
 	/* pairs with WRITE_ONCE() in netif_set_gso(_ipv4)_max_size() */
-	return skb->protocol == htons(ETH_P_IPV6) ?
+	return protocol == htons(ETH_P_IPV6) ?
 	       READ_ONCE(dev->gso_max_size) :
 	       READ_ONCE(dev->gso_ipv4_max_size);
 }
diff --git a/net/core/dev.c b/net/core/dev.c
index c67900354fa64..16685888b2812 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3834,16 +3834,18 @@ static bool skb_gso_has_extension_hdr(const struct sk_buff *skb)
 			 skb_inner_network_header_len(skb) != sizeof(struct ipv6hdr)));
 }
 
+static bool gso_within_device_limits(const struct sk_buff *skb,
+				     const struct net_device *dev)
+{
+	return skb_shinfo(skb)->gso_segs <= READ_ONCE(dev->gso_max_segs) &&
+	       skb->len < netif_get_gso_max_size(dev, skb->protocol);
+}
+
 static netdev_features_t gso_features_check(const struct sk_buff *skb,
 					    struct net_device *dev,
 					    netdev_features_t features)
 {
-	u16 gso_segs = skb_shinfo(skb)->gso_segs;
-
-	if (gso_segs > READ_ONCE(dev->gso_max_segs))
-		return features & ~NETIF_F_GSO_MASK;
-
-	if (unlikely(skb->len >= netif_get_gso_max_size(dev, skb)))
+	if (!gso_within_device_limits(skb, dev))
 		return features & ~NETIF_F_GSO_MASK;
 
 	if (!skb_shinfo(skb)->gso_type) {
-- 
2.47.3

[PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[Wang Zhan]

The bounded resegmentation added by the next patch splits an oversized TCP
GSO skb into several GSO skbs which fit the device limits. That needs the
GSO engine to group several MSS segments into one output skb, so let
callers bound the number of MSS segments each output skb carries and pass
the bound through the existing __skb_gso_segment() entry point. Ordinary
callers use zero for no limit.

skb_segment() only groups several MSS into one output skb when the device
advertises NETIF_F_GSO_PARTIAL, or when the skb has a frag_list which can
be split into uniform pieces, and falls back to one segment per skb
otherwise. A caller which passes a bound asks for that grouping
regardless, so the frag_list check is skipped when max_segs is set. Every
other caller keeps it, and the bounded path is only used for skbs which do
not carry a frag_list.

The output stays a GSO skb: gso_size is the original MSS and gso_segs is
the number of MSS it holds, so a downstream device can still perform
ordinary TSO. Store the bound in the existing skb_gso_cb scratch context,
alongside the call-local data_offset and mac_offset fields, so that the
segmentation methods keep their signature. A zero max_segs value means
that no bound is active; it is not a persistent skb flag. Clear the value
when each output skb copies the input header so the temporary limit is not
propagated to the next GSO call.

Assisted-by: LLM
Signed-off-by: Wang Zhan <wang.zhan@smartx.com>

---
v2:
- wrap the tap.c declaration and skb_gso_cb comment to 80 columns
v1: https://lore.kernel.org/20260917063854.2011613-3-wang.zhan@smartx.com/
---
 drivers/net/tap.c          |  3 ++-
 include/net/gso.h          |  6 ++++--
 include/net/udp.h          |  2 +-
 net/core/gso.c             |  5 ++++-
 net/core/skbuff.c          | 14 ++++++++++++--
 net/ipv4/tcp_offload.c     |  3 ++-
 net/openvswitch/datapath.c |  2 +-
 7 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index ff67d99deb39e..bc111495ebbce 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -278,9 +278,10 @@ rx_handler_result_t tap_handle_frame(struct sk_buff **pskb)
 	if (q->flags & IFF_VNET_HDR)
 		features |= tap->tap_features;
 	if (netif_needs_gso(skb, features)) {
-		struct sk_buff *segs = __skb_gso_segment(skb, features, false);
+		struct sk_buff *segs;
 		struct sk_buff *next;
 
+		segs = __skb_gso_segment(skb, features, false, 0);
 		if (IS_ERR(segs)) {
 			drop_reason = SKB_DROP_REASON_SKB_GSO_SEG;
 			goto drop;
diff --git a/include/net/gso.h b/include/net/gso.h
index 29975440cad51..fccb37889965f 100644
--- a/include/net/gso.h
+++ b/include/net/gso.h
@@ -19,6 +19,7 @@ struct skb_gso_cb {
 	int	encap_level;
 	__wsum	csum;
 	__u16	csum_start;
+	__u16	max_segs;	/* Max MSS segs per output skb, 0 = no limit */
 };
 #define SKB_GSO_CB_OFFSET	32
 #define SKB_GSO_CB(skb) ((struct skb_gso_cb *)((skb)->cb + SKB_GSO_CB_OFFSET))
@@ -75,12 +76,13 @@ static inline __sum16 gso_make_checksum(struct sk_buff *skb, __wsum res)
 }
 
 struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
-				  netdev_features_t features, bool tx_path);
+				  netdev_features_t features, bool tx_path,
+				  unsigned int max_segs);
 
 static inline struct sk_buff *skb_gso_segment(struct sk_buff *skb,
 					      netdev_features_t features)
 {
-	return __skb_gso_segment(skb, features, true);
+	return __skb_gso_segment(skb, features, true, 0);
 }
 
 struct sk_buff *skb_eth_gso_segment(struct sk_buff *skb,
diff --git a/include/net/udp.h b/include/net/udp.h
index 1fee17274745f..5bc25dcf25fba 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -613,7 +613,7 @@ static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
 	/* the GSO CB lays after the UDP one, no need to save and restore any
 	 * CB fragment
 	 */
-	segs = __skb_gso_segment(skb, features, false);
+	segs = __skb_gso_segment(skb, features, false, 0);
 	if (IS_ERR_OR_NULL(segs)) {
 		drop_count = skb_shinfo(skb)->gso_segs;
 		goto drop;
diff --git a/net/core/gso.c b/net/core/gso.c
index bcd156372f4df..157f2bfdca128 100644
--- a/net/core/gso.c
+++ b/net/core/gso.c
@@ -77,6 +77,7 @@ static bool skb_needs_check(const struct sk_buff *skb, bool tx_path)
  *	@skb: buffer to segment
  *	@features: features for the output path (see dev->features)
  *	@tx_path: whether it is called in TX path
+ *	@max_segs: maximum MSS segments per output GSO skb, 0 means no limit
  *
  *	This function segments the given skb and returns a list of segments.
  *
@@ -86,7 +87,8 @@ static bool skb_needs_check(const struct sk_buff *skb, bool tx_path)
  *	Segmentation preserves SKB_GSO_CB_OFFSET bytes of previous skb cb.
  */
 struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
-				  netdev_features_t features, bool tx_path)
+				  netdev_features_t features, bool tx_path,
+				  unsigned int max_segs)
 {
 	struct sk_buff *segs;
 
@@ -117,6 +119,7 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
 
 	SKB_GSO_CB(skb)->mac_offset = skb_headroom(skb);
 	SKB_GSO_CB(skb)->encap_level = 0;
+	SKB_GSO_CB(skb)->max_segs = min_t(unsigned int, max_segs, U16_MAX);
 
 	skb_reset_mac_header(skb);
 	skb_reset_mac_len(skb);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index dbbe10277d51d..9c0d140236bc6 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4793,6 +4793,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
 	struct sk_buff *segs = NULL;
 	struct sk_buff *tail = NULL;
 	struct sk_buff *list_skb = skb_shinfo(head_skb)->frag_list;
+	unsigned int max_segs = SKB_GSO_CB(head_skb)->max_segs;
 	unsigned int mss = skb_shinfo(head_skb)->gso_size;
 	bool gso_by_frags = mss == GSO_BY_FRAGS;
 	unsigned int doffset = head_skb->data - skb_mac_header(head_skb);
@@ -4839,7 +4840,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
 	csum = !!can_checksum_protocol(features, proto);
 
 	if (sg && csum && !gso_by_frags)  {
-		if (!(features & NETIF_F_GSO_PARTIAL)) {
+		if (!max_segs && !(features & NETIF_F_GSO_PARTIAL)) {
 			struct sk_buff *iter;
 			unsigned int frag_len;
 
@@ -4874,7 +4875,10 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
 		 * now.
 		 */
 		DEBUG_NET_WARN_ON_ONCE(len / mss > GSO_MAX_SEGS);
-		partial_segs = min(len / mss, GSO_MAX_SEGS);
+		if (max_segs)
+			partial_segs = min(len / mss, max_segs);
+		else
+			partial_segs = min(len / mss, GSO_MAX_SEGS);
 		if (partial_segs > 1)
 			mss *= partial_segs;
 		else
@@ -4975,6 +4979,12 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
 
 		__copy_skb_header(nskb, head_skb);
 
+		/*
+		 * max_segs is a per-call limit, so output skbs must not
+		 * inherit it from the input skb.
+		 */
+		SKB_GSO_CB(nskb)->max_segs = 0;
+
 		skb_headers_offset_update(nskb, skb_headroom(nskb) - headroom);
 		skb_reset_mac_len(nskb);
 
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index e74d99ca9face..a4076318c5352 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -164,7 +164,8 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb,
 	if (unlikely(skb->len <= mss))
 		goto out;
 
-	if (skb_gso_ok(skb, features | NETIF_F_GSO_ROBUST)) {
+	if (!SKB_GSO_CB(skb)->max_segs &&
+	    skb_gso_ok(skb, features | NETIF_F_GSO_ROBUST)) {
 		/* Packet is from an untrusted source, reset gso_segs. */
 
 		skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(skb->len, mss);
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 2187034143255..e793aead68372 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -375,7 +375,7 @@ static int queue_gso_packets(struct datapath *dp, struct sk_buff *skb,
 	int err;
 
 	BUILD_BUG_ON(sizeof(*OVS_CB(skb)) > SKB_GSO_CB_OFFSET);
-	segs = __skb_gso_segment(skb, NETIF_F_SG, false);
+	segs = __skb_gso_segment(skb, NETIF_F_SG, false, 0);
 	if (IS_ERR(segs))
 		return PTR_ERR(segs);
 	if (segs == NULL)
-- 
2.47.3

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[Willem de Bruijn]

Wang Zhan wrote:
> The bounded resegmentation added by the next patch splits an oversized TCP
> GSO skb into several GSO skbs which fit the device limits. That needs the
> GSO engine to group several MSS segments into one output skb, so let
> callers bound the number of MSS segments each output skb carries and pass
> the bound through the existing __skb_gso_segment() entry point. Ordinary
> callers use zero for no limit.
> 
> skb_segment() only groups several MSS into one output skb when the device
> advertises NETIF_F_GSO_PARTIAL, or when the skb has a frag_list which can
> be split into uniform pieces, and falls back to one segment per skb
> otherwise. A caller which passes a bound asks for that grouping
> regardless, so the frag_list check is skipped when max_segs is set. Every
> other caller keeps it, and the bounded path is only used for skbs which do
> not carry a frag_list.
> 
> The output stays a GSO skb: gso_size is the original MSS and gso_segs is
> the number of MSS it holds, so a downstream device can still perform
> ordinary TSO. Store the bound in the existing skb_gso_cb scratch context,
> alongside the call-local data_offset and mac_offset fields, so that the
> segmentation methods keep their signature. A zero max_segs value means
> that no bound is active; it is not a persistent skb flag. Clear the value
> when each output skb copies the input header so the temporary limit is not
> propagated to the next GSO call.
> 
> Assisted-by: LLM
> Signed-off-by: Wang Zhan <wang.zhan@smartx.com>
> 
> ---
> v2:
> - wrap the tap.c declaration and skb_gso_cb comment to 80 columns
> v1: https://lore.kernel.org/20260917063854.2011613-3-wang.zhan@smartx.com/
> ---
>  drivers/net/tap.c          |  3 ++-
>  include/net/gso.h          |  6 ++++--
>  include/net/udp.h          |  2 +-
>  net/core/gso.c             |  5 ++++-
>  net/core/skbuff.c          | 14 ++++++++++++--
>  net/ipv4/tcp_offload.c     |  3 ++-
>  net/openvswitch/datapath.c |  2 +-
>  7 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> index ff67d99deb39e..bc111495ebbce 100644
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -278,9 +278,10 @@ rx_handler_result_t tap_handle_frame(struct sk_buff **pskb)
>  	if (q->flags & IFF_VNET_HDR)
>  		features |= tap->tap_features;
>  	if (netif_needs_gso(skb, features)) {
> -		struct sk_buff *segs = __skb_gso_segment(skb, features, false);
> +		struct sk_buff *segs;
>  		struct sk_buff *next;
>  
> +		segs = __skb_gso_segment(skb, features, false, 0);

irrelevant?

> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index dbbe10277d51d..9c0d140236bc6 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -4793,6 +4793,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>  	struct sk_buff *segs = NULL;
>  	struct sk_buff *tail = NULL;
>  	struct sk_buff *list_skb = skb_shinfo(head_skb)->frag_list;
> +	unsigned int max_segs = SKB_GSO_CB(head_skb)->max_segs;

could this be computed inside skb_segment, rather than having to be
passed through SKB_GSO_CB. I haven't checked, but it would simplify.

>  	unsigned int mss = skb_shinfo(head_skb)->gso_size;
>  	bool gso_by_frags = mss == GSO_BY_FRAGS;
>  	unsigned int doffset = head_skb->data - skb_mac_header(head_skb);

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[Wang Zhan]

On Sat, 19 Sep 2026 11:35:17 -0400 Willem de Bruijn wrote:
> > -		struct sk_buff *segs = __skb_gso_segment(skb, features, false);
> > +		struct sk_buff *segs;
> >  		struct sk_buff *next;
> > +		segs = __skb_gso_segment(skb, features, false, 0);
>
> irrelevant?

Not unrelated: with the extra argument that line is 82 columns, so the
initializer moved to its own line.  The call itself is unchanged.

> > +	unsigned int max_segs = SKB_GSO_CB(head_skb)->max_segs;
>
> could this be computed inside skb_segment, rather than having to be
> passed through SKB_GSO_CB. I haven't checked, but it would simplify.

I tried it: https://github.com/zwtop/linux/pull/3

It does read better, but whether to resegment is the caller's choice: the
qdiscs strip the GSO bits to get one packet per segment (sch_netem.c:443),
and a device-derived limit groups that output instead - which sch_netem then
drops, because skb_checksum_help() on the first segment rejects a GSO skb
(sch_netem.c:538, net/core/dev.c:3626).  The features cannot tell the two
cases apart either: gso_features_check() clears the same bits for an
over-limit skb (net/core/dev.c:3843).

So the bound stays an input from the caller.

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[Willem de Bruijn]

Wang Zhan wrote:
> On Sat, 19 Sep 2026 11:35:17 -0400 Willem de Bruijn wrote:
> > > -		struct sk_buff *segs = __skb_gso_segment(skb, features, false);
> > > +		struct sk_buff *segs;
> > >  		struct sk_buff *next;
> > > +		segs = __skb_gso_segment(skb, features, false, 0);
> >
> > irrelevant?
> 
> Not unrelated: with the extra argument that line is 82 columns, so the
> initializer moved to its own line.  The call itself is unchanged.
> 
> > > +	unsigned int max_segs = SKB_GSO_CB(head_skb)->max_segs;
> >
> > could this be computed inside skb_segment, rather than having to be
> > passed through SKB_GSO_CB. I haven't checked, but it would simplify.
> 
> I tried it: https://github.com/zwtop/linux/pull/3
> 
> It does read better, but whether to resegment is the caller's choice: the
> qdiscs strip the GSO bits to get one packet per segment (sch_netem.c:443),
> and a device-derived limit groups that output instead - which sch_netem then
> drops, because skb_checksum_help() on the first segment rejects a GSO skb
> (sch_netem.c:538, net/core/dev.c:3626). 

So this is a rare netem edge case we need to handle.

In the hot path, we should be able to defer the decision whether to
segment entirely or segment to the capabilities of the device to
skb_segment itself.

> The features cannot tell the two
> cases apart either: gso_features_check() clears the same bits for an
> over-limit skb (net/core/dev.c:3843).

I wonder if we can refine this instead.

> So the bound stays an input from the caller.

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[Wang Zhan]

On Mon, 21 Sep 2026 16:36:37 -0400 Willem de Bruijn wrote:
> Wang Zhan wrote:
>> I tried it: https://github.com/zwtop/linux/pull/3
>>
>> It does read better, but whether to resegment is the caller's choice: the
>> qdiscs strip the GSO bits to get one packet per segment (sch_netem.c:443),
>> and a device-derived limit groups that output instead - which sch_netem then
>> drops, because skb_checksum_help() on the first segment rejects a GSO skb
>> (sch_netem.c:538, net/core/dev.c:3626).
>
> So this is a rare netem edge case we need to handle.

That is only one example.  I have not checked every caller, but at least tc
sched tbf/cake and the OVS upcall are as well.

> In the hot path, we should be able to defer the decision whether to
> segment entirely or segment to the capabilities of the device to
> skb_segment itself.

Deciding it entirely inside skb_segment(), without passing extra
information, is difficult: the oversize case overlaps with the cases above.
And the features we pass in may also have had NETIF_F_GSO_MASK cleared, so
it would have to take the device from skb->dev (that should not be much of a
problem if we only handle the TX path).

>> The features cannot tell the two
>> cases apart either: gso_features_check() clears the same bits for an
>> over-limit skb (net/core/dev.c:3843).
>
> I wonder if we can refine this instead.

Maybe we can add a bool to skb_gso_cb saying whether GSO output is allowed,
and leave it to skb_segment() to decide whether a GSO packet is actually
emitted, and the max-segs.

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[Willem de Bruijn]

Wang Zhan wrote:
> On Mon, 21 Sep 2026 16:36:37 -0400 Willem de Bruijn wrote:
> > Wang Zhan wrote:
> >> I tried it: https://github.com/zwtop/linux/pull/3
> >>
> >> It does read better, but whether to resegment is the caller's choice: the
> >> qdiscs strip the GSO bits to get one packet per segment (sch_netem.c:443),
> >> and a device-derived limit groups that output instead - which sch_netem then
> >> drops, because skb_checksum_help() on the first segment rejects a GSO skb
> >> (sch_netem.c:538, net/core/dev.c:3626).
> >
> > So this is a rare netem edge case we need to handle.
> 
> That is only one example.  I have not checked every caller, but at least tc
> sched tbf/cake and the OVS upcall are as well.
> 
> > In the hot path, we should be able to defer the decision whether to
> > segment entirely or segment to the capabilities of the device to
> > skb_segment itself.
> 
> Deciding it entirely inside skb_segment(), without passing extra
> information, is difficult: the oversize case overlaps with the cases above.
> And the features we pass in may also have had NETIF_F_GSO_MASK cleared, so
> it would have to take the device from skb->dev (that should not be much of a
> problem if we only handle the TX path).
> 
> >> The features cannot tell the two
> >> cases apart either: gso_features_check() clears the same bits for an
> >> over-limit skb (net/core/dev.c:3843).
> >
> > I wonder if we can refine this instead.
> 
> Maybe we can add a bool to skb_gso_cb saying whether GSO output is allowed,
> and leave it to skb_segment() to decide whether a GSO packet is actually
> emitted, and the max-segs.

Precomputing in the caller, as the current series does, is fine too,
if some caller-specific context is needed.

I'm mostly concerned about duplicating logic and the number of
functions touched in this series. But skb_segment itself is too
complex already, so preferable to minimize complication there.
(The reuse of partial for this purpose is very neat.)

If only validate_xmit_skb allows this, because all other segmentation
callers do want full segmentation (not checked, but I can believe
that), the current approach is fine. If we can clean up the repeated
tests and simplify the code in general.

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[Wang Zhan]

On Wed, 23 Sep 2026 12:42:59 -0400 Willem de Bruijn wrote:
> Precomputing in the caller, as the current series does, is fine too,
> if some caller-specific context is needed.
>
> I'm mostly concerned about duplicating logic and the number of
> functions touched in this series. But skb_segment itself is too
> complex already, so preferable to minimize complication there.
> (The reuse of partial for this purpose is very neat.)
>
> If only validate_xmit_skb allows this, because all other segmentation
> callers do want full segmentation (not checked, but I can believe
> that), the current approach is fine. If we can clean up the repeated
> tests and simplify the code in general.

OK, I will try to simplify the code and reduce the hot path cost in v3.
By the way, changing the signature of __skb_gso_segment() brings three
extra call site changes, and that is a trade-off.  __skb_gso_segment() and
skb_gso_segment() already exist, and adding another function like
skb_gso_segment_max_segs() would make the call chain confusing.

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[Willem de Bruijn]

Wang Zhan wrote:
> On Sat, 19 Sep 2026 11:35:17 -0400 Willem de Bruijn wrote:
> > > -		struct sk_buff *segs = __skb_gso_segment(skb, features, false);
> > > +		struct sk_buff *segs;
> > >  		struct sk_buff *next;
> > > +		segs = __skb_gso_segment(skb, features, false, 0);
> >
> > irrelevant?
> 
> Not unrelated: with the extra argument that line is 82 columns, so the
> initializer moved to its own line.  The call itself is unchanged.
> 
> > > +	unsigned int max_segs = SKB_GSO_CB(head_skb)->max_segs;
> >
> > could this be computed inside skb_segment, rather than having to be
> > passed through SKB_GSO_CB. I haven't checked, but it would simplify.
> 
> I tried it: https://github.com/zwtop/linux/pull/3
> 
> It does read better, but whether to resegment is the caller's choice: the
> qdiscs strip the GSO bits to get one packet per segment (sch_netem.c:443),
> and a device-derived limit groups that output instead - which sch_netem then
> drops, because skb_checksum_help() on the first segment rejects a GSO skb
> (sch_netem.c:538, net/core/dev.c:3626).  The features cannot tell the two
> cases apart either: gso_features_check() clears the same bits for an
> over-limit skb (net/core/dev.c:3843).
> 
> So the bound stays an input from the caller.

Somewhat tangential, but an entirely different practical approach
could be to disable BIG-TCP when such a patch from BIG-TCP capable to
non-capable device is encountered.

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[Wang Zhan]

On Mon, 21 Sep 2026 17:07:09 -0400 Willem de Bruijn wrote:
> Somewhat tangential, but an entirely different practical approach
> could be to disable BIG-TCP when such a patch from BIG-TCP capable to
> non-capable device is encountered.

That means a single device in the topology which does not support BIG TCP
forces every other device to turn BIG TCP off, to avoid the degradation.
So with mixed devices and a dynamic topology, BIG TCP cannot be enabled at all.

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[Willem de Bruijn]

Wang Zhan wrote:
> On Mon, 21 Sep 2026 17:07:09 -0400 Willem de Bruijn wrote:
> > Somewhat tangential, but an entirely different practical approach
> > could be to disable BIG-TCP when such a patch from BIG-TCP capable to
> > non-capable device is encountered.
> 
> That means a single device in the topology which does not support BIG TCP
> forces every other device to turn BIG TCP off, to avoid the degradation.
> So with mixed devices and a dynamic topology, BIG TCP cannot be enabled at all.

True. A question is how likely such a scenario with mixed devices is.
I.e., is it worth the code complexity.

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[Wang Zhan]

On Wed, 23 Sep 2026 12:44:27 -0400 Willem de Bruijn wrote:
> Wang Zhan wrote:
>> On Mon, 21 Sep 2026 17:07:09 -0400 Willem de Bruijn wrote:
>> > Somewhat tangential, but an entirely different practical approach
>> > could be to disable BIG-TCP when such a patch from BIG-TCP capable to
>> > non-capable device is encountered.
>> 
>> That means a single device in the topology which does not support BIG TCP
>> forces every other device to turn BIG TCP off, to avoid the degradation.
>> So with mixed devices and a dynamic topology, BIG TCP cannot be enabled at all.
> 
> True. A question is how likely such a scenario with mixed devices is.
> I.e., is it worth the code complexity.

I cannot say how common the mixed case is, but with this patch enabling
BIG TCP on a device becomes a local decision: if the peer supports it the
path is faster, and if it does not the path stays at what it would be
without BIG TCP.

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] skb_segment(): a non-zero SKB_GSO_CB(head_skb)->max_segs
  disables the *entire* frag_list eligibility block at…
- [Medium] __skb_gso_segment() now advertises a protocol-agnostic per-
  call bound ("@max_segs: maximum MSS segments per output GSO skb, 0…

--

net: gso: support bounded TCP segmentation

This adds a max_segs bound to __skb_gso_segment(), carried in the
skb_gso_cb scratch area, so callers can ask the GSO engine to group
several MSS segments into each output GSO skb. All existing callers pass
zero, and skb_segment() uses the bound instead of GSO_MAX_SEGS when
computing partial_segs.

> diff --git a/include/net/gso.h b/include/net/gso.h
> index 29975440cad5..fccb37889965 100644
> --- a/include/net/gso.h
> +++ b/include/net/gso.h
> @@ -19,6 +19,7 @@ struct skb_gso_cb {
>  	int	encap_level;
>  	__wsum	csum;
>  	__u16	csum_start;
> +	__u16	max_segs;	/* Max MSS segs per output skb, 0 = no limit */
>  };
>  #define SKB_GSO_CB_OFFSET	32
>  #define SKB_GSO_CB(skb) ((struct skb_gso_cb *)((skb)->cb + SKB_GSO_CB_OFFSET))

[ ... ]

> diff --git a/net/core/gso.c b/net/core/gso.c
> index bcd156372f4d..157f2bfdca12 100644
> --- a/net/core/gso.c
> +++ b/net/core/gso.c
> @@ -77,6 +77,7 @@ static bool skb_needs_check(const struct sk_buff *skb, bool tx_path)
>   *	@skb: buffer to segment
>   *	@features: features for the output path (see dev->features)
>   *	@tx_path: whether it is called in TX path
> + *	@max_segs: maximum MSS segments per output GSO skb, 0 means no limit

[Severity: Medium]
This kernel-doc describes max_segs as a protocol-agnostic bound on an
exported entry point, but only tcp_gso_segment() was taught to look at it.

The other gso_segment implementers reached through the very same
__skb_gso_segment() path still take their unconditional "nothing to do"
fast path. In __udp_gso_segment():

	if (skb_gso_ok(gso_skb, features | NETIF_F_GSO_ROBUST)) {
		/* Packet is from an untrusted source, reset gso_segs. */
		skb_shinfo(gso_skb)->gso_segs = DIV_ROUND_UP(gso_skb->len - sizeof(*uh), mss);
		return NULL;
	}

and in sctp_gso_segment():

	if (skb_gso_ok(skb, features | NETIF_F_GSO_ROBUST)) {
		/* Packet is from an untrusted source, reset gso_segs. */
		...
		segs = NULL;
		goto out;
	}

Neither consults SKB_GSO_CB(skb)->max_segs, which is exactly the check
tcp_gso_segment() gained in this patch.

If a caller passes a non-zero max_segs for a UDP or SCTP GSO skb, doesn't
the bound get silently dropped and the original oversized skb returned as
"no segmentation needed"?  The caller cannot tell that apart from the
device being able to take the skb as-is: tap_handle_frame() queues the
original skb on a NULL return, and queue_gso_packets() turns NULL into
-EINVAL.

No current caller passes a non-zero bound, so this is latent today. Should
the kernel-doc say the bound is honoured for TCP only, or should the other
implementers check it too?

>   *
>   *	This function segments the given skb and returns a list of segments.
>   *
> @@ -86,7 +87,8 @@ static bool skb_needs_check(const struct sk_buff *skb, bool tx_path)
>   *	Segmentation preserves SKB_GSO_CB_OFFSET bytes of previous skb cb.
>   */
>  struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
> -				  netdev_features_t features, bool tx_path)
> +				  netdev_features_t features, bool tx_path,
> +				  unsigned int max_segs)
>  {
>  	struct sk_buff *segs;
>  
> @@ -117,6 +119,7 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
>  
>  	SKB_GSO_CB(skb)->mac_offset = skb_headroom(skb);
>  	SKB_GSO_CB(skb)->encap_level = 0;
> +	SKB_GSO_CB(skb)->max_segs = min_t(unsigned int, max_segs, U16_MAX);

[Severity: Medium]
The changelog says:

	"the bounded path is only used for skbs which do not carry a
	frag_list"

Is that precondition enforced or documented anywhere?  __skb_gso_segment()
is EXPORT_SYMBOL and records the caller-supplied bound for any skb here,
and the new @max_segs kernel-doc above states no restriction.  See the
related question on the skb_segment() hunk below.

>  
>  	skb_reset_mac_header(skb);
>  	skb_reset_mac_len(skb);
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index dbbe10277d51..9c0d140236bc 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -4793,6 +4793,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>  	struct sk_buff *segs = NULL;
>  	struct sk_buff *tail = NULL;
>  	struct sk_buff *list_skb = skb_shinfo(head_skb)->frag_list;
> +	unsigned int max_segs = SKB_GSO_CB(head_skb)->max_segs;
>  	unsigned int mss = skb_shinfo(head_skb)->gso_size;
>  	bool gso_by_frags = mss == GSO_BY_FRAGS;
>  	unsigned int doffset = head_skb->data - skb_mac_header(head_skb);
> @@ -4839,7 +4840,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>  	csum = !!can_checksum_protocol(features, proto);
>  
>  	if (sg && csum && !gso_by_frags)  {
> -		if (!(features & NETIF_F_GSO_PARTIAL)) {
> +		if (!max_segs && !(features & NETIF_F_GSO_PARTIAL)) {

[Severity: Medium]
A non-zero max_segs skips the whole block here, not just the uniform-length
test the changelog mentions.  Also skipped are the net_gso_ok() bail-out,
the per-member head_frag guard and the length accounting:

net/core/skbuff.c:skb_segment() {
	...
		if (!list_skb ||
		    !net_gso_ok(features, skb_shinfo(head_skb)->gso_type))
			goto normal;
		...
		frag_len = list_skb->len;
		skb_walk_frags(head_skb, iter) {
			if (frag_len != iter->len && iter->next)
				goto normal;
			if (skb_headlen(iter) && !iter->head_frag)
				goto normal;

			len -= iter->len;
		}

		if (len != frag_len)
			goto normal;
	...
}

Since len is then still the full head_skb->len, mss *= partial_segs can
produce a grouped MSS that is not aligned with the frag_list member
boundaries, so one output skb spans several members.  Can that reach the
BUG_ON() in the frag walk further down?

net/core/skbuff.c:skb_segment() {
	...
			if (!skb_headlen(list_skb)) {
				BUG_ON(!nfrags);
			} else {
				BUG_ON(!list_skb->head_frag);
	...
}

A frag_list member with kmalloc'ed linear data (as TCP GRO produces via
skb_gro_receive()) is not a head_frag, and previously such an skb took
"goto normal" instead.

I checked the rest of the series: the only in-tree bounded caller,
skb_can_gso_resegment() in net/core/dev.c added by "net: core: resegment
oversized TCP GSO skbs", refuses these skbs with

	if (skb_has_frag_list(skb))
		return false;

and net_test.c never combines max_segs with frag_skbs, so nothing panics
today.  Would it be worth keeping the head_frag and uniformity validation
(or ignoring the bound) when skb_shinfo(head_skb)->frag_list is set, and
stating the precondition in the @max_segs kernel-doc?

Dropping the net_gso_ok() check the same way also means a bounded call can
emit still-GSO skbs whose gso_type the output features do not support.  Is
that intentional?

>  			struct sk_buff *iter;
>  			unsigned int frag_len;
>  
> @@ -4874,7 +4875,10 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>  		 * now.
>  		 */
>  		DEBUG_NET_WARN_ON_ONCE(len / mss > GSO_MAX_SEGS);
> -		partial_segs = min(len / mss, GSO_MAX_SEGS);
> +		if (max_segs)
> +			partial_segs = min(len / mss, max_segs);
> +		else
> +			partial_segs = min(len / mss, GSO_MAX_SEGS);
>  		if (partial_segs > 1)
>  			mss *= partial_segs;
>  		else

[ ... ]

> diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
> index e74d99ca9fac..a4076318c535 100644
> --- a/net/ipv4/tcp_offload.c
> +++ b/net/ipv4/tcp_offload.c
> @@ -164,7 +164,8 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb,
>  	if (unlikely(skb->len <= mss))
>  		goto out;
>  
> -	if (skb_gso_ok(skb, features | NETIF_F_GSO_ROBUST)) {
> +	if (!SKB_GSO_CB(skb)->max_segs &&
> +	    skb_gso_ok(skb, features | NETIF_F_GSO_ROBUST)) {
>  		/* Packet is from an untrusted source, reset gso_segs. */
>  
>  		skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(skb->len, mss);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918084651.3022878-1-wang.zhan%40smartx.com

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[Willem de Bruijn]

Wang Zhan wrote:
> The bounded resegmentation added by the next patch splits an oversized TCP
> GSO skb into several GSO skbs which fit the device limits. That needs the
> GSO engine to group several MSS segments into one output skb, so let
> callers bound the number of MSS segments each output skb carries and pass
> the bound through the existing __skb_gso_segment() entry point. Ordinary
> callers use zero for no limit.
> 
> skb_segment() only groups several MSS into one output skb when the device
> advertises NETIF_F_GSO_PARTIAL, or when the skb has a frag_list which can
> be split into uniform pieces, and falls back to one segment per skb
> otherwise. A caller which passes a bound asks for that grouping
> regardless, so the frag_list check is skipped when max_segs is set. Every
> other caller keeps it, and the bounded path is only used for skbs which do
> not carry a frag_list.
> 
> The output stays a GSO skb: gso_size is the original MSS and gso_segs is
> the number of MSS it holds, so a downstream device can still perform
> ordinary TSO. Store the bound in the existing skb_gso_cb scratch context,
> alongside the call-local data_offset and mac_offset fields, so that the
> segmentation methods keep their signature. A zero max_segs value means
> that no bound is active; it is not a persistent skb flag. Clear the value
> when each output skb copies the input header so the temporary limit is not
> propagated to the next GSO call.
> 
> Assisted-by: LLM
> Signed-off-by: Wang Zhan <wang.zhan@smartx.com>
 
> @@ -117,6 +119,7 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
>  
>  	SKB_GSO_CB(skb)->mac_offset = skb_headroom(skb);
>  	SKB_GSO_CB(skb)->encap_level = 0;
> +	SKB_GSO_CB(skb)->max_segs = min_t(unsigned int, max_segs, U16_MAX);

Can this use GSO_MAX_SEGS rather than U16_MAX.
>  
>  	skb_reset_mac_header(skb);
>  	skb_reset_mac_len(skb);
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index dbbe10277d51d..9c0d140236bc6 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -4793,6 +4793,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>  	struct sk_buff *segs = NULL;
>  	struct sk_buff *tail = NULL;
>  	struct sk_buff *list_skb = skb_shinfo(head_skb)->frag_list;
> +	unsigned int max_segs = SKB_GSO_CB(head_skb)->max_segs;
>  	unsigned int mss = skb_shinfo(head_skb)->gso_size;
>  	bool gso_by_frags = mss == GSO_BY_FRAGS;
>  	unsigned int doffset = head_skb->data - skb_mac_header(head_skb);
> @@ -4839,7 +4840,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>  	csum = !!can_checksum_protocol(features, proto);
>  
>  	if (sg && csum && !gso_by_frags)  {
> -		if (!(features & NETIF_F_GSO_PARTIAL)) {
> +		if (!max_segs && !(features & NETIF_F_GSO_PARTIAL)) {
>  			struct sk_buff *iter;
>  			unsigned int frag_len;
>  
> @@ -4874,7 +4875,10 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>  		 * now.
>  		 */
>  		DEBUG_NET_WARN_ON_ONCE(len / mss > GSO_MAX_SEGS);
> -		partial_segs = min(len / mss, GSO_MAX_SEGS);
> +		if (max_segs)
> +			partial_segs = min(len / mss, max_segs);
> +		else
> +			partial_segs = min(len / mss, GSO_MAX_SEGS);
>  		if (partial_segs > 1)
>  			mss *= partial_segs;
>  		else
> @@ -4975,6 +4979,12 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>  
>  		__copy_skb_header(nskb, head_skb);
>  
> +		/*
> +		 * max_segs is a per-call limit, so output skbs must not
> +		 * inherit it from the input skb.
> +		 */
> +		SKB_GSO_CB(nskb)->max_segs = 0;
> +

cb has to be treated as uninitialized between layers. All callers of
__skb_gso_segment should (re)initialize this field. Is it necessary to
clear it here. Note that we do not do that for any other cb fields.

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[Wang Zhan]

On Wed, 23 Sep 2026 12:50:09 -0400 Willem de Bruijn wrote:
>> @@ -117,6 +119,7 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
>>  
>>  	SKB_GSO_CB(skb)->mac_offset = skb_headroom(skb);
>>  	SKB_GSO_CB(skb)->encap_level = 0;
>> +	SKB_GSO_CB(skb)->max_segs = min_t(unsigned int, max_segs, U16_MAX);
> 
> Can this use GSO_MAX_SEGS rather than U16_MAX.

Good idea.

>> @@ -4975,6 +4979,12 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>>  
>>  		__copy_skb_header(nskb, head_skb);
>>  
>> +		/*
>> +		 * max_segs is a per-call limit, so output skbs must not
>> +		 * inherit it from the input skb.
>> +		 */
>> +		SKB_GSO_CB(nskb)->max_segs = 0;
>> +
> 
> cb has to be treated as uninitialized between layers. All callers of
> __skb_gso_segment should (re)initialize this field. Is it necessary to
> clear it here. Note that we do not do that for any other cb fields.

I checked again, it is indeed unnecessary, and I will drop it in v3.

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[David Laight]

On Fri, 18 Sep 2026 16:46:49 +0800
Wang Zhan <wang.zhan@smartx.com> wrote:

> The bounded resegmentation added by the next patch splits an oversized TCP
> GSO skb into several GSO skbs which fit the device limits. That needs the
> GSO engine to group several MSS segments into one output skb, so let
> callers bound the number of MSS segments each output skb carries and pass
> the bound through the existing __skb_gso_segment() entry point. Ordinary
> callers use zero for no limit.
> 
> skb_segment() only groups several MSS into one output skb when the device
> advertises NETIF_F_GSO_PARTIAL, or when the skb has a frag_list which can
> be split into uniform pieces, and falls back to one segment per skb
> otherwise. A caller which passes a bound asks for that grouping
> regardless, so the frag_list check is skipped when max_segs is set. Every
> other caller keeps it, and the bounded path is only used for skbs which do
> not carry a frag_list.
> 
> The output stays a GSO skb: gso_size is the original MSS and gso_segs is
> the number of MSS it holds, so a downstream device can still perform
> ordinary TSO. Store the bound in the existing skb_gso_cb scratch context,
> alongside the call-local data_offset and mac_offset fields, so that the
> segmentation methods keep their signature. A zero max_segs value means
> that no bound is active; it is not a persistent skb flag. Clear the value
> when each output skb copies the input header so the temporary limit is not
> propagated to the next GSO call.
> 
...
> @@ -86,7 +87,8 @@ static bool skb_needs_check(const struct sk_buff *skb, bool tx_path)
>   *	Segmentation preserves SKB_GSO_CB_OFFSET bytes of previous skb cb.
>   */
>  struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
> -				  netdev_features_t features, bool tx_path)
> +				  netdev_features_t features, bool tx_path,
> +				  unsigned int max_segs)
>  {
>  	struct sk_buff *segs;
>  
> @@ -117,6 +119,7 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
>  
>  	SKB_GSO_CB(skb)->mac_offset = skb_headroom(skb);
>  	SKB_GSO_CB(skb)->encap_level = 0;
> +	SKB_GSO_CB(skb)->max_segs = min_t(unsigned int, max_segs, U16_MAX);

Why min_t() ??

David

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[Wang Zhan]

On Thu, 24 Sep 2026 11:53:12 +0100 David Laight wrote:
>> @@ -117,6 +119,7 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
>>  
>>  	SKB_GSO_CB(skb)->mac_offset = skb_headroom(skb);
>>  	SKB_GSO_CB(skb)->encap_level = 0;
>> +	SKB_GSO_CB(skb)->max_segs = min_t(unsigned int, max_segs, U16_MAX);
> 
> Why min_t ??

Fair point, min() is fine.

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[Paolo Abeni]

On 9/18/26 10:46, Wang Zhan wrote:
> The bounded resegmentation added by the next patch splits an oversized TCP
> GSO skb into several GSO skbs which fit the device limits. That needs the
> GSO engine to group several MSS segments into one output skb, so let
> callers bound the number of MSS segments each output skb carries and pass
> the bound through the existing __skb_gso_segment() entry point. Ordinary
> callers use zero for no limit.
> 
> skb_segment() only groups several MSS into one output skb when the device
> advertises NETIF_F_GSO_PARTIAL, or when the skb has a frag_list which can
> be split into uniform pieces, and falls back to one segment per skb
> otherwise. A caller which passes a bound asks for that grouping
> regardless, so the frag_list check is skipped when max_segs is set. Every
> other caller keeps it, and the bounded path is only used for skbs which do
> not carry a frag_list.
> 
> The output stays a GSO skb: gso_size is the original MSS and gso_segs is
> the number of MSS it holds, so a downstream device can still perform
> ordinary TSO. Store the bound in the existing skb_gso_cb scratch context,
> alongside the call-local data_offset and mac_offset fields, so that the
> segmentation methods keep their signature. A zero max_segs value means
> that no bound is active; it is not a persistent skb flag. Clear the value
> when each output skb copies the input header so the temporary limit is not
> propagated to the next GSO call.
> 
> Assisted-by: LLM
> Signed-off-by: Wang Zhan <wang.zhan@smartx.com>
I'm wondering if this patch is needed at all. Can't the special gso caller
set skb_shinfo(skb)->gso_size to ~64K and adjust the gso bits in the shared
info afterwards?

/P

Re: [PATCH net-next v2 2/4] net: gso: support bounded TCP segmentation

[Wang Zhan]

On Thu, 24 Sep 2026 16:09:39 +0200 Paolo Abeni wrote:
> I'm wondering if this patch is needed at all. Can't the special gso caller
> set skb_shinfo(skb)->gso_size to ~64K and adjust the gso bits in the shared
> info afterwards?

That would work, and it keeps the logic in one place without a new cb field,
but I don't think it's better than the current approach.

It would need skb_unclone() before writing skb_shinfo(), and a repeat of the
if (partial_segs) logic from skb_segment().  For DF=0 we would also have to
recompute ip.id in validate_xmit_skb(), or keep that case on the existing
full-segmentation path.

[PATCH net-next v2 3/4] net: core: resegment oversized TCP GSO skbs

[Wang Zhan]

A GSO skb which exceeds an egress device limit loses its GSO feature mask
and is segmented into individual packets. This is unnecessarily expensive
when the device can still offload smaller TCP GSO skbs, which is easy to
hit once one hop of a BIG TCP path raises gso_max_size and the next one
does not.

For an unencapsulated TCP GSO skb which exceeds gso_max_size or
gso_max_segs, work out how many MSS segments each output skb may carry and
resegment the skb with that bound instead. Keep the features computed
without the GSO limit checks, which say whether the device offloads the
GSO type at all. Encapsulated and frag-list skbs, GSO types the device
cannot offload, and bounds below two segments keep the existing full
segmentation path. The result obeys the GSO feature and limit contract the
device already advertises, so apply it automatically, without extra device
state or a userspace control.

The check runs on the skb which is handed to the driver, after
validate_xmit_vlan() and sk_validate_xmit_skb(), and costs one extra
ndo_features_check() on the oversized path, against segmenting the skb
into individual packets. That position is also why the limit follows the
L3 protocol rather than skb->protocol: validate_xmit_vlan() replaces the
latter with the VLAN ethertype when it pushes the tag inside the skb.

Measured on a veth -> bridge -> TAP -> guest virtio-net path, with BIG TCP
enabled on the veth endpoints and left off in the guest, so the skbs which
the veth hop accepts have to be segmented before the TAP device. A single
iperf3 TCP flow, six alternating runs per state (`-t 15 -O 5`, fixed CPU
affinity and port tuple). The middle column is the same tree with the
resegmentation disabled:

  protocol  no BIG TCP   mixed, no reseg  mixed, resegmented
  TCP/IPv4  51.550 Gbps  15.850 Gbps      52.617 Gbps
  TCP/IPv6  52.050 Gbps  15.783 Gbps      51.933 Gbps

Coefficient of variation for the two mixed columns was 0.48% and 0.82%
for IPv4 and 0.44% and 0.44% for IPv6. A BIG TCP hop which feeds a 64 KiB
hop loses 69% of the throughput of a path which never enables BIG TCP at
all; bounded resegmentation recovers it, 3.3x over the existing
segmentation path and within noise of the no BIG TCP baseline.

Assisted-by: LLM
Signed-off-by: Wang Zhan <wang.zhan@smartx.com>
---
 net/core/dev.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 106 insertions(+), 7 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 16685888b2812..548db4d4e874c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3834,18 +3834,24 @@ static bool skb_gso_has_extension_hdr(const struct sk_buff *skb)
 			 skb_inner_network_header_len(skb) != sizeof(struct ipv6hdr)));
 }
 
+/*
+ * Does @skb fit the GSO limits of @dev?  The size limit depends on the L3
+ * protocol, which validate_xmit_vlan() replaces with the VLAN ethertype when
+ * it pushes the tag inside the skb, so look behind the tag.
+ */
 static bool gso_within_device_limits(const struct sk_buff *skb,
 				     const struct net_device *dev)
 {
 	return skb_shinfo(skb)->gso_segs <= READ_ONCE(dev->gso_max_segs) &&
-	       skb->len < netif_get_gso_max_size(dev, skb->protocol);
+	       skb->len < netif_get_gso_max_size(dev, vlan_get_protocol(skb));
 }
 
 static netdev_features_t gso_features_check(const struct sk_buff *skb,
 					    struct net_device *dev,
-					    netdev_features_t features)
+					    netdev_features_t features,
+					    bool check_limits)
 {
-	if (!gso_within_device_limits(skb, dev))
+	if (check_limits && !gso_within_device_limits(skb, dev))
 		return features & ~NETIF_F_GSO_MASK;
 
 	if (!skb_shinfo(skb)->gso_type) {
@@ -3894,13 +3900,15 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb,
 	return features;
 }
 
-netdev_features_t netif_skb_features(struct sk_buff *skb)
+static netdev_features_t __netif_skb_features(struct sk_buff *skb,
+					      bool check_gso_limits)
 {
 	struct net_device *dev = skb->dev;
 	netdev_features_t features = dev->features;
 
 	if (skb_is_gso(skb))
-		features = gso_features_check(skb, dev, features);
+		features = gso_features_check(skb, dev, features,
+					      check_gso_limits);
 
 	/* If encapsulation offload request, verify we are testing
 	 * hardware encapsulation features instead of standard
@@ -3923,8 +3931,79 @@ netdev_features_t netif_skb_features(struct sk_buff *skb)
 
 	return harmonize_features(skb, features);
 }
+
+netdev_features_t netif_skb_features(struct sk_buff *skb)
+{
+	return __netif_skb_features(skb, true);
+}
 EXPORT_SYMBOL(netif_skb_features);
 
+static bool skb_can_gso_resegment(struct sk_buff *skb,
+				  netdev_features_t features)
+{
+	__be16 protocol;
+
+	if (!net_gso_ok(features | NETIF_F_GSO_ROBUST,
+			skb_shinfo(skb)->gso_type))
+		return false;
+
+	if (!(features & NETIF_F_SG))
+		return false;
+
+	protocol = skb_network_protocol(skb, NULL);
+	if (!protocol || !can_checksum_protocol(features, protocol))
+		return false;
+
+	/*
+	 * The TCP frag-list path does not carry the bounded segment
+	 * limit through skb_segment_list(). Keep bounded resegmentation
+	 * on the regular skb path until that support is added.
+	 */
+	if (skb_has_frag_list(skb))
+		return false;
+
+	return true;
+}
+
+static unsigned int
+skb_gso_resegment_max_segs(struct sk_buff *skb, struct net_device *dev,
+			   netdev_features_t features)
+{
+	unsigned int mss = skb_shinfo(skb)->gso_size;
+	unsigned int hdr_len, max_segs;
+	unsigned int gso_max_size;
+	struct tcphdr _tcph, *th;
+
+	gso_max_size = netif_get_gso_max_size(dev, vlan_get_protocol(skb));
+
+	if (!skb_is_gso(skb) || !skb_is_gso_tcp(skb) ||
+	    skb->encapsulation || mss == GSO_BY_FRAGS ||
+	    !skb_mac_header_was_set(skb) ||
+	    !skb_transport_header_was_set(skb) ||
+	    !skb_can_gso_resegment(skb, features))
+		return 0;
+
+	th = skb_header_pointer(skb, skb_transport_offset(skb), sizeof(_tcph),
+				&_tcph);
+	if (!th || th->doff < sizeof(*th) / 4)
+		return 0;
+
+	hdr_len = skb_transport_header(skb) - skb_mac_header(skb) +
+		  th->doff * 4;
+	if (gso_max_size <= hdr_len + mss)
+		return 0;
+
+	/*
+	 * gso_within_device_limits() accepts gso_segs == gso_max_segs but
+	 * rejects skb->len >= gso_max_size, so only the size bound needs - 1.
+	 */
+	max_segs = (gso_max_size - hdr_len - 1) / mss;
+	max_segs = min_t(unsigned int, max_segs,
+			 READ_ONCE(dev->gso_max_segs));
+
+	return max_segs > 1 ? max_segs : 0;
+}
+
 static int xmit_one(struct sk_buff *skb, struct net_device *dev,
 		    struct netdev_queue *txq, bool more)
 {
@@ -4073,6 +4152,7 @@ static struct sk_buff *validate_xmit_unreadable_skb(struct sk_buff *skb,
  */
 static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device *dev, bool *again)
 {
+	unsigned int resegment_max_segs = 0;
 	netdev_features_t features;
 
 	skb = validate_xmit_unreadable_skb(skb, dev);
@@ -4088,10 +4168,29 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device
 	if (unlikely(!skb))
 		goto out_null;
 
-	if (netif_needs_gso(skb, features)) {
+	/*
+	 * An oversized skb loses its GSO feature bits and is segmented
+	 * down to MSS sized skbs below.  A TCP skb can instead be split
+	 * into GSO skbs which do fit the device, so keep the bits and
+	 * bound the resegmentation.  The features computed without the
+	 * limit checks say whether the device offloads the GSO type at
+	 * all.
+	 */
+	if (skb_is_gso(skb) && skb_is_gso_tcp(skb) && !skb->encapsulation &&
+	    !gso_within_device_limits(skb, dev)) {
+		netdev_features_t offload = __netif_skb_features(skb, false);
+
+		resegment_max_segs =
+			skb_gso_resegment_max_segs(skb, dev, offload);
+		if (resegment_max_segs)
+			features = offload;
+	}
+
+	if (resegment_max_segs || netif_needs_gso(skb, features)) {
 		struct sk_buff *segs;
 
-		segs = skb_gso_segment(skb, features);
+		segs = __skb_gso_segment(skb, features, true,
+					 resegment_max_segs);
 		if (IS_ERR(segs)) {
 			goto out_kfree_skb;
 		} else if (segs) {
-- 
2.47.3

Re: [PATCH net-next v2 3/4] net: core: resegment oversized TCP GSO skbs

[Willem de Bruijn]

Wang Zhan wrote:
> A GSO skb which exceeds an egress device limit loses its GSO feature mask
> and is segmented into individual packets. This is unnecessarily expensive
> when the device can still offload smaller TCP GSO skbs, which is easy to
> hit once one hop of a BIG TCP path raises gso_max_size and the next one
> does not.
> 
> For an unencapsulated TCP GSO skb which exceeds gso_max_size or
> gso_max_segs, work out how many MSS segments each output skb may carry and
> resegment the skb with that bound instead. Keep the features computed
> without the GSO limit checks, which say whether the device offloads the
> GSO type at all. Encapsulated and frag-list skbs, GSO types the device
> cannot offload, and bounds below two segments keep the existing full
> segmentation path. The result obeys the GSO feature and limit contract the
> device already advertises, so apply it automatically, without extra device
> state or a userspace control.
> 
> The check runs on the skb which is handed to the driver, after
> validate_xmit_vlan() and sk_validate_xmit_skb(), and costs one extra
> ndo_features_check() on the oversized path, against segmenting the skb
> into individual packets. That position is also why the limit follows the
> L3 protocol rather than skb->protocol: validate_xmit_vlan() replaces the
> latter with the VLAN ethertype when it pushes the tag inside the skb.
> 
> Measured on a veth -> bridge -> TAP -> guest virtio-net path, with BIG TCP
> enabled on the veth endpoints and left off in the guest, so the skbs which
> the veth hop accepts have to be segmented before the TAP device. A single
> iperf3 TCP flow, six alternating runs per state (`-t 15 -O 5`, fixed CPU
> affinity and port tuple). The middle column is the same tree with the
> resegmentation disabled:
> 
>   protocol  no BIG TCP   mixed, no reseg  mixed, resegmented
>   TCP/IPv4  51.550 Gbps  15.850 Gbps      52.617 Gbps
>   TCP/IPv6  52.050 Gbps  15.783 Gbps      51.933 Gbps
> 
> Coefficient of variation for the two mixed columns was 0.48% and 0.82%
> for IPv4 and 0.44% and 0.44% for IPv6. A BIG TCP hop which feeds a 64 KiB
> hop loses 69% of the throughput of a path which never enables BIG TCP at
> all; bounded resegmentation recovers it, 3.3x over the existing
> segmentation path and within noise of the no BIG TCP baseline.
> 
> Assisted-by: LLM
> Signed-off-by: Wang Zhan <wang.zhan@smartx.com>
> ---
>  net/core/dev.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 106 insertions(+), 7 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 16685888b2812..548db4d4e874c 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3834,18 +3834,24 @@ static bool skb_gso_has_extension_hdr(const struct sk_buff *skb)
>  			 skb_inner_network_header_len(skb) != sizeof(struct ipv6hdr)));
>  }
>  
> +/*
> + * Does @skb fit the GSO limits of @dev?  The size limit depends on the L3
> + * protocol, which validate_xmit_vlan() replaces with the VLAN ethertype when
> + * it pushes the tag inside the skb, so look behind the tag.
> + */
>  static bool gso_within_device_limits(const struct sk_buff *skb,
>  				     const struct net_device *dev)
>  {
>  	return skb_shinfo(skb)->gso_segs <= READ_ONCE(dev->gso_max_segs) &&
> -	       skb->len < netif_get_gso_max_size(dev, skb->protocol);
> +	       skb->len < netif_get_gso_max_size(dev, vlan_get_protocol(skb));

If this change is needed, it is not new for this feature and should be
a separate commit.

> +static bool skb_can_gso_resegment(struct sk_buff *skb,
> +				  netdev_features_t features)
> +{
> +	__be16 protocol;
> +
> +	if (!net_gso_ok(features | NETIF_F_GSO_ROBUST,
> +			skb_shinfo(skb)->gso_type))
> +		return false;
> +
> +	if (!(features & NETIF_F_SG))
> +		return false;
> +
> +	protocol = skb_network_protocol(skb, NULL);
> +	if (!protocol || !can_checksum_protocol(features, protocol))
> +		return false;
> +
> +	/*
> +	 * The TCP frag-list path does not carry the bounded segment
> +	 * limit through skb_segment_list(). Keep bounded resegmentation
> +	 * on the regular skb path until that support is added.
> +	 */
> +	if (skb_has_frag_list(skb))
> +		return false;
> +
> +	return true;
> +}
> +
> +static unsigned int
> +skb_gso_resegment_max_segs(struct sk_buff *skb, struct net_device *dev,
> +			   netdev_features_t features)
> +{
> +	unsigned int mss = skb_shinfo(skb)->gso_size;
> +	unsigned int hdr_len, max_segs;
> +	unsigned int gso_max_size;
> +	struct tcphdr _tcph, *th;
> +
> +	gso_max_size = netif_get_gso_max_size(dev, vlan_get_protocol(skb));
> +
> +	if (!skb_is_gso(skb) || !skb_is_gso_tcp(skb) ||
> +	    skb->encapsulation || mss == GSO_BY_FRAGS ||
> +	    !skb_mac_header_was_set(skb) ||
> +	    !skb_transport_header_was_set(skb) ||
> +	    !skb_can_gso_resegment(skb, features))
> +		return 0;

Is this duplicating/extending skb_can_gso_resegment

> +
> +	th = skb_header_pointer(skb, skb_transport_offset(skb), sizeof(_tcph),
> +				&_tcph);
> +	if (!th || th->doff < sizeof(*th) / 4)
> +		return 0;
> +
> +	hdr_len = skb_transport_header(skb) - skb_mac_header(skb) +
> +		  th->doff * 4;
> +	if (gso_max_size <= hdr_len + mss)
> +		return 0;
> +
> +	/*
> +	 * gso_within_device_limits() accepts gso_segs == gso_max_segs but
> +	 * rejects skb->len >= gso_max_size, so only the size bound needs - 1.
> +	 */
> +	max_segs = (gso_max_size - hdr_len - 1) / mss;
> +	max_segs = min_t(unsigned int, max_segs,
> +			 READ_ONCE(dev->gso_max_segs));
> +
> +	return max_segs > 1 ? max_segs : 0;
> +}
> +
>  static int xmit_one(struct sk_buff *skb, struct net_device *dev,
>  		    struct netdev_queue *txq, bool more)
>  {
> @@ -4073,6 +4152,7 @@ static struct sk_buff *validate_xmit_unreadable_skb(struct sk_buff *skb,
>   */
>  static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device *dev, bool *again)
>  {
> +	unsigned int resegment_max_segs = 0;
>  	netdev_features_t features;
>  
>  	skb = validate_xmit_unreadable_skb(skb, dev);
> @@ -4088,10 +4168,29 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device
>  	if (unlikely(!skb))
>  		goto out_null;
>  
> -	if (netif_needs_gso(skb, features)) {
> +	/*
> +	 * An oversized skb loses its GSO feature bits and is segmented
> +	 * down to MSS sized skbs below.  A TCP skb can instead be split
> +	 * into GSO skbs which do fit the device, so keep the bits and
> +	 * bound the resegmentation.  The features computed without the
> +	 * limit checks say whether the device offloads the GSO type at
> +	 * all.
> +	 */
> +	if (skb_is_gso(skb) && skb_is_gso_tcp(skb) && !skb->encapsulation &&
> +	    !gso_within_device_limits(skb, dev)) {
> +		netdev_features_t offload = __netif_skb_features(skb, false);
> +
> +		resegment_max_segs =
> +			skb_gso_resegment_max_segs(skb, dev, offload);
> +		if (resegment_max_segs)
> +			features = offload;
> +	}
> +

This is a lot to put in the hot path for a rare use case. Consider how
to make this less expensive.

> +	if (resegment_max_segs || netif_needs_gso(skb, features)) {
>  		struct sk_buff *segs;
>  
> -		segs = skb_gso_segment(skb, features);
> +		segs = __skb_gso_segment(skb, features, true,
> +					 resegment_max_segs);
>  		if (IS_ERR(segs)) {
>  			goto out_kfree_skb;
>  		} else if (segs) {
> -- 
> 2.47.3
> 

Re: [PATCH net-next v2 3/4] net: core: resegment oversized TCP GSO skbs

[Wang Zhan]

On Sat, 19 Sep 2026 11:37:45 -0400 Willem de Bruijn wrote:
> >  	return skb_shinfo(skb)->gso_segs <= READ_ONCE(dev->gso_max_segs) &&
> > -	       skb->len < netif_get_gso_max_size(dev, skb->protocol);
> > +	       skb->len < netif_get_gso_max_size(dev, vlan_get_protocol(skb));
>
> If this change is needed, it is not new for this feature and should be
> a separate commit.

Okay, will do it in v3 as patch 1/5, with a Fixes tag.

> > +	if (!skb_is_gso(skb) || !skb_is_gso_tcp(skb) ||
> > +	    skb->encapsulation || mss == GSO_BY_FRAGS ||
> > +	    !skb_mac_header_was_set(skb) ||
> > +	    !skb_transport_header_was_set(skb) ||
> > +	    !skb_can_gso_resegment(skb, features))
> > +		return 0;
>
> Is this duplicating/extending skb_can_gso_resegment

Okay, v3 folds skb_can_gso_resegment() into skb_gso_resegment_max_segs():
one caller, so the checks stay in one list.

> > +	if (skb_is_gso(skb) && skb_is_gso_tcp(skb) && !skb->encapsulation &&
> > +	    !gso_within_device_limits(skb, dev)) {
> > +		netdev_features_t offload = __netif_skb_features(skb, false);
> > +
> > +		resegment_max_segs =
> > +			skb_gso_resegment_max_segs(skb, dev, offload);
> > +		if (resegment_max_segs)
> > +			features = offload;
> > +	}
>
> This is a lot to put in the hot path for a rare use case. Consider how
> to make this less expensive.

Okay, v3 enters on

	if (unlikely(skb_is_gso(skb) && !(features & NETIF_F_GSO_MASK))) {

features is what netif_skb_features() returned a few lines above, and
gso_features_check() has already cleared the GSO bits for an over-limit
skb, so the common path reads two bits of a value which is already loaded.
An oversized skb then pays one __netif_skb_features(skb, false) call.
Could you take a look whether that is cheap enough?

Re: [PATCH net-next v2 3/4] net: core: resegment oversized TCP GSO skbs

[Paolo Abeni]

On 9/19/26 17:37, Willem de Bruijn wrote:
>> @@ -4088,10 +4168,29 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device
>>   	if (unlikely(!skb))
>>   		goto out_null;
>>   
>> -	if (netif_needs_gso(skb, features)) {
>> +	/*
>> +	 * An oversized skb loses its GSO feature bits and is segmented
>> +	 * down to MSS sized skbs below.  A TCP skb can instead be split
>> +	 * into GSO skbs which do fit the device, so keep the bits and
>> +	 * bound the resegmentation.  The features computed without the
>> +	 * limit checks say whether the device offloads the GSO type at
>> +	 * all.
>> +	 */
>> +	if (skb_is_gso(skb) && skb_is_gso_tcp(skb) && !skb->encapsulation &&
>> +	    !gso_within_device_limits(skb, dev)) {
>> +		netdev_features_t offload = __netif_skb_features(skb, false);
>> +
>> +		resegment_max_segs =
>> +			skb_gso_resegment_max_segs(skb, dev, offload);
>> +		if (resegment_max_segs)
>> +			features = offload;
>> +	}
>> +
> 
> This is a lot to put in the hot path for a rare use case. 

+1

> Consider how to make this less expensive.
I think that performing all the resegs check only in the

	netif_needs_gso(skb, features)

case would address this concern.

/P

Re: [PATCH net-next v2 3/4] net: core: resegment oversized TCP GSO skbs

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Bounded resegmentation can emit GSO skbs whose L3 length exceeds
  65535, which inet_gso_segment()/ipv6_gso_segment() then silently…
- [Medium] Changing gso_within_device_limits() from skb->protocol to
  vlan_get_protocol(skb) (net/core/dev.c:3842-3848) silently alters the…

--

net: core: resegment oversized TCP GSO skbs

An unencapsulated TCP GSO skb which exceeds the egress device gso_max_size
or gso_max_segs is no longer fully segmented down to MSS sized packets.
Instead a per-output segment bound is computed and the skb is resegmented
into smaller GSO skbs which still fit the device limits.

> diff --git a/net/core/dev.c b/net/core/dev.c
> index 16685888b281..548db4d4e874 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3834,18 +3834,24 @@ static bool skb_gso_has_extension_hdr(const struct sk_buff *skb)
>  			 skb_inner_network_header_len(skb) != sizeof(struct ipv6hdr)));
>  }
>  
> +/*
> + * Does @skb fit the GSO limits of @dev?  The size limit depends on the L3
> + * protocol, which validate_xmit_vlan() replaces with the VLAN ethertype when
> + * it pushes the tag inside the skb, so look behind the tag.
> + */
>  static bool gso_within_device_limits(const struct sk_buff *skb,
>  				     const struct net_device *dev)
>  {
>  	return skb_shinfo(skb)->gso_segs <= READ_ONCE(dev->gso_max_segs) &&
> -	       skb->len < netif_get_gso_max_size(dev, skb->protocol);
> +	       skb->len < netif_get_gso_max_size(dev, vlan_get_protocol(skb));
>  }
>  
>  static netdev_features_t gso_features_check(const struct sk_buff *skb,
>  					    struct net_device *dev,
> -					    netdev_features_t features)
> +					    netdev_features_t features,
> +					    bool check_limits)
>  {
> -	if (!gso_within_device_limits(skb, dev))
> +	if (check_limits && !gso_within_device_limits(skb, dev))
>  		return features & ~NETIF_F_GSO_MASK;

[Severity: Medium]

Does switching gso_within_device_limits() from skb->protocol to
vlan_get_protocol(skb) also change behaviour for the pre-existing limit
check, and not only for the new resegmentation path?

gso_features_check() is still reached with check_limits = true from
netif_skb_features(), which is an exported symbol used by the first
features computation in validate_xmit_skb(), by bonding/team, and by
qdiscs which segment via skb_gso_segment(skb, netif_skb_features(skb)).

For an skb with an in-band VLAN tag (skb->protocol == ETH_P_8021Q or
ETH_P_8021AD, e.g. a bridge-forwarded frame, or a frame after an earlier
__vlan_hwaccel_push_inside()) carrying IPv6, the limit selected by
netif_get_gso_max_size() changes:

  before: protocol == ETH_P_8021Q   -> dev->gso_ipv4_max_size
  after:  vlan_get_protocol() == ETH_P_IPV6 -> dev->gso_max_size

On a device with BIG TCP enabled for IPv6 only (gso_max_size >
gso_ipv4_max_size), such an in-band-tagged IPv6 TSO frame above
gso_ipv4_max_size is now handed to the driver where it was previously
software segmented.

The commit message only explains the change as a consequence of where the
new check sits:

  "That position is also why the limit follows the L3 protocol rather than
  skb->protocol: validate_xmit_vlan() replaces the latter with the VLAN
  ethertype when it pushes the tag inside the skb."

and patch 1 of the series, "net: core: factor out the GSO device limit
check", stated "No functional changes".

Could the change to the existing netif_skb_features() path be described
explicitly, or split into its own patch with a Fixes: tag if it is meant
as a fix?

>  
>  	if (!skb_shinfo(skb)->gso_type) {

[ ... ]

> @@ -3894,13 +3900,15 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb,
>  	return features;
>  }
>  
> -netdev_features_t netif_skb_features(struct sk_buff *skb)
> +static netdev_features_t __netif_skb_features(struct sk_buff *skb,
> +					      bool check_gso_limits)
>  {
>  	struct net_device *dev = skb->dev;
>  	netdev_features_t features = dev->features;
>  
>  	if (skb_is_gso(skb))
> -		features = gso_features_check(skb, dev, features);
> +		features = gso_features_check(skb, dev, features,
> +					      check_gso_limits);
>  

[ ... ]

> @@ -3923,8 +3931,79 @@ netdev_features_t netif_skb_features(struct sk_buff *skb)
>  
>  	return harmonize_features(skb, features);
>  }
> +
> +netdev_features_t netif_skb_features(struct sk_buff *skb)
> +{
> +	return __netif_skb_features(skb, true);
> +}
>  EXPORT_SYMBOL(netif_skb_features);
>  

[ ... ]

> +static unsigned int
> +skb_gso_resegment_max_segs(struct sk_buff *skb, struct net_device *dev,
> +			   netdev_features_t features)
> +{
> +	unsigned int mss = skb_shinfo(skb)->gso_size;
> +	unsigned int hdr_len, max_segs;
> +	unsigned int gso_max_size;
> +	struct tcphdr _tcph, *th;
> +
> +	gso_max_size = netif_get_gso_max_size(dev, vlan_get_protocol(skb));
> +

[ ... ]

> +	hdr_len = skb_transport_header(skb) - skb_mac_header(skb) +
> +		  th->doff * 4;
> +	if (gso_max_size <= hdr_len + mss)
> +		return 0;
> +
> +	/*
> +	 * gso_within_device_limits() accepts gso_segs == gso_max_segs but
> +	 * rejects skb->len >= gso_max_size, so only the size bound needs - 1.
> +	 */
> +	max_segs = (gso_max_size - hdr_len - 1) / mss;
> +	max_segs = min_t(unsigned int, max_segs,
> +			 READ_ONCE(dev->gso_max_segs));
> +
> +	return max_segs > 1 ? max_segs : 0;
> +}

[Severity: High]

Should this bound also be clamped so that hdr_len + max_segs * mss stays
within what a 16-bit IPv4 tot_len or IPv6 payload_len can express?

The bound here derives only from netif_get_gso_max_size(dev, ...) and
dev->gso_max_segs. If the egress device itself has a BIG TCP limit above
64 KiB (veth calls netif_set_tso_max_size(dev, GSO_MAX_SIZE), so
"ip link set dev vethX gso_max_size 120000" is accepted) and the incoming
skb is larger still, the output skbs come out at roughly gso_max_size
bytes.

skb_segment() only tags the outputs as SKB_GSO_PARTIAL when features carry
NETIF_F_GSO_PARTIAL:

  net/core/skbuff.c:skb_segment() {
      ...
      type |= (features & NETIF_F_GSO_PARTIAL) / NETIF_F_GSO_PARTIAL * SKB_GSO_PARTIAL;
      ...
  }

So for software devices the outputs are ordinary GSO skbs, and the L3
segmenters take the non-partial branch, which truncates through htons():

  net/ipv4/af_inet.c:inet_gso_segment() {
      ...
          else
              tot_len = skb->len - nhoff;
      ...
      iph->tot_len = htons(tot_len);
  }

  net/ipv6/ip6_offload.c:ipv6_gso_segment() {
      ...
          else
              payload_len = skb->len - nhoff - sizeof(*ipv6h);
      ipv6h->payload_len = htons(payload_len);
  }

Neither uses the >64 KiB encodings the rest of the tree expects, where
iph_set_totlen() writes tot_len = 0 above IP_MAX_MTU and ip6_xmit() writes
payload_len = 0 above IPV6_MAXPLEN. Can the declared length therefore end
up as (len mod 65536), with a software receiver trimming the frame in
ip6_rcv_core() via ipv6_payload_len() and pskb_trim_rcsum(), and the
equivalent iph_totlen() path in ip_rcv_core()?

> +
>  static int xmit_one(struct sk_buff *skb, struct net_device *dev,
>  		    struct netdev_queue *txq, bool more)
>  {

[ ... ]

> @@ -4088,10 +4168,29 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device
>  	if (unlikely(!skb))
>  		goto out_null;
>  
> -	if (netif_needs_gso(skb, features)) {
> +	/*
> +	 * An oversized skb loses its GSO feature bits and is segmented
> +	 * down to MSS sized skbs below.  A TCP skb can instead be split
> +	 * into GSO skbs which do fit the device, so keep the bits and
> +	 * bound the resegmentation.  The features computed without the
> +	 * limit checks say whether the device offloads the GSO type at
> +	 * all.
> +	 */
> +	if (skb_is_gso(skb) && skb_is_gso_tcp(skb) && !skb->encapsulation &&
> +	    !gso_within_device_limits(skb, dev)) {
> +		netdev_features_t offload = __netif_skb_features(skb, false);
> +
> +		resegment_max_segs =
> +			skb_gso_resegment_max_segs(skb, dev, offload);
> +		if (resegment_max_segs)
> +			features = offload;
> +	}
> +
> +	if (resegment_max_segs || netif_needs_gso(skb, features)) {
>  		struct sk_buff *segs;
>  
> -		segs = skb_gso_segment(skb, features);
> +		segs = __skb_gso_segment(skb, features, true,
> +					 resegment_max_segs);
>  		if (IS_ERR(segs)) {
>  			goto out_kfree_skb;
>  		} else if (segs) {

Is there anything between this point and dev_hard_start_xmit() which
re-validates the length of the resulting segments? With gso_max_size =
120000 on the egress device and a 200000 byte incoming skb,
skb_segment() computes partial_segs = min(len/mss, max_segs) and
mss *= partial_segs, so the outputs land around 119 KiB and reach the
driver unchecked, where the previous code produced MSS sized packets with
correct 16-bit lengths.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918084651.3022878-1-wang.zhan%40smartx.com

[PATCH net-next v2 4/4] net: net_test: add tests for bounded GSO segmentation

[Wang Zhan]

The GSO engine can now be asked to bound the number of MSS segments which
go into each output skb. Add KUnit coverage for the bound itself and for a
TCP skb segmented with one.

The parameterized GSO test gains a max_segs input and three cases: a bound
which splits the skb into two outputs, one which spans three outputs, and
a bound of a single MSS, which must leave the ungrouped output of the
unbounded path in place. Output skbs which remain GSO skbs carry the
original gso_size and no more than max_segs segments. It drives
skb_segment() directly, because the synthetic protocol it uses has no
gso_segment callback, and stores the bound in the GSO control block
itself.

The TCP test drives __skb_gso_segment() with a bound of two MSS and checks
that every output skb stays GSO, keeps its gso_size, and stays within the
bound.

The limit test checks that the GSO size limit which netif_skb_features()
applies follows the packet's L3 protocol, also after validate_xmit_vlan()
has pushed the VLAN tag inside the skb and skb->protocol is the VLAN
ethertype. It contrasts the two ways the IPv4 and IPv6 limits can be
skewed, and the skb with and without the tag.

Assisted-by: LLM
Signed-off-by: Wang Zhan <wang.zhan@smartx.com>

---
v2:
- wrap the .frags/.segs initializers and the two header macros to 80 columns
- use KUNIT_ASSERT_TRUE() for the __be16 check, EQ warns in sparse
v1: https://lore.kernel.org/20260917063854.2011613-5-wang.zhan@smartx.com/
---
 net/core/net_test.c | 243 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 243 insertions(+)

diff --git a/net/core/net_test.c b/net/core/net_test.c
index 9c3a590865d26..6ca0cbe3a3653 100644
--- a/net/core/net_test.c
+++ b/net/core/net_test.c
@@ -4,7 +4,14 @@
 
 /* GSO */
 
+#include <linux/if_ether.h>
+#include <linux/if_vlan.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <linux/netdevice.h>
 #include <linux/skbuff.h>
+#include <linux/tcp.h>
+#include <net/gso.h>
 
 static const char hdr[] = "abcdefgh";
 #define GSO_TEST_SIZE 1000
@@ -34,6 +41,9 @@ enum gso_test_nr {
 	GSO_TEST_FRAG_LIST_PURE,
 	GSO_TEST_FRAG_LIST_NON_UNIFORM,
 	GSO_TEST_GSO_BY_FRAGS,
+	GSO_TEST_BOUNDED,
+	GSO_TEST_BOUNDED_MULTI,
+	GSO_TEST_BOUNDED_ONE_MSS,
 };
 
 struct gso_test_case {
@@ -46,10 +56,12 @@ struct gso_test_case {
 	const unsigned int *frags;
 	unsigned int nr_frag_skbs;
 	const unsigned int *frag_skbs;
+	unsigned int max_segs;
 
 	/* output as expected */
 	unsigned int nr_segs;
 	const unsigned int *segs;
+	bool segs_are_gso;
 };
 
 static struct gso_test_case cases[] = {
@@ -135,6 +147,54 @@ static struct gso_test_case cases[] = {
 		.nr_segs = 4,
 		.segs = (const unsigned int[]) { 100, 200, 300, 400 },
 	},
+	{
+		.id = GSO_TEST_BOUNDED,
+		.name = "bounded",
+		.linear_len = GSO_TEST_SIZE,
+		.nr_frags = 3,
+		.frags = (const unsigned int[]) {
+			GSO_TEST_SIZE, GSO_TEST_SIZE, 3,
+		},
+		.max_segs = 2,
+		.nr_segs = 2,
+		.segs = (const unsigned int[]) {
+			2 * GSO_TEST_SIZE, GSO_TEST_SIZE + 3,
+		},
+		.segs_are_gso = true,
+	},
+	{
+		.id = GSO_TEST_BOUNDED_MULTI,
+		.name = "bounded_multi",
+		.linear_len = 2 * GSO_TEST_SIZE,
+		.nr_frags = 4,
+		.frags = (const unsigned int[]) {
+			GSO_TEST_SIZE, GSO_TEST_SIZE, GSO_TEST_SIZE, 3,
+		},
+		.max_segs = 2,
+		.nr_segs = 3,
+		.segs = (const unsigned int[]) {
+			2 * GSO_TEST_SIZE, 2 * GSO_TEST_SIZE, GSO_TEST_SIZE + 3,
+		},
+		.segs_are_gso = true,
+	},
+	{
+		/*
+		 * One MSS per skb is what the unbounded path produces, so a
+		 * bound of a single segment must not change the output.
+		 */
+		.id = GSO_TEST_BOUNDED_ONE_MSS,
+		.name = "bounded_one_mss",
+		.linear_len = GSO_TEST_SIZE,
+		.nr_frags = 3,
+		.frags = (const unsigned int[]) {
+			GSO_TEST_SIZE, GSO_TEST_SIZE, 3,
+		},
+		.max_segs = 1,
+		.nr_segs = 4,
+		.segs = (const unsigned int[]) {
+			GSO_TEST_SIZE, GSO_TEST_SIZE, GSO_TEST_SIZE, 3,
+		},
+	},
 };
 
 static void gso_test_case_to_desc(struct gso_test_case *t, char *desc)
@@ -226,6 +286,7 @@ static void gso_test_func(struct kunit *test)
 	if (tcase->id == GSO_TEST_FRAG_LIST_NON_UNIFORM)
 		features &= ~NETIF_F_SG;
 
+	SKB_GSO_CB(skb)->max_segs = tcase->max_segs;
 	segs = skb_segment(skb, features);
 	if (IS_ERR(segs)) {
 		KUNIT_FAIL(test, "segs error %pe", segs);
@@ -247,6 +308,15 @@ static void gso_test_func(struct kunit *test)
 
 		/* header was copied to all segs */
 		KUNIT_ASSERT_EQ(test, memcmp(skb_mac_header(cur), hdr, sizeof(hdr)), 0);
+		if (tcase->segs_are_gso) {
+			KUNIT_EXPECT_TRUE(test, skb_is_gso(cur));
+			KUNIT_EXPECT_EQ(test, skb_shinfo(cur)->gso_size,
+					GSO_TEST_SIZE);
+			KUNIT_EXPECT_LE(test, skb_shinfo(cur)->gso_segs,
+					tcase->max_segs);
+			KUNIT_EXPECT_FALSE(test, skb_shinfo(cur)->gso_type &
+					 SKB_GSO_PARTIAL);
+		}
 
 		/* last seg can be found through segs->prev pointer */
 		if (!next)
@@ -261,6 +331,177 @@ static void gso_test_func(struct kunit *test)
 	consume_skb(skb);
 }
 
+#define GSO_TCP_HDR_LEN \
+	(ETH_HLEN + sizeof(struct iphdr) + sizeof(struct tcphdr))
+
+static struct sk_buff *gso_tcp_skb_new(unsigned int payload_len)
+{
+	struct sk_buff *skb;
+	struct ethhdr *eth;
+	struct tcphdr *th;
+	struct iphdr *iph;
+
+	skb = alloc_skb(GSO_TCP_HDR_LEN + payload_len, GFP_KERNEL);
+	if (!skb)
+		return NULL;
+	skb_put_zero(skb, GSO_TCP_HDR_LEN + payload_len);
+
+	skb_reset_mac_header(skb);
+	eth = eth_hdr(skb);
+	eth->h_proto = htons(ETH_P_IP);
+	skb->protocol = eth->h_proto;
+
+	skb_set_network_header(skb, ETH_HLEN);
+	iph = ip_hdr(skb);
+	iph->version = 4;
+	iph->ihl = sizeof(*iph) / 4;
+	iph->protocol = IPPROTO_TCP;
+	iph->tot_len = htons(sizeof(*iph) + sizeof(*th) + payload_len);
+
+	skb_set_transport_header(skb, ETH_HLEN + sizeof(*iph));
+	th = tcp_hdr(skb);
+	th->doff = sizeof(*th) / 4;
+
+	skb->ip_summed = CHECKSUM_PARTIAL;
+	skb->csum_start = skb_transport_header(skb) - skb->head;
+	skb->csum_offset = offsetof(struct tcphdr, check);
+	skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
+	skb_shinfo(skb)->gso_size = GSO_TEST_SIZE;
+	skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(payload_len, GSO_TEST_SIZE);
+
+	return skb;
+}
+
+static void gso_test_tcp_bounded_segment(struct kunit *test)
+{
+	netdev_features_t features = NETIF_F_SG | NETIF_F_HW_CSUM |
+				     NETIF_F_TSO;
+	const unsigned int payload_len = 3 * GSO_TEST_SIZE + 3;
+	struct sk_buff *skb, *segs, *cur, *next;
+	const unsigned int expected[] = {
+		2 * GSO_TEST_SIZE, GSO_TEST_SIZE + 3,
+	};
+	const unsigned int max_segs = 2;
+	int i = 0;
+
+	skb = gso_tcp_skb_new(payload_len);
+	KUNIT_ASSERT_NOT_NULL(test, skb);
+
+	segs = __skb_gso_segment(skb, features, true, max_segs);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, segs);
+
+	for (cur = segs; cur; cur = next, i++) {
+		next = cur->next;
+
+		KUNIT_ASSERT_LT(test, i, ARRAY_SIZE(expected));
+		KUNIT_EXPECT_EQ(test, cur->len,
+				GSO_TCP_HDR_LEN + expected[i]);
+		KUNIT_EXPECT_TRUE(test, skb_is_gso(cur));
+		KUNIT_EXPECT_EQ(test, skb_shinfo(cur)->gso_size,
+				GSO_TEST_SIZE);
+		KUNIT_EXPECT_LE(test, skb_shinfo(cur)->gso_segs, max_segs);
+
+		consume_skb(cur);
+	}
+
+	KUNIT_EXPECT_EQ(test, i, ARRAY_SIZE(expected));
+	consume_skb(skb);
+}
+
+#define GSO_TCP6_HDR_LEN \
+	(ETH_HLEN + sizeof(struct ipv6hdr) + sizeof(struct tcphdr))
+
+static struct sk_buff *gso_tcp6_skb_new(unsigned int payload_len)
+{
+	struct ipv6hdr *ip6h;
+	struct sk_buff *skb;
+	struct ethhdr *eth;
+	struct tcphdr *th;
+
+	skb = alloc_skb(GSO_TCP6_HDR_LEN + payload_len, GFP_KERNEL);
+	if (!skb)
+		return NULL;
+	skb_put_zero(skb, GSO_TCP6_HDR_LEN + payload_len);
+
+	skb_reset_mac_header(skb);
+	eth = eth_hdr(skb);
+	eth->h_proto = htons(ETH_P_IPV6);
+	skb->protocol = eth->h_proto;
+
+	skb_set_network_header(skb, ETH_HLEN);
+	ip6h = ipv6_hdr(skb);
+	ip6h->version = 6;
+	ip6h->nexthdr = IPPROTO_TCP;
+	ip6h->payload_len = htons(sizeof(*th) + payload_len);
+
+	skb_set_transport_header(skb, ETH_HLEN + sizeof(*ip6h));
+	th = tcp_hdr(skb);
+	th->doff = sizeof(*th) / 4;
+
+	skb->ip_summed = CHECKSUM_PARTIAL;
+	skb->csum_start = skb_transport_header(skb) - skb->head;
+	skb->csum_offset = offsetof(struct tcphdr, check);
+	skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
+	skb_shinfo(skb)->gso_size = GSO_TEST_SIZE;
+	skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(payload_len, GSO_TEST_SIZE);
+
+	return skb;
+}
+
+/* The device GSO size limit is per L3 protocol, and it has to survive the
+ * VLAN tag which validate_xmit_vlan() can push inside the skb, because that
+ * tag replaces skb->protocol with the VLAN ethertype.
+ */
+static void gso_test_tcp_limit_l3_proto(struct kunit *test)
+{
+	static const struct net_device_ops dummy_netdev_ops = { };
+	const unsigned int payload_len = 100 * 1024;
+	netdev_features_t features;
+	struct net_device *dev;
+	struct sk_buff *skb;
+
+	dev = alloc_etherdev(0);
+	KUNIT_ASSERT_NOT_NULL(test, dev);
+	dev->netdev_ops = &dummy_netdev_ops;
+	dev->hw_features = NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_TSO6;
+	dev->features = dev->hw_features;
+	dev->vlan_features = dev->hw_features;
+
+	skb = gso_tcp6_skb_new(payload_len);
+	KUNIT_ASSERT_NOT_NULL(test, skb);
+	skb->dev = dev;
+
+	/* The skb fits the IPv6 limit but not the IPv4 one. */
+	dev->gso_max_size = GSO_MAX_SIZE;
+	dev->gso_ipv4_max_size = GSO_LEGACY_MAX_SIZE;
+	features = netif_skb_features(skb);
+	KUNIT_EXPECT_TRUE(test, features & NETIF_F_GSO_MASK);
+
+	/* ...and the other way around. */
+	dev->gso_max_size = GSO_LEGACY_MAX_SIZE;
+	dev->gso_ipv4_max_size = GSO_MAX_SIZE;
+	features = netif_skb_features(skb);
+	KUNIT_EXPECT_FALSE(test, features & NETIF_F_GSO_MASK);
+
+	/* Pushing the tag inside must not change either answer. */
+	skb = vlan_insert_tag_set_proto(skb, htons(ETH_P_8021Q), 0);
+	KUNIT_ASSERT_NOT_NULL(test, skb);
+	KUNIT_ASSERT_TRUE(test, skb->protocol == htons(ETH_P_8021Q));
+
+	dev->gso_max_size = GSO_MAX_SIZE;
+	dev->gso_ipv4_max_size = GSO_LEGACY_MAX_SIZE;
+	features = netif_skb_features(skb);
+	KUNIT_EXPECT_TRUE(test, features & NETIF_F_GSO_MASK);
+
+	dev->gso_max_size = GSO_LEGACY_MAX_SIZE;
+	dev->gso_ipv4_max_size = GSO_MAX_SIZE;
+	features = netif_skb_features(skb);
+	KUNIT_EXPECT_FALSE(test, features & NETIF_F_GSO_MASK);
+
+	consume_skb(skb);
+	free_netdev(dev);
+}
+
 /* IP tunnel flags */
 
 #include <net/ip_tunnels.h>
@@ -372,6 +613,8 @@ static void ip_tunnel_flags_test_run(struct kunit *test)
 
 static struct kunit_case net_test_cases[] = {
 	KUNIT_CASE_PARAM(gso_test_func, gso_test_gen_params),
+	KUNIT_CASE(gso_test_tcp_bounded_segment),
+	KUNIT_CASE(gso_test_tcp_limit_l3_proto),
 	KUNIT_CASE_PARAM(ip_tunnel_flags_test_run,
 			 ip_tunnel_flags_test_gen_params),
 	{ },
-- 
2.47.3

Re: [PATCH net-next v2 4/4] net: net_test: add tests for bounded GSO segmentation

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 6 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 5 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] gso_test_tcp_bounded_segment() (net/core/net_test.c:375-409)
  introduces an undeclared CONFIG_INET dependency into the NET_TEST…
- [Low] gso_test_tcp_limit_l3_proto() (net/core/net_test.c:455-503) makes
  the VLAN step depend on a large high-order GFP_ATOMIC allocation.
- [Low] The in-tree comment on the GSO_TEST_BOUNDED_ONE_MSS case claims
  'One MSS per skb is what the unbounded path produces, so a bound of a…
- [Low] gso_test_tcp_limit_l3_proto() never constructs an IPv4 skb, so it
  does not verify that IPv4 packets select dev->gso_ipv4_max_size.
- [Low] gso_test_tcp_limit_l3_proto() leaks the net_device (and, later,
  also the skb) whenever a fatal KUnit assertion aborts the case.
- [Low] gso_test_tcp_bounded_segment() leaks the input GSO skb (and, mid-
  loop, the untraversed remainder of the output list) when a fatal KUnit…

Pre-existing issues:
- [Low] In gso_test_func() the verification loop is bounded by the output
  skb list (`for (cur = segs, i = 0; cur; cur = next, i++)`) and indexes…

--

net: net_test: add tests for bounded GSO segmentation

This adds KUnit coverage for the new max_segs bound on GSO segmentation:
three new parameterized skb_segment() cases, a TCP test driving
__skb_gso_segment() with a bound of two MSS, and a test that the device
GSO size limit follows the packet's L3 protocol across a pushed VLAN tag.

A few questions below.

> diff --git a/net/core/net_test.c b/net/core/net_test.c
> index 9c3a590865d2..6ca0cbe3a365 100644
> --- a/net/core/net_test.c
> +++ b/net/core/net_test.c

[ ... ]

> @@ -135,6 +147,54 @@ static struct gso_test_case cases[] = {
>  		.nr_segs = 4,
>  		.segs = (const unsigned int[]) { 100, 200, 300, 400 },
>  	},

[ ... ]

> +	{
> +		/*
> +		 * One MSS per skb is what the unbounded path produces, so a
> +		 * bound of a single segment must not change the output.
> +		 */
> +		.id = GSO_TEST_BOUNDED_ONE_MSS,
> +		.name = "bounded_one_mss",
> +		.linear_len = GSO_TEST_SIZE,
> +		.nr_frags = 3,
> +		.frags = (const unsigned int[]) {
> +			GSO_TEST_SIZE, GSO_TEST_SIZE, 3,
> +		},
> +		.max_segs = 1,
> +		.nr_segs = 4,
> +		.segs = (const unsigned int[]) {
> +			GSO_TEST_SIZE, GSO_TEST_SIZE, GSO_TEST_SIZE, 3,
> +		},
> +	},

[Severity: Low]
The comment says a bound of a single segment "must not change the output",
but does anything in the case actually check the form of the output?  This
case leaves segs_are_gso unset, and in gso_test_func() every GSO-state
check is gated on it:

	if (tcase->segs_are_gso) {
		KUNIT_EXPECT_TRUE(test, skb_is_gso(cur));
		...
	}

There is no negative assertion, so only the four lengths are compared.

In skb_segment(), max_segs == 1 gives partial_segs = min(len / mss, 1) == 1,
and then:

	if (partial_segs > 1)
		mss *= partial_segs;
	else
		partial_segs = 0;

If that guard ever became ">= 1", the grouping epilogue would set
gso_size = 1000 and gso_segs = 1 on each output, yet the segment lengths
(1000/1000/1000/3) and nr_segs would be unchanged and the case would still
pass.  Would adding an explicit check that these outputs are not GSO skbs
pin the documented behaviour down?

[ ... ]

> @@ -247,6 +308,15 @@ static void gso_test_func(struct kunit *test)
>  
>  		/* header was copied to all segs */
>  		KUNIT_ASSERT_EQ(test, memcmp(skb_mac_header(cur), hdr, sizeof(hdr)), 0);
> +		if (tcase->segs_are_gso) {
> +			KUNIT_EXPECT_TRUE(test, skb_is_gso(cur));
> +			KUNIT_EXPECT_EQ(test, skb_shinfo(cur)->gso_size,
> +					GSO_TEST_SIZE);
> +			KUNIT_EXPECT_LE(test, skb_shinfo(cur)->gso_segs,
> +					tcase->max_segs);
> +			KUNIT_EXPECT_FALSE(test, skb_shinfo(cur)->gso_type &
> +					 SKB_GSO_PARTIAL);
> +		}

[Severity: Low]
This isn't a bug introduced by this patch, but while adding cases here,
can this loop read past tcase->segs[]?  The loop in gso_test_func() is
bounded by the returned skb list, not by nr_segs:

	for (cur = segs, i = 0; cur; cur = next, i++) {
		next = cur->next;

		KUNIT_ASSERT_EQ(test, cur->len, sizeof(hdr) + tcase->segs[i]);

tcase->segs points at a compound literal with exactly nr_segs entries (2 for
bounded, 3 for bounded_multi, 4 for bounded_one_mss).  If skb_segment()
returns more outputs than expected while the leading lengths still match,
segs[nr_segs] is read past the end of the literal and the garbage value ends
up in the failure message.  The new TCP test uses the guard that would close
this:

	KUNIT_ASSERT_LT(test, i, ARRAY_SIZE(expected));

Would the same assertion fit in the parameterized loop?

> @@ -261,6 +331,177 @@ static void gso_test_func(struct kunit *test)
>  	consume_skb(skb);
>  }
>  
> +#define GSO_TCP_HDR_LEN \
> +	(ETH_HLEN + sizeof(struct iphdr) + sizeof(struct tcphdr))
> +

[ ... ]

> +static void gso_test_tcp_bounded_segment(struct kunit *test)
> +{
> +	netdev_features_t features = NETIF_F_SG | NETIF_F_HW_CSUM |
> +				     NETIF_F_TSO;
> +	const unsigned int payload_len = 3 * GSO_TEST_SIZE + 3;
> +	struct sk_buff *skb, *segs, *cur, *next;
> +	const unsigned int expected[] = {
> +		2 * GSO_TEST_SIZE, GSO_TEST_SIZE + 3,
> +	};
> +	const unsigned int max_segs = 2;
> +	int i = 0;
> +
> +	skb = gso_tcp_skb_new(payload_len);
> +	KUNIT_ASSERT_NOT_NULL(test, skb);
> +
> +	segs = __skb_gso_segment(skb, features, true, max_segs);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, segs);

[Severity: Medium]
Does this test add an undeclared CONFIG_INET dependency to the NET_TEST
suite?  The skb carries skb->protocol = htons(ETH_P_IP) and gso_type
SKB_GSO_TCPV4, and __skb_gso_segment() dispatches through
skb_mac_gso_segment():

	struct sk_buff *segs = ERR_PTR(-EPROTONOSUPPORT);
	...
	list_for_each_entry_rcu(ptype, &net_hotdata.offload_base, list) {
		if (ptype->type == type && ptype->callbacks.gso_segment) {

The only registrar of the ETH_P_IP offload is ipv4_offload_init(), which is
built only for CONFIG_INET.  net/Kconfig has:

	config NET_TEST
		tristate "KUnit tests for networking" if !KUNIT_ALL_TESTS
		depends on KUNIT
		default KUNIT_ALL_TESTS

so CONFIG_NET=y with CONFIG_INET=n is still a valid configuration, and there
the assertion above fires with -EPROTONOSUPPORT on every run.  Note that
KUNIT_ASSERT_NOT_ERR_OR_NULL() aborts the case as a failure rather than
skipping it.  Would "depends on INET" (or a kunit_skip() guard) be
appropriate here?  The existing cases in this file deliberately use a
protocol with no gso_segment callback and call skb_segment() directly.

[Severity: Low]
Does the input skb leak when this assertion fires?  skb is freed only by the
consume_skb(skb) at the end of the function, and KUNIT_ASSERT_* unwinds the
test-case thread, so nothing reaches that call.  Beyond the CONFIG_INET=n
case above, skb_segment() allocates its output skbs with GFP_ATOMIC and can
return ERR_PTR(-ENOMEM).

The in-loop KUNIT_ASSERT_LT() below has the same property: it runs before
consume_skb(cur), so an over-producing GSO engine would leave the current
segment, the rest of the segs list, and the input skb unfreed.

gso_test_func() in the same file avoids this with KUNIT_FAIL() plus
"goto free_gso_skb;".  Would the same pattern (or kunit_add_action()) work
here?

> +
> +	for (cur = segs; cur; cur = next, i++) {
> +		next = cur->next;
> +
> +		KUNIT_ASSERT_LT(test, i, ARRAY_SIZE(expected));
> +		KUNIT_EXPECT_EQ(test, cur->len,
> +				GSO_TCP_HDR_LEN + expected[i]);
> +		KUNIT_EXPECT_TRUE(test, skb_is_gso(cur));
> +		KUNIT_EXPECT_EQ(test, skb_shinfo(cur)->gso_size,
> +				GSO_TEST_SIZE);
> +		KUNIT_EXPECT_LE(test, skb_shinfo(cur)->gso_segs, max_segs);
> +
> +		consume_skb(cur);
> +	}
> +
> +	KUNIT_EXPECT_EQ(test, i, ARRAY_SIZE(expected));
> +	consume_skb(skb);
> +}
> +
> +#define GSO_TCP6_HDR_LEN \
> +	(ETH_HLEN + sizeof(struct ipv6hdr) + sizeof(struct tcphdr))
> +
> +static struct sk_buff *gso_tcp6_skb_new(unsigned int payload_len)
> +{
> +	struct ipv6hdr *ip6h;
> +	struct sk_buff *skb;
> +	struct ethhdr *eth;
> +	struct tcphdr *th;
> +
> +	skb = alloc_skb(GSO_TCP6_HDR_LEN + payload_len, GFP_KERNEL);
> +	if (!skb)
> +		return NULL;
> +	skb_put_zero(skb, GSO_TCP6_HDR_LEN + payload_len);

[Severity: Low]
Should this reserve some headroom?  There is no skb_reserve() here, so
skb_headroom() is 0 on the ~102 KB linear skb, and the later
vlan_insert_tag_set_proto() in gso_test_tcp_limit_l3_proto() goes
vlan_insert_tag() -> __vlan_insert_inner_tag() -> skb_cow_head(skb,
VLAN_HLEN), which with zero headroom does:

	pskb_expand_head(skb, ALIGN(VLAN_HLEN, NET_SKB_PAD), 0, GFP_ATOMIC);

That is a fresh order-5 GFP_ATOMIC allocation plus a full copy.  GFP_ATOMIC
cannot direct-reclaim or compact, so under fragmentation it fails,
vlan_insert_tag() frees the skb and returns NULL, and the test reports a
failure unrelated to what it is checking.  Would skb_reserve(skb, VLAN_HLEN)
(or NET_SKB_PAD) in this constructor exercise the same path without the
atomic reallocation?

[ ... ]

> +static void gso_test_tcp_limit_l3_proto(struct kunit *test)
> +{
> +	static const struct net_device_ops dummy_netdev_ops = { };
> +	const unsigned int payload_len = 100 * 1024;
> +	netdev_features_t features;
> +	struct net_device *dev;
> +	struct sk_buff *skb;
> +
> +	dev = alloc_etherdev(0);
> +	KUNIT_ASSERT_NOT_NULL(test, dev);
> +	dev->netdev_ops = &dummy_netdev_ops;
> +	dev->hw_features = NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_TSO6;
> +	dev->features = dev->hw_features;
> +	dev->vlan_features = dev->hw_features;
> +
> +	skb = gso_tcp6_skb_new(payload_len);
> +	KUNIT_ASSERT_NOT_NULL(test, skb);
> +	skb->dev = dev;

[Severity: Low]
Does the net_device leak here?  dev is released only by the free_netdev(dev)
at the end of the function, and three fatal assertions sit between the
alloc_etherdev(0) and that call: this KUNIT_ASSERT_NOT_NULL() on the ~102 KB
skb, and the KUNIT_ASSERT_NOT_NULL()/KUNIT_ASSERT_TRUE() pair after
vlan_insert_tag_set_proto() below, which also leaks the skb.  No
kunit_add_action() or managed allocation is registered for either object.

> +
> +	/* The skb fits the IPv6 limit but not the IPv4 one. */
> +	dev->gso_max_size = GSO_MAX_SIZE;
> +	dev->gso_ipv4_max_size = GSO_LEGACY_MAX_SIZE;
> +	features = netif_skb_features(skb);
> +	KUNIT_EXPECT_TRUE(test, features & NETIF_F_GSO_MASK);
> +
> +	/* ...and the other way around. */
> +	dev->gso_max_size = GSO_LEGACY_MAX_SIZE;
> +	dev->gso_ipv4_max_size = GSO_MAX_SIZE;
> +	features = netif_skb_features(skb);
> +	KUNIT_EXPECT_FALSE(test, features & NETIF_F_GSO_MASK);

[Severity: Low]
All four checks in this test operate on the IPv6 skb from
gso_tcp6_skb_new(), so is the IPv4 half of the contract covered?  The
selector is:

	return protocol == htons(ETH_P_IPV6) ?
	       READ_ONCE(dev->gso_max_size) :
	       READ_ONCE(dev->gso_ipv4_max_size);

An implementation of gso_within_device_limits() that always passed
ETH_P_IPV6, or simply always used dev->gso_max_size, would satisfy every
assertion here while breaking IPv4.  Adding an IPv4 skb (tagged and
untagged, with the limits skewed the other way) would need NETIF_F_TSO in
dev->hw_features as well, since only NETIF_F_TSO6 is set above.

[ ... ]

> +	/* Pushing the tag inside must not change either answer. */
> +	skb = vlan_insert_tag_set_proto(skb, htons(ETH_P_8021Q), 0);
> +	KUNIT_ASSERT_NOT_NULL(test, skb);
> +	KUNIT_ASSERT_TRUE(test, skb->protocol == htons(ETH_P_8021Q));

[ ... ]

> +	consume_skb(skb);
> +	free_netdev(dev);
> +}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918084651.3022878-1-wang.zhan%40smartx.com