Merge "[OVN] Enhance port's extra DHCP options support" into stable/ussuri

doc/source/ovn/dhcp_opts.rst

@@ -0,0 +1,138 @@
+.. _ovn_dhcp_opts:
+
+OVN supported DHCP options
+==========================
+
+This is a list of the current supported DHCP options in ML2/OVN:
+
+IP version 4
+~~~~~~~~~~~~
+
+========================== ============================
+Option name / code         OVN value
+========================== ============================
+arp-timeout                arp_cache_timeout
+bootfile-name              bootfile_name
+classless-static-route     classless_static_route
+default-ttl                default_ttl
+dns-server                 dns_server
+domain-name                domain_name
+ethernet-encap             ethernet_encap
+ip-forward-enable          ip_forward_enable
+lease-time                 lease_time
+log-server                 log_server
+lpr-server                 lpr_server
+ms-classless-static-route  ms_classless_static_route
+mtu                        mtu
+netmask                    netmask
+nis-server                 nis_server
+ntp-server                 ntp_server
+path-prefix                path_prefix
+policy-filter              policy_filter
+router-discovery           router_discovery
+router                     router
+router-solicitation        router_solicitation
+server-id                  server_id
+server-ip-address          tftp_server_address
+swap-server                swap_server
+T1                         T1
+T2                         T2
+tcp-ttl                    tcp_ttl
+tcp-keepalive              tcp_keepalive_interval
+tftp-server-address        tftp_server_address
+tftp-server                tftp_server
+wpad                       wpad
+1                          netmask
+3                          router
+6                          dns_server
+7                          log_server
+9                          lpr_server
+15                         domain_name
+16                         swap_server
+19                         ip_forward_enable
+21                         policy_filter
+23                         default_ttl
+26                         mtu
+31                         router_discovery
+32                         router_solicitation
+35                         arp_cache_timeout
+36                         ethernet_encap
+37                         tcp_ttl
+38                         tcp_keepalive_interval
+41                         nis_server
+42                         ntp_server
+51                         lease_time
+54                         server_id
+58                         T1
+59                         T2
+66                         tftp_server
+67                         bootfile_name
+121                        classless_static_route
+150                        tftp_server_address
+210                        path_prefix
+249                        ms_classless_static_route
+252                        wpad
+========================== ============================
+
+IP version 6
+~~~~~~~~~~~~
+
+==================  =============
+Option name / code  OVN value
+==================  =============
+dns-server          dns_server
+domain-search       domain_search
+ia-addr             ip_addr
+server-id           server_id
+2                   server_id
+5                   ia_addr
+23                  dns_server
+24                  domain_search
+==================  =============
+
+OVN Database information
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+In OVN the DHCP options are stored on a table called ``DHCP_Options``
+in the OVN Northbound database.
+
+Let's add a DHCP option to a Neutron port:
+
+.. code-block:: bash
+
+    $ neutron port-update --extra-dhcp-opt opt_name='server-ip-address',opt_value='10.0.0.1' b4c3f265-369e-4bf5-8789-7caa9a1efb9c
+    Updated port: b4c3f265-369e-4bf5-8789-7caa9a1efb9c
+
+.. end
+
+To find that port in OVN we can use command below:
+
+.. code-block:: bash
+
+   $ ovn-nbctl find Logical_Switch_Port name=b4c3f265-369e-4bf5-8789-7caa9a1efb9c
+   ...
+   dhcpv4_options      : 5f00d1a2-c57d-4d1f-83ea-09bf8be13288
+   dhcpv6_options      : []
+   ...
+
+.. end
+
+For DHCP, the columns that we care about are the ``dhcpv4_options``
+and ``dhcpv6_options``. These columns has the uuids of entries in the
+``DHCP_Options`` table with the DHCP information for this port.
+
+.. code-block:: bash
+
+   $ ovn-nbctl list DHCP_Options 5f00d1a2-c57d-4d1f-83ea-09bf8be13288
+   _uuid               : 5f00d1a2-c57d-4d1f-83ea-09bf8be13288
+   cidr                : "10.0.0.0/26"
+   external_ids        : {"neutron:revision_number"="0", port_id="b4c3f265-369e-4bf5-8789-7caa9a1efb9c", subnet_id="5157ed8b-e7f1-4c56-b789-fa420098a687"}
+   options             : {classless_static_route="{169.254.169.254/32,10.0.0.2, 0.0.0.0/0,10.0.0.1}", dns_server="{8.8.8.8}", domain_name="\"openstackgate.local\"", lease_time="43200", log_server="127.0.0.3", mtu="1442", router="10.0.0.1", server_id="10.0.0.1", server_mac="fa:16:3e:dc:57:22", tftp_server_address="10.0.0.1"}
+
+.. end
+
+Here you can see that the option ``tftp_server_address`` has been set in
+the **options** column. Note that, the ``tftp_server_address`` option is
+the OVN translated name for ``server-ip-address`` (option 150). Take a
+look at the table in this document to find out more about the supported
+options and their counterpart names in OVN.

