[PATCH v1 net-next 0/8] tcp: Prevent tcp_collapse() from bloating queue.

[PATCH v1 net-next 0/8] tcp: Prevent tcp_collapse() from bloating queue.

[Kuniyuki Iwashima]

This is a follow-up for commit 031c403d13e5 ("tcp: do not let
tcp_collapse() drive sk_forward_alloc negative").

The core algorithm in tcp_collapse() is so simple and has not
been changed since commit 1da177e4c3f4 ("Linux-2.6.12-rc2");
finding a collapsible skb and repacking it into up-to-SKB_MAX_ORDER()
skb.

Now that we use 4K MSS and tp->scaling_ratio, the simple algorithm
does not work well, and it could even end up consuming more memory
contrary to the intention.

For example, the scaling ratio of SKB_MAX_ORDER() is 222, but
typically tp->scaling_ratio for 4K MSS flow is 252, and repacking
data rather bloats high-density skb.

Also, once a low-density skb starts collapsing, it drags all the
following skbs into collapsing regardless of the density.

This series changes the algorithm so that it skips high-density
skb and a single low-density skb between high-density skbs, batches
memory accounting at the end, and removes sk_forced_mem_schedule().


Kuniyuki Iwashima (8):
  tcp: Clean up the first loop in tcp_collapse().
  tcp: Don't collapse high density skb.
  tcp: Stop collapsing if not bloated.
  net: Move SKB_SMALL_HEAD_CACHE_SIZE to skbuff.h.
  tcp: Skip collapsing if collapsed skb is less-dense.
  tcp: Allocate all new skb before collapsing.
  tcp: Batch memory accounting in tcp_collapse().
  selftest: packetdrill: Add test for tcp_collapse().

 include/linux/skbuff.h                        |  25 +++
 include/net/gro.h                             |   3 -
 include/net/tcp.h                             |   1 -
 net/core/skbuff.c                             |  13 --
 net/ipv4/tcp_input.c                          | 171 +++++++++++++-----
 .../selftests/net/packetdrill/ksft_runner.sh  |   9 +
 .../net/packetdrill/tcp_collapse.pkt          | 135 ++++++++++++++
 7 files changed, 292 insertions(+), 65 deletions(-)
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_collapse.pkt

-- 
2.56.0.611.g1030f0b940-goog

[PATCH v1 net-next 1/8] tcp: Clean up the first loop in tcp_collapse().

[Kuniyuki Iwashima]

tcp_collapse() detects skb to start collapsing from in the
first loop.

If the last skb has SYN or FIN, the condition is tested
again after the loop.

skb_frags_readable() is placed inside and outside of the
loop as well.

If we goto skip_this in both cases, we can remove the second
tests and simply replace end_of_skbs with the inverted loop
condition.

Then, we do not need a test for tcp_collapse_one() too.

Also, skb_frags_readable() should be tested after the fully
overlapping check.

Let's clean up the loop.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/tcp_input.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index a2074a1af422..32e63d089db8 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5839,23 +5839,17 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 {
 	struct sk_buff *skb = head, *n;
 	struct sk_buff_head tmp;
-	bool end_of_skbs;
 
 	/* First, check that queue is collapsible and find
 	 * the point where collapsing can be useful.
 	 */
 restart:
-	for (end_of_skbs = true; skb != NULL && skb != tail; skb = n) {
+	for (; skb && skb != tail; skb = n) {
 		n = tcp_skb_next(skb, list);
 
-		if (!skb_frags_readable(skb))
-			goto skip_this;
-
 		/* No new bits? It is possible on ofo queue. */
 		if (!before(start, TCP_SKB_CB(skb)->end_seq)) {
 			skb = tcp_collapse_one(sk, skb, list, root);
-			if (!skb)
-				break;
 			goto restart;
 		}
 
@@ -5864,27 +5858,25 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 		 * - bloated or contains data before "start" or
 		 *   overlaps to the next one and mptcp allow collapsing.
 		 */
-		if (!(TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)) &&
-		    (tcp_win_from_space(sk, skb->truesize) > skb->len ||
-		     before(TCP_SKB_CB(skb)->seq, start))) {
-			end_of_skbs = false;
+		if (!skb_frags_readable(skb) ||
+		    TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN))
+			goto skip_this;
+
+		if (tcp_win_from_space(sk, skb->truesize) > skb->len ||
+		    before(TCP_SKB_CB(skb)->seq, start))
 			break;
-		}
 
 		if (n && n != tail && skb_frags_readable(n) &&
 		    tcp_skb_can_collapse_rx(skb, n) &&
-		    TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(n)->seq) {
-			end_of_skbs = false;
+		    TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(n)->seq)
 			break;
-		}
 
 skip_this:
 		/* Decided to skip this, advance start seq. */
 		start = TCP_SKB_CB(skb)->end_seq;
 	}
-	if (end_of_skbs ||
-	    (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)) ||
-	    !skb_frags_readable(skb))
+
+	if (!skb || skb == tail)
 		return;
 
 	__skb_queue_head_init(&tmp);
-- 
2.56.0.611.g1030f0b940-goog

Re: [PATCH v1 net-next 1/8] tcp: Clean up the first loop in tcp_collapse().

[Eric Dumazet]

Le sam. 14 nov. 2026 à 05:01, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
>
> tcp_collapse() detects skb to start collapsing from in the
> first loop.
>
> If the last skb has SYN or FIN, the condition is tested
> again after the loop.
>
> skb_frags_readable() is placed inside and outside of the
> loop as well.
>
> If we goto skip_this in both cases, we can remove the second
> tests and simply replace end_of_skbs with the inverted loop
> condition.
>
> Then, we do not need a test for tcp_collapse_one() too.
>
> Also, skb_frags_readable() should be tested after the fully
> overlapping check.
>
> Let's clean up the loop.

This is not only a cleanup.

Before this patch, when a SYN/FIN skb was overlapping with the next
skb, the loop was left with end_of_skbs == false, and the test
following the loop made tcp_collapse() return: nothing after this skb
was collapsed.

After this patch, this skb is skipped and the following skbs are
considered.

I think the new behavior is fine, but please mention it in the
changelog.

Reviewed-by: Eric Dumazet <edumazet@kernel.org>

Re: [PATCH v1 net-next 1/8] tcp: Clean up the first loop in tcp_collapse().

[Kuniyuki Iwashima]

On Sat, Nov 14, 2026 at 7:32 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> Le sam. 14 nov. 2026 à 05:01, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
> >
> > tcp_collapse() detects skb to start collapsing from in the
> > first loop.
> >
> > If the last skb has SYN or FIN, the condition is tested
> > again after the loop.
> >
> > skb_frags_readable() is placed inside and outside of the
> > loop as well.
> >
> > If we goto skip_this in both cases, we can remove the second
> > tests and simply replace end_of_skbs with the inverted loop
> > condition.
> >
> > Then, we do not need a test for tcp_collapse_one() too.
> >
> > Also, skb_frags_readable() should be tested after the fully
> > overlapping check.
> >
> > Let's clean up the loop.
>
> This is not only a cleanup.
>
> Before this patch, when a SYN/FIN skb was overlapping with the next
> skb, the loop was left with end_of_skbs == false, and the test
> following the loop made tcp_collapse() return: nothing after this skb
> was collapsed.
>
> After this patch, this skb is skipped and the following skbs are
> considered.
>
> I think the new behavior is fine, but please mention it in the
> changelog.

Will add more description in v2.

>
> Reviewed-by: Eric Dumazet <edumazet@kernel.org>

Thank you!

Re: [PATCH v1 net-next 1/8] tcp: Clean up the first loop in tcp_collapse().

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The patch is titled a clean-up, and its message presents the
  change as equivalent to the old code ("If we goto skip_this in both…

--

tcp: Clean up the first loop in tcp_collapse().

This commit restructures the first loop of tcp_collapse(). The SYN/FIN
and skb_frags_readable() checks are folded into one goto skip_this test,
and the end_of_skbs flag is replaced with the inverted loop condition.
It also moves the readability check after the fully overlapping check.

> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index a2074a1af4222..32e63d089db89 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -5839,23 +5839,17 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,

[ ... ]

>  restart:
> -	for (end_of_skbs = true; skb != NULL && skb != tail; skb = n) {
> +	for (; skb && skb != tail; skb = n) {
>  		n = tcp_skb_next(skb, list);
>  
> -		if (!skb_frags_readable(skb))
> -			goto skip_this;
> -
>  		/* No new bits? It is possible on ofo queue. */
>  		if (!before(start, TCP_SKB_CB(skb)->end_seq)) {
>  			skb = tcp_collapse_one(sk, skb, list, root);
> -			if (!skb)
> -				break;
>  			goto restart;
>  		}
>  
> @@ -5864,27 +5858,25 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
>  		 * - bloated or contains data before "start" or
>  		 *   overlaps to the next one and mptcp allow collapsing.
>  		 */
> -		if (!(TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)) &&
> -		    (tcp_win_from_space(sk, skb->truesize) > skb->len ||
> -		     before(TCP_SKB_CB(skb)->seq, start))) {
> -			end_of_skbs = false;
> +		if (!skb_frags_readable(skb) ||
> +		    TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN))
> +			goto skip_this;

[Severity: Low]
The commit message describes this as an equivalent clean-up:

  If we goto skip_this in both cases, we can remove the second
  tests and simply replace end_of_skbs with the inverted loop
  condition.

Is that true for SYN/FIN skbs? Before this patch, take a SYN or FIN skb
whose readable successor n overlaps it and is allowed by
tcp_skb_can_collapse_rx(). That skb reached the second break with
end_of_skbs = false:

	if (n && n != tail && skb_frags_readable(n) &&
	    tcp_skb_can_collapse_rx(skb, n) &&
	    TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(n)->seq) {
		end_of_skbs = false;
		break;
	}

The post-loop test then returned early:

	if (end_of_skbs ||
	    (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)) ||
	    !skb_frags_readable(skb))
		return;

So nothing in the head..tail range was collapsed in that call. The
post-loop SYN/FIN test was not redundant.

With this patch every SYN/FIN skb goes to skip_this. start advances and
scanning continues, so later skbs in the same range can now be collapsed.
A peer can reach this path, for example with a FIN segment followed by an
overlapping segment in the ofo queue.

The readability reordering also changes behaviour. Unreadable (devmem)
skbs that lie entirely below start used to be skipped. Now they are
unlinked and freed by tcp_collapse_one(), which also increments
LINUX_MIB_TCPRCVCOLLAPSED. The message only says:

  Also, skb_frags_readable() should be tested after the fully
  overlapping check.

It gives no reason for the change.

The new behaviour looks safe as far as I can tell. The copy loop still
stops at SYN/FIN skbs, and offset stays non-negative.

Could the changelog state both the SYN/FIN change and the devmem change
explicitly, rather than presenting the patch as a pure clean-up? The first
loop at the end of the series still behaves this way, so this patch looks
like the right place to describe it.

> +
> +		if (tcp_win_from_space(sk, skb->truesize) > skb->len ||
> +		    before(TCP_SKB_CB(skb)->seq, start))
>  			break;
> -		}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261114040101.1043546-1-kuniyu%40google.com

[PATCH v1 net-next 2/8] tcp: Don't collapse high density skb.

[Kuniyuki Iwashima]

tcp_collapse() starts collapsing skb to a new skb if either
of the following is true:

  1. skb is less dense compared to tp->scaling_ratio
  2. skb has read data before @start

However, this could make situation worse.

The new skb size is SKB_TRUESIZE(SKB_MAX_ORDER(0, 0)) at most,
and with PAGE_SIZE being 4K, its scaling_ratio is 222 (86%).

If 4K-MSS GRO skb updates tp->scaling_ratio, it can be larger
than 222.

Then, even the newly collapsed skb could be detected as bloated
by the next tcp_collapse().

Also, even when only a few bytes are read from GRO skb, it is
collapsed by the 2nd rule, making less-efficient skb.

Let's cap tp->scaling_ratio with the collapsed skb's maximum
scaling_ratio and use unread bytes for bloat detection.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/tcp_input.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 32e63d089db8..e07f43347409 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5808,6 +5808,19 @@ static struct sk_buff *tcp_skb_next(struct sk_buff *skb, struct sk_buff_head *li
 	return skb_rb_next(skb);
 }
 
