Merge "NEC plugin: Allow to apply Packet filter on OFC router interface"

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
 

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',

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,
+    def create_filter(self, context, filter_dict, filter_id=None):
-                      portinfo=None, filter_id=None, apply_ports=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<tenant_id>[^/]+)/routers/(?P<router_id>[^/]+)"
+        "/interfaces/(?P<router_inf_id>[^/]+)$")
+
+    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)

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,
+    def create_filter(self, context, filter_dict, filter_id=None):
-                      portinfo=None, filter_id=None, apply_ports=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"]:

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",
+        ofc_pf_id = self.driver.create_filter(context, filter_dict, filter_id)
-                                      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)
         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):

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})

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,
+    def create_filter(self, context, filter_dict, filter_id=None):
-                      portinfo=None, filter_id=None, apply_ports=None):
         return "ofc-" + filter_id[:-4]
 
     def delete_filter(self, ofc_filter_id):

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)

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(
+            self.assertEqual(2, self.ofc.create_ofc_packet_filter.call_count)
-                ctx, pf_id, pf_dict)
+            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(2, self.ofc.create_ofc_packet_filter.call_count)
-        self.assertEqual(self.ofc.delete_ofc_packet_filter.call_count, 1)
+        self.assertEqual(1, self.ofc.delete_ofc_packet_filter.call_count)
 
     def test_activate_pf_while_exists_on_ofc(self):
         ctx = mock.ANY

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:
+        with mock.patch('neutron.plugins.nec.db.api.get_active_ports_on_ofc',
-            ret = self.driver.create_filter(net_path, f, p,
+                        return_value=apply_ports) as active_ports:
-                                            apply_ports=apply_ports)
+            ret = self.driver.create_filter(mock.sentinel.ctx, f)
-        else:
-            ret = self.driver.create_filter(net_path, f, p)
         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

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': ''},