doc/source/ovn/index.rst

@@ -10,4 +10,5 @@ OVN Driver
 
    migration.rst
    gaps.rst
+   dhcp_opts.rst
    faq/index.rst

neutron/common/ovn/constants.py

@@ -87,14 +87,88 @@ ACL_ACTION_ALLOW = 'allow'
 # unhosted router gateways to schedule.
 OVN_GATEWAY_INVALID_CHASSIS = 'neutron-ovn-invalid-chassis'
 
-SUPPORTED_DHCP_OPTS = {
+# NOTE(lucasagomes): These options were last synced from
-    4: ['netmask', 'router', 'dns-server', 'log-server',
+# https://github.com/ovn-org/ovn/blob/feb5d6e81d5a0290aa3618a229c860d01200422e/lib/ovn-l7.h
-        'lpr-server', 'swap-server', 'ip-forward-enable',
+#
-        'policy-filter', 'default-ttl', 'mtu', 'router-discovery',
+# NOTE(lucasagomes): Whenever we update these lists please also update
-        'router-solicitation', 'arp-timeout', 'ethernet-encap',
+# the related documentation at doc/source/ovn/dhcp_opts.rst
-        'tcp-ttl', 'tcp-keepalive', 'nis-server', 'ntp-server',
+#
-        'tftp-server'],
+# Mappping between Neutron option names and OVN ones
-    6: ['server-id', 'dns-server', 'domain-search']}
+SUPPORTED_DHCP_OPTS_MAPPING = {
+    4: {'arp-timeout': 'arp_cache_timeout',
+        'tcp-keepalive': 'tcp_keepalive_interval',
+        'netmask': 'netmask',
+        'router': 'router',
+        'dns-server': 'dns_server',
+        'log-server': 'log_server',
+        'lpr-server': 'lpr_server',
+        'domain-name': 'domain_name',
+        'swap-server': 'swap_server',
+        'policy-filter': 'policy_filter',
+        'router-solicitation': 'router_solicitation',
+        'nis-server': 'nis_server',
+        'ntp-server': 'ntp_server',
+        'server-id': 'server_id',
+        'tftp-server': 'tftp_server',
+        'classless-static-route': 'classless_static_route',
+        'ms-classless-static-route': 'ms_classless_static_route',
+        'ip-forward-enable': 'ip_forward_enable',
+        'router-discovery': 'router_discovery',
+        'ethernet-encap': 'ethernet_encap',
+        'default-ttl': 'default_ttl',
+        'tcp-ttl': 'tcp_ttl',
+        'mtu': 'mtu',
+        'lease-time': 'lease_time',
+        'T1': 'T1',
+        'T2': 'T2',
+        'bootfile-name': 'bootfile_name',
+        'wpad': 'wpad',
+        'path-prefix': 'path_prefix',
+        'tftp-server-address': 'tftp_server_address',
+        'server-ip-address': 'tftp_server_address',
+        '1': 'netmask',
+        '3': 'router',
+        '6': 'dns_server',
+        '7': 'log_server',
+        '9': 'lpr_server',
+        '15': 'domain_name',
+        '16': 'swap_server',
+        '21': 'policy_filter',
+        '32': 'router_solicitation',
+        '35': 'arp_cache_timeout',
+        '38': 'tcp_keepalive_interval',
+        '41': 'nis_server',
+        '42': 'ntp_server',
+        '54': 'server_id',
+        '66': 'tftp_server',
+        '121': 'classless_static_route',
+        '249': 'ms_classless_static_route',
+        '19': 'ip_forward_enable',
+        '31': 'router_discovery',
+        '36': 'ethernet_encap',
+        '23': 'default_ttl',
+        '37': 'tcp_ttl',
+        '26': 'mtu',
+        '51': 'lease_time',
+        '58': 'T1',
+        '59': 'T2',
+        '67': 'bootfile_name',
+        '252': 'wpad',
+        '210': 'path_prefix',
+        '150': 'tftp_server_address'},
+    6: {'server-id': 'server_id',
+        'dns-server': 'dns_server',
+        'domain-search': 'domain_search',
+        'ia-addr': 'ip_addr',
+        '2': 'server_id',
+        '5': 'ia_addr',
+        '24': 'domain_search',
+        '23': 'dns_server'},
+}
+
+# Special option for disabling DHCP via extra DHCP options
+DHCP_DISABLED_OPT = 'dhcp_disabled'
+
 DHCPV6_STATELESS_OPT = 'dhcpv6_stateless'
 
 # When setting global DHCP options, these options will be ignored

