[OVN] Add baremetal support without Neutron DHCP agent for IPv4

doc/source/ovn/gaps.rst

@@ -32,23 +32,25 @@ at [1]_.
   can announce host routes for both floating and fixed IP addresses. These
   functions are not supported in OVN.
 
-* Baremetal provisioning with iPXE
+* Baremetal provisioning with iPXE without Neutron DHCP agent for IPv6
 
-  The core OVN DHCP server implementation does not have support for
-  sending different boot options based on the ``gpxe`` DHCP Option
-  (no. 175). Also, Ironic uses dnsmasq syntax when configuring the DHCP
-  options for Neutron [2]_ which is not understood by the OVN driver.
+  The core OVN built-in DHCP server implementation does not
+  yet support PXE booting for IPv6. This can be achieved at
+  the moment if used with the Neutron DHCP agent by deploying it
+  on OVN gateway nodes and disabling the OVN DHCP by setting the
+  ``[ovn]/disable_ovn_dhcp_for_baremetal_ports`` configuration option
+  to True.
 
 * QoS minimum bandwidth allocation in Placement API
 
   ML2/OVN integration with the Nova placement API to provide guaranteed
-  minimum bandwidth for ports [3]_.
+  minimum bandwidth for ports [4]_. Work in progress, see [5]_
 
 * IPv6 Prefix Delegation
 
   Currently ML2/OVN doesn't implement IPv6 prefix delegation. OVN logical
-  routers have this capability implemented in [4]_ and we have an open RFE to
-  fill this gap [5]_.
+  routers have this capability implemented in [6]_ and we have an open RFE to
+  fill this gap [7]_.
 
 * East/West Fragmentation
 
@@ -62,11 +64,12 @@ at [1]_.
   from instances to reach the DHCP agent. For OVN this traffic has to be explicitly
   allowed by security group rules attached to the instance. Note that the default
   security group does allow all outgoing traffic, so this only becomes relevant
-  when using custom security groups [6]_.
+  when using custom security groups [8]_. Proposed patch is [9]_ but it
+  needs to be revived and updated.
 
 * DNS resolution for instances
 
-  OVN cannot use the host's networking for DNS resolution, so Case 2b in [7]_ can
+  OVN cannot use the host's networking for DNS resolution, so Case 2b in [10]_ can
   only be used when additional DHCP agents are deployed. For Case 2a a different
   configuration option has to be used in ``ml2_conf.ini``::
 
@@ -82,11 +85,12 @@ References
 ----------
 
 .. [1] https://github.com/ovn-org/ovn/blob/master/TODO.rst
-.. [2] https://github.com/openstack/ironic/blob/123cb22c731f93d0c608d791b41e05884fe18c04/ironic/common/pxe_utils.py#L447-L462>
-.. [3] https://specs.openstack.org/openstack/neutron-specs/specs/rocky/minimum-bandwidth-allocation-placement-api.html
-.. [4] https://patchwork.ozlabs.org/project/openvswitch/patch/6aec0fb280f610a2083fbb6c61e251b1d237b21f.1576840560.git.lorenzo.bianconi@redhat.com/
-.. [5] https://bugs.launchpad.net/neutron/+bug/1895972
-.. [6] https://bugs.launchpad.net/neutron/+bug/1926515
-.. [7] https://docs.openstack.org/neutron/latest/admin/config-dns-res.html
-.. [8] https://bugs.launchpad.net/neutron/+bug/1951816
-.. [9] https://bugs.launchpad.net/neutron/+bug/1950686
+.. [2] https://bugzilla.redhat.com/show_bug.cgi?id=2060310
+.. [3] https://review.opendev.org/c/openstack/neutron/+/842292
+.. [4] https://specs.openstack.org/openstack/neutron-specs/specs/rocky/minimum-bandwidth-allocation-placement-api.html
+.. [5] https://review.opendev.org/c/openstack/neutron/+/786478
+.. [6] https://patchwork.ozlabs.org/project/openvswitch/patch/6aec0fb280f610a2083fbb6c61e251b1d237b21f.1576840560.git.lorenzo.bianconi@redhat.com/
+.. [7] https://bugs.launchpad.net/neutron/+bug/1895972
+.. [8] https://bugs.launchpad.net/neutron/+bug/1926515
+.. [9] https://review.opendev.org/c/openstack/neutron/+/788594
+.. [10] https://docs.openstack.org/neutron/latest/admin/config-dns-res.html

neutron/common/ovn/constants.py

@@ -11,6 +11,7 @@
 #    under the License.
 
 import collections
+import copy
 import re
 import uuid
 
