Re: [PATCH net-next] tcp: tweak len/truesize ratio for coalesce candidates

[Soheil Hassas Yeganeh <soheil@google.com>]

On Wed, Jul 21, 2021 at 6:15 AM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> From: Eric Dumazet <edumazet@google.com>
>
> tcp_grow_window() is using skb->len/skb->truesize to increase tp->rcv_ssthresh
> which has a direct impact on advertized window sizes.
>
> We added TCP coalescing in linux-3.4 & linux-3.5:
>
> Instead of storing skbs with one or two MSS in receive queue (or OFO queue),
> we try to append segments together to reduce memory overhead.
>
> High performance network drivers tend to cook skb with 3 parts :
>
> 1) sk_buff structure (256 bytes)
> 2) skb->head contains room to copy headers as needed, and skb_shared_info
> 3) page fragment(s) containing the ~1514 bytes frame (or more depending on MTU)
>
> Once coalesced into a previous skb, 1) and 2) are freed.
>
> We can therefore tweak the way we compute len/truesize ratio knowing
> that skb->truesize is inflated by 1) and 2) soon to be freed.
>
> This is done only for in-order skb, or skb coalesced into OFO queue.
>
> The result is that low rate flows no longer pay the memory price of having
> low GRO aggregation factor. Same result for drivers not using GRO.
>
> This is critical to allow a big enough receiver window,
> typically tcp_rmem[2] / 2.
>
> We have been using this at Google for about 5 years, it is due time
> to make it upstream.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Soheil Hassas Yeganeh <soheil@google.com>
> Cc: Neal Cardwell <ncardwell@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>

Acked-by: Soheil Hassas Yeganeh <soheil@google.com>

Thank you, Eric!

> ---
>  net/ipv4/tcp_input.c | 38 ++++++++++++++++++++++++++++++--------
>  1 file changed, 30 insertions(+), 8 deletions(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index bef2c8b64d83a0f3d4cca90f9b12912bf3d00807..501d8d4d4ba46f9a5de322ab690c320757e0990c 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -454,11 +454,12 @@ static void tcp_sndbuf_expand(struct sock *sk)
>   */
>
>  /* Slow part of check#2. */
> -static int __tcp_grow_window(const struct sock *sk, const struct sk_buff *skb)
> +static int __tcp_grow_window(const struct sock *sk, const struct sk_buff *skb,
> +                            unsigned int skbtruesize)
>  {
>         struct tcp_sock *tp = tcp_sk(sk);
>         /* Optimize this! */
> -       int truesize = tcp_win_from_space(sk, skb->truesize) >> 1;
> +       int truesize = tcp_win_from_space(sk, skbtruesize) >> 1;
>         int window = tcp_win_from_space(sk, sock_net(sk)->ipv4.sysctl_tcp_rmem[2]) >> 1;
>
>         while (tp->rcv_ssthresh <= window) {
> @@ -471,7 +472,27 @@ static int __tcp_grow_window(const struct sock *sk, const struct sk_buff *skb)
>         return 0;
>  }
>
> -static void tcp_grow_window(struct sock *sk, const struct sk_buff *skb)
> +/* Even if skb appears to have a bad len/truesize ratio, TCP coalescing
> + * can play nice with us, as sk_buff and skb->head might be either
> + * freed or shared with up to MAX_SKB_FRAGS segments.
> + * Only give a boost to drivers using page frag(s) to hold the frame(s),
> + * and if no payload was pulled in skb->head before reaching us.
> + */
> +static u32 truesize_adjust(bool adjust, const struct sk_buff *skb)
> +{
> +       u32 truesize = skb->truesize;
> +
> +       if (adjust && !skb_headlen(skb)) {
> +               truesize -= SKB_TRUESIZE(skb_end_offset(skb));
> +               /* paranoid check, some drivers might be buggy */
> +               if (unlikely((int)truesize < (int)skb->len))
> +                       truesize = skb->truesize;
> +       }
> +       return truesize;
> +}
> +
> +static void tcp_grow_window(struct sock *sk, const struct sk_buff *skb,
> +                           bool adjust)
>  {
>         struct tcp_sock *tp = tcp_sk(sk);
>         int room;
> @@ -480,15 +501,16 @@ static void tcp_grow_window(struct sock *sk, const struct sk_buff *skb)
>
>         /* Check #1 */
>         if (room > 0 && !tcp_under_memory_pressure(sk)) {
> +               unsigned int truesize = truesize_adjust(adjust, skb);
>                 int incr;
>
>                 /* Check #2. Increase window, if skb with such overhead
>                  * will fit to rcvbuf in future.
>                  */
> -               if (tcp_win_from_space(sk, skb->truesize) <= skb->len)
> +               if (tcp_win_from_space(sk, truesize) <= skb->len)
>                         incr = 2 * tp->advmss;
>                 else
> -                       incr = __tcp_grow_window(sk, skb);
> +                       incr = __tcp_grow_window(sk, skb, truesize);
>
>                 if (incr) {
>                         incr = max_t(int, incr, 2 * skb->len);
> @@ -782,7 +804,7 @@ static void tcp_event_data_recv(struct sock *sk, struct sk_buff *skb)
>         tcp_ecn_check_ce(sk, skb);
>
>         if (skb->len >= 128)
> -               tcp_grow_window(sk, skb);
> +               tcp_grow_window(sk, skb, true);
>  }
>
>  /* Called to compute a smoothed rtt estimate. The data fed to this
> @@ -4769,7 +4791,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
>                  * and trigger fast retransmit.
>                  */
>                 if (tcp_is_sack(tp))
> -                       tcp_grow_window(sk, skb);
> +                       tcp_grow_window(sk, skb, true);
>                 kfree_skb_partial(skb, fragstolen);
>                 skb = NULL;
>                 goto add_sack;
> @@ -4857,7 +4879,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
>                  * and trigger fast retransmit.
>                  */
>                 if (tcp_is_sack(tp))
> -                       tcp_grow_window(sk, skb);
> +                       tcp_grow_window(sk, skb, false);
>                 skb_condense(skb);
>                 skb_set_owner_r(skb, sk);
>         }
> --
> 2.32.0.402.g57bb445576-goog
>