From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tonghao Zhang Subject: Re: Compare length of req sock queue with sk_max_ack_backlog Date: Fri, 2 Feb 2018 13:41:54 +0800 Message-ID: References: <1517493607.3715.117.camel@gmail.com> <1517545166.3715.124.camel@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: Eric Dumazet , Linux Kernel Network Developers To: Eric Dumazet Return-path: Received: from mail-ot0-f173.google.com ([74.125.82.173]:46856 "EHLO mail-ot0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750714AbeBBFl4 (ORCPT ); Fri, 2 Feb 2018 00:41:56 -0500 Received: by mail-ot0-f173.google.com with SMTP id f56so2811256otj.13 for ; Thu, 01 Feb 2018 21:41:55 -0800 (PST) In-Reply-To: <1517545166.3715.124.camel@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On Fri, Feb 2, 2018 at 12:19 PM, Eric Dumazet wrote: > On Fri, 2018-02-02 at 09:55 +0800, Tonghao Zhang wrote: >> On Thu, Feb 1, 2018 at 10:00 PM, Eric Dumazet wrote: >> > On Thu, 2018-02-01 at 20:34 +0800, Tonghao Zhang wrote: >> > > Hi Eric >> > > One question for you, In the patch ef547f2ac16 ("tcp: remove >> > > max_qlen_log"), why we compared the length of req sock queue with >> > > sk_max_ack_backlog. If we remove the max_qlen_log, we should check the >> > > length of req sock queue with tcp_max_syn_backlog, right ? >> > > >> > > With this patch, the option "/proc/sys/net/ipv4/tcp_max_syn_backlog" >> > > will be unsed anymore, right ? >> > >> > Not right. >> > >> > Please "git grep -n sysctl_max_syn_backlog" to convince it is still >> > used. >> > >> >> In the tcp_conn_request(), we call the inet_csk_reqsk_queue_is_full() >> to check the length of req sock queue. But >> inet_csk_reqsk_queue_is_full() >> compares it with sk_max_ack_backlog. >> >> inet_csk_reqsk_queue_is_full >> inet_csk_reqsk_queue_len(sk) >= sk->sk_max_ack_backlog; >> >> >> inet_csk_reqsk_queue_len >> reqsk_queue_len(&inet_csk(sk)->icsk_accept_queue); >> >> If we set sysctl_max_syn_backlog to 1024 via >> /proc/sys/net/ipv4/tcp_max_syn_backlog, and >> backlog(sk_max_ack_backlog) to 128 via listen() , tcp_conn_request >> will drop the packets when >> length of req sock queue > 128, but not 1024. >> >> tcp_conn_request >> if ((net->ipv4.sysctl_tcp_syncookies == 2 || >> inet_csk_reqsk_queue_is_full(sk)) && !isn) { >> want_cookie = tcp_syn_flood_action(sk, skb, rsk_ops->slab_name); >> if (!want_cookie) >> goto drop; // drop the packets >> } > > It seems to work as intended and documented in the changelog. > > A socket listen backlog is determined at the time listen() system call > is issued, using the standard somaxconn as safe guard, for all socket > types. > > We want to get rid of tcp_max_syn_backlog eventually, since TCP > listeners can use listen(fd, 10000000) these days if they want, > it is a matter of provisioning the needed memory. In some case, the user may use different value for tcp_max_syn_backlog and sk_max_ack_backlog. Then in the tcp_conn_request, we should compare the length of req sock queue with tcp_max_syn_backlog, but not sk_max_ack_backlog. We can update the tcp_max_syn_backlog for specific socket via ioctl, such like listen updating the ack backlog. inet_csk_reqsk_queue_is_full inet_csk_reqsk_queue_len(sk) >= sk->sk_max_ack_backlog; to: inet_csk_reqsk_queue_is_full inet_csk_reqsk_queue_len(sk) >= tcp_max_syn_backlog; // for example The sk_acceptq_is_full is ok in the sk_acceptq_is_full(). sk_acceptq_is_full sk->sk_ack_backlog > sk->sk_max_ack_backlog; > Old mechanism was not allowing a listener to reduce or increase its > backlog dynamically (listener was using a private hash table, sized at > the time of first listen() and not resized later) > > Having a global sysctl to magically change behavior on all TCP > listeners is not very practical, unless you had dedicated hosts to deal > with one service. I got it.