neutron/common/ovn/utils.py

@@ -27,6 +27,7 @@ from neutron_lib import context as n_context
 from neutron_lib import exceptions as n_exc
 from neutron_lib.plugins import directory
 from neutron_lib.utils import net as n_utils
+from oslo_log import log
 from oslo_utils import netutils
 from oslo_utils import strutils
 from ovs.db import idl
@@ -37,12 +38,17 @@ from neutron._i18n import _
 from neutron.common.ovn import constants
 from neutron.common.ovn import exceptions as ovn_exc
 
+LOG = log.getLogger(__name__)
+
 
 DNS_RESOLVER_FILE = "/etc/resolv.conf"
 
 AddrPairsDiff = collections.namedtuple(
     'AddrPairsDiff', ['added', 'removed', 'changed'])
 
+PortExtraDHCPValidation = collections.namedtuple(
+    'PortExtraDHCPValidation', ['failed', 'invalid_ipv4', 'invalid_ipv6'])
+
 
 def ovn_name(id):
     # The name of the OVN entry will be neutron-<UUID>
@@ -109,6 +115,39 @@ def is_network_device_port(port):
         const.DEVICE_OWNER_PREFIXES)
 
 
+def _is_dhcp_disabled(dhcp_opt):
+    return (dhcp_opt['opt_name'] == constants.DHCP_DISABLED_OPT and
+            dhcp_opt.get('opt_value', '').lower() == 'true')
+
+
+def validate_port_extra_dhcp_opts(port):
+    """Validate port's extra DHCP options.
+
+    :param port: A neutron port.
+    :returns: A PortExtraDHCPValidation object.
+    """
+    invalid = {const.IP_VERSION_4: [], const.IP_VERSION_6: []}
+    failed = False
+    for edo in port.get(edo_ext.EXTRADHCPOPTS, []):
+        ip_version = edo['ip_version']
+        opt_name = edo['opt_name']
+
+        # If DHCP is disabled for this port via this special option,
+        # always succeed the validation
+        if _is_dhcp_disabled(edo):
+            failed = False
+            break
+
+        if opt_name not in constants.SUPPORTED_DHCP_OPTS_MAPPING[ip_version]:
+            invalid[ip_version].append(opt_name)
+            failed = True
+
+    return PortExtraDHCPValidation(
+        failed=failed,
+        invalid_ipv4=invalid[const.IP_VERSION_4] if failed else [],
+        invalid_ipv6=invalid[const.IP_VERSION_6] if failed else [])
+
+
 def get_lsp_dhcp_opts(port, ip_version):
     # Get dhcp options from Neutron port, for setting DHCP_Options row
     # in OVN.
