openstack/neutron
Code Issues Proposed changes

Correct arguments to nfct_open

Browse Source 
While working on this module, I noticed a couple of inconsistencies
in how we were calling nfct. Specifically, the NFNL_SUBSYS_CTNETLINK
value is supposed to be 1[1], and the order of arguments to nfct_open
is subsys_id then subscriptions[2]. We were passing them in the
opposite order, which didn't particularly matter because both were
defined to be 0. Now that the subsystem identifier is correctly
defined it does matter though.

Change-Id: I9fb74a9ef7a83cd630afa1e1ea0e2fc0c6df3943
1: https://git.netfilter.org/libnfnetlink/tree/include/libnfnetlink/linux_nfnetlink.h#n45
2: https://git.netfilter.org/libnetfilter_conntrack/tree/src/main.c#n68
This commit is contained in:
Ben Nemec 2019-01-09 00:03:26 +00:00 committed by Slawek Kaplonski
parent 53bfd949bd
commit 85e1b1b0dc
3 changed files with 29 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
neutron/privileged/agent/linux/netlink_constants.py
View File

@ -76,7 +76,7 @@ NFCT_T_ALL = NFCT_T_NEW | NFCT_T_UPDATE | NFCT_T_DESTROY
NFCT_CB_CONTINUE = 1
NFCT_CB_FAILURE = -1
NFNL_SUBSYS_CTNETLINK = 0
NFNL_SUBSYS_CTNETLINK = 1
BUFFER = 1024
# IPv6 address memory buffer

4
neutron/privileged/agent/linux/netlink_lib.py
View File

@ -231,8 +231,8 @@ class ConntrackManager(object):
def __enter__(self):
self.conntrack_handler = nfct.nfct_open(
nl_constants.CONNTRACK,
nl_constants.NFNL_SUBSYS_CTNETLINK)
nl_constants.NFNL_SUBSYS_CTNETLINK,
nl_constants.CONNTRACK)
if not self.conntrack_handler:
msg = _("Failed to open new conntrack handler")
LOG.critical(msg)

52
neutron/tests/unit/privileged/agent/linux/test_netlink_lib.py
View File

