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