@@ -117,12 +156,12 @@ def get_lsp_dhcp_opts(port, ip_version):
     if is_network_device_port(port):
         lsp_dhcp_disabled = True
     else:
+        mapping = constants.SUPPORTED_DHCP_OPTS_MAPPING[ip_version]
         for edo in port.get(edo_ext.EXTRADHCPOPTS, []):
             if edo['ip_version'] != ip_version:
                 continue
 
-            if edo['opt_name'] == 'dhcp_disabled' and (
+            if _is_dhcp_disabled(edo):
-                    edo['opt_value'] in ['True', 'true']):
                 # OVN native DHCP is disabled on this port
                 lsp_dhcp_disabled = True
                 # Make sure return value behavior not depends on the order and
@@ -130,11 +169,13 @@ def get_lsp_dhcp_opts(port, ip_version):
                 lsp_dhcp_opts.clear()
                 break
 
-            if edo['opt_name'] not in (
+            if edo['opt_name'] not in mapping:
-                    constants.SUPPORTED_DHCP_OPTS[ip_version]):
+                LOG.warning('The DHCP option %(opt_name)s on port %(port)s '
+                            'is not suppported by OVN, ignoring it',
+                            {'opt_name': edo['opt_name'], 'port': port['id']})
                 continue
 
-            opt = edo['opt_name'].replace('-', '_')
+            opt = mapping[edo['opt_name']]
             lsp_dhcp_opts[opt] = edo['opt_value']
 
     return (lsp_dhcp_disabled, lsp_dhcp_opts)

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

@@ -456,6 +456,19 @@ class OVNMechanismDriver(api.MechanismDriver):
         self._ovn_client.delete_subnet(context._plugin_context,
                                        context.current['id'])
 
