openstack
/
neutron
Code Issues Proposed changes

Use cidr during tenant network rule deletion 

If a distributed router has interfaces on multiple tenant networks, with
'fast exit' functionality policy based rules are created in qrouter
namespace for every tenant network subnet and 'from <cidr>' is included
into an 'ip rule' command invocation.

When a port on a tenant network is deleted 'from <cidr>' part is not
included and a first rule matching specified parameters gets deleted.

For example with the following layout

ip netns exec qrouter-4f9ca9ef-303b-4082-abbc-e50782d9b800 ip rule
0:      from all lookup local
32766:  from all lookup main
32767:  from all lookup default
80000:  from 192.168.100.0/24 lookup 16
80000:  from 192.168.200.0/24 lookup 16

and neutron l3 agent will use this command

ip netns exec qrouter-4f9ca9ef-303b-4082-abbc-e50782d9b800 ip -4 rule\
del priority 80000 table 16 type unicast

and 192.168.100.0/24 rule will get deleted even if you actually removed
a port on 192.168.200.0.

This results in an extra rule present and not cleaned up and the right
rule removed. It is only recreated if a router is disabled and enabled
again.

additional changes:

1) Floating IP rules are identified by priority only as implemented
currently - for this reason this change adds fixed_ip to the rule
removal code. Rule priorities are 32-bit values in iproute2 so,
in theory, those should be not be used to cover IPv6.

2) IP protocol information for 'from all' rules is currently
derived from link-local address IP version. The same approach
is preserved by using version-specific /0 addresses without
changing the API provided by ip_lib.

Change-Id: I0ea6dddd26e17771be223a1fbdf21792c90f3e9c
Closes-Bug: #1759956
(cherry picked from commit 81db328b2d)
This commit is contained in:
Dmitrii Shcherbakov 2018-03-29 17:32:01 -04:00 committed by Swaminathan Vasudevan
parent 48ae4b7ed1
commit 0224dcfea4
5 changed files with 27 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
neutron/agent/l3/dvr_fip_ns.py
View File

