openstack
/
neutron
Code Issues Proposed changes

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
This commit is contained in:
Brian Haley 2014-11-24 21:33:20 -05:00
parent dbb72fa3fb
commit 4d774ef13c
4 changed files with 46 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
neutron/agent/l3/dvr.py
View File

@ -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'))

14
neutron/agent/linux/ip_lib.py
View File

@ -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

6
neutron/tests/unit/test_l3_agent.py
View File

@ -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!')

35
neutron/tests/unit/test_linux_ip_lib.py
View File

@ -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):