linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Dumazet <edumazet@google.com>
To: Richard Cochran <richardcochran@gmail.com>
Cc: Jouni Malinen <jkmalinen@gmail.com>,
	rcochran@linutronix.de, Thomas Gleixner <tglx@linutronix.de>,
	Rik van Riel <riel@redhat.com>, Len Brown <lenb@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	George Spelvin <linux@sciencehorizons.net>,
	Josh Triplett <josh@joshtriplett.org>, Chris Mason <clm@fb.com>,
	rt@linutronix.de, Paul McKenney <paulmck@linux.vnet.ibm.com>,
	Ingo Molnar <mingo@kernel.org>,
	Arjan van de Ven <arjan@infradead.org>, j <j@w1.fi>
Subject: Re: [PREEMPT-RT] [patch 4 14/22] timer: Switch to a non cascading wheel
Date: Tue, 16 Aug 2016 10:35:22 -0400	[thread overview]
Message-ID: <CANn89iJ6YmfgYZ=d4k1XoomhY7y5z1TxxXS-fg=Nsnyiva0A9g@mail.gmail.com> (raw)
In-Reply-To: <20160816094600.GC23077@localhost.localdomain>

On Tue, Aug 16, 2016 at 5:46 AM, Richard Cochran
<richardcochran@gmail.com> wrote:
> Jouni,
>
> If I understand the test correctly, then the slightly different kernel
> timer behavior is ok, but the test isn't quite right.  Let explain
> what I mean.
>
> First off, reading test_ap_wps.py, the point of the test is to see if
> ten simultaneous connections are possible.  I guess the server
> implements a hard coded limit on the number of clients.  (BTW where is
> the server loop?)
>
> You said that the server also sets 'backlog' to ten.  The backlog
> controls the size of the queue holding incoming connections that are
> in the SYN_RCVD or ESTABLISHED state but have not yet been
> accept(2)-ed by the server.  This is *not* the same as the number of
> possible simultaneous connections.
>
> On Sat, Aug 13, 2016 at 12:12:26PM +0300, Jouni Malinen wrote:
>> Yes, it looks like a TCP connect() timeout. I use a significantly
>> reduced timeout in the test scripts since they are run unattended and
>> are supposed to terminate in reasonable amount of time.. That said,
>
> I did not find where the client sets the one second timeout.  Where
> does this happen?
>
>> If I increase that 20 to 50, I get more of such about 1.03 second
>> results at i=17, i=34, i=48..
>
> Can you provide the timings when the test runs on the older kernel?
>
>> Looking more at what exactly is happening at the TCP layer, this is
>> likely related to the server behavior since listen() backlog is set to
>> 10 and if there are 10 parallel connections, the last one if
>> immediately closed before reading anything.
>
> To clarify, when the backlog is exceed, the new connection is not
> closed.  Instead, the SYN is simply ignored, and the client is expect
> to re-transmit the SYN in the normal TCP fashion.
>
>> Looking at a sniffer capture (*), the three-way TCP connection goes
>> through fine for the first 15 connect() calls, but the 15th one does
>> not get a response to SYN. This SYN is the frame 47 in the capture
>> file with srcport == 60802. There is no SYN,ACK for it. The about one
>> second unexpected time for connect() comes from this, i.e., the
>> connection is completed only after the client side does TCP
>> retransmission of the SYN (frame #77) a second later and the server
>> side replies with RST,ACK (frame #78).
>
> This is the expected behavior.
>
>> So it looks like the issue is in one of the SYN,ACK frames getting
>> completely lost..
>
> No, the frame is not missing.  It was never sent because the backlog
> was exceeded.
>
> Here is what I suspect is happening.  By sending 20 SYN frames to a
> port with a backlog of 10, it saturates the queue.  One SYN is ignored
> by the kernel, and a race begins between the connect() timeout and the
> SYN re-transmission.  If the client's re-transmitted SYN and then the
> server's SYN,ACK returns before the connect timeout, then the call to
> connect() succeeds.  With the new timer wheel, the result of the race
> is different.
>
> There a couple of ways to deal with this.  One is to increase the
> backlog on the server side.  Another is to increase the connect()
> timeout to a multiple of the re-transmission interval.
>
> Thoughts?
>

I am coming late to the party, but yes, test looks flaky.

(Relying on having very precise SYN retransmits when listen backlog on
server side is full)

  reply	other threads:[~2016-08-16 14:35 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-04  9:50 [patch 4 00/22] timer: Refactor the timer wheel Thomas Gleixner
2016-07-04  9:50 ` [patch 4 01/22] timer: Make pinned a timer property Thomas Gleixner
2016-07-07  8:39   ` [tip:timers/core] timers: Make 'pinned' " tip-bot for Thomas Gleixner
2016-07-04  9:50 ` [patch 4 02/22] x86/apic/uv: Initialize timer as pinned Thomas Gleixner
2016-07-07  8:40   ` [tip:timers/core] timers, x86/apic/uv: Initialize the UV heartbeat " tip-bot for Thomas Gleixner
2016-07-04  9:50 ` [patch 4 03/22] x86/mce: Initialize " Thomas Gleixner
2016-07-07  8:40   ` [tip:timers/core] timers, x86/mce: Initialize MCE restart " tip-bot for Thomas Gleixner
2016-07-04  9:50 ` [patch 4 04/22] cpufreq/powernv: Initialize " Thomas Gleixner
2016-07-07  8:41   ` [tip:timers/core] timers, cpufreq/powernv: Initialize the gpstate " tip-bot for Thomas Gleixner
2016-07-04  9:50 ` [patch 4 05/22] driver/net/ethernet/tile: Initialize " Thomas Gleixner
2016-07-07  8:41   ` [tip:timers/core] timers, driver/net/ethernet/tile: Initialize the egress " tip-bot for Thomas Gleixner
2016-07-04  9:50 ` [patch 4 06/22] drivers/tty/metag_da: Initialize " Thomas Gleixner
2016-07-07  8:42   ` [tip:timers/core] timers, drivers/tty/metag_da: Initialize the poll " tip-bot for Thomas Gleixner
2016-07-04  9:50 ` [patch 4 07/22] drivers/tty/mips_ejtag: Initialize " Thomas Gleixner
2016-07-07  8:42   ` [tip:timers/core] timers, drivers/tty/mips_ejtag: Initialize the poll " tip-bot for Thomas Gleixner
2016-07-04  9:50 ` [patch 4 08/22] net/ipv4/inet: Initialize timers " Thomas Gleixner
2016-07-07  8:43   ` [tip:timers/core] timers, net/ipv4/inet: Initialize connection request " tip-bot for Thomas Gleixner
2016-07-04  9:50 ` [patch 4 09/22] timer: Remove mod_timer_pinned Thomas Gleixner
2016-07-07  8:43   ` [tip:timers/core] timers: Remove the deprecated mod_timer_pinned() API tip-bot for Thomas Gleixner
2016-07-04  9:50 ` [patch 4 10/22] signal: Use hrtimer for sigtimedwait Thomas Gleixner
2016-07-07  8:43   ` [tip:timers/core] signals: Use hrtimer for sigtimedwait() tip-bot for Thomas Gleixner
2016-07-04  9:50 ` [patch 4 11/22] hlist: Add hlist_is_singular_node() helper Thomas Gleixner
2016-07-07  8:44   ` [tip:timers/core] " tip-bot for Thomas Gleixner
2016-07-04  9:50 ` [patch 4 12/22] timer: Give a few structs and members proper names Thomas Gleixner
2016-07-07  8:44   ` [tip:timers/core] timers: " tip-bot for Thomas Gleixner
2016-07-04  9:50 ` [patch 4 13/22] timer: Reduce the CPU index space to 256k Thomas Gleixner
2016-07-07  8:45   ` [tip:timers/core] timers: " tip-bot for Thomas Gleixner
2016-07-04  9:50 ` [patch 4 14/22] timer: Switch to a non cascading wheel Thomas Gleixner
2016-07-07  8:45   ` [tip:timers/core] timers: Switch to a non-cascading wheel tip-bot for Thomas Gleixner
2016-08-11 15:21   ` [patch 4 14/22] timer: Switch to a non cascading wheel Jouni Malinen
2016-08-11 20:25     ` [PREEMPT-RT] " rcochran
2016-08-13  9:12       ` Jouni Malinen
2016-08-16  9:46         ` Richard Cochran
2016-08-16 14:35           ` Eric Dumazet [this message]
2016-08-17  9:05           ` Jouni Malinen
2016-08-17  9:23             ` rcochran
2016-08-12 17:50     ` Rik van Riel
2016-08-12 19:14       ` Paul E. McKenney
2016-08-16  8:55         ` Richard Cochran
2016-08-16  7:57       ` Richard Cochran
2016-07-04  9:50 ` [patch 4 15/22] timer: Remove slack leftovers Thomas Gleixner
2016-07-07  8:46   ` [tip:timers/core] timers: Remove set_timer_slack() leftovers tip-bot for Thomas Gleixner
2016-07-22 11:31   ` [patch 4 15/22] timer: Remove slack leftovers Jason A. Donenfeld
2016-07-22 13:04     ` Thomas Gleixner
2016-07-22 15:18       ` Jason A. Donenfeld
2016-07-22 22:54         ` Jason A. Donenfeld
2016-07-04  9:50 ` [patch 4 16/22] timer: Move __run_timers() function Thomas Gleixner
2016-07-07  8:46   ` [tip:timers/core] timers: " tip-bot for Anna-Maria Gleixner
2016-07-04  9:50 ` [patch 4 17/22] timer: Optimize collect timers for NOHZ Thomas Gleixner
2016-07-07  8:47   ` [tip:timers/core] timers: Optimize collect_expired_timers() " tip-bot for Anna-Maria Gleixner
2016-07-04  9:50 ` [patch 4 18/22] tick/sched: Remove pointless empty function Thomas Gleixner
2016-07-07  8:47   ` [tip:timers/core] timers/nohz: Remove pointless tick_nohz_kick_tick() function tip-bot for Thomas Gleixner
2016-07-04  9:50 ` [patch 4 19/22] timer: Forward wheel clock whenever possible Thomas Gleixner
2016-07-07  8:48   ` [tip:timers/core] timers: Forward the " tip-bot for Thomas Gleixner
2016-07-04  9:50 ` [patch 4 20/22] timer: Only wake softirq if necessary Thomas Gleixner
2016-07-07  8:48   ` [tip:timers/core] timers: " tip-bot for Thomas Gleixner
2016-07-04  9:50 ` [patch 4 21/22] timer: Split out index calculation Thomas Gleixner
2016-07-07  8:48   ` [tip:timers/core] timers: " tip-bot for Anna-Maria Gleixner
2016-07-04  9:50 ` [patch 4 22/22] timer: Optimization for same expiry time in mod_timer() Thomas Gleixner
2016-07-07  8:49   ` [tip:timers/core] timers: Implement optimization " tip-bot for Anna-Maria Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CANn89iJ6YmfgYZ=d4k1XoomhY7y5z1TxxXS-fg=Nsnyiva0A9g@mail.gmail.com' \
    --to=edumazet@google.com \
    --cc=arjan@infradead.org \
    --cc=clm@fb.com \
    --cc=fweisbec@gmail.com \
    --cc=j@w1.fi \
    --cc=jkmalinen@gmail.com \
    --cc=josh@joshtriplett.org \
    --cc=lenb@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@sciencehorizons.net \
    --cc=mingo@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=rcochran@linutronix.de \
    --cc=richardcochran@gmail.com \
    --cc=riel@redhat.com \
    --cc=rt@linutronix.de \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).