From ab94c6b02116ccc24c67c1ed8e09c6840d092424 Mon Sep 17 00:00:00 2001 From: Dirk Mueller Date: Thu, 15 Nov 2018 17:19:35 +0100 Subject: [PATCH] Enable liberal TCP connection tracking for SNAT namespaces This can avoid connections rarely hanging due to tcp window scaling not correctly being observed by the TCP connection tracking. this seems to happen when retransmits are occurring occassionally. Setting this parameter turns off validating the window scaling checks for the purpose of matching whether a packet matches an existing connection tracked flow, which avoids the SNAT namespace from interfering and letting the connection peers recover the connection via retransmits/Selective ACKs instead of the SNAT terminating one side of the connection and letting it stall permanently. Closes-Bug: #1804327 Change-Id: I5e58bb2850bfa8e974e62215af0b4d7bc0592c13 --- neutron/agent/l3/namespaces.py | 29 ++++++++++--------- neutron/tests/unit/agent/l3/test_agent.py | 18 +++++------- .../tests/unit/agent/l3/test_dvr_fip_ns.py | 9 +++--- ...track_tcp_be_liberal-00432039c9e7ab9d.yaml | 18 ++++++++++++ 4 files changed, 47 insertions(+), 27 deletions(-) create mode 100644 releasenotes/notes/enable-nf_conntrack_tcp_be_liberal-00432039c9e7ab9d.yaml diff --git a/neutron/agent/l3/namespaces.py b/neutron/agent/l3/namespaces.py index a6a95e5ea3a..10d3922be6e 100644 --- a/neutron/agent/l3/namespaces.py +++ b/neutron/agent/l3/namespaces.py @@ -89,23 +89,26 @@ class Namespace(object): self.use_ipv6 = use_ipv6 def create(self, ipv6_forwarding=True): + self.ip_wrapper_root.ensure_namespace(self.name) # See networking (netdev) tree, file # Documentation/networking/ip-sysctl.txt for an explanation of # these sysctl values. - ip_wrapper = self.ip_wrapper_root.ensure_namespace(self.name) - cmd = ['sysctl', '-w', 'net.ipv4.ip_forward=1'] - ip_wrapper.netns.execute(cmd, privsep_exec=True) - # 1. Reply only if the target IP address is local address configured - # on the incoming interface; and - # 2. Always use the best local address - cmd = ['sysctl', '-w', 'net.ipv4.conf.all.arp_ignore=1'] - ip_wrapper.netns.execute(cmd, privsep_exec=True) - cmd = ['sysctl', '-w', 'net.ipv4.conf.all.arp_announce=2'] - ip_wrapper.netns.execute(cmd, privsep_exec=True) + # Here's what we are setting: + # 1) nf_conntrack_tcp_be_liberal=1 - Be liberal in the state tracking + # to avoid issues with TCP window scaling + # 2) ip_forward=1 - Turn on IP forwarding + # 3) arp_ignore=1 - Reply only if the target IP address is local + # address configured on the incoming interface + # 4) arp_announce=2 - Always use the best local address + # 5) forwarding=0/1 - Turn on/off IPv6 forwarding + cmd = ['net.netfilter.nf_conntrack_tcp_be_liberal=1', + 'net.ipv4.ip_forward=1', + 'net.ipv4.conf.all.arp_ignore=1', + 'net.ipv4.conf.all.arp_announce=2'] if self.use_ipv6: - cmd = ['sysctl', '-w', - 'net.ipv6.conf.all.forwarding=%d' % int(ipv6_forwarding)] - ip_wrapper.netns.execute(cmd, privsep_exec=True) + cmd.append( + 'net.ipv6.conf.all.forwarding=%d' % int(ipv6_forwarding)) + ip_lib.sysctl(cmd, namespace=self.name) def delete(self): try: diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index 0d51040a86e..1db3c5505ca 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -2700,18 +2700,16 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): 'qrouter-bar', self.conf, agent.driver, agent.use_ipv6) ns.create() - calls = [mock.call(['sysctl', '-w', 'net.ipv4.ip_forward=1'], - privsep_exec=True), - mock.call(['sysctl', '-w', 'net.ipv4.conf.all.arp_ignore=1'], - privsep_exec=True), - mock.call( - ['sysctl', '-w', 'net.ipv4.conf.all.arp_announce=2'], - privsep_exec=True)] + sysctl_settings = ['net.netfilter.nf_conntrack_tcp_be_liberal=1', + 'net.ipv4.ip_forward=1', + 'net.ipv4.conf.all.arp_ignore=1', + 'net.ipv4.conf.all.arp_announce=2'] if agent.use_ipv6: - calls.append(mock.call( - ['sysctl', '-w', 'net.ipv6.conf.all.forwarding=1'], - privsep_exec=True)) + sysctl_settings.append('net.ipv6.conf.all.forwarding=1') + calls = [mock.call( + ['sysctl', '-w'] + sysctl_settings, + run_as_root=True, log_fail_as_error=True, privsep_exec=True)] self.mock_ip.netns.execute.assert_has_calls(calls) def test_destroy_namespace(self): diff --git a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py index d33142cc840..c3a768a464b 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py +++ b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py @@ -216,9 +216,10 @@ class TestDvrFipNs(base.BaseTestCase): @mock.patch.object(ip_lib.IpNetnsCommand, 'exists') def _test_create(self, old_kernel, exists, execute, IPTables): exists.return_value = True - # There are up to six sysctl calls - two to enable forwarding, - # two for arp_ignore and arp_announce, and two for ip_nonlocal_bind - execute.side_effect = [None, None, None, None, + # There are up to 3 sysctl calls - one to enable forwarding, + # arp_ignore and arp_announce, one for ip_nonlocal_bind, and + # one for nf_conntrack_tcp_be_liberal. + execute.side_effect = [None, RuntimeError if old_kernel else None, None] self.fip_ns._iptables_manager = IPTables() @@ -239,7 +240,7 @@ class TestDvrFipNs(base.BaseTestCase): run_as_root=True, privsep_exec=True)) - execute.assert_has_calls(expected) + execute.assert_has_calls(expected, any_order=True) def test_create_old_kernel(self): self._test_create(True) diff --git a/releasenotes/notes/enable-nf_conntrack_tcp_be_liberal-00432039c9e7ab9d.yaml b/releasenotes/notes/enable-nf_conntrack_tcp_be_liberal-00432039c9e7ab9d.yaml new file mode 100644 index 00000000000..184257d8ce1 --- /dev/null +++ b/releasenotes/notes/enable-nf_conntrack_tcp_be_liberal-00432039c9e7ab9d.yaml @@ -0,0 +1,18 @@ +--- +fixes: + - | + Liberal TCP connection tracking is now enabled in SNAT namespaces, + (``sysctl net.netfilter.nf_conntrack_tcp_be_liberal=1``). + + In some cases, when a TCP connection that is NAT-ed ends up + re-transmitting, a packet could be outside what the Linux kernel + connection tracking considers part of the valid TCP window. When + this happens, a TCP Reset (RST) is triggered, terminating the connection + on the sender side, while leaving the receiver side (the Neutron + port attached VM) hanging. + + Since a number of firewall vendors typically turn this on by default + to avoid unnecessary resets, we now do it in the Neutron router as well. + + See bug `1804327 `_ + for more information.