@@ -129,10 +130,11 @@
         'T1': 'T1',
         'T2': 'T2',
         'bootfile-name': 'bootfile_name',
+        'bootfile-name-alt': 'bootfile_name_alt',
         'wpad': 'wpad',
         'path-prefix': 'path_prefix',
         'tftp-server-address': 'tftp_server_address',
-        'server-ip-address': 'tftp_server_address',
+        'server-ip-address': 'next_server',
         '1': 'netmask',
         '3': 'router',
         '6': 'dns_server',
@@ -173,10 +175,20 @@
         '23': 'dns_server'},
 }
 
+# Baremetal specific DHCP options for VNIC_BAREMETAL ports
+SUPPORTED_BM_DHCP_OPTS_MAPPING = copy.deepcopy(
+    SUPPORTED_DHCP_OPTS_MAPPING)
+SUPPORTED_BM_DHCP_OPTS_MAPPING[4].update({
+    'tag:ipxe,bootfile-name': 'bootfile_name',
+    'tag:ipxe,67': 'bootfile_name',
+    'tag:!ipxe,bootfile-name': 'bootfile_name_alt',
+    'tag:!ipxe,67': 'bootfile_name_alt'})
+
 # OVN string type DHCP options
 OVN_STR_TYPE_DHCP_OPTS = [
     'domain_name',
     'bootfile_name',
+    'bootfile_name_alt',
     'path_prefix',
     'wpad',
     'tftp_server']

neutron/common/ovn/utils.py

