Browse Source

[OVN] Enhance port's extra DHCP options support

Prior to this patch OVN did not validate any extra DHCP option passed
to the port leading to confusion because the user of the API could just
input any value and OVN would accept it (returning 200) but ignoring the
option internally.

This patch now adds such validations on port creation and update.

This patch also sync with the latest supported DHCP options from OVN and
create a map between the different names and option codes to their OVN
counterpart.

Closes-bug: #1874282
Change-Id: I99799e54e941cdd8da2614feecad1ef6299703fc
Signed-off-by: Lucas Alvares Gomes <lucasagomes@gmail.com>
changes/25/722025/8
Lucas Alvares Gomes 2 years ago
parent
commit
f6010f6042
  1. 138
      doc/source/ovn/dhcp_opts.rst
  2. 1
      doc/source/ovn/index.rst
  3. 90
      neutron/common/ovn/constants.py
  4. 51
      neutron/common/ovn/utils.py
  5. 15
      neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py
  6. 113
      neutron/tests/unit/common/ovn/test_utils.py
  7. 51
      neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py

138
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.

1
doc/source/ovn/index.rst

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

90
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 = {
4: ['netmask', 'router', 'dns-server', 'log-server',
'lpr-server', 'swap-server', 'ip-forward-enable',
'policy-filter', 'default-ttl', 'mtu', 'router-discovery',
'router-solicitation', 'arp-timeout', 'ethernet-encap',
'tcp-ttl', 'tcp-keepalive', 'nis-server', 'ntp-server',
'tftp-server'],
6: ['server-id', 'dns-server', 'domain-search']}
# NOTE(lucasagomes): These options were last synced from
# https://github.com/ovn-org/ovn/blob/feb5d6e81d5a0290aa3618a229c860d01200422e/lib/ovn-l7.h
#
# NOTE(lucasagomes): Whenever we update these lists please also update
# the related documentation at doc/source/ovn/dhcp_opts.rst
#
# Mappping between Neutron option names and OVN ones
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

51
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 (
edo['opt_value'] in ['True', 'true']):
if _is_dhcp_disabled(edo):
# 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 (
constants.SUPPORTED_DHCP_OPTS[ip_version]):
if edo['opt_name'] not in mapping:
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)

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

@ -481,6 +481,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.
@ -495,6 +508,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'])
@ -609,6 +623,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,

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

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

@ -18,6 +18,7 @@ from unittest import mock
import uuid
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
@ -349,6 +350,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:

Loading…
Cancel
Save