x/vmware-nsx
Code Issues Proposed changes

NSX|V3: Do not add SNAT rules if same address scope

Browse Source 
In case the router GW and interface belong to the same address scope
there is not need to add SNAT rules for this interface, as there are no
overlapping addresses.

For this end, we need to add specific SNAT rule for each of the router
relevant subnets, instead of just 1 SNAT all rule.
Those NAT rules will be deleted when the GW is removed, changed to no-SNAT
or when an interface is removed.

Depends-on: Id43a2ced7d6526f538f485f345c20ba44673c7b2
Change-Id: Ie528503ec77b397a19fdcf90cfced738fdfc7a22
This commit is contained in:
Adit Sarfaty 2017-06-28 07:54:58 +03:00
parent 2033fe37c1
commit 3887d898b4
4 changed files with 165 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
vmware_nsx/plugins/common/plugin.py
View File

@ -20,6 +20,7 @@ from neutron.db import address_scope_db
from neutron.db import api as db_api
from neutron.db import db_base_plugin_v2
from neutron.db import l3_db
from neutron.db import models_v2
from neutron.extensions import address_scope as ext_address_scope
from neutron_lib.api.definitions import network as net_def
from neutron_lib.api.definitions import port as port_def
@ -126,3 +127,31 @@ class NsxPluginBase(db_base_plugin_v2.NeutronDbPluginV2,
port_filters = {'device_id': [router_id],
'device_owner': [l3_db.DEVICE_OWNER_ROUTER_INTF]}
return self.get_ports(context, filters=port_filters)
def _find_router_subnets_cidrs(self, context, router_id):
"""Retrieve cidrs of subnets attached to the specified router."""
subnets = self._find_router_subnets_and_cidrs(context, router_id)
return [subnet['cidr'] for subnet in subnets]
def _get_port_by_device_id(self, context, device_id, device_owner):
"""Retrieve ports associated with a specific device id.
Used for retrieving all neutron ports attached to a given router.
"""
port_qry = context.session.query(models_v2.Port)
return port_qry.filter_by(
device_id=device_id,
device_owner=device_owner,).all()
def _find_router_subnets_and_cidrs(self, context, router_id):
"""Retrieve subnets attached to the specified router."""
ports = self._get_port_by_device_id(context, router_id,
l3_db.DEVICE_OWNER_ROUTER_INTF)
# No need to check for overlapping CIDRs
subnets = []
for port in ports:
for ip in port.get('fixed_ips', []):
subnet_qry = context.session.query(models_v2.Subnet)
subnet = subnet_qry.filter_by(id=ip.subnet_id).one()
subnets.append({'id': subnet.id, 'cidr': subnet.cidr})
return subnets

28
vmware_nsx/plugins/nsx_v/plugin.py
View File

@ -3243,34 +3243,6 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
address_groups.append(address_group)
return address_groups
def _get_port_by_device_id(self, context, device_id, device_owner):
"""Retrieve ports associated with a specific device id.
Used for retrieving all neutron ports attached to a given router.
"""
port_qry = context.session.query(models_v2.Port)
return port_qry.filter_by(
device_id=device_id,
device_owner=device_owner,).all()
def _find_router_subnets_cidrs(self, context, router_id):
"""Retrieve cidrs of subnets attached to the specified router."""
subnets = self._find_router_subnets_and_cidrs(context, router_id)
return [subnet['cidr'] for subnet in subnets]
def _find_router_subnets_and_cidrs(self, context, router_id):
"""Retrieve subnets attached to the specified router."""
ports = self._get_port_by_device_id(context, router_id,
l3_db.DEVICE_OWNER_ROUTER_INTF)
# No need to check for overlapping CIDRs
subnets = []
for port in ports:
for ip in port.get('fixed_ips', []):
subnet_qry = context.session.query(models_v2.Subnet)
subnet = subnet_qry.filter_by(id=ip.subnet_id).one()
subnets.append({'id': subnet.id, 'cidr': subnet.cidr})
return subnets
def _get_nat_rules(self, context, router):
fip_qry = context.session.query(l3_db_models.FloatingIP)
fip_db = fip_qry.filter_by(router_id=router['id']).all()

55
vmware_nsx/plugins/nsx_v3/plugin.py
View File

@ -2737,7 +2737,7 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
# TODO(berlin): revocate bgp announce on org tier0 router
pass
if remove_snat_rules:
self._routerlib.delete_gw_snat_rule(nsx_router_id, orgaddr)
self._routerlib.delete_gw_snat_rules(nsx_router_id, orgaddr)
if remove_router_link_port:
self._routerlib.remove_router_link_port(
nsx_router_id, org_tier0_uuid)
@ -2752,8 +2752,15 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
self._routerlib.add_router_link_port(nsx_router_id, new_tier0_uuid,
tags=tags)
if add_snat_rules:
self._routerlib.add_gw_snat_rule(nsx_router_id, newaddr,
bypass_firewall=False)
# Add SNAT rules for all the subnets which are in different scope
# than the gw
gw_address_scope = self._get_network_address_scope(
context, router.gw_port.network_id)
subnets = self._find_router_subnets_and_cidrs(context.elevated(),
router_id)
for subnet in subnets:
self._add_subnet_snat_rule(context, router_id, nsx_router_id,
subnet, gw_address_scope, newaddr)
if bgp_announce:
# TODO(berlin): bgp announce on new tier0 router
pass
@ -2762,6 +2769,26 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
advertise_route_nat_flag,
advertise_route_connected_flag)
def _add_subnet_snat_rule(self, context, router_id, nsx_router_id, subnet,
gw_address_scope, gw_ip):
# if the subnets address scope is the same as the gateways:
# no need for SNAT
if gw_address_scope:
subnet_address_scope = self._get_subnet_address_scope(
context, subnet['id'])
if (gw_address_scope == subnet_address_scope):
LOG.info("No need for SNAT rule for router %(router)s "
"and subnet %(subnet)s because they use the "
"same address scope %(addr_scope)s.",
{'router': router_id,
'subnet': subnet['id'],
'addr_scope': gw_address_scope})
return
self._routerlib.add_gw_snat_rule(nsx_router_id, gw_ip,
source_net=subnet['cidr'],
bypass_firewall=False)
def _process_extra_attr_router_create(self, context, router_db, r):
for extra_attr in l3_attrs_db.get_attr_info().keys():
if extra_attr in r:
@ -3070,11 +3097,12 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
nsx_net_id, nsx_port_id = nsx_db.get_nsx_switch_and_port_id(
context.session, port['id'])
router_db = self._get_router(context, router_id)
gw_network_id = (router_db.gw_port.network_id if router_db.gw_port
else None)
# If it is a no-snat router, interface address scope must be the
# same as the gateways
if not router_db.enable_snat:
gw_network_id = router_db.gw_port.network_id
if not router_db.enable_snat and gw_network_id:
self._validate_address_scope_for_router_interface(
context, router_id, gw_network_id, subnet['id'])
@ -3106,6 +3134,15 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
# its DHCP port), by creating it if needed.
nsx_rpc.handle_router_metadata_access(self, context, router_id,
interface=info)
# add the SNAT rule for this interface
if (router_db.enable_snat and gw_network_id and
router_db.gw_port.get('fixed_ips')):
gw_ip = router_db.gw_port['fixed_ips'][0]['ip_address']
gw_address_scope = self._get_network_address_scope(
context, gw_network_id)
self._add_subnet_snat_rule(context, router_id, nsx_router_id,
subnet, gw_address_scope, gw_ip)
except Exception:
with excutils.save_and_reraise_exception():
LOG.error("Neutron failed to add_router_interface on "
@ -3176,6 +3213,14 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin,
else:
self.nsxlib.logical_router_port.delete_by_lswitch_id(
nsx_net_id)
# try to delete the SNAT rule of this subnet
if (router_db.gw_port and router_db.enable_snat and
router_db.gw_port.get('fixed_ips')):
gw_ip = router_db.gw_port['fixed_ips'][0]['ip_address']
self._routerlib.delete_gw_snat_rule_by_source(
nsx_router_id, gw_ip, subnet['cidr'],
skip_not_found=True)
except nsx_lib_exc.ResourceNotFound:
LOG.error("router port on router %(router_id)s for net "
"%(net_id)s not found at the backend",

86
vmware_nsx/tests/unit/nsx_v3/test_plugin.py
View File

@ -891,6 +891,92 @@ class TestL3NatTestCase(L3NatTest,
int_subnet['subnet']['id'],
None)
def test_router_address_scope_snat_rules(self):
"""Test that if the router interface had the same address scope
as the gateway - snat rule is not added.
"""
# create an external network on one address scope
with self.address_scope(name='as1') as addr_scope, \
self.network() as ext_net:
self._set_net_external(ext_net['network']['id'])
as_id = addr_scope['address_scope']['id']
subnet = netaddr.IPNetwork('10.10.10.0/21')
subnetpool = self._test_create_subnetpool(
[subnet.cidr], name='sp1',
min_prefixlen='24', address_scope_id=as_id)
subnetpool_id = subnetpool['subnetpool']['id']
data = {'subnet': {
'network_id': ext_net['network']['id'],
'subnetpool_id': subnetpool_id,
'ip_version': 4,
'enable_dhcp': False,
'tenant_id': ext_net['network']['tenant_id']}}
req = self.new_create_request('subnets', data)
ext_subnet = self.deserialize(self.fmt, req.get_response(self.api))
# create a regular network on the same address scope
with self.network() as net:
data = {'subnet': {
'network_id': net['network']['id'],
'subnetpool_id': subnetpool_id,
'ip_version': 4,
'tenant_id': net['network']['tenant_id']}}
req = self.new_create_request('subnets', data)
int_subnet = self.deserialize(
self.fmt, req.get_response(self.api))
# create a router with this gateway
with self.router() as r:
self._add_external_gateway_to_router(
r['router']['id'],
ext_subnet['subnet']['network_id'])
with mock.patch("vmware_nsxlib.v3.router.RouterLib."
"add_gw_snat_rule") as add_nat:
# Add the interface
self._router_interface_action(
'add',
r['router']['id'],
int_subnet['subnet']['id'],
None)
# make sure snat rules are not added
add_nat.assert_not_called()
# create a regular network on a different address scope
with self.address_scope(name='as2') as addr_scope2, \
self.network() as net:
as_id2 = addr_scope2['address_scope']['id']
subnet2 = netaddr.IPNetwork('20.10.10.0/24')
subnetpool2 = self._test_create_subnetpool(
[subnet2.cidr], name='sp2',
min_prefixlen='24', address_scope_id=as_id2)
subnetpool_id2 = subnetpool2['subnetpool']['id']
data = {'subnet': {
'network_id': net['network']['id'],
'subnetpool_id': subnetpool_id2,
'ip_version': 4,
'tenant_id': net['network']['tenant_id']}}
req = self.new_create_request('subnets', data)
int_subnet = self.deserialize(
self.fmt, req.get_response(self.api))
# create a router with this gateway
with self.router() as r:
self._add_external_gateway_to_router(
r['router']['id'],
ext_subnet['subnet']['network_id'])
with mock.patch("vmware_nsxlib.v3.router.RouterLib."
"add_gw_snat_rule") as add_nat:
# Add the interface
self._router_interface_action(
'add',
r['router']['id'],
int_subnet['subnet']['id'],
None)
# make sure snat rules are added
add_nat.assert_called_once()
class ExtGwModeTestCase(test_ext_gw_mode.ExtGwModeIntTestCase,
L3NatTest):