@@ -137,6 +137,12 @@ def validate_port_extra_dhcp_opts(port):
     :param port: A neutron port.
     :returns: A PortExtraDHCPValidation object.
     """
+    # Get the right option mappings according to the port's vnic_type
+    vnic_type = port.get(portbindings.VNIC_TYPE, portbindings.VNIC_NORMAL)
+    mapping = constants.SUPPORTED_DHCP_OPTS_MAPPING
+    if vnic_type == portbindings.VNIC_BAREMETAL:
+        mapping = constants.SUPPORTED_BM_DHCP_OPTS_MAPPING
+
     invalid = {const.IP_VERSION_4: [], const.IP_VERSION_6: []}
     failed = False
     for edo in port.get(edo_ext.EXTRADHCPOPTS, []):
@@ -149,7 +155,7 @@ def validate_port_extra_dhcp_opts(port):
             failed = False
             break
 
-        if opt_name not in constants.SUPPORTED_DHCP_OPTS_MAPPING[ip_version]:
+        if opt_name not in mapping[ip_version]:
             invalid[ip_version].append(opt_name)
             failed = True
 
@@ -169,14 +175,16 @@ def get_lsp_dhcp_opts(port, ip_version):
     lsp_dhcp_disabled = False
     lsp_dhcp_opts = {}
     vnic_type = port.get(portbindings.VNIC_TYPE, portbindings.VNIC_NORMAL)
+    is_baremetal = vnic_type == portbindings.VNIC_BAREMETAL
 
-    # NOTE(lucasagomes): Baremetal does not yet work with OVN's built-in
-    # DHCP server, disable it for now
-    if (is_network_device_port(port) or
-            vnic_type == portbindings.VNIC_BAREMETAL):
+    if is_network_device_port(port):
+        lsp_dhcp_disabled = True
+    elif is_baremetal and ovn_conf.is_ovn_dhcp_disabled_for_baremetal():
         lsp_dhcp_disabled = True
     else:
-        mapping = constants.SUPPORTED_DHCP_OPTS_MAPPING[ip_version]
+        mapping = (constants.SUPPORTED_BM_DHCP_OPTS_MAPPING[ip_version]
+                   if is_baremetal else
+                   constants.SUPPORTED_DHCP_OPTS_MAPPING[ip_version])
         for edo in port.get(edo_ext.EXTRADHCPOPTS, []):
             if edo['ip_version'] != ip_version:
                 continue

neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py

@@ -197,6 +197,12 @@
                        'or by checking the output of the following command: \n'
                        'ovs-appctl -t ovs-vswitchd dpif/show-dp-features '
                        'br-int | grep "Check pkt length action".')),
+    cfg.BoolOpt('disable_ovn_dhcp_for_baremetal_ports',
+                default=False,
+                help=_('Disable OVN\'s built-in DHCP for baremetal ports '
+                       '(VNIC type "baremetal"). This alllow operators to '
+                       'plug their own DHCP server of choice for PXE booting '
+                       'baremetal nodes. Defaults to False.')),
 ]
 
 cfg.CONF.register_opts(ovn_opts, group='ovn')
@@ -304,3 +310,7 @@ def is_ovn_emit_need_to_frag_enabled():
 
 def is_igmp_snooping_enabled():
     return cfg.CONF.OVS.igmp_snooping_enable
+
+
+def is_ovn_dhcp_disabled_for_baremetal():
+    return cfg.CONF.ovn.disable_ovn_dhcp_for_baremetal_ports

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py

@@ -21,6 +21,7 @@
 
 from futurist import periodics
 from neutron_lib.api.definitions import external_net
+from neutron_lib.api.definitions import portbindings
 from neutron_lib.api.definitions import provider_net as pnet
 from neutron_lib.api.definitions import segment as segment_def
 from neutron_lib import constants as n_const
@@ -840,6 +841,58 @@ def update_port_virtual_type(self):
                     txn.add(cmd)
         raise periodics.NeverAgain()
 
+    # A static spacing value is used here, but this method will only run
+    # once per lock due to the use of periodics.NeverAgain().
+    @periodics.periodic(spacing=600, run_immediately=True)
+    def check_baremetal_ports_dhcp_options(self):
+        """Update baremetal ports DHCP options
+
+        Update baremetal ports DHCP options based on the
+        "disable_ovn_dhcp_for_baremetal_ports" configuration option.
+        """
+        # If external ports is not supported stop running
+        # this periodic task
+        if not self._ovn_client.is_external_ports_supported():
+            raise periodics.NeverAgain()
+
+        if not self.has_lock:
+            return
+
+        context = n_context.get_admin_context()
+        ports = self._ovn_client._plugin.get_ports(
+            context,
+            filters={portbindings.VNIC_TYPE: portbindings.VNIC_BAREMETAL})
+        if not ports:
+            raise periodics.NeverAgain()
+
+        with self._nb_idl.transaction(check_error=True) as txn:
+            for port in ports:
+                lsp = self._nb_idl.lsp_get(port['id']).execute(
+                    check_error=True)
+                if not lsp:
+                    continue
+
+                update_dhcp = False
+                if ovn_conf.is_ovn_dhcp_disabled_for_baremetal():
+                    if lsp.dhcpv4_options or lsp.dhcpv6_options:
+                        update_dhcp = True
+                else:
+                    if not lsp.dhcpv4_options and not lsp.dhcpv6_options:
+                        update_dhcp = True
+
+                if update_dhcp:
+                    port_info = self._ovn_client._get_port_options(port)
+                    dhcpv4_options, dhcpv6_options = (
+                        self._ovn_client.update_port_dhcp_options(
+                            port_info, txn))
+                    txn.add(self._nb_idl.set_lswitch_port(
+                                lport_name=port['id'],
+                                dhcpv4_options=dhcpv4_options,
+                                dhcpv6_options=dhcpv6_options,
+                                if_exists=False))
+
+        raise periodics.NeverAgain()
+
 
 class HashRingHealthCheckPeriodics(object):
 

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py

@@ -475,6 +475,24 @@ def sync_ha_chassis_group(self, context, network_id, txn):
 
         return ha_ch_grp.uuid
 
+    def update_port_dhcp_options(self, port_info, txn):
+        dhcpv4_options = []
+        dhcpv6_options = []
+        if not port_info.dhcpv4_options:
+            dhcpv4_options = []
+        elif 'cmd' in port_info.dhcpv4_options:
+            dhcpv4_options = txn.add(port_info.dhcpv4_options['cmd'])
+        else:
+            dhcpv4_options = [port_info.dhcpv4_options['uuid']]
+        if not port_info.dhcpv6_options:
+            dhcpv6_options = []
+        elif 'cmd' in port_info.dhcpv6_options:
+            dhcpv6_options = txn.add(port_info.dhcpv6_options['cmd'])
+        else:
+            dhcpv6_options = [port_info.dhcpv6_options['uuid']]
+
+        return (dhcpv4_options, dhcpv6_options)
+
     def create_port(self, context, port):
         if utils.is_lsp_ignored(port):
             return
@@ -505,18 +523,8 @@ def create_port(self, context, port):
             'Logical_Switch', 'name', lswitch_name)
 
         with self._nb_idl.transaction(check_error=True) as txn:
-            if not port_info.dhcpv4_options:
-                dhcpv4_options = []
-            elif 'cmd' in port_info.dhcpv4_options:
-                dhcpv4_options = txn.add(port_info.dhcpv4_options['cmd'])
-            else:
-                dhcpv4_options = [port_info.dhcpv4_options['uuid']]
-            if not port_info.dhcpv6_options:
-                dhcpv6_options = []
-            elif 'cmd' in port_info.dhcpv6_options:
-                dhcpv6_options = txn.add(port_info.dhcpv6_options['cmd'])
-            else:
-                dhcpv6_options = [port_info.dhcpv6_options['uuid']]
+            dhcpv4_options, dhcpv6_options = self.update_port_dhcp_options(
+                port_info, txn=txn)
             # The lport_name *must* be neutron port['id'].  It must match the
             # iface-id set in the Interfaces table of the Open_vSwitch
             # database which nova sets to be the port ID.
@@ -642,18 +650,9 @@ def update_port(self, context, port, port_object=None):
             else:
                 columns_dict['type'] = port_info.type
                 columns_dict['addresses'] = port_info.addresses
-            if not port_info.dhcpv4_options:
-                dhcpv4_options = []
-            elif 'cmd' in port_info.dhcpv4_options:
-                dhcpv4_options = txn.add(port_info.dhcpv4_options['cmd'])
-            else:
-                dhcpv4_options = [port_info.dhcpv4_options['uuid']]
-            if not port_info.dhcpv6_options:
-                dhcpv6_options = []
-            elif 'cmd' in port_info.dhcpv6_options:
-                dhcpv6_options = txn.add(port_info.dhcpv6_options['cmd'])
-            else:
-                dhcpv6_options = [port_info.dhcpv6_options['uuid']]
+
+            dhcpv4_options, dhcpv6_options = self.update_port_dhcp_options(
+                port_info, txn=txn)
 
             if self.is_metadata_port(port):
                 context = n_context.get_admin_context()

neutron/tests/unit/common/ovn/test_utils.py

@@ -264,6 +264,10 @@ def test_gateway_chassis_for_chassis_not_in_gw_chassis_list(self):
 
 class TestDHCPUtils(base.BaseTestCase):
 
+    def setUp(self):
+        ovn_conf.register_opts()
+        super(TestDHCPUtils, self).setUp()
+
     def test_validate_port_extra_dhcp_opts_empty(self):
         port = {edo_ext.EXTRADHCPOPTS: []}
         result = utils.validate_port_extra_dhcp_opts(port)
@@ -367,11 +371,49 @@ def test_get_lsp_dhcp_opts(self):
         dhcp_disabled, options = utils.get_lsp_dhcp_opts(port, 4)
         self.assertFalse(dhcp_disabled)
         # Assert the names got translated to their OVN names
-        expected_options = {'tftp_server_address': '10.0.0.1',
+        expected_options = {'next_server': '10.0.0.1',
                             'ntp_server': '10.0.2.1',
                             'bootfile_name': '"homer_simpson.bin"'}
         self.assertEqual(expected_options, options)
 
+    def test_get_lsp_dhcp_opts_for_baremetal(self):
+        opt0 = {'opt_name': 'tag:ipxe,bootfile-name',
+                'opt_value': 'http://172.7.27.29/ipxe',
+                'ip_version': 4}
+        opt1 = {'opt_name': 'tag:!ipxe,bootfile-name',
+                'opt_value': 'undionly.kpxe',
+                'ip_version': 4}
+        opt2 = {'opt_name': 'tftp-server',
+                'opt_value': '"172.7.27.29"',
+                'ip_version': 4}
+        port = {portbindings.VNIC_TYPE: portbindings.VNIC_BAREMETAL,
+                edo_ext.EXTRADHCPOPTS: [opt0, opt1, opt2]}
+
+        dhcp_disabled, options = utils.get_lsp_dhcp_opts(port, 4)
+        self.assertFalse(dhcp_disabled)
+        # Assert the names got translated to their OVN names and the
+        # options that weren't double-quoted are now double-quoted
+        expected_options = {'tftp_server': '"172.7.27.29"',
+                            'bootfile_name': '"http://172.7.27.29/ipxe"',
+                            'bootfile_name_alt': '"undionly.kpxe"'}
+        self.assertEqual(expected_options, options)
+
+    def test_get_lsp_dhcp_opts_dhcp_disabled_for_baremetal(self):
+        cfg.CONF.set_override(
+            'disable_ovn_dhcp_for_baremetal_ports', True, group='ovn')
+
+        opt = {'opt_name': 'tag:ipxe,bootfile-name',
+               'opt_value': 'http://172.7.27.29/ipxe',
+               'ip_version': 4}
+        port = {portbindings.VNIC_TYPE: portbindings.VNIC_BAREMETAL,
+                edo_ext.EXTRADHCPOPTS: [opt]}
+
+        dhcp_disabled, options = utils.get_lsp_dhcp_opts(port, 4)
+        # Assert DHCP is disabled for this port
+        self.assertTrue(dhcp_disabled)
+        # Assert no options were passed
+        self.assertEqual({}, options)
+
 
 class TestGetDhcpDnsServers(base.BaseTestCase):