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

diff --git a/neutron/agent/l3/dvr.py b/neutron/agent/l3/dvr.py
index e64a4007978870f562fa02bda2c19ea1cd3770b8..ab35d9920cde99d240d670f810e8f44184f93eff 100644 (file)
--- 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 dea22c466e95b1e6ea312f6ab5e312d32bba8f61..1b0ef401afd89e53664976a1ed831b274eef7048 100644 (file)
--- 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 7a1a067e8031e872336859378420e99cc75440be..afd87a2121a63cecee4c6ea7c7df1dc68728f745 100644 (file)
--- 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 954dd7666cec2a1f0d84ffe2d3cea255c6d47132..840d3b773eb2a0f0cd3a2aa58486b0198966774b 100644 (file)
--- 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):