Hello Eric,

        We have applied that commit e358f4af19db ("tcp: tcp_fragment() should 
apply sane memory limits")
        as a hotpatch in production environment. We found that it will make
        tcp long connection reset during sending out packet when applying
        that commit. 
        
        Our applications which in A/B test have suffered that
        and made them retransmit large data, and then caused retransmission
        storm and lower the performance and increase RT.

        Therefore we discontinued to apply this hotpatch in A/B test.

        After invesgation, we found this patch already fix this issue in
        stable. Before applying this patch, we have some questions:

        1. This commit in stable hard coded a magic number 0x20000. I am
        wondering this value and if there any better solution.
        2. Is there any known or unknown side effect? If any, we could test
        it in some suspicious scenarios before testing in prod env.

        Thanks.

Cheers,
Tony Lu

On Fri, Jun 21, 2019 at 06:09:55AM -0700, Eric Dumazet wrote:
> tcp_fragment() might be called for skbs in the write queue.
> 
> Memory limits might have been exceeded because tcp_sendmsg() only
> checks limits at full skb (64KB) boundaries.
> 
> Therefore, we need to make sure tcp_fragment() wont punish applications
> that might have setup very low SO_SNDBUF values.
> 
> Fixes: f070ef2ac667 ("tcp: tcp_fragment() should apply sane memory limits")
> Signed-off-by: Eric Dumazet <eduma...@google.com>
> Reported-by: Christoph Paasch <cpaa...@apple.com>
> ---
>  net/ipv4/tcp_output.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 
> 00c01a01b547ec67c971dc25a74c9258563cf871..0ebc33d1c9e5099d163a234930e213ee35e9fbd1
>  100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -1296,7 +1296,8 @@ int tcp_fragment(struct sock *sk, enum tcp_queue 
> tcp_queue,
>       if (nsize < 0)
>               nsize = 0;
>  
> -     if (unlikely((sk->sk_wmem_queued >> 1) > sk->sk_sndbuf)) {
> +     if (unlikely((sk->sk_wmem_queued >> 1) > sk->sk_sndbuf &&
> +                  tcp_queue != TCP_FRAG_IN_WRITE_QUEUE)) {
>               NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPWQUEUETOOBIG);
>               return -ENOMEM;
>       }
> -- 
> 2.22.0.410.gd8fdbe21b5-goog

Reply via email to