From 4d774ef13cc4292f4fd95082ec1ceb0db24b39c7 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Mon, 24 Nov 2014 21:33:20 -0500 Subject: [PATCH] Add address family to 'ip rule' calls Without an address family, 'ip rule' won't work with IPv6 arguments because it assumes IPv4. This causes the l3-agent to throw an error when adding a rule in DVR mode. Also changed these functions to be more symmetrical and take the same arguments, which required a little tweaking, but it looks much cleaner now. Change-Id: I85718d8d6ffcf3dec2a6b92641a731af813114aa Closes-bug: #1376325 --- neutron/agent/l3/dvr.py | 16 ++++++----- neutron/agent/linux/ip_lib.py | 14 +++++----- neutron/tests/unit/test_l3_agent.py | 6 ++--- neutron/tests/unit/test_linux_ip_lib.py | 35 ++++++++++++++++++------- 4 files changed, 46 insertions(+), 25 deletions(-) diff --git a/neutron/agent/l3/dvr.py b/neutron/agent/l3/dvr.py index e64a40079..ab35d9920 100644 --- a/neutron/agent/l3/dvr.py +++ b/neutron/agent/l3/dvr.py @@ -358,7 +358,7 @@ class AgentMixin(object): ri.floating_ips_dict[floating_ip] = rule_pr fip_2_rtr_name = self.get_fip_int_device_name(ri.router_id) ip_rule = ip_lib.IpRule(self.root_helper, namespace=ri.ns_name) - ip_rule.add_rule_from(fixed_ip, FIP_RT_TBL, rule_pr) + ip_rule.add(fixed_ip, FIP_RT_TBL, rule_pr) #Add routing rule in fip namespace fip_ns_name = self.get_fip_ns_name(str(fip['floating_network_id'])) rtr_2_fip, _ = ri.rtr_fip_subnet.get_pair() @@ -384,10 +384,10 @@ class AgentMixin(object): ri.rtr_fip_subnet = self.local_subnets.allocate(ri.router_id) rtr_2_fip, fip_2_rtr = ri.rtr_fip_subnet.get_pair() fip_ns_name = self.get_fip_ns_name(str(self._fetch_external_net_id())) - ip_rule_rtr = ip_lib.IpRule(self.root_helper, namespace=ri.ns_name) if floating_ip in ri.floating_ips_dict: rule_pr = ri.floating_ips_dict[floating_ip] - ip_rule_rtr.delete_rule_priority(rule_pr) + ip_rule = ip_lib.IpRule(self.root_helper, namespace=ri.ns_name) + ip_rule.delete(floating_ip, FIP_RT_TBL, rule_pr) self.fip_priorities.add(rule_pr) #TODO(rajeev): Handle else case - exception/log? @@ -415,12 +415,13 @@ class AgentMixin(object): def _snat_redirect_add(self, ri, gateway, sn_port, sn_int): """Adds rules and routes for SNAT redirection.""" try: - snat_idx = self._get_snat_idx(sn_port['ip_cidr']) + ip_cidr = sn_port['ip_cidr'] + snat_idx = self._get_snat_idx(ip_cidr) ns_ipr = ip_lib.IpRule(self.root_helper, namespace=ri.ns_name) ns_ipd = ip_lib.IPDevice(sn_int, self.root_helper, namespace=ri.ns_name) ns_ipd.route.add_gateway(gateway, table=snat_idx) - ns_ipr.add_rule_from(sn_port['ip_cidr'], snat_idx, snat_idx) + ns_ipr.add(ip_cidr, snat_idx, snat_idx) ns_ipr.netns.execute(['sysctl', '-w', 'net.ipv4.conf.%s.' 'send_redirects=0' % sn_int]) except Exception: @@ -429,12 +430,13 @@ class AgentMixin(object): def _snat_redirect_remove(self, ri, sn_port, sn_int): """Removes rules and routes for SNAT redirection.""" try: - snat_idx = self._get_snat_idx(sn_port['ip_cidr']) + ip_cidr = sn_port['ip_cidr'] + snat_idx = self._get_snat_idx(ip_cidr) ns_ipr = ip_lib.IpRule(self.root_helper, namespace=ri.ns_name) ns_ipd = ip_lib.IPDevice(sn_int, self.root_helper, namespace=ri.ns_name) ns_ipd.route.delete_gateway(table=snat_idx) - ns_ipr.delete_rule_priority(snat_idx) + ns_ipr.delete(ip_cidr, snat_idx, snat_idx) except Exception: LOG.exception(_LE('DVR: removed snat failed')) diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index dea22c466..1b0ef401a 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -206,14 +206,16 @@ class IPWrapper(SubProcessBase): class IpRule(IPWrapper): - def add_rule_from(self, ip, table, rule_pr): - args = ['add', 'from', ip, 'lookup', table, 'priority', rule_pr] - ip = self._as_root('', 'rule', tuple(args)) + def add(self, ip, table, rule_pr): + ip_version = netaddr.IPNetwork(ip).version + args = ['add', 'from', ip, 'table', table, 'priority', rule_pr] + ip = self._as_root([ip_version], 'rule', tuple(args)) return ip - def delete_rule_priority(self, rule_pr): - args = ['del', 'priority', rule_pr] - ip = self._as_root('', 'rule', tuple(args)) + def delete(self, ip, table, rule_pr): + ip_version = netaddr.IPNetwork(ip).version + args = ['del', 'table', table, 'priority', rule_pr] + ip = self._as_root([ip_version], 'rule', tuple(args)) return ip diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index 7a1a067e8..afd87a212 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -2010,8 +2010,7 @@ class TestBasicRouterOperations(base.BaseTestCase): ri.dist_fip_count = 0 ip_cidr = common_utils.ip_to_cidr(fip['floating_ip_address']) agent.floating_ip_added_dist(ri, fip, ip_cidr) - self.mock_rule.add_rule_from.assert_called_with('192.168.0.1', - 16, FIP_PRI) + self.mock_rule.add.assert_called_with('192.168.0.1', 16, FIP_PRI) # TODO(mrsmith): add more asserts @mock.patch.object(l3_agent.L3NATAgent, '_fip_ns_unsubscribe') @@ -2039,7 +2038,8 @@ class TestBasicRouterOperations(base.BaseTestCase): s = lla.LinkLocalAddressPair('169.254.30.42/31') ri.rtr_fip_subnet = s agent.floating_ip_removed_dist(ri, fip_cidr) - self.mock_rule.delete_rule_priority.assert_called_with(FIP_PRI) + floating_ip = fip_cidr.split('/')[0] + self.mock_rule.delete.assert_called_with(floating_ip, 16, FIP_PRI) self.mock_ip_dev.route.delete_route.assert_called_with(fip_cidr, str(s.ip)) self.assertFalse(unsubscribe.called, '_fip_ns_unsubscribe called!') diff --git a/neutron/tests/unit/test_linux_ip_lib.py b/neutron/tests/unit/test_linux_ip_lib.py index 954dd7666..840d3b773 100644 --- a/neutron/tests/unit/test_linux_ip_lib.py +++ b/neutron/tests/unit/test_linux_ip_lib.py @@ -16,6 +16,7 @@ import os import mock +import netaddr from neutron.agent.linux import ip_lib from neutron.common import exceptions @@ -449,21 +450,37 @@ class TestIpRule(base.BaseTestCase): self.execute_p = mock.patch.object(ip_lib.IpRule, '_execute') self.execute = self.execute_p.start() - def test_add_rule_from(self): - ip_lib.IpRule('sudo').add_rule_from('192.168.45.100', 2, 100) - self.execute.assert_called_once_with('', 'rule', - ('add', 'from', '192.168.45.100', - 'lookup', 2, 'priority', 100), + def _test_add_rule(self, ip, table, priority): + ip_version = netaddr.IPNetwork(ip).version + ip_lib.IpRule('sudo').add(ip, table, priority) + self.execute.assert_called_once_with([ip_version], 'rule', + ('add', 'from', ip, + 'table', table, + 'priority', priority), 'sudo', None, log_fail_as_error=True) - def test_delete_rule_priority(self): - ip_lib.IpRule('sudo').delete_rule_priority(100) - self.execute.assert_called_once_with('', 'rule', - ('del', 'priority', 100), + def _test_delete_rule(self, ip, table, priority): + ip_version = netaddr.IPNetwork(ip).version + ip_lib.IpRule('sudo').delete(ip, table, priority) + self.execute.assert_called_once_with([ip_version], 'rule', + ('del', 'table', table, + 'priority', priority), 'sudo', None, log_fail_as_error=True) + def test_add_rule_v4(self): + self._test_add_rule('192.168.45.100', 2, 100) + + def test_add_rule_v6(self): + self._test_add_rule('2001:db8::1', 3, 200) + + def test_delete_rule_v4(self): + self._test_delete_rule('192.168.45.100', 2, 100) + + def test_delete_rule_v6(self): + self._test_delete_rule('2001:db8::1', 3, 200) + class TestIPDevice(base.BaseTestCase): def test_eq_same_name(self): -- 2.45.2