From 75f96f12bada67e89d6062a9eba410ac580eb504 Mon Sep 17 00:00:00 2001 From: Akihiro Motoki Date: Thu, 17 Apr 2014 16:52:36 +0900 Subject: [PATCH] NEC plugin: Allow to apply Packet filter on OFC router interface Config parameter support_packet_filter_on_ofc_router is added only to make the pluign work with the old version of PFC v5 which has no support of packet filter on vrouter interface. Closes-Bug: #1384263 Change-Id: I2f54419e0b7c84c554ab2039ebaebdb065f9e502 --- etc/neutron/plugins/nec/nec.ini | 3 ++ neutron/plugins/nec/common/config.py | 2 + neutron/plugins/nec/drivers/pfc.py | 45 ++++++++++++++++---- neutron/plugins/nec/drivers/trema.py | 17 +++++++- neutron/plugins/nec/ofc_manager.py | 17 +------- neutron/plugins/nec/packet_filter.py | 8 ++-- neutron/tests/unit/nec/stub_ofc_driver.py | 3 +- neutron/tests/unit/nec/test_config.py | 1 + neutron/tests/unit/nec/test_packet_filter.py | 18 +++++--- neutron/tests/unit/nec/test_pfc_driver.py | 38 ++++++++++++++--- neutron/tests/unit/nec/test_trema_driver.py | 17 +++++++- 11 files changed, 122 insertions(+), 47 deletions(-) diff --git a/etc/neutron/plugins/nec/nec.ini b/etc/neutron/plugins/nec/nec.ini index aa4171da78..798a5a61a0 100644 --- a/etc/neutron/plugins/nec/nec.ini +++ b/etc/neutron/plugins/nec/nec.ini @@ -36,6 +36,9 @@ firewall_driver = neutron.agent.linux.iptables_firewall.OVSHybridIptablesFirewal # and supported by the driver. # enable_packet_filter = true +# Support PacketFilter on OFC router interface +# support_packet_filter_on_ofc_router = true + # Use SSL to connect # use_ssl = false diff --git a/neutron/plugins/nec/common/config.py b/neutron/plugins/nec/common/config.py index 3626472625..f673b9e116 100644 --- a/neutron/plugins/nec/common/config.py +++ b/neutron/plugins/nec/common/config.py @@ -41,6 +41,8 @@ ofc_opts = [ help=_("Driver to use.")), cfg.BoolOpt('enable_packet_filter', default=True, help=_("Enable packet filter.")), + cfg.BoolOpt('support_packet_filter_on_ofc_router', default=True, + help=_("Support packet filter on OFC router interface.")), cfg.BoolOpt('use_ssl', default=False, help=_("Use SSL to connect.")), cfg.StrOpt('key_file', diff --git a/neutron/plugins/nec/drivers/pfc.py b/neutron/plugins/nec/drivers/pfc.py index faf6c2d181..12f15fb0db 100644 --- a/neutron/plugins/nec/drivers/pfc.py +++ b/neutron/plugins/nec/drivers/pfc.py @@ -22,7 +22,9 @@ from neutron.common import constants from neutron.common import exceptions as qexc from neutron.common import log as call_log from neutron import manager +from neutron.plugins.nec.common import config from neutron.plugins.nec.common import ofc_client +from neutron.plugins.nec.db import api as ndb from neutron.plugins.nec.extensions import packetfilter as ext_pf from neutron.plugins.nec import ofc_driver_base @@ -168,6 +170,21 @@ class PFCFilterDriverMixin(object): elif not create: body[key] = "" + def _extract_ofc_filter_port_id(self, ofc_port_id): + """Return ofc port id description for packet filter. + + It returns either of the following format: + {'tenant': xxxx, 'network': xxxx, 'port': xxxx} or + {'tenant': xxxx, 'router': xxxx, 'interface': xxxx} + If no matching ofc id is found, InvalidOFCIdFormat is raised. + """ + if config.OFC.support_packet_filter_on_ofc_router: + try: + return self._extract_ofc_router_inf_id(ofc_port_id) + except InvalidOFCIdFormat: + pass + return self._extract_ofc_port_id(ofc_port_id) + def _generate_body(self, filter_dict, apply_ports=None, create=True): body = {} @@ -214,7 +231,8 @@ class PFCFilterDriverMixin(object): body['apply_ports'] = [] for p in apply_ports: try: - body['apply_ports'].append(self._extract_ofc_port_id(p[1])) + _ofc_id = self._extract_ofc_filter_port_id(p[1]) + body['apply_ports'].append(_ofc_id) except InvalidOFCIdFormat: pass @@ -257,8 +275,11 @@ class PFCFilterDriverMixin(object): self._validate_filter_common(filter_dict) @call_log.log - def create_filter(self, ofc_network_id, filter_dict, - portinfo=None, filter_id=None, apply_ports=None): + def create_filter(self, context, filter_dict, filter_id=None): + in_port_id = filter_dict.get('in_port') + apply_ports = ndb.get_active_ports_on_ofc( + context, filter_dict['network_id'], in_port_id) + body = self._generate_body(filter_dict, apply_ports, create=True) res = self.client.post(self.filters_path, body=body) # filter_id passed from a caller is not used. @@ -282,17 +303,25 @@ class PFCFilterDriverMixin(object): return match.group('filter_id') raise InvalidOFCIdFormat(resource='filter', ofc_id=ofc_filter_id) - def convert_ofc_filter_id(self, context, ofc_filter_id): - # PFC Packet Filter is supported after the format of mapping tables - # are changed, so it is enough just to return ofc_filter_id - return ofc_filter_id - class PFCRouterDriverMixin(object): router_supported = True router_nat_supported = False + match_ofc_router_inf_id = re.compile( + "^/tenants/(?P[^/]+)/routers/(?P[^/]+)" + "/interfaces/(?P[^/]+)$") + + def _extract_ofc_router_inf_id(self, ofc_router_inf_id): + match = self.match_ofc_router_inf_id.match(ofc_router_inf_id) + if match: + return {'tenant': match.group('tenant_id'), + 'router': match.group('router_id'), + 'interface': match.group('router_inf_id')} + raise InvalidOFCIdFormat(resource='router-interface', + ofc_id=ofc_router_inf_id) + def create_router(self, ofc_tenant_id, router_id, description): path = '%s/routers' % ofc_tenant_id res = self.client.post(path, body=None) diff --git a/neutron/plugins/nec/drivers/trema.py b/neutron/plugins/nec/drivers/trema.py index cf39008346..a13028d964 100644 --- a/neutron/plugins/nec/drivers/trema.py +++ b/neutron/plugins/nec/drivers/trema.py @@ -13,7 +13,9 @@ # under the License. from neutron.openstack.common import uuidutils +from neutron.plugins.nec.common import exceptions as nexc from neutron.plugins.nec.common import ofc_client +from neutron.plugins.nec.db import api as ndb from neutron.plugins.nec import ofc_driver_base @@ -66,8 +68,19 @@ class TremaFilterDriverMixin(object): def filter_supported(cls): return True - def create_filter(self, ofc_network_id, filter_dict, - portinfo=None, filter_id=None, apply_ports=None): + def create_filter(self, context, filter_dict, filter_id=None): + ofc_network_id = ndb.get_ofc_id(context.session, "ofc_network", + filter_dict['network_id']) + # Prepare portinfo + in_port_id = filter_dict.get('in_port') + if in_port_id: + portinfo = ndb.get_portinfo(context.session, in_port_id) + if not portinfo: + raise nexc.PortInfoNotFound(id=in_port_id) + else: + portinfo = None + + # Prepare filter body if filter_dict['action'].upper() in ["ACCEPT", "ALLOW"]: ofc_action = "ALLOW" elif filter_dict['action'].upper() in ["DROP", "DENY"]: diff --git a/neutron/plugins/nec/ofc_manager.py b/neutron/plugins/nec/ofc_manager.py index 86960c8b93..68d605de6e 100644 --- a/neutron/plugins/nec/ofc_manager.py +++ b/neutron/plugins/nec/ofc_manager.py @@ -112,26 +112,11 @@ class OFCManager(object): self._del_ofc_item(context, "ofc_port", port_id) def create_ofc_packet_filter(self, context, filter_id, filter_dict): - ofc_net_id = self._get_ofc_id(context, "ofc_network", - filter_dict['network_id']) - in_port_id = filter_dict.get('in_port') - portinfo = None - if in_port_id: - portinfo = ndb.get_portinfo(context.session, in_port_id) - if not portinfo: - raise nexc.PortInfoNotFound(id=in_port_id) - - # Collect ports to be associated with the filter - apply_ports = ndb.get_active_ports_on_ofc( - context, filter_dict['network_id'], in_port_id) - ofc_pf_id = self.driver.create_filter(ofc_net_id, - filter_dict, portinfo, filter_id, - apply_ports) + ofc_pf_id = self.driver.create_filter(context, filter_dict, filter_id) self._add_ofc_item(context, "ofc_packet_filter", filter_id, ofc_pf_id) def update_ofc_packet_filter(self, context, filter_id, filter_dict): ofc_pf_id = self._get_ofc_id(context, "ofc_packet_filter", filter_id) - ofc_pf_id = self.driver.convert_ofc_filter_id(context, ofc_pf_id) self.driver.update_filter(ofc_pf_id, filter_dict) def exists_ofc_packet_filter(self, context, filter_id): diff --git a/neutron/plugins/nec/packet_filter.py b/neutron/plugins/nec/packet_filter.py index c36148b570..0b73dc783c 100644 --- a/neutron/plugins/nec/packet_filter.py +++ b/neutron/plugins/nec/packet_filter.py @@ -16,7 +16,6 @@ from neutron.openstack.common import excutils from neutron.openstack.common import log as logging from neutron.plugins.nec.common import config from neutron.plugins.nec.common import exceptions as nexc -from neutron.plugins.nec.db import api as ndb from neutron.plugins.nec.db import packetfilter as pf_db @@ -162,16 +161,12 @@ class PacketFilterMixin(pf_db.PacketFilterDbMixin): "packet_filter=%s."), packet_filter) pf_id = packet_filter['id'] - in_port_id = packet_filter.get('in_port') current = packet_filter['status'] pf_status = current if not packet_filter['admin_state_up']: LOG.debug(_("activate_packet_filter_if_ready(): skip pf_id=%s, " "packet_filter.admin_state_up is False."), pf_id) - elif in_port_id and not ndb.get_portinfo(context.session, in_port_id): - LOG.debug(_("activate_packet_filter_if_ready(): skip " - "pf_id=%s, no portinfo for the in_port."), pf_id) elif self.ofc.exists_ofc_packet_filter(context, packet_filter['id']): LOG.debug(_("_activate_packet_filter_if_ready(): skip, " "ofc_packet_filter already exists.")) @@ -182,6 +177,9 @@ class PacketFilterMixin(pf_db.PacketFilterDbMixin): self.ofc.create_ofc_packet_filter(context, pf_id, packet_filter) pf_status = pf_db.PF_STATUS_ACTIVE + except nexc.PortInfoNotFound: + LOG.debug(_("Skipped to create a packet filter pf_id=%s " + "on OFC, no portinfo for the in_port."), pf_id) except (nexc.OFCException, nexc.OFCMappingNotFound) as exc: LOG.error(_("Failed to create packet_filter id=%(id)s on " "OFC: %(exc)s"), {'id': pf_id, 'exc': exc}) diff --git a/neutron/tests/unit/nec/stub_ofc_driver.py b/neutron/tests/unit/nec/stub_ofc_driver.py index da28f8735f..fb34fe8944 100644 --- a/neutron/tests/unit/nec/stub_ofc_driver.py +++ b/neutron/tests/unit/nec/stub_ofc_driver.py @@ -142,8 +142,7 @@ class StubOFCDriver(ofc_driver_base.OFCDriverBase): def filter_supported(cls): return True - def create_filter(self, ofc_network_id, filter_dict, - portinfo=None, filter_id=None, apply_ports=None): + def create_filter(self, context, filter_dict, filter_id=None): return "ofc-" + filter_id[:-4] def delete_filter(self, ofc_filter_id): diff --git a/neutron/tests/unit/nec/test_config.py b/neutron/tests/unit/nec/test_config.py index 6df371a2f0..fef70ad5e4 100644 --- a/neutron/tests/unit/nec/test_config.py +++ b/neutron/tests/unit/nec/test_config.py @@ -29,6 +29,7 @@ class ConfigurationTest(base.BaseTestCase): self.assertEqual('', config.CONF.OFC.path_prefix) self.assertEqual('trema', config.CONF.OFC.driver) self.assertTrue(config.CONF.OFC.enable_packet_filter) + self.assertTrue(config.CONF.OFC.support_packet_filter_on_ofc_router) self.assertFalse(config.CONF.OFC.use_ssl) self.assertIsNone(config.CONF.OFC.key_file) self.assertIsNone(config.CONF.OFC.cert_file) diff --git a/neutron/tests/unit/nec/test_packet_filter.py b/neutron/tests/unit/nec/test_packet_filter.py index 782460af05..ba7992cc46 100644 --- a/neutron/tests/unit/nec/test_packet_filter.py +++ b/neutron/tests/unit/nec/test_packet_filter.py @@ -408,16 +408,24 @@ class TestNecPluginPacketFilter(TestNecPluginPacketFilterBase): def test_activate_pf_on_port_triggered_by_update_port(self): ctx = mock.ANY pf_dict = mock.ANY + self.ofc.set_raise_exc('create_ofc_packet_filter', + nexc.PortInfoNotFound(id='fake_id')) with self.packet_filter_on_port(set_portinfo=False) as pf: pf_id = pf['packet_filter']['id'] in_port_id = pf['packet_filter']['in_port'] - self.assertFalse(self.ofc.create_ofc_packet_filter.called) + # create_ofc_packet_filter is now called even when + # in_port does not exists yet. In this case + # PortInfoNotFound exception is raised. + self.assertEqual(1, self.ofc.create_ofc_packet_filter.call_count) portinfo = {'id': in_port_id, 'port_no': 123} kw = {'added': [portinfo]} + self.ofc.set_raise_exc('create_ofc_packet_filter', None) self.rpcapi_update_ports(**kw) - self.ofc.create_ofc_packet_filter.assert_called_once_with( - ctx, pf_id, pf_dict) + self.assertEqual(2, self.ofc.create_ofc_packet_filter.call_count) + self.ofc.assert_has_calls([ + mock.call.create_ofc_packet_filter(ctx, pf_id, pf_dict), + ]) self.assertFalse(self.ofc.delete_ofc_packet_filter.called) kw = {'removed': [in_port_id]} @@ -441,8 +449,8 @@ class TestNecPluginPacketFilter(TestNecPluginPacketFilterBase): mock.call.delete_ofc_packet_filter(ctx, pf_id), ] self.ofc.assert_has_calls(expected) - self.assertEqual(self.ofc.create_ofc_packet_filter.call_count, 1) - self.assertEqual(self.ofc.delete_ofc_packet_filter.call_count, 1) + self.assertEqual(2, self.ofc.create_ofc_packet_filter.call_count) + self.assertEqual(1, self.ofc.delete_ofc_packet_filter.call_count) def test_activate_pf_while_exists_on_ofc(self): ctx = mock.ANY diff --git a/neutron/tests/unit/nec/test_pfc_driver.py b/neutron/tests/unit/nec/test_pfc_driver.py index c7516d5cb8..ad2ea7ae79 100644 --- a/neutron/tests/unit/nec/test_pfc_driver.py +++ b/neutron/tests/unit/nec/test_pfc_driver.py @@ -18,6 +18,7 @@ import uuid import mock import netaddr +from oslo.config import cfg from neutron.common import constants from neutron.openstack.common import uuidutils @@ -369,24 +370,24 @@ class PFCFilterDriverTestMixin: t, n, p = self.get_ofc_item_random_params() filter_id = uuidutils.generate_uuid() - f = {'priority': 123, 'action': "ACCEPT"} + f = {'priority': 123, 'action': "ACCEPT", + 'network_id': n} if filter_dict: f.update(filter_dict) - net_path = "/networks/%s" % n body = {'action': 'pass', 'priority': 123} if filter_post: body.update(filter_post) self.do_request.return_value = {'id': filter_id} - if apply_ports is not None: - ret = self.driver.create_filter(net_path, f, p, - apply_ports=apply_ports) - else: - ret = self.driver.create_filter(net_path, f, p) + with mock.patch('neutron.plugins.nec.db.api.get_active_ports_on_ofc', + return_value=apply_ports) as active_ports: + ret = self.driver.create_filter(mock.sentinel.ctx, f) self.do_request.assert_called_once_with("POST", "/filters", body=body) self.assertEqual(ret, '/filters/%s' % filter_id) + active_ports.assert_called_once_with(mock.sentinel.ctx, n, + filter_dict.get('in_port')) def test_create_filter_accept(self): self._test_create_filter(filter_dict={'action': 'ACCEPT'}) @@ -476,6 +477,29 @@ class PFCFilterDriverTestMixin: self._test_create_filter(filter_dict={}, apply_ports=apply_ports, filter_post=filter_post) + def test_create_filter_apply_ports_with_router_interface(self): + apply_ports = [ + ('p1', '/tenants/tenant-1/networks/network-1/ports/port-1'), + ('p2', '/tenants/tenant-2/routers/router-3/interfaces/if-4')] + filter_post = {'apply_ports': [ + {'tenant': 'tenant-1', 'network': 'network-1', 'port': 'port-1'}, + {'tenant': 'tenant-2', 'router': 'router-3', 'interface': 'if-4'} + ]} + self._test_create_filter(filter_dict={}, apply_ports=apply_ports, + filter_post=filter_post) + + def test_create_filter_apply_ports_no_router_interface_support(self): + cfg.CONF.set_override('support_packet_filter_on_ofc_router', + False, 'OFC') + apply_ports = [ + ('p1', '/tenants/tenant-1/networks/network-1/ports/port-1'), + ('p2', '/tenants/tenant-2/routers/router-3/interfaces/if-4')] + filter_post = {'apply_ports': [ + {'tenant': 'tenant-1', 'network': 'network-1', 'port': 'port-1'}, + ]} + self._test_create_filter(filter_dict={}, apply_ports=apply_ports, + filter_post=filter_post) + def _test_update_filter(self, filter_dict=None, filter_post=None): filter_id = uuidutils.generate_uuid() ofc_filter_id = '/filters/%s' % filter_id diff --git a/neutron/tests/unit/nec/test_trema_driver.py b/neutron/tests/unit/nec/test_trema_driver.py index 2d095fb005..766f3b233b 100644 --- a/neutron/tests/unit/nec/test_trema_driver.py +++ b/neutron/tests/unit/nec/test_trema_driver.py @@ -18,6 +18,7 @@ import mock from six import moves from neutron.openstack.common import uuidutils +from neutron.plugins.nec.common import exceptions as nexc from neutron.plugins.nec.common import ofc_client from neutron.plugins.nec.db import models as nmodels from neutron.plugins.nec import drivers @@ -246,7 +247,14 @@ class TremaFilterDriverTest(TremaDriverTestBase): if non_ofp_wildcards: body['wildcards'] = set(non_ofp_wildcards) - ret = self.driver.create_filter(net_path, f, p, f['id']) + ctx = mock.Mock() + ctx.session = mock.sentinel.session + with mock.patch('neutron.plugins.nec.db.api.get_portinfo', + return_value=p) as get_portinfo: + with mock.patch('neutron.plugins.nec.db.api.get_ofc_id', + return_value=net_path) as get_ofc_id: + ret = self.driver.create_filter(ctx, f, f['id']) + # The content of 'body' is checked below. self.do_request.assert_called_once_with("POST", "/filters", body=mock.ANY) @@ -263,6 +271,10 @@ class TremaFilterDriverTest(TremaDriverTestBase): actual_body['wildcards'] = set(actual_body['wildcards'].split(',')) self.assertEqual(body, actual_body) + get_ofc_id.assert_called_once_with(mock.sentinel.session, + 'ofc_network', n) + get_portinfo.assert_called_once_with(mock.sentinel.session, p.id) + def test_create_filter_accept(self): self._test_create_filter(filter_dict={'action': 'ACCEPT'}) @@ -278,7 +290,8 @@ class TremaFilterDriverTest(TremaDriverTestBase): filter_post={'action': 'DENY'}) def test_create_filter_no_port(self): - self._test_create_filter(no_portinfo=True) + self.assertRaises(nexc.PortInfoNotFound, + self._test_create_filter, no_portinfo=True) def test_create_filter_src_mac_wildcard(self): self._test_create_filter(filter_dict={'src_mac': ''},