@ -50,15 +50,15 @@ class NetlinkLibTestCase(base.BaseTestCase):
def test_open_new_conntrack_handler_pass(self):
with nl_lib.ConntrackManager():
nl_lib.nfct.nfct_open.assert_called_once_with(
nl_constants.CONNTRACK, nl_constants.NFNL_SUBSYS_CTNETLINK)
nl_constants.NFNL_SUBSYS_CTNETLINK, nl_constants.CONNTRACK)
nl_lib.nfct.nfct_close.assert_called_once_with(nl_lib.nfct.nfct_open(
nl_constants.CONNTRACK, nl_constants.NFNL_SUBSYS_CTNETLINK))
nl_constants.NFNL_SUBSYS_CTNETLINK, nl_constants.CONNTRACK))
def test_conntrack_list_entries(self):
with nl_lib.ConntrackManager() as conntrack:
nl_lib.nfct.nfct_open.assert_called_once_with(
nl_constants.CONNTRACK, nl_constants.NFNL_SUBSYS_CTNETLINK)
nl_constants.NFNL_SUBSYS_CTNETLINK, nl_constants.CONNTRACK)
conntrack.list_entries()
@ -67,33 +67,33 @@ class NetlinkLibTestCase(base.BaseTestCase):
mock.ANY, None)])
nl_lib.nfct.nfct_query.assert_called_once_with(
nl_lib.nfct.nfct_open(
nl_constants.CONNTRACK,
nl_constants.NFNL_SUBSYS_CTNETLINK),
nl_constants.NFNL_SUBSYS_CTNETLINK,
nl_constants.CONNTRACK),
nl_constants.NFCT_Q_DUMP,
mock.ANY)
nl_lib.nfct.nfct_close.assert_called_once_with(nl_lib.nfct.nfct_open(
nl_constants.CONNTRACK, nl_constants.NFNL_SUBSYS_CTNETLINK))
nl_constants.NFNL_SUBSYS_CTNETLINK, nl_constants.CONNTRACK))
def test_conntrack_new_failed(self):
nl_lib.nfct.nfct_new.return_value = None
with nl_lib.ConntrackManager() as conntrack:
nl_lib.nfct.nfct_open.assert_called_once_with(
nl_constants.CONNTRACK,
nl_constants.NFNL_SUBSYS_CTNETLINK)
nl_constants.NFNL_SUBSYS_CTNETLINK,
nl_constants.CONNTRACK)
conntrack.delete_entries([FAKE_ICMP_ENTRY])
nl_lib.nfct.nfct_new.assert_called_once_with()
nl_lib.nfct.nfct_destroy.assert_called_once_with(None)
nl_lib.nfct.nfct_close.assert_called_once_with(nl_lib.nfct.nfct_open(
nl_constants.CONNTRACK,
nl_constants.NFNL_SUBSYS_CTNETLINK))
nl_constants.NFNL_SUBSYS_CTNETLINK,
nl_constants.CONNTRACK))
def test_conntrack_delete_icmp_entry(self):
conntrack_filter = mock.Mock()
nl_lib.nfct.nfct_new.return_value = conntrack_filter
with nl_lib.ConntrackManager() as conntrack:
nl_lib.nfct.nfct_open.assert_called_once_with(
nl_constants.CONNTRACK,
nl_constants.NFNL_SUBSYS_CTNETLINK)
nl_constants.NFNL_SUBSYS_CTNETLINK,
nl_constants.CONNTRACK)
conntrack.delete_entries([FAKE_ICMP_ENTRY])
calls = [
mock.call(conntrack_filter,
@ -136,16 +136,16 @@ class NetlinkLibTestCase(base.BaseTestCase):
nl_lib.nfct.nfct_set_attr.assert_has_calls(calls, any_order=True)
nl_lib.nfct.nfct_destroy.assert_called_once_with(conntrack_filter)
nl_lib.nfct.nfct_close.assert_called_once_with(nl_lib.nfct.nfct_open(
nl_constants.CONNTRACK,
nl_constants.NFNL_SUBSYS_CTNETLINK))
nl_constants.NFNL_SUBSYS_CTNETLINK,
nl_constants.CONNTRACK))
def test_conntrack_delete_udp_entry(self):
conntrack_filter = mock.Mock()
nl_lib.nfct.nfct_new.return_value = conntrack_filter
with nl_lib.ConntrackManager() as conntrack:
nl_lib.nfct.nfct_open.assert_called_once_with(
nl_constants.CONNTRACK,
nl_constants.NFNL_SUBSYS_CTNETLINK)
nl_constants.NFNL_SUBSYS_CTNETLINK,
nl_constants.CONNTRACK)
conntrack.delete_entries([FAKE_UDP_ENTRY])
calls = [
mock.call(conntrack_filter,
@ -185,16 +185,16 @@ class NetlinkLibTestCase(base.BaseTestCase):
nl_lib.nfct.nfct_set_attr.assert_has_calls(calls, any_order=True)
nl_lib.nfct.nfct_destroy.assert_called_once_with(conntrack_filter)
nl_lib.nfct.nfct_close.assert_called_once_with(nl_lib.nfct.nfct_open(
nl_constants.CONNTRACK,
nl_constants.NFNL_SUBSYS_CTNETLINK))
nl_constants.NFNL_SUBSYS_CTNETLINK,
nl_constants.CONNTRACK))
def test_conntrack_delete_tcp_entry(self):
conntrack_filter = mock.Mock()
nl_lib.nfct.nfct_new.return_value = conntrack_filter
with nl_lib.ConntrackManager() as conntrack:
nl_lib.nfct.nfct_open.assert_called_once_with(
nl_constants.CONNTRACK,
nl_constants.NFNL_SUBSYS_CTNETLINK)
nl_constants.NFNL_SUBSYS_CTNETLINK,
nl_constants.CONNTRACK)
conntrack.delete_entries([FAKE_TCP_ENTRY])
calls = [
mock.call(conntrack_filter,
@ -235,16 +235,16 @@ class NetlinkLibTestCase(base.BaseTestCase):
nl_lib.nfct.nfct_set_attr.assert_has_calls(calls, any_order=True)
nl_lib.nfct.nfct_destroy.assert_called_once_with(conntrack_filter)
nl_lib.nfct.nfct_close.assert_called_once_with(nl_lib.nfct.nfct_open(
nl_constants.CONNTRACK,
nl_constants.NFNL_SUBSYS_CTNETLINK))
nl_constants.NFNL_SUBSYS_CTNETLINK,
nl_constants.CONNTRACK))
def test_conntrack_delete_entries(self):
conntrack_filter = mock.Mock()
nl_lib.nfct.nfct_new.return_value = conntrack_filter
with nl_lib.ConntrackManager() as conntrack:
nl_lib.nfct.nfct_open.assert_called_once_with(
nl_constants.CONNTRACK,
nl_constants.NFNL_SUBSYS_CTNETLINK)
nl_constants.NFNL_SUBSYS_CTNETLINK,
nl_constants.CONNTRACK)
conntrack.delete_entries([FAKE_ICMP_ENTRY,
FAKE_TCP_ENTRY,
FAKE_UDP_ENTRY])
@ -338,5 +338,5 @@ class NetlinkLibTestCase(base.BaseTestCase):
nl_lib.nfct.nfct_set_attr.assert_has_calls(calls, any_order=True)
nl_lib.nfct.nfct_destroy.assert_called_once_with(conntrack_filter)
nl_lib.nfct.nfct_close.assert_called_once_with(nl_lib.nfct.nfct_open(
nl_constants.CONNTRACK,
nl_constants.NFNL_SUBSYS_CTNETLINK))
nl_constants.NFNL_SUBSYS_CTNETLINK,
nl_constants.CONNTRACK))