<html><body><p><tt>John McDowall <jmcdowall@paloaltonetworks.com> wrote on 05/10/2016 11:11:57 AM:<br><br>> From: John McDowall <jmcdowall@paloaltonetworks.com></tt><br><tt>> To: Ryan Moats/Omaha/IBM@IBMUS</tt><br><tt>> Cc: "discuss@openvswitch.org" <discuss@openvswitch.org>, "OpenStack <br>> Development Mailing List" <openstack-dev@lists.openstack.org></tt><br><tt>> Date: 05/10/2016 11:12 AM</tt><br><tt>> Subject: Re: [OVN] [networking-ovn] [networking-sfc] SFC and OVN</tt><br><tt>> <br>> Ryan,</tt><br><tt>> <br>> Let me do that – I assume adding them to plugin.py is the right approach. </tt><br><tt>> <br>> I have cleaned up <a href="https://github.com/doonhammer/networking-ovn">https://github.com/doonhammer/networking-ovn</a> and <br>> did a merge so it should be a lot easier to see the changes. I am <br>> going to cleanup ovs/ovn next. Once I have everything cleaned up and<br>> make sure it is still working I will move the code over to the port-<br>> pair/port-chain model.</tt><br><tt>> <br>> Let me know if that works for you.</tt><br><tt>> <br>> Regards</tt><br><tt>> <br>> John</tt><br><tt>> <br>> From: Ryan Moats <rmoats@us.ibm.com><br>> Date: Tuesday, May 10, 2016 at 7:38 AM<br>> To: John McDowall <jmcdowall@paloaltonetworks.com><br>> Cc: "discuss@openvswitch.org" <discuss@openvswitch.org>, OpenStack <br>> Development Mailing List <openstack-dev@lists.openstack.org><br>> Subject: Re: [OVN] [networking-ovn] [networking-sfc] SFC and OVN</tt><br><tt>> <br>> John McDowall <jmcdowall@paloaltonetworks.com> wrote on 05/09/2016 <br>> 10:46:41 AM:<br>> <br>> > From: John McDowall <jmcdowall@paloaltonetworks.com><br>> > To: Ryan Moats/Omaha/IBM@IBMUS<br>> > Cc: "discuss@openvswitch.org" <discuss@openvswitch.org>, "OpenStack <br>> > Development Mailing List" <openstack-dev@lists.openstack.org><br>> > Date: 05/09/2016 10:46 AM<br>> > Subject: Re: [OVN] [networking-ovn] [networking-sfc] SFC and OVN<br>> > <br>> > Ryan,<br>> > <br>> > Thanks – let me try and get the code cleaned up and rebased. One <br>> > area that I could use your insight on is the interface to <br>> > networking-ovn and how it should look. <br>> > <br>> > Regards<br>> > <br>> > John<br>> <br>> Looking at this, the initial code that I think should move over are <br>> _create_ovn_vnf and _delete_ovn_vnf and maybe rename them to<br>> create_vnf and delete_vnf.<br>> <br>> What I haven't figured out at this point is:<br>> 1) Is the above enough?<br>> 2) Do we need to refactor some of OVNPlugin's calls to provide hooks<br>> for the SFC<br>>    driver to use for when the OVNPlugin inheritance goes away.<br>> <br>> Ryan</tt><br><br><tt>I like the idea, but unfortunately, I'm still not able to apply the series</tt><br><tt>cleanly to openstack:master:</tt><br><tt>- the list of supported extensions has moved from plugin.py to </tt><br><tt>  commons/extensions.py</tt><br><tt>- the patches that create extensions/__init__.py and </tt><br><tt>  networking_ovn/db/migration/__init__.py complain of garbage</tt><br><tt>- the patch that adds Logging on fixed ips doesn't align with the new</tt><br><tt>  code structure for handling same</tt><br><tt>- the patch that removes experimental code also has a whole lot of </tt><br><tt>  changes that were part of the master devstack/plugin.sh and so </tt><br><tt>  confuses patch.  Worse, almost 90% of the changes to plugin.py fail</tt><br><tt>  because of the same commingling.</tt><br><br><tt>After walking through all the patches, I'm still seeing what looks like</tt><br><tt>some cruft: I'm not sure what networking_ovn/db/migration</tt><br><tt>and networking_ovn/extensions are doing anymore, as they only have</tt><br><tt>__init__.py files in them.</tt><br><br><tt>Even with that, I *think* the following patch below all the changes against</tt><br><tt>tip of the tree master...</tt><br><br><tt>Ryan</tt><br><tt><br>---<br> networking_ovn/common/extensions.py     |    1 +<br> networking_ovn/ovsdb/commands.py        |   78 +++++++++++++++++++++++++++++++<br> networking_ovn/ovsdb/impl_idl_ovn.py    |   18 +++++++<br> networking_ovn/ovsdb/ovn_api.py         |   49 +++++++++++++++++++<br> networking_ovn/plugin.py                |   71 ++++++++++++++++++++++++++++<br> 5 files changed, 217 insertions(+), 0 deletions(-)<br> create mode 100644 networking_ovn/db/migration/__init__.py<br> create mode 100644 networking_ovn/extensions/__init__.py</tt><br><br><tt>diff --git a/networking_ovn/common/extensions.py b/networking_ovn/common/extensions.py<br>index c171e11..55fc147 100644<br>--- a/networking_ovn/common/extensions.py<br>+++ b/networking_ovn/common/extensions.py<br>@@ -37,4 +37,5 @@ SUPPORTED_API_EXTENSIONS = [<br>     'subnet_allocation',<br>     'port-security',<br>     'allowed-address-pairs',<br>+    'sfi',<br> ]<br>diff --git a/networking_ovn/db/migration/__init__.py b/networking_ovn/db/migration/__init__.py<br>new file mode 100644<br>index 0000000..e69de29<br>diff --git a/networking_ovn/extensions/__init__.py b/networking_ovn/extensions/__init__.py<br>new file mode 100644<br>index 0000000..e69de29<br>diff --git a/networking_ovn/ovsdb/commands.py b/networking_ovn/ovsdb/commands.py<br>index 7ea7a6f..68a747f 100644<br>--- a/networking_ovn/ovsdb/commands.py<br>+++ b/networking_ovn/ovsdb/commands.py<br>@@ -164,6 +164,84 @@ class DelLogicalPortCommand(BaseCommand):<br>         setattr(lswitch, 'ports', ports)<br>         self.api._tables['Logical_Port'].rows[lport.uuid].delete()<br> <br>+class AddLogicalServiceCommand(BaseCommand):</tt><br><tt>+    def __init__(self, api, lservice, lswitch, may_exist, **columns):<br>+        super(AddLogicalServiceCommand, self).__init__(api)<br>+        self.lservice = lservice<br>+        self.lswitch = lswitch<br>+        self.may_exist = may_exist<br>+        self.columns = columns<br>+<br>+    def run_idl(self, txn):<br>+        try:<br>+            lswitch = idlutils.row_by_value(self.api.idl, 'Logical_Switch',<br>+                                            'name', self.lswitch)<br>+            services= getattr(lswitch, 'services', [])<br>+        except idlutils.RowNotFound:<br>+            msg = _("Logical Switch %s does not exist") % self.lswitch<br>+            raise RuntimeError(msg)<br>+        if self.may_exist:<br>+            service = idlutils.row_by_value(self.api.idl,<br>+                                         'Logical_Service', 'name',<br>+                                         self.lservice, None)<br>+            if service:<br>+                return<br>+<br>+        lswitch.verify('services')<br>+<br>+        service = txn.insert(self.api._tables['Logical_Service'])</tt><br><tt>+        service.name = self.lservice<br>+        for col, val in self.columns.items():<br>+            setattr(service, col, val)<br>+        # add the newly created service to existing lswitch<br>+        services.append(service.uuid)<br>+        setattr(lswitch, 'services', services)<br>+<br>+class SetLogicalServiceCommand(BaseCommand):<br>+    def __init__(self, api, lservice, if_exists, **columns):<br>+        super(SetLogicalServiceCommand, self).__init__(api)<br>+        self.lservice = lservice<br>+        self.columns = columns<br>+        self.if_exists = if_exists<br>+<br>+    def run_idl(self, txn):<br>+        try:<br>+            service = idlutils.row_by_value(self.api.idl, 'Logical_Service',<br>+                                            'name', self.lservice)<br>+        except idlutils.RowNotFound:<br>+            if self.if_exists:<br>+                return<br>+            msg = _("Logical Service %s does not exist") % self.lservice<br>+            raise RuntimeError(msg)<br>+<br>+        for col, val in self.columns.items():<br>+            setattr(service, col, val)</tt><br><tt>+<br>+class DelLogicalServiceCommand(BaseCommand):<br>+    def __init__(self, api, lservice, lswitch, if_exists):<br>+        super(DelLogicalServiceCommand, self).__init__(api)<br>+        self.lservice = lservice<br>+        self.lswitch = lswitch<br>+        self.if_exists = if_exists<br>+<br>+    def run_idl(self, txn):<br>+        try:<br>+            lservice = idlutils.row_by_value(self.api.idl, 'Logical_Service',<br>+                                             'name', self.lservice)<br>+            lswitch = idlutils.row_by_value(self.api.idl, 'Logical_Switch',<br>+                                            'name', self.lswitch)<br>+            services = getattr(lswitch, 'services', [])<br>+        except idlutils.RowNotFound:<br>+            if self.if_exists:<br>+                return<br>+            msg = _("Service %s does not exist") % self.lservice<br>+            raise RuntimeError(msg)<br>+<br>+        lswitch.verify('services')<br>+<br>+        services.remove(lservice)<br>+        setattr(lswitch, 'services', services)<br>+        self.api._tables['Logical_Service'].rows[lservice.uuid].delete()</tt><br><tt> <br> class AddLRouterCommand(BaseCommand):<br>     def __init__(self, api, name, may_exist, **columns):<br>diff --git a/networking_ovn/ovsdb/impl_idl_ovn.py b/networking_ovn/ovsdb/impl_idl_ovn.py<br>index c3411f0..38623ac 100644<br>--- a/networking_ovn/ovsdb/impl_idl_ovn.py<br>+++ b/networking_ovn/ovsdb/impl_idl_ovn.py<br>@@ -77,6 +77,24 @@ class OvsdbOvnIdl(ovn_api.API):<br>                                                ext_id[0], ext_id[1],<br>                                                if_exists)<br> <br>+    def create_lservice(self, lservice_name, lswitch_name, may_exist=True,<br>+                     **columns):<br>+        return cmd.AddLogicalServiceCommand(self, lservice_name, lswitch_name,<br>+                                         may_exist, **columns)<br>+<br>+    def set_lservice(self, lservice_name, if_exists=True, **columns):<br>+        return cmd.SetLogicalServiceCommand(self, lservice_name,<br>+                                         if_exists, **columns)<br>+<br>+    def delete_lservice(self, lservice_name=None, lswitch=None,<br>+                     ext_id=None, if_exists=True):</tt><br><tt>+        if lservice_name is not None:<br>+            return cmd.DelLogicalServiceCommand(self, lservice_name,<br>+                                             lswitch, if_exists)<br>+        else:<br>+            raise RuntimeError(_("Currently only supports "<br>+                                 "delete by lservice-name"))<br>+<br>     def create_lport(self, lport_name, lswitch_name, may_exist=True,<br>                      **columns):<br>         return cmd.AddLogicalPortCommand(self, lport_name, lswitch_name,<br>diff --git a/networking_ovn/ovsdb/ovn_api.py b/networking_ovn/ovsdb/ovn_api.py<br>index feca916..067488c 100644<br>--- a/networking_ovn/ovsdb/ovn_api.py<br>+++ b/networking_ovn/ovsdb/ovn_api.py<br>@@ -69,6 +69,55 @@ class API(object):<br>         :returns:         :class:`Command` with no result<br>         """<br> <br>+<br>+<br>+<br>+    @abc.abstractmethod<br>+    def create_lservice(self, name, lswitch_name, may_exist=True, **columns):<br>+        """Create a command to add an OVN lservice<br>+<br>+        :param name:          The name of the lservice<br>+        :type name:           string</tt><br><tt>+        :param lswitch_name:  The name of the lswitch the lservice is created on<br>+        :type lswitch_name:   string<br>+        :param may_exist:     Do not fail if lservice already exists<br>+        :type may_exist:      bool<br>+        :param columns:       Dictionary of service columns<br>+                              Supported columns: app_port, in_port, out_port<br>+        :type columns:        dictionary<br>+        :returns:             :class:`Command` with no result<br>+        """<br>+<br>+    @abc.abstractmethod<br>+    def set_lservice(self, lservice_name, if_exists=True, **columns):<br>+        """Create a command to set OVN lservice fields<br>+<br>+        :param lservice_name:    The name of the lservice<br>+        :type lservice_name:     string<br>+        :param columns:       Dictionary of service columns<br>+                              Supported columns: app_port, in_port, out_port<br>+        :param if_exists:     Do not fail if lservice does not exist<br>+        :type if_exists:      bool<br>+        :type columns:        dictionary</tt><br><tt>+        :returns:             :class:`Command` with no result<br>+        """<br>+<br>+    @abc.abstractmethod<br>+    def delete_lservice(self, name=None, lswitch=None, ext_id=None,<br>+                     if_exists=True):<br>+        """Create a command to delete an OVN lservice<br>+<br>+        :param name:      The name of the lservice<br>+        :type name:       string<br>+        :param lswitch:   The name of the lswitch<br>+        :type lswitch:    string<br>+        :param ext_id:    The external id of the lservice<br>+        :type ext_id:     pair of <ext_id_key ,ext_id_value><br>+        :param if_exists: Do not fail if the lservice does not exists<br>+        :type if_exists:  bool<br>+        :returns:         :class:`Command` with no result<br>+        """<br>+<br>     @abc.abstractmethod<br>     def create_lport(self, name, lswitch_name, may_exist=True, **columns):<br>         """Create a command to add an OVN lport<br>diff --git a/networking_ovn/plugin.py b/networking_ovn/plugin.py<br>index d626b15..58dcc35 100644<br>--- a/networking_ovn/plugin.py<br>+++ b/networking_ovn/plugin.py</tt><br><tt>@@ -171,6 +171,10 @@ class OVNPlugin(db_base_plugin_v2.NeutronDbPluginV2,<br>             if not config.is_ovn_l3():<br>                 self.start_periodic_l3_agent_status_check()<br> <br>+            # start periodic check task for L3 agent<br>+            if not config.is_ovn_l3():<br>+                self.start_periodic_l3_agent_status_check()<br>+<br>     def _setup_rpc(self):<br>         self.endpoints = [dhcp_rpc.DhcpRpcCallback(),<br>                           agents_db.AgentExtRpcCallback(),<br>@@ -649,6 +653,58 @@ class OVNPlugin(db_base_plugin_v2.NeutronDbPluginV2,<br>             if security_groups:<br>                 raise psec.PortSecurityPortHasSecurityGroup()<br> <br>+    def _portsec_ext_port_create_processing(self, context, port_data, port):<br>+        # This function  is borrowed from ml2 plugin<br>+        port_security = ((port_data.get(psec.PORTSECURITY) is None) or<br>+                         port_data[psec.PORTSECURITY])<br>+<br>+        # allowed address pair checks<br>+        if self._check_update_has_allowed_address_pairs(port):<br>+            if not port_security:</tt><br><tt>+                raise addr_pair.AddressPairAndPortSecurityRequired()<br>+        else:<br>+            # remove ATTR_NOT_SPECIFIED<br>+            port['port'][addr_pair.ADDRESS_PAIRS] = []<br>+<br>+        if port_security:<br>+            self._ensure_default_security_group_on_port(context, port)<br>+        elif self._check_update_has_security_groups(port):<br>+            raise psec.PortSecurityAndIPRequiredForSecurityGroups()<br>+<br>+    def _portsec_ext_port_update_processing(self, updated_port, context, port):<br>+        # This function  is borrowed from ml2 plugin<br>+        port_security = ((updated_port.get(psec.PORTSECURITY) is None) or<br>+                         updated_port[psec.PORTSECURITY])<br>+<br>+        if port_security:<br>+            return<br>+<br>+        # check the address-pairs<br>+        if self._check_update_has_allowed_address_pairs(port):<br>+            #  has address pairs in request<br>+            raise addr_pair.AddressPairAndPortSecurityRequired()<br>+        elif (not self._check_update_deletes_allowed_address_pairs(port)):<br>+            # not a request for deleting the address-pairs</tt><br><tt>+            updated_port[addr_pair.ADDRESS_PAIRS] = (<br>+                self.get_allowed_address_pairs(context, updated_port['id']))<br>+<br>+            if updated_port[addr_pair.ADDRESS_PAIRS]:<br>+                raise addr_pair.AddressPairAndPortSecurityRequired()<br>+<br>+        # checks if security groups were updated adding/modifying<br>+        # security groups, port security is set<br>+        if self._check_update_has_security_groups(port):<br>+            raise psec.PortSecurityAndIPRequiredForSecurityGroups()<br>+        elif (not self._check_update_deletes_security_groups(port)):<br>+            # Update did not have security groups passed in. Check<br>+            # that port does not have any security groups already on it.<br>+            filters = {'port_id': [updated_port['id']]}<br>+            security_groups = self._get_port_security_group_bindings(<br>+                context, filters)<br>+<br>+            if security_groups:<br>+                raise psec.PortSecurityPortHasSecurityGroup()<br>+<br>     def _update_port_in_ovn(self, context, original_port, port,</tt><br><tt>                             ovn_port_info):<br>         external_ids = {<br>@@ -1284,6 +1340,21 @@ class OVNPlugin(db_base_plugin_v2.NeutronDbPluginV2,<br>                                              external_ids=external_ids<br>                                              ))<br> <br>+    def create_lrouter_in_ovn(self, router):<br>+        """Create lrouter in OVN<br>+<br>+        @param router: Router to be created in OVN<br>+        @return: Nothing<br>+        """<br>+<br>+        router_name = utils.ovn_name(router['id'])<br>+        external_ids = {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:<br>+                        router.get('name', 'no_router_name')}<br>+        with self._ovn.transaction(check_error=True) as txn:<br>+            txn.add(self._ovn.create_lrouter(router_name,<br>+                                             external_ids=external_ids<br>+                                             ))<br>+<br>     def delete_router(self, context, router_id):<br>         router_name = utils.ovn_name(router_id)<br>         ret_val = super(OVNPlugin, self).delete_router(context,</tt><br><tt>-- <br>1.7.1</tt><br><br><br><br><br><BR>
</body></html>