From fb9ec1afb6545def3130952008ee7f20dbaafd2c Mon Sep 17 00:00:00 2001 From: Dmitrii Shcherbakov Date: Thu, 29 Mar 2018 17:32:01 -0400 Subject: [PATCH] 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 ' is included into an 'ip rule' command invocation. When a port on a tenant network is deleted 'from ' 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 81db328b2df08f2b4adcc80104cf05ad8966c019) --- neutron/agent/l3/dvr_fip_ns.py | 9 ++++++--- neutron/agent/l3/dvr_local_router.py | 6 +++--- neutron/agent/linux/ip_lib.py | 11 ++++++++--- .../unit/agent/l3/test_dvr_local_router.py | 17 +++++++++-------- neutron/tests/unit/agent/linux/test_ip_lib.py | 2 +- 5 files changed, 27 insertions(+), 18 deletions(-) diff --git a/neutron/agent/l3/dvr_fip_ns.py b/neutron/agent/l3/dvr_fip_ns.py index a77e7a3239d..21879b78426 100644 --- a/neutron/agent/l3/dvr_fip_ns.py +++ b/neutron/agent/l3/dvr_fip_ns.py @@ -394,9 +394,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) diff --git a/neutron/agent/l3/dvr_local_router.py b/neutron/agent/l3/dvr_local_router.py index 5a3d2fc54c4..4d6d3e8d1e0 100644 --- a/neutron/agent/l3/dvr_local_router.py +++ b/neutron/agent/l3/dvr_local_router.py @@ -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) diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index 2354ac12ab1..aeda696820b 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -469,7 +469,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'): @@ -485,8 +485,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) diff --git a/neutron/tests/unit/agent/l3/test_dvr_local_router.py b/neutron/tests/unit/agent/l3/test_dvr_local_router.py index 6a41b851727..f10a47ab040 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_local_router.py +++ b/neutron/tests/unit/agent/l3/test_dvr_local_router.py @@ -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 @@ -361,7 +360,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, @@ -373,7 +373,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 @@ -382,7 +382,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() @@ -391,20 +391,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) diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py index e3cc049fa8c..9e5992487be 100644 --- a/neutron/tests/unit/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/agent/linux/test_ip_lib.py @@ -678,7 +678,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):