+#define TCP_COLLAPSE_MAX_SIZE	SKB_MAX_ORDER(0, 0)
+#define TCP_COLLAPSE_SCALING_RATIO				\
+	((TCP_COLLAPSE_MAX_SIZE << TCP_RMEM_TO_WIN_SCALE) /	\
+	 (SKB_TRUESIZE(TCP_COLLAPSE_MAX_SIZE)))
+
+static bool tcp_collapse_skb_bloated(const struct sk_buff *skb,
+				     u32 start, u8 scaling_ratio)
+{
+	int unread = TCP_SKB_CB(skb)->end_seq - start;
+
+	return __tcp_win_from_space(scaling_ratio, skb->truesize) > unread;
+}
+
 static struct sk_buff *tcp_collapse_one(struct sock *sk, struct sk_buff *skb,
 					struct sk_buff_head *list,
 					struct rb_root *root)
@@ -5839,6 +5852,10 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 {
 	struct sk_buff *skb = head, *n;
 	struct sk_buff_head tmp;
+	u8 scaling_ratio;
+
+	scaling_ratio = min(tcp_sk(sk)->scaling_ratio,
+			    TCP_COLLAPSE_SCALING_RATIO);
 
 	/* First, check that queue is collapsible and find
 	 * the point where collapsing can be useful.
@@ -5855,15 +5872,14 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 
 		/* The first skb to collapse is:
 		 * - not SYN/FIN and
-		 * - bloated or contains data before "start" or
+		 * - the unread portion starting from "start" is bloated, or
 		 *   overlaps to the next one and mptcp allow collapsing.
 		 */
 		if (!skb_frags_readable(skb) ||
 		    TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN))
 			goto skip_this;
 
-		if (tcp_win_from_space(sk, skb->truesize) > skb->len ||
-		    before(TCP_SKB_CB(skb)->seq, start))
+		if (tcp_collapse_skb_bloated(skb, start, scaling_ratio))
 			break;
 
 		if (n && n != tail && skb_frags_readable(n) &&
@@ -5882,7 +5898,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 	__skb_queue_head_init(&tmp);
 
 	while (before(start, end)) {
-		int copy = min_t(int, SKB_MAX_ORDER(0, 0), end - start);
+		int copy = min_t(int, TCP_COLLAPSE_MAX_SIZE, end - start);
 		struct sk_buff *nskb;
 
 		nskb = alloc_skb(copy, GFP_ATOMIC);
-- 
2.56.0.611.g1030f0b940-goog

Re: [PATCH v1 net-next 2/8] tcp: Don't collapse high density skb.

[Eric Dumazet]

Le sam. 14 nov. 2026 à 05:01, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
>
> tcp_collapse() starts collapsing skb to a new skb if either
> of the following is true:
>
>   1. skb is less dense compared to tp->scaling_ratio
>   2. skb has read data before @start
>
> However, this could make situation worse.
>
> The new skb size is SKB_TRUESIZE(SKB_MAX_ORDER(0, 0)) at most,
> and with PAGE_SIZE being 4K, its scaling_ratio is 222 (86%).
>
> If 4K-MSS GRO skb updates tp->scaling_ratio, it can be larger
> than 222.
>
> Then, even the newly collapsed skb could be detected as bloated
> by the next tcp_collapse().
>
> Also, even when only a few bytes are read from GRO skb, it is
> collapsed by the 2nd rule, making less-efficient skb.
>
> Let's cap tp->scaling_ratio with the collapsed skb's maximum
> scaling_ratio and use unread bytes for bloat detection.
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>

Reviewed-by: Eric Dumazet <edumazet@kernel.org>

[PATCH v1 net-next 3/8] tcp: Stop collapsing if not bloated.

[Kuniyuki Iwashima]

Even when a skb is not bloated, tcp_collapse() starts collapsing
if the next skb has overlapping data.

Also, tcp_collapse() does not stop until it reaches the tail or
unmergable skb (SYN/FIN, unreadable frags).

Assume we detect a low-density skb followed by fully-packed 4K-MSS
GRO skbs:

             skb->len  truesize  scaling_ratio
  skb    #1      1448      2944      125 (49%)
  skb    #2     65536     66432      252 (98%)
  skb    #3     65536     66432      252 (98%)
  ...
  skb    #9     65536     66432      252 (98%)
  --------------------------------------------
  total                  534400

Then, even the high-density skbs are repacked into lower-density
skbs, and we end up consuming much more memory.

             skb->len  truesize  scaling_ratio
  nskb   #1      3776      4352      222 (86%)
  nskb   #2      3776      4352      222 (86%)
  ...
  nskb #139      3776      4352      222 (86%)
  nskb #140       872      2304       96 (37%)
  --------------------------------------------
  total                  607232 (+72832!)

Even if we find a collapsible skb, we should not collapse
subsequent skbs with scaling_ratio >= TCP_COLLAPSE_SCALING_RATIO.

Conversely, even if tp->scaling_ratio is smaller than
TCP_COLLAPSE_SCALING_RATIO, we should repack skbs once we
start collapsing, which results in higher-density skb.

Let's remove the next skb condition in the first loop,
search for collapsible bloated skb range, and repeat
collapsing for each range.

Note that even when the next skb is not bloated, we must
collapse it for OOO rbtree if it has overlapping data.

	if ((list || !before(TCP_SKB_CB(n)->seq, sub_end)) &&
	    !tcp_collapse_skb_bloated(n, sub_end,
				      TCP_COLLAPSE_SCALING_RATIO))

For example, bloated skb_1 and non-bloated skb_2 have overlapping
data and only skb_1 is collapsed to nskb_1 and nskb_2.

   skb_1 = [1000, 5000]
   skb_2 = [2000, 66000]

Then, nskb_2 will be inserted after skb_2 in rbtree, wrongly
setting tp->ooo_last_skb to 5000.

  nskb_1 = [1000, 4776]
   skb_2 = [2000, 66000]
  nskb_2 = [4776, 5000]

This case is still inefficient, but a later patch handles this.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/tcp_input.c | 66 +++++++++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 29 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index e07f43347409..5bd42a74c6f5 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5839,7 +5839,7 @@ static struct sk_buff *tcp_collapse_one(struct sock *sk, struct sk_buff *skb,
 }
 
 /* Collapse contiguous sequence of skbs head..tail with
- * sequence numbers start..end.
+ * sequence numbers start..sub_end.
  *
  * If tail is NULL, this means until the end of the queue.
  *
@@ -5848,15 +5848,18 @@ static struct sk_buff *tcp_collapse_one(struct sock *sk, struct sk_buff *skb,
  */
 static void
 tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
-	     struct sk_buff *head, struct sk_buff *tail, u32 start, u32 end)
+	     struct sk_buff *head, struct sk_buff *tail, u32 start)
 {
 	struct sk_buff *skb = head, *n;
 	struct sk_buff_head tmp;
 	u8 scaling_ratio;
+	u32 sub_end;
 
 	scaling_ratio = min(tcp_sk(sk)->scaling_ratio,
 			    TCP_COLLAPSE_SCALING_RATIO);
 
+	__skb_queue_head_init(&tmp);
+
 	/* First, check that queue is collapsible and find
 	 * the point where collapsing can be useful.
 	 */
@@ -5872,38 +5875,48 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 
 		/* The first skb to collapse is:
 		 * - not SYN/FIN and
-		 * - the unread portion starting from "start" is bloated, or
-		 *   overlaps to the next one and mptcp allow collapsing.
+		 * - the unread portion starting from "start" is bloated.
 		 */
-		if (!skb_frags_readable(skb) ||
-		    TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN))
-			goto skip_this;
-
-		if (tcp_collapse_skb_bloated(skb, start, scaling_ratio))
-			break;
-
-		if (n && n != tail && skb_frags_readable(n) &&
-		    tcp_skb_can_collapse_rx(skb, n) &&
-		    TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(n)->seq)
+		if (skb_frags_readable(skb) &&
+		    !(TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)) &&
+		    tcp_collapse_skb_bloated(skb, start, scaling_ratio))
 			break;
 
-skip_this:
 		/* Decided to skip this, advance start seq. */
 		start = TCP_SKB_CB(skb)->end_seq;
 	}
 
 	if (!skb || skb == tail)
-		return;
+		goto end;
 
-	__skb_queue_head_init(&tmp);
+	/* Second, find consecutive collapsible skb. */
+	sub_end = TCP_SKB_CB(skb)->end_seq;
+
+	for (n = tcp_skb_next(skb, list); n && n != tail; n = tcp_skb_next(n, list)) {
+		if (!skb_frags_readable(n) ||
+		    TCP_SKB_CB(n)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN) ||
+		    !tcp_skb_can_collapse_rx(skb, n))
+			break;
+
+		if (!after(TCP_SKB_CB(n)->end_seq, sub_end))
+			continue;
+
+		/* seq and end_seq must increase monotonically for rbtree. */
+		if ((list || !before(TCP_SKB_CB(n)->seq, sub_end)) &&
+		    !tcp_collapse_skb_bloated(n, sub_end,
+					      TCP_COLLAPSE_SCALING_RATIO))
+			break;
 
-	while (before(start, end)) {
-		int copy = min_t(int, TCP_COLLAPSE_MAX_SIZE, end - start);
+		sub_end = TCP_SKB_CB(n)->end_seq;
+	}
+
+	while (before(start, sub_end)) {
+		int copy = min_t(int, TCP_COLLAPSE_MAX_SIZE, sub_end - start);
 		struct sk_buff *nskb;
 
 		nskb = alloc_skb(copy, GFP_ATOMIC);
 		if (!nskb)
-			break;
+			goto end;
 
 		memcpy(nskb->cb, skb->cb, sizeof(skb->cb));
 		TCP_SKB_CB(nskb)->has_rxtstamp = false;
@@ -5942,15 +5955,11 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 			}
 			if (!before(start, TCP_SKB_CB(skb)->end_seq)) {
 				skb = tcp_collapse_one(sk, skb, list, root);
-				if (!skb ||
-				    skb == tail ||
-				    !tcp_skb_can_collapse_rx(nskb, skb) ||
-				    (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)) ||
-				    !skb_frags_readable(skb))
-					goto end;
 			}
 		}
 	}
+
+	goto restart;
 end:
 	skb_queue_walk_safe(&tmp, skb, n)
 		tcp_rbtree_insert(root, skb);
@@ -5989,7 +5998,7 @@ static void tcp_collapse_ofo_queue(struct sock *sk)
 			if (range_truesize != head->truesize ||
 			    end - start >= SKB_WITH_OVERHEAD(PAGE_SIZE)) {
 				tcp_collapse(sk, NULL, &tp->out_of_order_queue,
-					     head, skb, start, end);
+					     head, skb, start);
 			} else {
 				sum_tiny += range_truesize;
 				if (sum_tiny > sk->sk_rcvbuf >> 3)
@@ -6103,8 +6112,7 @@ static int tcp_prune_queue(struct sock *sk, const struct sk_buff *in_skb)
 		if (!skb_queue_empty(&sk->sk_receive_queue))
 			tcp_collapse(sk, &sk->sk_receive_queue, NULL,
 				     skb_peek(&sk->sk_receive_queue),
-				     NULL,
-				     tp->copied_seq, tp->rcv_nxt);
+				     NULL, tp->copied_seq);
 
 		if (tcp_can_ingest(sk, in_skb))
 			return 0;
-- 
2.56.0.611.g1030f0b940-goog

Re: [PATCH v1 net-next 3/8] tcp: Stop collapsing if not bloated.

[Eric Dumazet]

