netdev - Re: [PATCH -next] net: tcp: add mib counters to track zero window transitions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140219193038.GC1179@order.stressinduktion.org>
Date:	Wed, 19 Feb 2014 20:30:38 +0100
From:	Hannes Frederic Sowa <hannes@...essinduktion.org>
To:	Florian Westphal <fw@...len.de>
Cc:	Eric Dumazet <eric.dumazet@...il.com>,
	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH -next] net: tcp: add mib counters to track zero window transitions

On Wed, Feb 19, 2014 at 08:18:31PM +0100, Florian Westphal wrote:
> Hannes Frederic Sowa <hannes@...essinduktion.org> wrote:
> > On Wed, Feb 19, 2014 at 10:49:47AM -0800, Eric Dumazet wrote:
> > > On Wed, 2014-02-19 at 13:17 -0500, David Miller wrote:
> > > > From: Florian Westphal <fw@...len.de>
> > > > Date: Mon, 17 Feb 2014 22:57:48 +0100
> > > > 
> > > > > Three counters are added:
> > > > > - one to track when we went from non-zero to zero window
> > > > > - one to track the reverse
> > > > > - one counter incremented when we want to announce zero window.
> > > > > 
> > > > > The latter is added because it can show cases where we want to close the
> > > > > window but can't because we would shrink window.
> > > > > 
> > > > > Suggested-by: Eric Dumazet <edumazet@...gle.com>
> > > > > Signed-off-by: Florian Westphal <fw@...len.de>
> > > > > ---
> > > > >  Eric, is this what you had in mind?
> > > > > 
> > > > >  I re-ran my 'slow-sender-with-reader-that-does-not-drain-socket'
> > > > >  scenario and, as expected, only TCPWANTZEROWINDOW increases.
> > > > 
> > > > Eric, ping?
> > > 
> > > Thanks, I missed this patch. Let me think about it, thanks !
> > 
> > We need NET_INC_STATS instead of NET_INC_STATS_BH, no?
> 
> sure, I can send v2 but I'll wait for (n)ack from Eric first
> before considering it.
> 
> tcp_transmit_skb (caller of tcp_select_window) also uses _BH
> which is why it ended up in tcp_select_window.

NET_STATS only use 32 bit counter and thus need not be protected with a
seqlock on 32 bit platforms. As such, it does not matter, but e.g. the IP
counter are prone to deadlocks if used with wrong postfix because of 64 bit
counter thus protected by seqlock.

A pitty that the _STATS_BH postfixes have the opposite meaning of the bottom
_bh postfixes.

Basically always safe is _STATS and if we are sure we can omit the bh disable
call because we only call the function from bh, we can use _STATS_BH calls.

Greetings,

  Hannnes

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html