@ -393,9 +393,12 @@ class FipNamespace(namespaces.Namespace):
fg_device.route.flush(lib_constants.IP_VERSION_6,
table=tbl_index)
# Remove the rule lookup
# IP is ignored in delete, but we still require it
# for getting the ip_version.
fip_rt_rule.rule.delete(ip=fip_2_rtr.ip,
# /0 addresses for IPv4 and IPv6 are used to pass
# IP protocol version information based on a
# link-local address IP version. Using any of those
# is equivalent to using 'from all' for iproute2.
rule_ip = lib_constants.IP_ANY[fip_2_rtr.ip.version]
fip_rt_rule.rule.delete(ip=rule_ip,
iif=fip_2_rtr_name,
table=tbl_index,
priority=tbl_index)

6
neutron/agent/l3/dvr_local_router.py
View File

@ -142,7 +142,7 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
def _add_floating_ip_rule(self, floating_ip, fixed_ip):
rule_pr = self.fip_ns.allocate_rule_priority(floating_ip)
self.floating_ips_dict[floating_ip] = rule_pr
self.floating_ips_dict[floating_ip] = (fixed_ip, rule_pr)
ip_rule = ip_lib.IPRule(namespace=self.ns_name)
ip_rule.rule.add(ip=fixed_ip,
table=dvr_fip_ns.FIP_RT_TBL,
@ -150,9 +150,9 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
def _remove_floating_ip_rule(self, floating_ip):
if floating_ip in self.floating_ips_dict:
rule_pr = self.floating_ips_dict[floating_ip]
fixed_ip, rule_pr = self.floating_ips_dict[floating_ip]
ip_rule = ip_lib.IPRule(namespace=self.ns_name)
ip_rule.rule.delete(ip=floating_ip,
ip_rule.rule.delete(ip=fixed_ip,
table=dvr_fip_ns.FIP_RT_TBL,
priority=rule_pr)
self.fip_ns.deallocate_rule_priority(floating_ip)

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

@ -473,7 +473,7 @@ class IpRuleCommand(IpCommandBase):
def add(self, ip, **kwargs):
ip_version = common_utils.get_ip_version(ip)
# In case if we need to add in a rule based on incoming
# In case we need to add a rule based on an incoming
# interface, pass the "any" IP address, for example, 0.0.0.0/0,
# else pass the given IP.
if kwargs.get('iif'):
@ -489,8 +489,13 @@ class IpRuleCommand(IpCommandBase):
def delete(self, ip, **kwargs):
ip_version = common_utils.get_ip_version(ip)
# TODO(Carl) ip ignored in delete, okay in general?
# In case we need to delete a rule based on an incoming
# interface, pass the "any" IP address, for example, 0.0.0.0/0,
# else pass the given IP.
if kwargs.get('iif'):
kwargs.update({'from': constants.IP_ANY[ip_version]})
else:
kwargs.update({'from': ip})
canonical_kwargs = self._make_canonical(ip_version, kwargs)
args_tuple = self._make__flat_args_tuple('del', **canonical_kwargs)

17
neutron/tests/unit/agent/l3/test_dvr_local_router.py
View File

@ -13,7 +13,6 @@
# under the License.
import mock
import netaddr
from neutron_lib.api.definitions import portbindings
from neutron_lib import constants as lib_constants
from oslo_config import cfg
@ -362,7 +361,8 @@ class TestDvrRouterOperations(base.BaseTestCase):
ri = self._create_router(router)
ri.ex_gw_port = ri.router['gw_port']
subnet_id = _uuid()
agent_gw_port = {'fixed_ips': [{'ip_address': '20.0.0.30',
fixed_ip = '20.0.0.30'
agent_gw_port = {'fixed_ips': [{'ip_address': fixed_ip,
'prefixlen': 24,
'subnet_id': subnet_id}],
'subnets': [{'id': subnet_id,
@ -374,7 +374,7 @@ class TestDvrRouterOperations(base.BaseTestCase):
fip_cidr = '11.22.33.44/24'
ri.fip_ns = mock.Mock()
ri.fip_ns.get_name.return_value = 'fip_ns_name'
ri.floating_ips_dict['11.22.33.44'] = FIP_PRI
ri.floating_ips_dict['11.22.33.44'] = (fixed_ip, FIP_PRI)
ri.fip_2_rtr = '11.22.33.42'
ri.rtr_2_fip = '11.22.33.40'
ri.fip_ns.agent_gateway_port = agent_gw_port
@ -383,7 +383,7 @@ class TestDvrRouterOperations(base.BaseTestCase):
ri.fip_ns.local_subnets = mock.Mock()
ri.floating_ip_removed_dist(fip_cidr)
mIPRule().rule.delete.assert_called_with(
ip=str(netaddr.IPNetwork(fip_cidr).ip), table=16, priority=FIP_PRI)
ip=fixed_ip, table=16, priority=FIP_PRI)
mIPDevice().route.delete_route.assert_called_with(fip_cidr, str(s.ip))
ri.fip_ns.local_subnets.allocate.assert_not_called()
@ -392,20 +392,21 @@ class TestDvrRouterOperations(base.BaseTestCase):
router = mock.MagicMock()
ri = self._create_router(router)
floating_ip_address = '15.1.2.3'
fixed_ip = '192.168.0.1'
fip = {'floating_ip_address': floating_ip_address,
'fixed_ip_address': '192.168.0.1'}
ri.floating_ips_dict['15.1.2.3'] = FIP_PRI
'fixed_ip_address': fixed_ip}
ri.floating_ips_dict['15.1.2.3'] = (fixed_ip, FIP_PRI)
ri.fip_ns = mock.Mock()
ri.fip_ns.allocate_rule_priority.return_value = FIP_PRI
ri.floating_ip_moved_dist(fip)
mIPRule().rule.delete.assert_called_once_with(
ip=floating_ip_address, table=16, priority=FIP_PRI)
ip=fixed_ip, table=16, priority=FIP_PRI)
ri.fip_ns.deallocate_rule_priority.assert_called_once_with(
floating_ip_address)
ri.fip_ns.allocate_rule_priority.assert_called_once_with(
floating_ip_address)
mIPRule().rule.add.assert_called_with(ip='192.168.0.1',
mIPRule().rule.add.assert_called_with(ip=fixed_ip,
table=16,
priority=FIP_PRI)

2
neutron/tests/unit/agent/linux/test_ip_lib.py
View File

@ -689,7 +689,7 @@ class TestIpRuleCommand(TestIPCmdBase):
ip_version = netaddr.IPNetwork(ip).version
self.rule_cmd.delete(ip, table=table, priority=priority)
self._assert_sudo([ip_version],
('del', 'priority', str(priority),
('del', 'from', ip, 'priority', str(priority),
'table', str(table), 'type', 'unicast'))
def test__parse_line(self):