Le sam. 14 nov. 2026 à 05:01, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
>
> Even when a skb is not bloated, tcp_collapse() starts collapsing
> if the next skb has overlapping data.
>
> Also, tcp_collapse() does not stop until it reaches the tail or
> unmergable skb (SYN/FIN, unreadable frags).
>
> Assume we detect a low-density skb followed by fully-packed 4K-MSS
> GRO skbs:
>
>              skb->len  truesize  scaling_ratio
>   skb    #1      1448      2944      125 (49%)
>   skb    #2     65536     66432      252 (98%)
>   skb    #3     65536     66432      252 (98%)
>   ...
>   skb    #9     65536     66432      252 (98%)
>   --------------------------------------------
>   total                  534400
>
> Then, even the high-density skbs are repacked into lower-density
> skbs, and we end up consuming much more memory.
>
>              skb->len  truesize  scaling_ratio
>   nskb   #1      3776      4352      222 (86%)
>   nskb   #2      3776      4352      222 (86%)
>   ...
>   nskb #139      3776      4352      222 (86%)
>   nskb #140       872      2304       96 (37%)
>   --------------------------------------------
>   total                  607232 (+72832!)
>
> Even if we find a collapsible skb, we should not collapse
> subsequent skbs with scaling_ratio >= TCP_COLLAPSE_SCALING_RATIO.
>
> Conversely, even if tp->scaling_ratio is smaller than
> TCP_COLLAPSE_SCALING_RATIO, we should repack skbs once we
> start collapsing, which results in higher-density skb.
>
> Let's remove the next skb condition in the first loop,
> search for collapsible bloated skb range, and repeat
> collapsing for each range.
>
> Note that even when the next skb is not bloated, we must
> collapse it for OOO rbtree if it has overlapping data.
>
>         if ((list || !before(TCP_SKB_CB(n)->seq, sub_end)) &&
>             !tcp_collapse_skb_bloated(n, sub_end,
>                                       TCP_COLLAPSE_SCALING_RATIO))
>
> For example, bloated skb_1 and non-bloated skb_2 have overlapping
> data and only skb_1 is collapsed to nskb_1 and nskb_2.
>
>    skb_1 = [1000, 5000]
>    skb_2 = [2000, 66000]
>
> Then, nskb_2 will be inserted after skb_2 in rbtree, wrongly
> setting tp->ooo_last_skb to 5000.
>
>   nskb_1 = [1000, 4776]
>    skb_2 = [2000, 66000]
>   nskb_2 = [4776, 5000]
>
> This case is still inefficient, but a later patch handles this.
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
>  net/ipv4/tcp_input.c | 66 +++++++++++++++++++++++++-------------------
>  1 file changed, 37 insertions(+), 29 deletions(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index e07f43347409..5bd42a74c6f5 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -5839,7 +5839,7 @@ static struct sk_buff *tcp_collapse_one(struct sock *sk, struct sk_buff *skb,
>  }
>
>  /* Collapse contiguous sequence of skbs head..tail with
> - * sequence numbers start..end.
> + * sequence numbers start..sub_end.
>   *
>   * If tail is NULL, this means until the end of the queue.
>   *
> @@ -5848,15 +5848,18 @@ static struct sk_buff *tcp_collapse_one(struct sock *sk, struct sk_buff *skb,
>   */
>  static void
>  tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
> -            struct sk_buff *head, struct sk_buff *tail, u32 start, u32 end)
> +            struct sk_buff *head, struct sk_buff *tail, u32 start)
>  {
>         struct sk_buff *skb = head, *n;
>         struct sk_buff_head tmp;
>         u8 scaling_ratio;
> +       u32 sub_end;
>
>         scaling_ratio = min(tcp_sk(sk)->scaling_ratio,
>                             TCP_COLLAPSE_SCALING_RATIO);
>
> +       __skb_queue_head_init(&tmp);
> +
>         /* First, check that queue is collapsible and find
>          * the point where collapsing can be useful.
>          */
> @@ -5872,38 +5875,48 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
>
>                 /* The first skb to collapse is:
>                  * - not SYN/FIN and
> -                * - the unread portion starting from "start" is bloated, or
> -                *   overlaps to the next one and mptcp allow collapsing.
> +                * - the unread portion starting from "start" is bloated.
>                  */
> -               if (!skb_frags_readable(skb) ||
> -                   TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN))
> -                       goto skip_this;
> -
> -               if (tcp_collapse_skb_bloated(skb, start, scaling_ratio))
> -                       break;
> -
> -               if (n && n != tail && skb_frags_readable(n) &&
> -                   tcp_skb_can_collapse_rx(skb, n) &&
> -                   TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(n)->seq)
> +               if (skb_frags_readable(skb) &&
> +                   !(TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)) &&
> +                   tcp_collapse_skb_bloated(skb, start, scaling_ratio))
>                         break;
>
> -skip_this:
>                 /* Decided to skip this, advance start seq. */
>                 start = TCP_SKB_CB(skb)->end_seq;
>         }
>
>         if (!skb || skb == tail)
> -               return;
> +               goto end;
>
> -       __skb_queue_head_init(&tmp);
> +       /* Second, find consecutive collapsible skb. */
> +       sub_end = TCP_SKB_CB(skb)->end_seq;
> +
> +       for (n = tcp_skb_next(skb, list); n && n != tail; n = tcp_skb_next(n, list)) {
> +               if (!skb_frags_readable(n) ||
> +                   TCP_SKB_CB(n)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN) ||
> +                   !tcp_skb_can_collapse_rx(skb, n))
> +                       break;


Adding MPTCP maintainers.

Before this patch, each candidate was tested against the current new
skb:

        !tcp_skb_can_collapse_rx(nskb, skb)

and nskb is the result of mptcp_skb_ext_move(nskb, skb), done with
the skb that was being copied when nskb was allocated.

Now all candidates are tested against the first skb of the range,
before any extension has been moved.

mptcp_ext_matches() is not symmetric (a 'from' skb without extension
matches anything), so the two are not equivalent when the range needs
more than one new skb. For instance:

  skb1 (ext A, 6000 bytes), skb2 (ext A)

nskb1 gets ext A, nskb2 is allocated while skb1 is still being
copied and gets no extension. Before the patch, skb2 was not merged
into nskb2 (to_ext == NULL, from_ext != NULL). Now it is, and its
extension is dropped.

This might be fine since skb2 has the same mapping as nskb1, but
I would like MPTCP folks to confirm, and the changelog should
mention it.

The rest of the patch looks good to me.

> +
> +               if (!after(TCP_SKB_CB(n)->end_seq, sub_end))
> +                       continue;
> +
> +               /* seq and end_seq must increase monotonically for rbtree. */
> +               if ((list || !before(TCP_SKB_CB(n)->seq, sub_end)) &&
> +                   !tcp_collapse_skb_bloated(n, sub_end,
> +                                             TCP_COLLAPSE_SCALING_RATIO))
> +                       break;
>
> -       while (before(start, end)) {
> -               int copy = min_t(int, TCP_COLLAPSE_MAX_SIZE, end - start);
> +               sub_end = TCP_SKB_CB(n)->end_seq;
> +       }
> +
> +       while (before(start, sub_end)) {
> +               int copy = min_t(int, TCP_COLLAPSE_MAX_SIZE, sub_end - start);
>                 struct sk_buff *nskb;
>
>                 nskb = alloc_skb(copy, GFP_ATOMIC);
>                 if (!nskb)
> -                       break;
> +                       goto end;
>
>                 memcpy(nskb->cb, skb->cb, sizeof(skb->cb));
>                 TCP_SKB_CB(nskb)->has_rxtstamp = false;
> @@ -5942,15 +5955,11 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
>                         }
>                         if (!before(start, TCP_SKB_CB(skb)->end_seq)) {
>                                 skb = tcp_collapse_one(sk, skb, list, root);
> -                               if (!skb ||
> -                                   skb == tail ||
> -                                   !tcp_skb_can_collapse_rx(nskb, skb) ||
> -                                   (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)) ||
> -                                   !skb_frags_readable(skb))
> -                                       goto end;
>                         }
>                 }
>         }
> +
> +       goto restart;
>  end:
>         skb_queue_walk_safe(&tmp, skb, n)
>                 tcp_rbtree_insert(root, skb);
> @@ -5989,7 +5998,7 @@ static void tcp_collapse_ofo_queue(struct sock *sk)
>                         if (range_truesize != head->truesize ||
>                             end - start >= SKB_WITH_OVERHEAD(PAGE_SIZE)) {
>                                 tcp_collapse(sk, NULL, &tp->out_of_order_queue,
> -                                            head, skb, start, end);
> +                                            head, skb, start);
>                         } else {
>                                 sum_tiny += range_truesize;
>                                 if (sum_tiny > sk->sk_rcvbuf >> 3)
> @@ -6103,8 +6112,7 @@ static int tcp_prune_queue(struct sock *sk, const struct sk_buff *in_skb)
>                 if (!skb_queue_empty(&sk->sk_receive_queue))
>                         tcp_collapse(sk, &sk->sk_receive_queue, NULL,
>                                      skb_peek(&sk->sk_receive_queue),
> -                                    NULL,
> -                                    tp->copied_seq, tp->rcv_nxt);
> +                                    NULL, tp->copied_seq);
>
>                 if (tcp_can_ingest(sk, in_skb))
>                         return 0;
> --
> 2.56.0.611.g1030f0b940-goog
>

Re: [PATCH v1 net-next 3/8] tcp: Stop collapsing if not bloated.

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The header comment on tcp_collapse() in net/ipv4/tcp_input.c
  (line 5841-5842) was edited by this patch to read "Collapse contiguous…

--

tcp: Stop collapsing if not bloated.

This commit changes tcp_collapse() so it only collapses ranges of bloated
skbs. It no longer starts collapsing because the next skb overlaps, and it
no longer runs on until the tail. The end parameter is dropped. A local
sub_end is computed for each bloated range, and the function restarts the
search after each range.

> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index e07f433474093..5bd42a74c6f58 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -5839,7 +5839,7 @@ static struct sk_buff *tcp_collapse_one(struct sock *sk, struct sk_buff *skb,
>  }
>  
>  /* Collapse contiguous sequence of skbs head..tail with
> - * sequence numbers start..end.
> + * sequence numbers start..sub_end.

[Severity: Low]
This isn't a bug, but should this header comment for tcp_collapse() be
reworded rather than just renamed?

sub_end is a local variable, not a parameter. The second loop recomputes
it for each bloated sub-range, and it changes after every goto restart.
Someone reading the function's contract won't know what sub_end refers to.

The comment also still says head..tail is collapsed as one contiguous run.
With this patch, non-bloated skbs are skipped, each sub-range stops at the
first non-bloated, non-overlapping skb, and only the bloated sub-ranges
inside head..tail are collapsed.

The comment above the caller, tcp_collapse_ofo_queue(), has the same
mismatch:

/* Collapse ofo queue. Algorithm: select contiguous sequence of skbs
 * and tcp_collapse() them until all the queue is collapsed.
 */

The last patch in the series still leaves the header comment as
start..sub_end.

>   *
>   * If tail is NULL, this means until the end of the queue.
>   *
> @@ -5848,15 +5848,18 @@ static struct sk_buff *tcp_collapse_one(struct sock *sk, struct sk_buff *skb,
>   */
>  static void
>  tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
> -	     struct sk_buff *head, struct sk_buff *tail, u32 start, u32 end)
> +	     struct sk_buff *head, struct sk_buff *tail, u32 start)
>  {
>  	struct sk_buff *skb = head, *n;
>  	struct sk_buff_head tmp;
>  	u8 scaling_ratio;
> +	u32 sub_end;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261114040101.1043546-1-kuniyu%40google.com

[PATCH v1 net-next 4/8] net: Move SKB_SMALL_HEAD_CACHE_SIZE to skbuff.h.

[Kuniyuki Iwashima]

The next patch needs SKB_SMALL_HEAD_CACHE_SIZE to precalculate
skb truesize for tcp_collapse().

Let's move SKB_SMALL_HEAD_CACHE_SIZE and its dependencies to
skbuff.h.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/linux/skbuff.h | 18 ++++++++++++++++++
 include/net/gro.h      |  3 ---
 include/net/tcp.h      |  1 -
 net/core/skbuff.c      | 13 -------------
 4 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 1d6832878930..d36fbb88b3f6 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -269,6 +269,24 @@
 #define SKB_MAX_HEAD(X)		(SKB_MAX_ORDER((X), 0))
 #define SKB_MAX_ALLOC		(SKB_MAX_ORDER(0, 2))
 
+#define MAX_TCP_HEADER	L1_CACHE_ALIGN(128 + MAX_HEADER)
+
+/* This should be increased if a protocol with a bigger head is added. */
+#define GRO_MAX_HEAD (MAX_HEADER + 128)
+#define GRO_MAX_HEAD_PAD (GRO_MAX_HEAD + NET_SKB_PAD + NET_IP_ALIGN)
+
+#define SKB_SMALL_HEAD_SIZE SKB_HEAD_ALIGN(max(MAX_TCP_HEADER,		\
+					       GRO_MAX_HEAD_PAD))
+
+/* SKB_SMALL_HEAD_CACHE_SIZE is the size used for the skbuff_small_head
+ * kmem_cache. The non-power-of-2 padding is kept for historical reasons and
+ * to avoid potential collisions with generic kmalloc bucket sizes.
+ */
+#define SKB_SMALL_HEAD_CACHE_SIZE			\
+	(is_power_of_2(SKB_SMALL_HEAD_SIZE) ?		\
+	 (SKB_SMALL_HEAD_SIZE + L1_CACHE_BYTES) :	\
+	 SKB_SMALL_HEAD_SIZE)
+
 /* return minimum truesize of one skb containing X bytes of data */
 #define SKB_TRUESIZE(X) ((X) +						\
 			 SKB_DATA_ALIGN(sizeof(struct sk_buff)) +	\
diff --git a/include/net/gro.h b/include/net/gro.h
index 7a19d4486456..4eb755c33a26 100644
--- a/include/net/gro.h
+++ b/include/net/gro.h
@@ -11,9 +11,6 @@
 #include <net/udp.h>
 #include <net/hotdata.h>
 