+    def _validate_port_extra_dhcp_opts(self, port):
+        result = ovn_utils.validate_port_extra_dhcp_opts(port)
+        if not result.failed:
+            return
+        ipv4_opts = ', '.join(result.invalid_ipv4)
+        ipv6_opts = ', '.join(result.invalid_ipv6)
+        msg = (_('The following extra DHCP options for port %(port_id)s '
+                 'are not supported by OVN. IPv4: "%(ipv4_opts)s" and '
+                 'IPv6: "%(ipv6_opts)s"') %
+               {'port_id': port['id'], 'ipv4_opts': ipv4_opts,
+                'ipv6_opts': ipv6_opts})
+        raise OVNPortUpdateError(resource='port', msg=msg)
+
     def create_port_precommit(self, context):
         """Allocate resources for a new port.
 
@@ -470,6 +483,7 @@ class OVNMechanismDriver(api.MechanismDriver):
         if ovn_utils.is_lsp_ignored(port):
             return
         ovn_utils.validate_and_get_data_from_binding_profile(port)
+        self._validate_port_extra_dhcp_opts(port)
         if self._is_port_provisioning_required(port, context.host):
             self._insert_port_provisioning_block(context._plugin_context,
                                                  port['id'])
@@ -584,6 +598,7 @@ class OVNMechanismDriver(api.MechanismDriver):
         original_port = context.original
         self._validate_ignored_port(port, original_port)
         ovn_utils.validate_and_get_data_from_binding_profile(port)
+        self._validate_port_extra_dhcp_opts(port)
         if self._is_port_provisioning_required(port, context.host,
                                                context.original_host):
             self._insert_port_provisioning_block(context._plugin_context,

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

@@ -14,6 +14,8 @@
 #    under the License.
 
 import fixtures
+import mock
+from neutron_lib.api.definitions import extra_dhcp_opt as edo_ext
 
 from neutron.common.ovn import constants
 from neutron.common.ovn import utils
@@ -118,3 +120,114 @@ class TestGateWayChassisValidity(base.BaseTestCase):
         self.assertTrue(utils.is_gateway_chassis_invalid(
             self.chassis_name, self.gw_chassis, self.physnet,
             self.chassis_physnets))
+
+
+class TestDHCPUtils(base.BaseTestCase):
+
+    def test_validate_port_extra_dhcp_opts_empty(self):
+        port = {edo_ext.EXTRADHCPOPTS: []}
+        result = utils.validate_port_extra_dhcp_opts(port)
+        self.assertFalse(result.failed)
+        self.assertEqual([], result.invalid_ipv4)
+        self.assertEqual([], result.invalid_ipv6)
+
+    def test_validate_port_extra_dhcp_opts_dhcp_disabled(self):
+        opt0 = {'opt_name': 'not-valid-ipv4',
+                'opt_value': 'joe rogan',
+                'ip_version': 4}
+        opt1 = {'opt_name': 'dhcp_disabled',
+                'opt_value': 'True',
+                'ip_version': 4}
+        port = {edo_ext.EXTRADHCPOPTS: [opt0, opt1]}
+
+        # Validation always succeeds if the "dhcp_disabled" option is enabled
+        result = utils.validate_port_extra_dhcp_opts(port)
+        self.assertFalse(result.failed)
+        self.assertEqual([], result.invalid_ipv4)
+        self.assertEqual([], result.invalid_ipv6)
+
+    def test_validate_port_extra_dhcp_opts(self):
+        opt0 = {'opt_name': 'bootfile-name',
+                'opt_value': 'homer_simpson.bin',
+                'ip_version': 4}
+        opt1 = {'opt_name': 'dns-server',
+                'opt_value': '2001:4860:4860::8888',
+                'ip_version': 6}
+        port = {edo_ext.EXTRADHCPOPTS: [opt0, opt1]}
+
+        result = utils.validate_port_extra_dhcp_opts(port)
+        self.assertFalse(result.failed)
+        self.assertEqual([], result.invalid_ipv4)
+        self.assertEqual([], result.invalid_ipv6)
+
+    def test_validate_port_extra_dhcp_opts_invalid(self):
+        # Two value options and two invalid, assert the validation
+        # will fail and only the invalid options will be returned as
+        # not supported
+        opt0 = {'opt_name': 'bootfile-name',
+                'opt_value': 'homer_simpson.bin',
+                'ip_version': 4}
+        opt1 = {'opt_name': 'dns-server',
+                'opt_value': '2001:4860:4860::8888',
+                'ip_version': 6}
+        opt2 = {'opt_name': 'not-valid-ipv4',
+                'opt_value': 'joe rogan',
+                'ip_version': 4}
+        opt3 = {'opt_name': 'not-valid-ipv6',
+                'opt_value': 'young jamie',
+                'ip_version': 6}
+        port = {edo_ext.EXTRADHCPOPTS: [opt0, opt1, opt2, opt3]}
+
+        result = utils.validate_port_extra_dhcp_opts(port)
+        self.assertTrue(result.failed)
+        self.assertEqual(['not-valid-ipv4'], result.invalid_ipv4)
+        self.assertEqual(['not-valid-ipv6'], result.invalid_ipv6)
+
+    def test_get_lsp_dhcp_opts_empty(self):
+        port = {edo_ext.EXTRADHCPOPTS: []}
+        dhcp_disabled, options = utils.get_lsp_dhcp_opts(port, 4)
+        self.assertFalse(dhcp_disabled)
+        self.assertEqual({}, options)
+
+    def test_get_lsp_dhcp_opts_empty_dhcp_disabled(self):
+        opt0 = {'opt_name': 'bootfile-name',
+                'opt_value': 'homer_simpson.bin',
+                'ip_version': 4}
+        opt1 = {'opt_name': 'dhcp_disabled',
+                'opt_value': 'True',
+                'ip_version': 4}
+        port = {edo_ext.EXTRADHCPOPTS: [opt0, opt1]}
+
+        # Validation always succeeds if the "dhcp_disabled" option is enabled
+        dhcp_disabled, options = utils.get_lsp_dhcp_opts(port, 4)
+        self.assertTrue(dhcp_disabled)
+        self.assertEqual({}, options)
+
+    @mock.patch.object(utils, 'is_network_device_port')
+    def test_get_lsp_dhcp_opts_is_network_device_port(self, mock_device_port):
+        mock_device_port.return_value = True
+        port = {}
+        dhcp_disabled, options = utils.get_lsp_dhcp_opts(port, 4)
+        # Assert OVN DHCP is disabled
+        self.assertTrue(dhcp_disabled)
+        self.assertEqual({}, options)
+
+    def test_get_lsp_dhcp_opts(self):
+        opt0 = {'opt_name': 'bootfile-name',
+                'opt_value': 'homer_simpson.bin',
+                'ip_version': 4}
+        opt1 = {'opt_name': 'server-ip-address',
+                'opt_value': '10.0.0.1',
+                'ip_version': 4}
+        opt2 = {'opt_name': '42',
+                'opt_value': '10.0.2.1',
+                'ip_version': 4}
+        port = {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
+        expected_options = {'tftp_server_address': '10.0.0.1',
+                            'ntp_server': '10.0.2.1',
+                            'bootfile_name': 'homer_simpson.bin'}
+        self.assertEqual(expected_options, options)

neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py

@@ -18,6 +18,7 @@ import uuid
 
 import mock
 from neutron_lib.api.definitions import external_net
+from neutron_lib.api.definitions import extra_dhcp_opt as edo_ext
 from neutron_lib.api.definitions import portbindings
 from neutron_lib.api.definitions import provider_net as pnet
 from neutron_lib.callbacks import events
@@ -306,6 +307,56 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase):
                           self.mech_driver._validate_ignored_port,
                           p, ori_p)
 
+    def test__validate_port_extra_dhcp_opts(self):
+        opt = {'opt_name': 'bootfile-name',
+               'opt_value': 'homer_simpson.bin',
+               'ip_version': 4}
+        port = {edo_ext.EXTRADHCPOPTS: [opt], 'id': 'fake-port'}
+        self.assertIsNone(
+            self.mech_driver._validate_port_extra_dhcp_opts(port))
+
+    def test__validate_port_extra_dhcp_opts_invalid(self):
+        opt = {'opt_name': 'not-valid',
+               'opt_value': 'spongebob squarepants',
+               'ip_version': 4}
+        port = {edo_ext.EXTRADHCPOPTS: [opt], 'id': 'fake-port'}
+        self.assertRaises(mech_driver.OVNPortUpdateError,
+                          self.mech_driver._validate_port_extra_dhcp_opts,
+                          port)
+
+    def test_create_port_invalid_extra_dhcp_opts(self):
+        extra_dhcp_opts = {
+            'extra_dhcp_opts': [{'ip_version': 4, 'opt_name': 'banana',
+                                 'opt_value': 'banana'},
+                                {'ip_version': 6, 'opt_name': 'orange',
+                                 'opt_value': 'orange'}]
+        }
+        with self.network() as n:
+            with self.subnet(n):
+                res = self._create_port(self.fmt, n['network']['id'],
+                                        arg_list=('extra_dhcp_opts',),
+                                        **extra_dhcp_opts)
+                # Assert 400 (BadRequest) was returned
+                self.assertEqual(400, res.status_code)
+                response = self.deserialize(self.fmt, res)
+                self.assertIn('banana', response['NeutronError']['message'])
+                self.assertIn('orange', response['NeutronError']['message'])
+
+    def test_update_port_invalid_extra_dhcp_opts(self):
+        data = {
+            'port': {'extra_dhcp_opts': [{'ip_version': 4, 'opt_name': 'apple',
+                                         'opt_value': 'apple'},
+                                         {'ip_version': 6, 'opt_name': 'grape',
+                                         'opt_value': 'grape'}]}}
+        with self.network(set_context=True, tenant_id='test') as net:
+            with self.subnet(network=net) as subnet:
+                with self.port(subnet=subnet,
+                               set_context=True, tenant_id='test') as port:
+                    res = self._update('ports', port['port']['id'], data,
+                                       expected_code=400)
+                    self.assertIn('apple', res['NeutronError']['message'])
+                    self.assertIn('grape', res['NeutronError']['message'])
+
     def test_create_and_update_ignored_fip_port(self):
         with self.network(set_context=True, tenant_id='test') as net1:
             with self.subnet(network=net1) as subnet1: