Re: [PATCH v2 4.14 0/5] netfilter: xt_connlimit: backport upstream fixes for race in connection counting

[Sasha Levin <sashal@kernel.org>]

On Wed, Jan 02, 2019 at 06:41:59PM -0200, Mauricio Faria de Oliveira wrote:
>From: Ubuntu <ubuntu@petilil.segmaas.1ss>
>
>[changelog]
> - v2: include patch 5/5 (a very recent fix to patch 4/5) which is
>       not yet in Linus's tree but it's in nf.git + linux-next.git,
>       thus should make it shortly.  Test results still consistent.
>       Thanks Florian Westphal for reviewing and pointing that out.
>
>Recently, Alakesh Haloi reported the following issue [1] with stable/4.14:
>
>  """
>  An iptable rule like the following on a multicore systems will result in
>  accepting more connections than set in the rule.
>
>  iptables  -A INPUT -p tcp -m tcp --syn --dport 7777 -m connlimit \
>        --connlimit-above 2000 --connlimit-mask 0 -j DROP
>  """
>
>And proposed a fix that is not in Linus's tree. The discussion went on to
>confirm whether the issue was still reproducible with mainline/nf.git tip,
>and to either identify the upstream fix or re-submit the non-upstream fix.
>
>Alakesh eventually was able to test with upstream, and reported that issue
>was still reproducible [2].
>On that, our findinds diverge, at least in my test environment:
>
>First, I verified that the suggested mainline fix for the issue [3] indeed
>fixes it, by testing with it applied and reverted on v4.18, a clean revert.
>(The issue is reproducible with the commit reverted).
>
>Then, with a consistent reproducer, I moved to nf.git, with HEAD on commit
>a007232 ("netfilter: nf_conncount: fix argument order to find_next_bit"),
>and the issues was not reproducible (even with 20+ threads on client side,
>the number Alakesh reported to achieve 2150+ connections [4], and I tried
>spreading the network interface IRQ affinity over more and more CPUs too.)
>
>Either way, the suggested mainline fix does actually fix the issue in 4.14
>for at least one environment. So, it might well be the case that Alakesh's
>test environment has differences/subtleties that leads to more connections
>accepted, and more commits are needed for that particular environment type.
>
>(v2 update: see Florian's reply to v1 thread [1]; these different results
> are probably explained by very recent fixes still missing back then.)
>
>But for now, with one bare-metal environment (24-core server, 4-core client)
>verified, I thought of submitting the patches for review/comments/testing,
>then looking for additional fixes for that environment separately.
>
>The fix is PATCH 4 (needs fix in PATCH 5), and PATCHes 1-3 are helpers for
>a cleaner backport.
>All backports are simple, and essentially consist of refresh context lines
>and use older struct/file names.
>
>Reviews from netfilter maintainers are very appreciated, as I've no previous
>experience in this area, and although the backports look simple and build/run
>correctly, there's usually stuff that only more experienced people may notice.

Queued for 4.14, thank you.