-/* This should be increased if a protocol with a bigger head is added. */
-#define GRO_MAX_HEAD (MAX_HEADER + 128)
-
 struct napi_gro_cb {
 	union {
 		struct {
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 25d11fab1b03..9fcc5e5223f8 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -67,7 +67,6 @@ static inline void tcp_orphan_count_dec(void)
 
 void tcp_time_wait(struct sock *sk, int state, int timeo);
 
-#define MAX_TCP_HEADER	L1_CACHE_ALIGN(128 + MAX_HEADER)
 #define MAX_TCP_OPTION_SPACE 40
 #define TCP_MIN_SND_MSS		48
 #define TCP_MIN_GSO_SIZE	(TCP_MIN_SND_MSS - MAX_TCP_OPTION_SPACE)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 2e81aacc998b..421776e3526e 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -105,19 +105,6 @@
 static struct kmem_cache *skbuff_ext_cache __ro_after_init;
 #endif
 
-#define GRO_MAX_HEAD_PAD (GRO_MAX_HEAD + NET_SKB_PAD + NET_IP_ALIGN)
-#define SKB_SMALL_HEAD_SIZE SKB_HEAD_ALIGN(max(MAX_TCP_HEADER, \
-					       GRO_MAX_HEAD_PAD))
-
-/* SKB_SMALL_HEAD_CACHE_SIZE is the size used for the skbuff_small_head
- * kmem_cache. The non-power-of-2 padding is kept for historical reasons and
- * to avoid potential collisions with generic kmalloc bucket sizes.
- */
-#define SKB_SMALL_HEAD_CACHE_SIZE					\
-	(is_power_of_2(SKB_SMALL_HEAD_SIZE) ?			\
-		(SKB_SMALL_HEAD_SIZE + L1_CACHE_BYTES) :	\
-		SKB_SMALL_HEAD_SIZE)
-
 #define SKB_SMALL_HEAD_HEADROOM						\
 	SKB_WITH_OVERHEAD(SKB_SMALL_HEAD_CACHE_SIZE)
 
-- 
2.56.0.611.g1030f0b940-goog

[PATCH v1 net-next 5/8] tcp: Skip collapsing if collapsed skb is less-dense.

[Kuniyuki Iwashima]

Once tcp_collapse() finds a collapsible skb, it collapses
consecutive bloated skbs.

However, the end result could be the same or larger than
as is in some cases:

  1. skb is already minimum

     If SKB_HEAD_ALIGN(len) <= SKB_SMALL_HEAD_CACHE_SIZE,
     skb is allocated from skbuff_small_head, and skb->truesize
     never changes.

  2. SKB_HEAD_ALIGN() and kmalloc_size_roundup() in alloc_skb()

     Assume two bloated skbs with len = 1448 B (truesize =
     2304 B, scaling_ratio = 160) and len = 300 B (truesize =
     896 B, scaling_ratio = 85), whose total truesize is
     3200 B.

     Collapsing them calls alloc_skb(1748), which rounds up
     SKB_HEAD_ALIGN(1748) = 2112 B to 4096 B and allocates a
     new skb with truesize = 4352 B (+1152 B!).

Even worse, in case 1, such a skb is reallocated repeatedly
by every tcp_collapse() in vain.

Let's calculate the final collapsed truesize before collapsing
skbs and skip the range if it does not shrink.

We could allocate skbs first and calculate the new truesize,
but given tcp_collapse() is called under memory pressure,
we want to avoid unnecessary allocation.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/linux/skbuff.h |  7 +++++++
 net/ipv4/tcp_input.c   | 24 +++++++++++++++++++++++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index d36fbb88b3f6..618e629df15a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -292,6 +292,13 @@
 			 SKB_DATA_ALIGN(sizeof(struct sk_buff)) +	\
 			 SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
 
+/* return estimated truesize allocated by alloc_skb(X) via kmalloc_reserve() */
+#define SKB_ALLOCSIZE(X)					\
+	((SKB_HEAD_ALIGN(X) <= SKB_SMALL_HEAD_CACHE_SIZE ?	\
+	  SKB_SMALL_HEAD_CACHE_SIZE :				\
+	  kmalloc_size_roundup(SKB_HEAD_ALIGN(X))) +		\
+	 SKB_DATA_ALIGN(sizeof(struct sk_buff)))
+
 struct net_device;
 struct scatterlist;
 struct pipe_inode_info;
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 5bd42a74c6f5..fe71305f6e67 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5850,8 +5850,10 @@ static void
 tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 	     struct sk_buff *head, struct sk_buff *tail, u32 start)
 {
+	int old_truesize, new_truesize;
 	struct sk_buff *skb = head, *n;
 	struct sk_buff_head tmp;
+	int remainder, nskbs;
 	u8 scaling_ratio;
 	u32 sub_end;
 
@@ -5891,6 +5893,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 
 	/* Second, find consecutive collapsible skb. */
 	sub_end = TCP_SKB_CB(skb)->end_seq;
+	old_truesize = skb->truesize;
 
 	for (n = tcp_skb_next(skb, list); n && n != tail; n = tcp_skb_next(n, list)) {
 		if (!skb_frags_readable(n) ||
@@ -5898,8 +5901,10 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 		    !tcp_skb_can_collapse_rx(skb, n))
 			break;
 
-		if (!after(TCP_SKB_CB(n)->end_seq, sub_end))
+		if (!after(TCP_SKB_CB(n)->end_seq, sub_end)) {
+			old_truesize += n->truesize;
 			continue;
+		}
 
 		/* seq and end_seq must increase monotonically for rbtree. */
 		if ((list || !before(TCP_SKB_CB(n)->seq, sub_end)) &&
@@ -5908,6 +5913,23 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 			break;
 
 		sub_end = TCP_SKB_CB(n)->end_seq;
+		old_truesize += n->truesize;
+	}
+
+	/* Third, check if the new skbs have high density. */
+	nskbs = (sub_end - start) / TCP_COLLAPSE_MAX_SIZE;
+	new_truesize = nskbs * SKB_TRUESIZE(TCP_COLLAPSE_MAX_SIZE);
+
+	remainder = (sub_end - start) % TCP_COLLAPSE_MAX_SIZE;
+	if (remainder) {
+		nskbs++;
+		new_truesize += SKB_ALLOCSIZE(remainder);
+	}
+
+	if (old_truesize <= new_truesize) {
+		start = sub_end;
+		skb = n;
+		goto restart;
 	}
 
 	while (before(start, sub_end)) {
-- 
2.56.0.611.g1030f0b940-goog

Re: [PATCH v1 net-next 5/8] tcp: Skip collapsing if collapsed skb is less-dense.

[Eric Dumazet]

Le sam. 14 nov. 2026 à 05:01, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
>
> Once tcp_collapse() finds a collapsible skb, it collapses
> consecutive bloated skbs.
>
> However, the end result could be the same or larger than
> as is in some cases:
>
>   1. skb is already minimum
>
>      If SKB_HEAD_ALIGN(len) <= SKB_SMALL_HEAD_CACHE_SIZE,
>      skb is allocated from skbuff_small_head, and skb->truesize
>      never changes.
>
>   2. SKB_HEAD_ALIGN() and kmalloc_size_roundup() in alloc_skb()
>
>      Assume two bloated skbs with len = 1448 B (truesize =
>      2304 B, scaling_ratio = 160) and len = 300 B (truesize =
>      896 B, scaling_ratio = 85), whose total truesize is
>      3200 B.
>
>      Collapsing them calls alloc_skb(1748), which rounds up
>      SKB_HEAD_ALIGN(1748) = 2112 B to 4096 B and allocates a
>      new skb with truesize = 4352 B (+1152 B!).
>
> Even worse, in case 1, such a skb is reallocated repeatedly
> by every tcp_collapse() in vain.
>
> Let's calculate the final collapsed truesize before collapsing
> skbs and skip the range if it does not shrink.
>
> We could allocate skbs first and calculate the new truesize,
> but given tcp_collapse() is called under memory pressure,
> we want to avoid unnecessary allocation.
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
>  include/linux/skbuff.h |  7 +++++++
>  net/ipv4/tcp_input.c   | 24 +++++++++++++++++++++++-
>  2 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index d36fbb88b3f6..618e629df15a 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -292,6 +292,13 @@
>                          SKB_DATA_ALIGN(sizeof(struct sk_buff)) +       \
>                          SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
>
> +/* return estimated truesize allocated by alloc_skb(X) via kmalloc_reserve() */
> +#define SKB_ALLOCSIZE(X)                                       \
> +       ((SKB_HEAD_ALIGN(X) <= SKB_SMALL_HEAD_CACHE_SIZE ?      \
> +         SKB_SMALL_HEAD_CACHE_SIZE :                           \
> +         kmalloc_size_roundup(SKB_HEAD_ALIGN(X))) +            \
> +        SKB_DATA_ALIGN(sizeof(struct sk_buff)))
> +

This duplicates the logic of kmalloc_reserve() in a header, far from
it. Nothing will remind us to update the macro when kmalloc_reserve()
changes.

This matters after patch 7: if the estimation is smaller than what
alloc_skb() really provides, @released can be negative, and
sk_forced_mem_schedule() is no longer there to protect
sk->sk_forward_alloc.

Please add a function in net/core/skbuff.c, next to
kmalloc_reserve(), returning the truesize of an skb allocated by
alloc_skb(size, GFP_ATOMIC), and ideally make kmalloc_reserve() and
this function share the size computation.

Then patch 4 is not needed.

>  struct net_device;
>  struct scatterlist;
>  struct pipe_inode_info;
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 5bd42a74c6f5..fe71305f6e67 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -5850,8 +5850,10 @@ static void
>  tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
>              struct sk_buff *head, struct sk_buff *tail, u32 start)
>  {
> +       int old_truesize, new_truesize;
>         struct sk_buff *skb = head, *n;
>         struct sk_buff_head tmp;
> +       int remainder, nskbs;
>         u8 scaling_ratio;
>         u32 sub_end;
>
> @@ -5891,6 +5893,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
>
>         /* Second, find consecutive collapsible skb. */
>         sub_end = TCP_SKB_CB(skb)->end_seq;
> +       old_truesize = skb->truesize;
>
>         for (n = tcp_skb_next(skb, list); n && n != tail; n = tcp_skb_next(n, list)) {
>                 if (!skb_frags_readable(n) ||
> @@ -5898,8 +5901,10 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
>                     !tcp_skb_can_collapse_rx(skb, n))
>                         break;
>
> -               if (!after(TCP_SKB_CB(n)->end_seq, sub_end))
> +               if (!after(TCP_SKB_CB(n)->end_seq, sub_end)) {
> +                       old_truesize += n->truesize;
>                         continue;
> +               }
>
>                 /* seq and end_seq must increase monotonically for rbtree. */
>                 if ((list || !before(TCP_SKB_CB(n)->seq, sub_end)) &&
> @@ -5908,6 +5913,23 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
>                         break;
>
>                 sub_end = TCP_SKB_CB(n)->end_seq;
> +               old_truesize += n->truesize;
> +       }
> +
> +       /* Third, check if the new skbs have high density. */
> +       nskbs = (sub_end - start) / TCP_COLLAPSE_MAX_SIZE;
> +       new_truesize = nskbs * SKB_TRUESIZE(TCP_COLLAPSE_MAX_SIZE);
> +
> +       remainder = (sub_end - start) % TCP_COLLAPSE_MAX_SIZE;
> +       if (remainder) {
> +               nskbs++;
> +               new_truesize += SKB_ALLOCSIZE(remainder);
> +       }
> +
> +       if (old_truesize <= new_truesize) {
> +               start = sub_end;
> +               skb = n;
> +               goto restart;
>         }
>
>         while (before(start, sub_end)) {
> --
> 2.56.0.611.g1030f0b940-goog
>

Re: [PATCH v1 net-next 5/8] tcp: Skip collapsing if collapsed skb is less-dense.

[Kuniyuki Iwashima]

On Sat, Nov 14, 2026 at 7:37 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> Le sam. 14 nov. 2026 à 05:01, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
> >
> > Once tcp_collapse() finds a collapsible skb, it collapses
> > consecutive bloated skbs.
> >
> > However, the end result could be the same or larger than
> > as is in some cases:
> >
> >   1. skb is already minimum
> >
> >      If SKB_HEAD_ALIGN(len) <= SKB_SMALL_HEAD_CACHE_SIZE,
> >      skb is allocated from skbuff_small_head, and skb->truesize
> >      never changes.
> >
> >   2. SKB_HEAD_ALIGN() and kmalloc_size_roundup() in alloc_skb()
> >
> >      Assume two bloated skbs with len = 1448 B (truesize =
> >      2304 B, scaling_ratio = 160) and len = 300 B (truesize =
> >      896 B, scaling_ratio = 85), whose total truesize is
> >      3200 B.
> >
> >      Collapsing them calls alloc_skb(1748), which rounds up
> >      SKB_HEAD_ALIGN(1748) = 2112 B to 4096 B and allocates a
> >      new skb with truesize = 4352 B (+1152 B!).
> >
> > Even worse, in case 1, such a skb is reallocated repeatedly
> > by every tcp_collapse() in vain.
> >
> > Let's calculate the final collapsed truesize before collapsing
> > skbs and skip the range if it does not shrink.
> >
> > We could allocate skbs first and calculate the new truesize,
> > but given tcp_collapse() is called under memory pressure,
> > we want to avoid unnecessary allocation.
> >
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > ---
> >  include/linux/skbuff.h |  7 +++++++
> >  net/ipv4/tcp_input.c   | 24 +++++++++++++++++++++++-
> >  2 files changed, 30 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > index d36fbb88b3f6..618e629df15a 100644
> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -292,6 +292,13 @@
> >                          SKB_DATA_ALIGN(sizeof(struct sk_buff)) +       \
> >                          SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
> >
> > +/* return estimated truesize allocated by alloc_skb(X) via kmalloc_reserve() */
> > +#define SKB_ALLOCSIZE(X)                                       \
> > +       ((SKB_HEAD_ALIGN(X) <= SKB_SMALL_HEAD_CACHE_SIZE ?      \
> > +         SKB_SMALL_HEAD_CACHE_SIZE :                           \
> > +         kmalloc_size_roundup(SKB_HEAD_ALIGN(X))) +            \
> > +        SKB_DATA_ALIGN(sizeof(struct sk_buff)))
> > +
>
> This duplicates the logic of kmalloc_reserve() in a header, far from
> it. Nothing will remind us to update the macro when kmalloc_reserve()
> changes.
>
> This matters after patch 7: if the estimation is smaller than what
> alloc_skb() really provides, @released can be negative, and
> sk_forced_mem_schedule() is no longer there to protect
> sk->sk_forward_alloc.
>
> Please add a function in net/core/skbuff.c, next to
> kmalloc_reserve(), returning the truesize of an skb allocated by
> alloc_skb(size, GFP_ATOMIC), and ideally make kmalloc_reserve() and
> this function share the size computation.

Sure, I will add a thin function.

I was a bit afraid that touching kmalloc_reserve()
might affect non-FDO users.


>
> Then patch 4 is not needed.
>
> >  struct net_device;
> >  struct scatterlist;
> >  struct pipe_inode_info;
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > index 5bd42a74c6f5..fe71305f6e67 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -5850,8 +5850,10 @@ static void
> >  tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
> >              struct sk_buff *head, struct sk_buff *tail, u32 start)
> >  {
> > +       int old_truesize, new_truesize;
> >         struct sk_buff *skb = head, *n;
> >         struct sk_buff_head tmp;
> > +       int remainder, nskbs;
> >         u8 scaling_ratio;
> >         u32 sub_end;
> >
> > @@ -5891,6 +5893,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
> >
> >         /* Second, find consecutive collapsible skb. */
> >         sub_end = TCP_SKB_CB(skb)->end_seq;
> > +       old_truesize = skb->truesize;
> >
> >         for (n = tcp_skb_next(skb, list); n && n != tail; n = tcp_skb_next(n, list)) {
> >                 if (!skb_frags_readable(n) ||
> > @@ -5898,8 +5901,10 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
> >                     !tcp_skb_can_collapse_rx(skb, n))
> >                         break;
> >
> > -               if (!after(TCP_SKB_CB(n)->end_seq, sub_end))
> > +               if (!after(TCP_SKB_CB(n)->end_seq, sub_end)) {
> > +                       old_truesize += n->truesize;
> >                         continue;
> > +               }
> >
> >                 /* seq and end_seq must increase monotonically for rbtree. */
> >                 if ((list || !before(TCP_SKB_CB(n)->seq, sub_end)) &&
> > @@ -5908,6 +5913,23 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
> >                         break;
> >
> >                 sub_end = TCP_SKB_CB(n)->end_seq;
> > +               old_truesize += n->truesize;
> > +       }
> > +
> > +       /* Third, check if the new skbs have high density. */
> > +       nskbs = (sub_end - start) / TCP_COLLAPSE_MAX_SIZE;
> > +       new_truesize = nskbs * SKB_TRUESIZE(TCP_COLLAPSE_MAX_SIZE);
> > +
> > +       remainder = (sub_end - start) % TCP_COLLAPSE_MAX_SIZE;
> > +       if (remainder) {
> > +               nskbs++;
> > +               new_truesize += SKB_ALLOCSIZE(remainder);
> > +       }
> > +
> > +       if (old_truesize <= new_truesize) {
> > +               start = sub_end;
> > +               skb = n;
> > +               goto restart;
> >         }
> >
> >         while (before(start, sub_end)) {
> > --
> > 2.56.0.611.g1030f0b940-goog
> >

[PATCH v1 net-next 6/8] tcp: Allocate all new skb before collapsing.

[Kuniyuki Iwashima]

At the final stage, tcp_collapse() iterates over collapsible
candidate skbs and repacks them to new skbs.

In each iteration, a new skb is allocated and inserted to the
receive queue or a temporary list for RB tree.

If alloc_skb() fails while splitting a large bloated skb across
multiple new skbs, the partially-copied skb is not removed and
new skbs remain in the queue as duplicates, consuming more memory.

It is only when all candidates are properly collapsed that
we can save memory.

To avoid complicated rollback, let's preallocate skbs before
collapsing.

Now all skb in [start, sub_end) must be allocated in a batch,
so the section size is normally capped at TCP_COLLAPSE_MAX_TOTAL_SIZE
(16 skbs), except when a single bloated skb or overlapping OOO skbs
must be collapsed together to free the original skb or keep the
rbtree monotonic.

Once the size exceeds the limit, the remaining skbs are re-evaluated
from restart:.

Note that the next skb can be skipped if its density already meets
tp->scaling_ratio, and each cut adds at most one remainder skb
(~3.4% worst-case truesize overhead on 4 KiB pages), which is a
small tradeoff to bound the temporary allocation.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/tcp_input.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index fe71305f6e67..9f32577b0b53 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5812,6 +5812,7 @@ static struct sk_buff *tcp_skb_next(struct sk_buff *skb, struct sk_buff_head *li
 #define TCP_COLLAPSE_SCALING_RATIO				\
 	((TCP_COLLAPSE_MAX_SIZE << TCP_RMEM_TO_WIN_SCALE) /	\
 	 (SKB_TRUESIZE(TCP_COLLAPSE_MAX_SIZE)))
+#define TCP_COLLAPSE_MAX_TOTAL_SIZE (TCP_COLLAPSE_MAX_SIZE * 16)
 
 static bool tcp_collapse_skb_bloated(const struct sk_buff *skb,
 				     u32 start, u8 scaling_ratio)
@@ -5852,8 +5853,9 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 {
 	int old_truesize, new_truesize;
 	struct sk_buff *skb = head, *n;
-	struct sk_buff_head tmp;
+	struct sk_buff_head tmp, pool;
 	int remainder, nskbs;
+	struct sk_buff *nskb;
 	u8 scaling_ratio;
 	u32 sub_end;
 
@@ -5861,6 +5863,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 			    TCP_COLLAPSE_SCALING_RATIO);
 
 	__skb_queue_head_init(&tmp);
+	__skb_queue_head_init(&pool);
 
 	/* First, check that queue is collapsible and find
 	 * the point where collapsing can be useful.
@@ -5908,8 +5911,9 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 
 		/* seq and end_seq must increase monotonically for rbtree. */
 		if ((list || !before(TCP_SKB_CB(n)->seq, sub_end)) &&
-		    !tcp_collapse_skb_bloated(n, sub_end,
-					      TCP_COLLAPSE_SCALING_RATIO))
+		    (TCP_SKB_CB(n)->end_seq - start >= TCP_COLLAPSE_MAX_TOTAL_SIZE ||
+		     !tcp_collapse_skb_bloated(n, sub_end,
+					       TCP_COLLAPSE_SCALING_RATIO)))
 			break;
 
 		sub_end = TCP_SKB_CB(n)->end_seq;
@@ -5924,6 +5928,8 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 	if (remainder) {
 		nskbs++;
 		new_truesize += SKB_ALLOCSIZE(remainder);
+	} else {
+		remainder = TCP_COLLAPSE_MAX_SIZE;
 	}
 
 	if (old_truesize <= new_truesize) {
@@ -5932,14 +5938,19 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 		goto restart;
 	}
 
-	while (before(start, sub_end)) {
-		int copy = min_t(int, TCP_COLLAPSE_MAX_SIZE, sub_end - start);
-		struct sk_buff *nskb;
-
-		nskb = alloc_skb(copy, GFP_ATOMIC);
+	while (nskbs--) {
+		nskb = alloc_skb(nskbs ? TCP_COLLAPSE_MAX_SIZE : remainder,
+				 GFP_ATOMIC);
 		if (!nskb)
 			goto end;
 
+		__skb_queue_tail(&pool, nskb);
+	}
+
+	while (before(start, sub_end)) {
+		int copy = min_t(int, TCP_COLLAPSE_MAX_SIZE, sub_end - start);
+
+		nskb = __skb_dequeue(&pool);
 		memcpy(nskb->cb, skb->cb, sizeof(skb->cb));
 		TCP_SKB_CB(nskb)->has_rxtstamp = false;
 		skb_copy_decrypted(nskb, skb);
@@ -5985,6 +5996,8 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 end:
 	skb_queue_walk_safe(&tmp, skb, n)
 		tcp_rbtree_insert(root, skb);
+
+	__skb_queue_purge_reason(&pool, SKB_CONSUMED);
 }
 
 /* Collapse ofo queue. Algorithm: select contiguous sequence of skbs
-- 
2.56.0.611.g1030f0b940-goog

Re: [PATCH v1 net-next 6/8] tcp: Allocate all new skb before collapsing.

[Eric Dumazet]

Le sam. 14 nov. 2026 à 05:01, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
>
> At the final stage, tcp_collapse() iterates over collapsible
> candidate skbs and repacks them to new skbs.
>
> In each iteration, a new skb is allocated and inserted to the
> receive queue or a temporary list for RB tree.
>
> If alloc_skb() fails while splitting a large bloated skb across
> multiple new skbs, the partially-copied skb is not removed and
> new skbs remain in the queue as duplicates, consuming more memory.
>
> It is only when all candidates are properly collapsed that
> we can save memory.
>
> To avoid complicated rollback, let's preallocate skbs before
> collapsing.
>
> Now all skb in [start, sub_end) must be allocated in a batch,
> so the section size is normally capped at TCP_COLLAPSE_MAX_TOTAL_SIZE
> (16 skbs), except when a single bloated skb or overlapping OOO skbs
> must be collapsed together to free the original skb or keep the
> rbtree monotonic.
>
> Once the size exceeds the limit, the remaining skbs are re-evaluated
> from restart:.
>
> Note that the next skb can be skipped if its density already meets
> tp->scaling_ratio, and each cut adds at most one remainder skb
> (~3.4% worst-case truesize overhead on 4 KiB pages), which is a
> small tradeoff to bound the temporary allocation.
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>

Reviewed-by: Eric Dumazet <edumazet@kernel.org>

Re: [PATCH v1 net-next 6/8] tcp: Allocate all new skb before collapsing.

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 · Low: 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] In tcp_collapse() (net/ipv4/tcp_input.c), the new
  TCP_COLLAPSE_MAX_TOTAL_SIZE cap sits inside the second operand of…
- [Low] Concern: preallocating every skb makes tcp_collapse() all-or-
  nothing per section.

--

tcp: Allocate all new skb before collapsing.

This commit changes tcp_collapse() to preallocate every replacement skb
for a section into a temporary pool before copying any data. An
alloc_skb() failure therefore no longer leaves partially-copied skbs and
duplicates in the queue. To bound the batch, the section size is capped at
TCP_COLLAPSE_MAX_TOTAL_SIZE (16 skbs). The exceptions are a single bloated
skb and overlapping OOO skbs.

> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index fe71305f6e673..9f32577b0b53c 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c

[ ... ]

> @@ -5908,8 +5911,9 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
>  
>  		/* seq and end_seq must increase monotonically for rbtree. */
>  		if ((list || !before(TCP_SKB_CB(n)->seq, sub_end)) &&
> -		    !tcp_collapse_skb_bloated(n, sub_end,
> -					      TCP_COLLAPSE_SCALING_RATIO))
> +		    (TCP_SKB_CB(n)->end_seq - start >= TCP_COLLAPSE_MAX_TOTAL_SIZE ||
> +		     !tcp_collapse_skb_bloated(n, sub_end,
> +					       TCP_COLLAPSE_SCALING_RATIO)))
>  			break;
>  
>  		sub_end = TCP_SKB_CB(n)->end_seq;

[Severity: Medium]

Can the TCP_COLLAPSE_MAX_TOTAL_SIZE cap ever stop the section on the OOO
overlap path?

For the OOO rbtree (list == NULL), suppose n partially overlaps the
section, so n->seq < sub_end and n->end_seq > sub_end. Then the first
operand of the && is false. Neither the cap nor tcp_collapse_skb_bloated()
is checked, and sub_end keeps growing.

The peer controls seq and end_seq. tcp_data_queue_ofo() keeps partially
overlapping segments in the tree:

net/ipv4/tcp_input.c:tcp_data_queue_ofo() {
    ...
			if (after(seq, TCP_SKB_CB(skb1)->seq)) {
				/* Partial overlap. */
				tcp_dsack_set(sk, seq, TCP_SKB_CB(skb1)->end_seq);
			} else {
    ...
}

So a chain of segments, each starting just before the previous one ends,
can turn into a single section. Its size is limited only by the OOO memory
of the socket.

Only the first skb has to be bloated. The overlap also inflates
old_truesize, so the old_truesize > new_truesize check is easy to pass.

> @@ -5932,14 +5938,19 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
>  		goto restart;
>  	}
>  
> -	while (before(start, sub_end)) {
> -		int copy = min_t(int, TCP_COLLAPSE_MAX_SIZE, sub_end - start);
> -		struct sk_buff *nskb;
> -
> -		nskb = alloc_skb(copy, GFP_ATOMIC);
> +	while (nskbs--) {
> +		nskb = alloc_skb(nskbs ? TCP_COLLAPSE_MAX_SIZE : remainder,
> +				 GFP_ATOMIC);
>  		if (!nskb)
>  			goto end;
>  
> +		__skb_queue_tail(&pool, nskb);
> +	}

This loop then allocates every replacement skb with GFP_ATOMIC. All of
them sit in pool before any source skb is freed or any new skb is charged
to the socket.

Before this patch, alloc_skb() alternated with
tcp_collapse_one()->__kfree_skb(), so only about one extra skb was in
flight at a time. Now the number in flight is about
ceil((sub_end - start) / TCP_COLLAPSE_MAX_SIZE). With one-byte overlaps and
typical MSS truesize, that is roughly 0.7x the OOO memory of the socket,
on top of the existing queue.

A larger all-or-nothing batch is also more likely to fail under the memory
pressure that triggers collapse. In that case goto end frees the pool and
tcp_prune_queue() falls back to tcp_prune_ofo_queue().

The commit message does mention this exception:

  except when a single bloated skb or overlapping OOO skbs
  must be collapsed together to free the original skb or keep the
  rbtree monotonic.

However, the size of the overlapping OOO case is controlled by the peer
and bounded only by the OOO queue. Should the overlap path be bounded as
well? If not, could the commit message say that this exception is not
covered by the bound on the temporary allocation?

[Severity: Low]

Each section is now all-or-nothing. A single alloc_skb() failure in this
loop jumps to end, and nothing is reclaimed for this section or for any
later one. Even in the capped case, up to 16 skbs (about 70 KiB on 4 KiB
pages) are held before any source skb is freed.

The unused skbs are released by __skb_queue_purge_reason() at end, so
nothing leaks.

The commit message describes this tradeoff ("It is only when all
candidates are properly collapsed that we can save memory" and "a small
tradeoff to bound the temporary allocation"). The goto end on allocation
failure was also already there before this patch.

Is it intended that one failed batch also skips the remaining sections,
rather than only the current one?

> +
> +	while (before(start, sub_end)) {
> +		int copy = min_t(int, TCP_COLLAPSE_MAX_SIZE, sub_end - start);
> +
> +		nskb = __skb_dequeue(&pool);

[ ... ]

> @@ -5985,6 +5996,8 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
>  end:
>  	skb_queue_walk_safe(&tmp, skb, n)
>  		tcp_rbtree_insert(root, skb);
> +
> +	__skb_queue_purge_reason(&pool, SKB_CONSUMED);
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261114040101.1043546-1-kuniyu%40google.com

[PATCH v1 net-next 7/8] tcp: Batch memory accounting in tcp_collapse().

[Kuniyuki Iwashima]

Once tcp_collapse_one() calls sock_rfree(), skb->truesize is
reclaimed to sk->sk_forward_alloc by sk_mem_uncharge().

However, sk_mem_uncharge() rounds down sk->sk_forward_alloc
immediately once it reaches 4K since commit 4890b686f408 ("net:
keep sk->sk_forward_alloc as small as possible").

This unnecessarily reclaims memory for newly allocated skbs,
and the following skb_set_owner_r() for them underflows
sk->sk_forward_alloc without sk_forced_mem_schedule().

Let's cache the delta during tcp_collapse() and batch accounting
at the end.

Then, we no longer need sk_forced_mem_schedule(), and instead
DEBUG_NET_WARN_ON_ONCE() is added to catch a future issue.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/tcp_input.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 9f32577b0b53..598463f33987 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5822,6 +5822,24 @@ static bool tcp_collapse_skb_bloated(const struct sk_buff *skb,
 	return __tcp_win_from_space(scaling_ratio, skb->truesize) > unread;
 }
 
+static int tcp_collapse_uncharge(struct sk_buff *skb)
+{
+	DEBUG_NET_WARN_ON_ONCE(skb->destructor != sock_rfree);
+
+	skb->sk = NULL;
+	skb->destructor = NULL;
+
+	return skb->truesize;
+}
+
+static int tcp_collapse_charge(struct sk_buff *skb, struct sock *sk)
+{
+	skb->sk = sk;
+	skb->destructor = sock_rfree;
+
+	return -skb->truesize;
+}
+
 static struct sk_buff *tcp_collapse_one(struct sock *sk, struct sk_buff *skb,
 					struct sk_buff_head *list,
 					struct rb_root *root)
@@ -5857,6 +5875,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 	int remainder, nskbs;
 	struct sk_buff *nskb;
 	u8 scaling_ratio;
+	int released = 0;
 	u32 sub_end;
 
 	scaling_ratio = min(tcp_sk(sk)->scaling_ratio,
@@ -5874,6 +5893,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 
 		/* No new bits? It is possible on ofo queue. */
 		if (!before(start, TCP_SKB_CB(skb)->end_seq)) {
+			released += tcp_collapse_uncharge(skb);
 			skb = tcp_collapse_one(sk, skb, list, root);
 			goto restart;
 		}
@@ -5959,11 +5979,8 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 			__skb_queue_before(list, skb, nskb);
 		else
 			__skb_queue_tail(&tmp, nskb); /* defer rbtree insertion */
-		/* nskb is charged before the skbs it replaces are freed.
-		 * Make sure sk->sk_forward_alloc can not become negative.
-		 */
-		sk_forced_mem_schedule(sk, nskb->truesize);
-		skb_set_owner_r(nskb, sk);
+
+		released += tcp_collapse_charge(nskb, sk);
 		mptcp_skb_ext_move(nskb, skb);
 
 		/* Copy data, releasing collapsed skbs. */
@@ -5987,6 +6004,7 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 				start += size;
 			}
 			if (!before(start, TCP_SKB_CB(skb)->end_seq)) {
+				released += tcp_collapse_uncharge(skb);
 				skb = tcp_collapse_one(sk, skb, list, root);
 			}
 		}
@@ -5998,6 +6016,12 @@ tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
 		tcp_rbtree_insert(root, skb);
 
 	__skb_queue_purge_reason(&pool, SKB_CONSUMED);
+
+	if (released) {
+		DEBUG_NET_WARN_ON_ONCE(released < 0);
+		atomic_sub(released, &sk->sk_rmem_alloc);
+		sk_mem_uncharge(sk, released);
+	}
 }
 
 /* Collapse ofo queue. Algorithm: select contiguous sequence of skbs
-- 
2.56.0.611.g1030f0b940-goog

Re: [PATCH v1 net-next 7/8] tcp: Batch memory accounting in tcp_collapse().

[Eric Dumazet]

Le sam. 14 nov. 2026 à 05:01, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
>
> Once tcp_collapse_one() calls sock_rfree(), skb->truesize is
> reclaimed to sk->sk_forward_alloc by sk_mem_uncharge().
>
> However, sk_mem_uncharge() rounds down sk->sk_forward_alloc
> immediately once it reaches 4K since commit 4890b686f408 ("net:
> keep sk->sk_forward_alloc as small as possible").
>
> This unnecessarily reclaims memory for newly allocated skbs,
> and the following skb_set_owner_r() for them underflows
> sk->sk_forward_alloc without sk_forced_mem_schedule().
>
> Let's cache the delta during tcp_collapse() and batch accounting
> at the end.
>
> Then, we no longer need sk_forced_mem_schedule(), and instead
> DEBUG_NET_WARN_ON_ONCE() is added to catch a future issue.
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>

Reviewed-by: Eric Dumazet <edumazet@kernel.org>

[PATCH v1 net-next 8/8] selftest: packetdrill: Add test for tcp_collapse().

[Kuniyuki Iwashima]

tcp_collapse() now does not collapse high-density skbs and a
low-density skb sandwitched by hihg-density skbs.

tcp_collapse.pkt verifies the behaviour by

1. set a large SO_RCVBUF
2. send one high-density skb with 100 bytes read()
3. send one high-density skb
4. send one low-density skb
5. send one high-density skb
6. send 30 low-density skbs
7. set a small SO_RCVBUF
8. send a small data to trigger tcp_collapse()
9. check TcpExtTCPRcvCollapsed via nstat

Without the series, the first high-density skb is collapsed.
In addition, the first low-density skb drags the following
high-density skb into collapsing, consuming more memory.

If we remove sk_forced_mem_schedule() without this series,
sk->sk_forwad_alloc becomes negative at the end of tcp_collapse().

Note that 30 segments are to make the memory savings large
enough so that the margin can absorb minor size delta of
struct sk_buff or L1_CACHE_BYTES across configs.

skb truesize, TCP_COLLAPSE_MAX_SIZE, TCP_COLLAPSE_SCALING_RATIO
depend on PAGE_SIZE being 4K, so the test is skipped if getconf
returns non-4K PAGE_SIZE.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 .../selftests/net/packetdrill/ksft_runner.sh  |   9 ++
 .../net/packetdrill/tcp_collapse.pkt          | 135 ++++++++++++++++++
 2 files changed, 144 insertions(+)
 create mode 100644 tools/testing/selftests/net/packetdrill/tcp_collapse.pkt

diff --git a/tools/testing/selftests/net/packetdrill/ksft_runner.sh b/tools/testing/selftests/net/packetdrill/ksft_runner.sh
index 0a97d5ae3469..250d143bb9f7 100755
--- a/tools/testing/selftests/net/packetdrill/ksft_runner.sh
+++ b/tools/testing/selftests/net/packetdrill/ksft_runner.sh
@@ -44,6 +44,15 @@ if [ -z "$(which packetdrill)" ]; then
 	exit "$KSFT_SKIP"
 fi
 
+page_size=$(grep -E '^--define="?PAGE_SIZE=' $script | sed -E 's/.*PAGE_SIZE="?([0-9]+)"?.*/\1/')
+if [[ -n "$page_size" ]]; then
+	cur_page_size=$(getconf PAGESIZE 2>/dev/null)
+	if [[ "$cur_page_size" != "$page_size" ]]; then
+		ktap_skip_all "requires PAGE_SIZE=$page_size (current: ${cur_page_size:-unknown})"
+		exit "$KSFT_SKIP"
+	fi
+fi
+
 declare -a optargs
 failfunc=ktap_test_fail
 
diff --git a/tools/testing/selftests/net/packetdrill/tcp_collapse.pkt b/tools/testing/selftests/net/packetdrill/tcp_collapse.pkt
new file mode 100644
index 000000000000..90e8be86968f
--- /dev/null
+++ b/tools/testing/selftests/net/packetdrill/tcp_collapse.pkt
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Test that tcp_collapse() only collapses bloated skb runs, skips leading
+// and trailing high-density skbs, skips isolated bloated skbs when collapsing
+// them would not shrink truesize, and properly recovers receive buffer space.
+
+--mss=1000
+--define="PAGE_SIZE=4096"
+
+`./defaults.sh`
+
+// Establish a connection with a large receive buffer.
+// __sock_set_rcvbuf() doubles, so 2000000
+   +0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+   +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+   +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [1000000], 4) = 0
+   +0 bind(3, ..., ...) = 0
+   +0 listen(3, 1) = 0
+
+   +0 < S 0:0(0) win 32792 <mss 1000,nop,nop,sackOK,nop,wscale 7>
+   +0 > S. 0:0(0) ack 1 win 65535 <mss 1460,nop,nop,sackOK,nop,wscale 4>
+   +0 < . 1:1(0) ack 1 win 65535
+
+   +0 accept(3, ..., ...) = 4
+
+// 1. Queue two high-density 62,000 B segments (each adds 15 pages of frag
+// truesize with enough margin across NET_SKB_PAD, L1_CACHE_BYTES, and
+// MAX_SKB_FRAGS configs).
+// Read 100 bytes from the first one so before(skb->seq, tp->copied_seq) is true.
+   +0 < P. 1:62001(62000) ack 1 win 65535
+    * > .  1:1(0) ack 62001
+   +0 read(4, ..., 100) = 100
+
+   +0 < P. 62001:124001(62000) ack 1 win 65535
+    * > .  1:1(0) ack 124001
+
+// 2. Queue an isolated bloated 1,000 B segment.
+   +0 < P. 124001:125001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 125001
+
+// 3. Queue another high-density 62,000 B segment.
+   +0 < P. 125001:187001(62000) ack 1 win 65535
+    * > .  1:1(0) ack 187001
+
+// 4. Queue a run of 30 bloated 1,000 B segments.
+   +0 < P. 187001:188001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 188001
+   +0 < P. 188001:189001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 189001
+   +0 < P. 189001:190001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 190001
+   +0 < P. 190001:191001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 191001
+   +0 < P. 191001:192001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 192001
+   +0 < P. 192001:193001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 193001
+   +0 < P. 193001:194001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 194001
+   +0 < P. 194001:195001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 195001
+   +0 < P. 195001:196001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 196001
+   +0 < P. 196001:197001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 197001
+   +0 < P. 197001:198001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 198001
+   +0 < P. 198001:199001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 199001
+   +0 < P. 199001:200001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 200001
+   +0 < P. 200001:201001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 201001
+   +0 < P. 201001:202001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 202001
+   +0 < P. 202001:203001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 203001
+   +0 < P. 203001:204001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 204001
+   +0 < P. 204001:205001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 205001
+   +0 < P. 205001:206001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 206001
+   +0 < P. 206001:207001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 207001
+   +0 < P. 207001:208001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 208001
+   +0 < P. 208001:209001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 209001
+   +0 < P. 209001:210001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 210001
+   +0 < P. 210001:211001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 211001
+   +0 < P. 211001:212001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 212001
+   +0 < P. 212001:213001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 213001
+   +0 < P. 213001:214001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 214001
+   +0 < P. 214001:215001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 215001
+   +0 < P. 215001:216001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 216001
+   +0 < P. 216001:217001(1000) ack 1 win 65535
+    * > .  1:1(0) ack 217001
+
+// 5. Queue a trailing high-density 62,000 B segment.
+   +0 < P. 217001:279001(62000) ack 1 win 65535
+    * > .  1:1(0) ack 279001
+
+// Reset SNMP counters.
+   +0 `nstat -n`
+
+// Shrink SO_RCVBUF so sk_rcvbuf (320,000) is smaller than current
+// sk_rmem_alloc (334,592 on x86_64 default), but larger than sk_rmem_alloc
+// after collapsing the 30 bloated skbs (300,288 on x86_64 default).
+   +0 setsockopt(4, SOL_SOCKET, SO_RCVBUF, [160000], 4) = 0
+
+// Inject the next 1,000 B segment with a changed window to bypass
+// header prediction.  This triggers tcp_prune_queue() -> tcp_collapse().
+// Only the 30 bloated segments should be collapsed; the high-density
+// segments and the isolated bloated segment are skipped.  sk_rmem_alloc
+// shrinks below sk_rcvbuf, so tcp_can_ingest() succeeds and the packet
+// is accepted.
+   +0 < P. 279001:280001(1000) ack 1 win 65534
+    * > .  1:1(0) ack 280001
+
+// Verify exactly 30 skbs were collapsed.
+   +0 `nstat -s | grep TcpExtTCPRcvCollapsed | grep -q " 30 "`
+
+// Verify no packets were dropped.
+   +0 `test -z "$(nstat -s | grep TcpExtTCPRcvQDrop)"`
+
+// Read all remaining queued data to verify data integrity.
+   +0 read(4, ..., 300000) = 279900
-- 
2.56.0.611.g1030f0b940-goog

Re: [PATCH v1 net-next 8/8] selftest: packetdrill: Add test for tcp_collapse().

[Eric Dumazet]

Le sam. 14 nov. 2026 à 05:01, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
>
> tcp_collapse() now does not collapse high-density skbs and a
> low-density skb sandwitched by hihg-density skbs.
>

s/sandwitched/sandwiched/
s/hihg/high/


> tcp_collapse.pkt verifies the behaviour by
>
> 1. set a large SO_RCVBUF
> 2. send one high-density skb with 100 bytes read()
> 3. send one high-density skb
> 4. send one low-density skb
> 5. send one high-density skb
> 6. send 30 low-density skbs
> 7. set a small SO_RCVBUF
> 8. send a small data to trigger tcp_collapse()
> 9. check TcpExtTCPRcvCollapsed via nstat
>
> Without the series, the first high-density skb is collapsed.
> In addition, the first low-density skb drags the following
> high-density skb into collapsing, consuming more memory.
>
> If we remove sk_forced_mem_schedule() without this series,
> sk->sk_forwad_alloc becomes negative at the end of tcp_collapse().

s/sk_forwad_alloc/sk_forward_alloc/

>
> Note that 30 segments are to make the memory savings large
> enough so that the margin can absorb minor size delta of
> struct sk_buff or L1_CACHE_BYTES across configs.
>
> skb truesize, TCP_COLLAPSE_MAX_SIZE, TCP_COLLAPSE_SCALING_RATIO
> depend on PAGE_SIZE being 4K, so the test is skipped if getconf
> returns non-4K PAGE_SIZE.
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
>  .../selftests/net/packetdrill/ksft_runner.sh  |   9 ++
>  .../net/packetdrill/tcp_collapse.pkt          | 135 ++++++++++++++++++
>  2 files changed, 144 insertions(+)
>  create mode 100644 tools/testing/selftests/net/packetdrill/tcp_collapse.pkt
>
> diff --git a/tools/testing/selftests/net/packetdrill/ksft_runner.sh b/tools/testing/selftests/net/packetdrill/ksft_runner.sh
> index 0a97d5ae3469..250d143bb9f7 100755
> --- a/tools/testing/selftests/net/packetdrill/ksft_runner.sh
> +++ b/tools/testing/selftests/net/packetdrill/ksft_runner.sh
> @@ -44,6 +44,15 @@ if [ -z "$(which packetdrill)" ]; then
>         exit "$KSFT_SKIP"
>  fi
>
> +page_size=$(grep -E '^--define="?PAGE_SIZE=' $script | sed -E 's/.*PAGE_SIZE="?([0-9]+)"?.*/\1/')
> +if [[ -n "$page_size" ]]; then
> +       cur_page_size=$(getconf PAGESIZE 2>/dev/null)
> +       if [[ "$cur_page_size" != "$page_size" ]]; then
> +               ktap_skip_all "requires PAGE_SIZE=$page_size (current: ${cur_page_size:-unknown})"
> +               exit "$KSFT_SKIP"
> +       fi
> +fi
> +
>  declare -a optargs
>  failfunc=ktap_test_fail
>
> diff --git a/tools/testing/selftests/net/packetdrill/tcp_collapse.pkt b/tools/testing/selftests/net/packetdrill/tcp_collapse.pkt
> new file mode 100644
> index 000000000000..90e8be86968f
> --- /dev/null
> +++ b/tools/testing/selftests/net/packetdrill/tcp_collapse.pkt
> @@ -0,0 +1,135 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Test that tcp_collapse() only collapses bloated skb runs, skips leading
> +// and trailing high-density skbs, skips isolated bloated skbs when collapsing
> +// them would not shrink truesize, and properly recovers receive buffer space.
> +
> +--mss=1000
> +--define="PAGE_SIZE=4096"
> +
> +`./defaults.sh`
> +
> +// Establish a connection with a large receive buffer.
> +// __sock_set_rcvbuf() doubles, so 2000000
> +   +0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
> +   +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
> +   +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [1000000], 4) = 0
> +   +0 bind(3, ..., ...) = 0
> +   +0 listen(3, 1) = 0
> +
> +   +0 < S 0:0(0) win 32792 <mss 1000,nop,nop,sackOK,nop,wscale 7>
> +   +0 > S. 0:0(0) ack 1 win 65535 <mss 1460,nop,nop,sackOK,nop,wscale 4>
> +   +0 < . 1:1(0) ack 1 win 65535
> +
> +   +0 accept(3, ..., ...) = 4
> +
> +// 1. Queue two high-density 62,000 B segments (each adds 15 pages of frag
> +// truesize with enough margin across NET_SKB_PAD, L1_CACHE_BYTES, and
> +// MAX_SKB_FRAGS configs).
> +// Read 100 bytes from the first one so before(skb->seq, tp->copied_seq) is true.
> +   +0 < P. 1:62001(62000) ack 1 win 65535
> +    * > .  1:1(0) ack 62001
> +   +0 read(4, ..., 100) = 100
> +
> +   +0 < P. 62001:124001(62000) ack 1 win 65535
> +    * > .  1:1(0) ack 124001
> +
> +// 2. Queue an isolated bloated 1,000 B segment.
> +   +0 < P. 124001:125001(1000) ack 1 win 65535
> +    * > .  1:1(0) ack 125001
> +
> +// 3. Queue another high-density 62,000 B segment.
> +   +0 < P. 125001:187001(62000) ack 1 win 65535
> +    * > .  1:1(0) ack 187001
> +
> +// 4. Queue a run of 30 bloated 1,000 B segments.

This test only covers the receive queue.

Most of the new logic in patch 3 is about the out-of-order queue
(overlapping skbs, monotonic seq/end_seq in the rbtree), and nothing
exercises it: before this series, TcpExtTCPRcvCollapsed stays at zero
for the whole packetdrill TCP suite.

I have a test collapsing the out-of-order queue (segments sent in
decreasing sequence order so that they are not coalesced when
queued), it passes on top of your series. I can send it after your
v2, or you can add something similar here.

Re: [PATCH v1 net-next 8/8] selftest: packetdrill: Add test for tcp_collapse().

[Kuniyuki Iwashima]

On Sat, Nov 14, 2026 at 7:40 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> Le sam. 14 nov. 2026 à 05:01, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
> >
> > tcp_collapse() now does not collapse high-density skbs and a
> > low-density skb sandwitched by hihg-density skbs.
> >
>
> s/sandwitched/sandwiched/
> s/hihg/high/

oops, will update.

>
>
> > tcp_collapse.pkt verifies the behaviour by
> >
> > 1. set a large SO_RCVBUF
> > 2. send one high-density skb with 100 bytes read()
> > 3. send one high-density skb
> > 4. send one low-density skb
> > 5. send one high-density skb
> > 6. send 30 low-density skbs
> > 7. set a small SO_RCVBUF
> > 8. send a small data to trigger tcp_collapse()
> > 9. check TcpExtTCPRcvCollapsed via nstat
> >
> > Without the series, the first high-density skb is collapsed.
> > In addition, the first low-density skb drags the following
> > high-density skb into collapsing, consuming more memory.
> >
> > If we remove sk_forced_mem_schedule() without this series,
> > sk->sk_forwad_alloc becomes negative at the end of tcp_collapse().
>
> s/sk_forwad_alloc/sk_forward_alloc/

will update.

>
> >
> > Note that 30 segments are to make the memory savings large
> > enough so that the margin can absorb minor size delta of
> > struct sk_buff or L1_CACHE_BYTES across configs.
> >
> > skb truesize, TCP_COLLAPSE_MAX_SIZE, TCP_COLLAPSE_SCALING_RATIO
> > depend on PAGE_SIZE being 4K, so the test is skipped if getconf
> > returns non-4K PAGE_SIZE.
> >
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > ---
> >  .../selftests/net/packetdrill/ksft_runner.sh  |   9 ++
> >  .../net/packetdrill/tcp_collapse.pkt          | 135 ++++++++++++++++++
> >  2 files changed, 144 insertions(+)
> >  create mode 100644 tools/testing/selftests/net/packetdrill/tcp_collapse.pkt
> >
> > diff --git a/tools/testing/selftests/net/packetdrill/ksft_runner.sh b/tools/testing/selftests/net/packetdrill/ksft_runner.sh
> > index 0a97d5ae3469..250d143bb9f7 100755
> > --- a/tools/testing/selftests/net/packetdrill/ksft_runner.sh
> > +++ b/tools/testing/selftests/net/packetdrill/ksft_runner.sh
> > @@ -44,6 +44,15 @@ if [ -z "$(which packetdrill)" ]; then
> >         exit "$KSFT_SKIP"
> >  fi
> >
> > +page_size=$(grep -E '^--define="?PAGE_SIZE=' $script | sed -E 's/.*PAGE_SIZE="?([0-9]+)"?.*/\1/')
> > +if [[ -n "$page_size" ]]; then
> > +       cur_page_size=$(getconf PAGESIZE 2>/dev/null)
> > +       if [[ "$cur_page_size" != "$page_size" ]]; then
> > +               ktap_skip_all "requires PAGE_SIZE=$page_size (current: ${cur_page_size:-unknown})"
> > +               exit "$KSFT_SKIP"
> > +       fi
> > +fi
> > +
> >  declare -a optargs
> >  failfunc=ktap_test_fail
> >
> > diff --git a/tools/testing/selftests/net/packetdrill/tcp_collapse.pkt b/tools/testing/selftests/net/packetdrill/tcp_collapse.pkt
> > new file mode 100644
> > index 000000000000..90e8be86968f
> > --- /dev/null
> > +++ b/tools/testing/selftests/net/packetdrill/tcp_collapse.pkt
> > @@ -0,0 +1,135 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// Test that tcp_collapse() only collapses bloated skb runs, skips leading
> > +// and trailing high-density skbs, skips isolated bloated skbs when collapsing
> > +// them would not shrink truesize, and properly recovers receive buffer space.
> > +
> > +--mss=1000
> > +--define="PAGE_SIZE=4096"
> > +
> > +`./defaults.sh`
> > +
> > +// Establish a connection with a large receive buffer.
> > +// __sock_set_rcvbuf() doubles, so 2000000
> > +   +0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
> > +   +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
> > +   +0 setsockopt(3, SOL_SOCKET, SO_RCVBUF, [1000000], 4) = 0
> > +   +0 bind(3, ..., ...) = 0
> > +   +0 listen(3, 1) = 0
> > +
> > +   +0 < S 0:0(0) win 32792 <mss 1000,nop,nop,sackOK,nop,wscale 7>
> > +   +0 > S. 0:0(0) ack 1 win 65535 <mss 1460,nop,nop,sackOK,nop,wscale 4>
> > +   +0 < . 1:1(0) ack 1 win 65535
> > +
> > +   +0 accept(3, ..., ...) = 4
> > +
> > +// 1. Queue two high-density 62,000 B segments (each adds 15 pages of frag
> > +// truesize with enough margin across NET_SKB_PAD, L1_CACHE_BYTES, and
> > +// MAX_SKB_FRAGS configs).
> > +// Read 100 bytes from the first one so before(skb->seq, tp->copied_seq) is true.
> > +   +0 < P. 1:62001(62000) ack 1 win 65535
> > +    * > .  1:1(0) ack 62001
> > +   +0 read(4, ..., 100) = 100
> > +
> > +   +0 < P. 62001:124001(62000) ack 1 win 65535
> > +    * > .  1:1(0) ack 124001
> > +
> > +// 2. Queue an isolated bloated 1,000 B segment.
> > +   +0 < P. 124001:125001(1000) ack 1 win 65535
> > +    * > .  1:1(0) ack 125001
> > +
> > +// 3. Queue another high-density 62,000 B segment.
> > +   +0 < P. 125001:187001(62000) ack 1 win 65535
> > +    * > .  1:1(0) ack 187001
> > +
> > +// 4. Queue a run of 30 bloated 1,000 B segments.
>
> This test only covers the receive queue.
>
> Most of the new logic in patch 3 is about the out-of-order queue
> (overlapping skbs, monotonic seq/end_seq in the rbtree), and nothing
> exercises it: before this series, TcpExtTCPRcvCollapsed stays at zero
> for the whole packetdrill TCP suite.

Yes, indeed.

>
> I have a test collapsing the out-of-order queue (segments sent in
> decreasing sequence order so that they are not coalesced when
> queued), it passes on top of your series. I can send it after your
> v2, or you can add something similar here.

Thank you for the new test !

I'd avoid duplicating it and wait for your test.

Re: [PATCH v1 net-next 8/8] selftest: packetdrill: Add test for tcp_collapse().

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message does not accurately describe the test in
  tools/testing/selftests/net/packetdrill/tcp_collapse.pkt.

--

selftest: packetdrill: Add test for tcp_collapse().

This adds a packetdrill test, tcp_collapse.pkt. It checks that
tcp_collapse() only collapses runs of low-density skbs, and leaves
high-density skbs and an isolated low-density skb alone. It also makes
ksft_runner.sh skip any script whose PAGE_SIZE define does not match
getconf PAGESIZE.

> tcp_collapse() now does not collapse high-density skbs and a
> low-density skb sandwitched by hihg-density skbs.
>
> tcp_collapse.pkt verifies the behaviour by
>
> 1. set a large SO_RCVBUF
> 2. send one high-density skb with 100 bytes read()
> 3. send one high-density skb
> 4. send one low-density skb
> 5. send one high-density skb
> 6. send 30 low-density skbs
> 7. set a small SO_RCVBUF
> 8. send a small data to trigger tcp_collapse()
> 9. check TcpExtTCPRcvCollapsed via nstat
>
> Without the series, the first high-density skb is collapsed.
> In addition, the first low-density skb drags the following
> high-density skb into collapsing, consuming more memory.
>
> If we remove sk_forced_mem_schedule() without this series,
> sk->sk_forwad_alloc becomes negative at the end of tcp_collapse().

[Severity: Low]
Is a step missing from this list? Between steps 6 and 7,
tcp_collapse.pkt queues one more 62,000 B high-density segment:

// 5. Queue a trailing high-density 62,000 B segment.
   +0 < P. 217001:279001(62000) ack 1 win 65535
    * > .  1:1(0) ack 279001

The file header says the test checks that tcp_collapse() "skips leading
and trailing high-density skbs". The SO_RCVBUF shrink comment gives
sk_rmem_alloc as 334,592, and that figure seems to count four
high-density skbs, not three.

The last paragraph says sk_forward_alloc goes negative if
sk_forced_mem_schedule() is removed without the series. Does this test
catch that case?

The test only passes or fails on three checks:

- TcpExtTCPRcvCollapsed equals 30
- TcpExtTCPRcvQDrop does not appear
- the final read() returns 279900:

   +0 read(4, ..., 300000) = 279900

None of these checks sk_forward_alloc. Could the message say that the
test does not cover this case?

The message also has a few typos: sandwitched (sandwiched), hihg-density
(high-density), and sk_forwad_alloc (sk_forward_alloc).

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261114040101.1043546-1-kuniyu%40google.com

Re: [PATCH v1 net-next 0/8] tcp: Prevent tcp_collapse() from bloating queue.

[Eric Dumazet]

Le sam. 14 nov. 2026 à 05:01, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
>
> This is a follow-up for commit 031c403d13e5 ("tcp: do not let
> tcp_collapse() drive sk_forward_alloc negative").
>
> The core algorithm in tcp_collapse() is so simple and has not
> been changed since commit 1da177e4c3f4 ("Linux-2.6.12-rc2");
> finding a collapsible skb and repacking it into up-to-SKB_MAX_ORDER()
> skb.
>
> Now that we use 4K MSS and tp->scaling_ratio, the simple algorithm
> does not work well, and it could even end up consuming more memory
> contrary to the intention.
>
> For example, the scaling ratio of SKB_MAX_ORDER() is 222, but
> typically tp->scaling_ratio for 4K MSS flow is 252, and repacking
> data rather bloats high-density skb.
>
> Also, once a low-density skb starts collapsing, it drags all the
> following skbs into collapsing regardless of the density.
>
> This series changes the algorithm so that it skips high-density
> skb and a single low-density skb between high-density skbs, batches
> memory accounting at the end, and removes sk_forced_mem_schedule().
>

Thanks for working on this.

I went through the series and ran it on a KASAN + DEBUG_NET kernel:
your packetdrill test, a packetdrill test collapsing the out-of-order
queue, and a loopback stress test (1-byte writes, small SO_RCVBUF,
payload checked by the reader). No splat, no corrupted payload.

I have comments on patches 1, 3, 4, 5 and 8.

One general remark: the series keeps linear skbs of
SKB_MAX_ORDER(0, 0) bytes as the collapse target, and uses their
density (86 %) as the definition of a dense skb.

I have been working on a patch storing the collapsed payload in
order-0 page frags instead: alloc_skb(0), then up to MAX_SKB_FRAGS
pages added while copying. A full skb is then about 98 % dense, like
a 4K MSS GRO packet, and:

- There are fewer skbs in the receive queue and in the out-of-order
  rbtree (one skb per 68 KB instead of one per 3.7 KB).

- Payload sits in full page-aligned frags, usable by
  TCP_ZEROCOPY_RECEIVE.

This does not replace your series: the selection logic (skip dense
skbs, stop at the first non bloated skb, skip ranges that would not
shrink) is still needed. But TCP_COLLAPSE_MAX_SIZE,
TCP_COLLAPSE_SCALING_RATIO, the estimation added in patch 5 and the
pool added in patch 6 are all built around the linear format, so I
prefer to mention it now.

Re: [PATCH v1 net-next 0/8] tcp: Prevent tcp_collapse() from bloating queue.

[Kuniyuki Iwashima]

On Sat, Nov 14, 2026 at 7:31 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> Le sam. 14 nov. 2026 à 05:01, Kuniyuki Iwashima <kuniyu@google.com> a écrit :
> >
> > This is a follow-up for commit 031c403d13e5 ("tcp: do not let
> > tcp_collapse() drive sk_forward_alloc negative").
> >
> > The core algorithm in tcp_collapse() is so simple and has not
> > been changed since commit 1da177e4c3f4 ("Linux-2.6.12-rc2");
> > finding a collapsible skb and repacking it into up-to-SKB_MAX_ORDER()
> > skb.
> >
> > Now that we use 4K MSS and tp->scaling_ratio, the simple algorithm
> > does not work well, and it could even end up consuming more memory
> > contrary to the intention.
> >
> > For example, the scaling ratio of SKB_MAX_ORDER() is 222, but
> > typically tp->scaling_ratio for 4K MSS flow is 252, and repacking
> > data rather bloats high-density skb.
> >
> > Also, once a low-density skb starts collapsing, it drags all the
> > following skbs into collapsing regardless of the density.
> >
> > This series changes the algorithm so that it skips high-density
> > skb and a single low-density skb between high-density skbs, batches
> > memory accounting at the end, and removes sk_forced_mem_schedule().
> >
>
> Thanks for working on this.
>
> I went through the series and ran it on a KASAN + DEBUG_NET kernel:
> your packetdrill test, a packetdrill test collapsing the out-of-order
> queue, and a loopback stress test (1-byte writes, small SO_RCVBUF,
> payload checked by the reader). No splat, no corrupted payload.
>
> I have comments on patches 1, 3, 4, 5 and 8.
>
> One general remark: the series keeps linear skbs of
> SKB_MAX_ORDER(0, 0) bytes as the collapse target, and uses their
> density (86 %) as the definition of a dense skb.
>
> I have been working on a patch storing the collapsed payload in
> order-0 page frags instead: alloc_skb(0), then up to MAX_SKB_FRAGS
> pages added while copying. A full skb is then about 98 % dense, like
> a 4K MSS GRO packet, and:
>
> - There are fewer skbs in the receive queue and in the out-of-order
>   rbtree (one skb per 68 KB instead of one per 3.7 KB).
>
> - Payload sits in full page-aligned frags, usable by
>   TCP_ZEROCOPY_RECEIVE.

Wow, this sounds great !


>
> This does not replace your series: the selection logic (skip dense
> skbs, stop at the first non bloated skb, skip ranges that would not
> shrink) is still needed. But TCP_COLLAPSE_MAX_SIZE,
> TCP_COLLAPSE_SCALING_RATIO, the estimation added in patch 5 and the
> pool added in patch 6 are all built around the linear format, so I
> prefer to mention it now.

I actually avoided touching the last loop to avoid complication,
and your series will complete the revamp :)