Re: Multi-thread udp 4.7 regression, bisected to 71d8c47fc653

[Pablo Neira Ayuso <pablo@netfilter.org>]

[-- Attachment #1: Type: text/plain, Size: 1834 bytes --]

On Sun, Jul 10, 2016 at 04:48:26PM -0300, Marc Dionne wrote:
> An update here since I've had some interactions with Pablo off list.
> 
> Further testing shows that the underlying cause of the different test
> results is a udp packet that has a bogus source port number.  In the
> test the server process tries to send an ack to the bogus port and the
> flow is disrupted.
> 
> Notes:
> - The packet with the bad source port is from a sendmsg() call that
> has hit the connection tracker clash code introduced by 71d8c47fc653
> - Packets are successfully sent after the bad one, from the same
> socket, with the correct source port number
> - The problem does not reproduce with 71d8c47fc653 reverted, or
> without nf_conntrack loaded
> - The bogus port numbers start at 1024, bumping up by 1 every few
> times the problem occurs (1025, 1026, etc.)
> - The patch above does not change the behaviour
> - Enabling lockdep does not show anything
> 
> Our workaround for the original race was to retry sendmsg() once on
> EPERM errors, and that had been effective.
> I can trigger the insertion clash easily with some simple test code,
> but I have not been able so far to reproduce the packets with bad
> source port numbers with some simpler code that I could share.

The NAT nul-binding is triggering the source port mangling, even if
there is no NAT rules in place. The following patch skips the clash
resolution for NAT by now since we don't see a simple solution for
this case at the moment.

Could you give a try to this patch in these two cases?

1) No NAT in place: Make sure iptable_nat module is not there. Or if
   you're using nf_tables, just make sure you have no nat chains at
   all.

2) With NAT in place, you hit back the EPERM errors that you've
   observed so far.

Please, test both scenarios and report back. Thanks.

[-- Attachment #2: 0001-netfilter-conntrack-skip-clash-resolution-if-nat-is-.patch --]
[-- Type: text/x-diff, Size: 1559 bytes --]

>From c3b9dfbcf35ea38a3dce22daf7450fc23c620aea Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Mon, 11 Jul 2016 17:28:54 +0200
Subject: [PATCH] netfilter: conntrack: skip clash resolution if nat is in
 place

The clash resolution is not easy to apply if the NAT table is
registered. Even if no NAT rules are installed, the nul-binding ensures
that a unique tuple is used, thus, the packet that loses race gets a
different source port number, as described by:

http://marc.info/?l=netfilter-devel&m=146818011604484&w=2

Clash resolution with NAT is also problematic if addresses/port range
ports are used since the conntrack that wins race may describe a
different mangling that we may have earlier applied to the packet via
nf_nat_setup_info().

Fixes: 71d8c47fc653 ("netfilter: conntrack: introduce clash resolution on insertion race")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_conntrack_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index e0e9c9a..2b0f80a 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -657,6 +657,7 @@ static int nf_ct_resolve_clash(struct net *net, struct sk_buff *skb,
 
 	l4proto = __nf_ct_l4proto_find(nf_ct_l3num(ct), nf_ct_protonum(ct));
 	if (l4proto->allow_clash &&
+	    !nfct_nat(ct) &&
 	    !nf_ct_is_dying(ct) &&
 	    atomic_inc_not_zero(&ct->ct_general.use)) {
 		nf_ct_acct_merge(ct, ctinfo, (struct nf_conn *)skb->nfct);
-- 
2.1.4