Neutron port hints

Introduce the hints port attribute that allows passing in backend specific hints mainly to allow backend specific performance tuning. Enable: openstack port create --hint ALIAS=VALUE openstack port set --hint ALIAS=VALUE openstack port unset --hints Required neutron extension: port-hints port-hint-ovs-tx-steering Valid hint aliases and values: ovs-tx-steering=hash ovs-tx-steering=thread The same hints in JSON format, as expected by the Neutron API: {"openvswitch": {"other_config": {"tx-steering": "hash"}}} {"openvswitch": {"other_config": {"tx-steering": "thread"}}} Change-Id: I4c7142909b1e4fb26fc77ad9ba08ec994cc450b2 Depends-On: https://review.opendev.org/c/openstack/neutron/+/873113 Partial-Bug: #1990842 Related-Change (server side): https://review.opendev.org/c/openstack/neutron/+/873113 Related-Change (spec): https://review.opendev.org/c/openstack/neutron-specs/+/862133
2023-04-25 14:22:07 +02:00 · 2023-04-25 14:22:07 +02:00 · 22d1a26d1d
commit 22d1a26d1d
parent f3a51b0051
4 changed files with 366 additions and 2 deletions
--- a/openstackclient/network/v2/port.py
+++ b/openstackclient/network/v2/port.py
@ -322,6 +322,22 @@ def _add_updatable_args(parser):
        action='store_true',
        help=_("NUMA affinity policy using legacy mode to schedule this port"),
    )
+    parser.add_argument(
+        '--hint',
+        metavar='<alias=value>',
+        action=JSONKeyValueAction,
+        default={},
+        help=_(
+            'Port hints as ALIAS=VALUE or as JSON. '
+            'Valid hint aliases/values: '
+            'ovs-tx-steering=thread, ovs-tx-steering=hash. '
+            'Valid JSON values are as specified by the Neutron API. '
+            '(requires port-hints extension) '
+            '(requires port-hint-ovs-tx-steering extension for alias: '
+            'ovs-tx-steering) '
+            '(repeat option to set multiple hints)'
+        ),
+    )


 # TODO(abhiraut): Use the SDK resource mapped attribute names once the
@ -350,6 +366,34 @@ def _convert_extra_dhcp_options(parsed_args):
    return dhcp_options


+# When we have multiple hints, we'll need to refactor this to allow
+# arbitrary combinations. But until then let's have it as simple as possible.
+def _validate_port_hints(hints):
+    if hints not in (
+        {},
+        # by hint alias
+        {'ovs-tx-steering': 'thread'},
+        {'ovs-tx-steering': 'hash'},
+        # by fully specified value of the port's hints field
+        {'openvswitch': {'other_config': {'tx-steering': 'thread'}}},
+        {'openvswitch': {'other_config': {'tx-steering': 'hash'}}},
+    ):
+        msg = _("Invalid value to --hints, see --help for valid values.")
+        raise argparse.ArgumentTypeError(msg)
+
+
+# When we have multiple hints, we'll need to refactor this to expand aliases
+# without losing other hints. But until then let's have it as simple as
+# possible.
+def _expand_port_hint_aliases(hints):
+    if hints == {'ovs-tx-steering': 'thread'}:
+        return {'openvswitch': {'other_config': {'tx-steering': 'thread'}}}
+    elif hints == {'ovs-tx-steering': 'hash'}:
+        return {'openvswitch': {'other_config': {'tx-steering': 'hash'}}}
+    else:
+        return hints
+
+
 class CreatePort(command.ShowOne, common.NeutronCommandWithExtraArgs):
    _description = _("Create a new port")

@ -527,6 +571,29 @@ class CreatePort(command.ShowOne, common.NeutronCommandWithExtraArgs):
                parsed_args.qos_policy, ignore_missing=False
            ).id

+        if parsed_args.hint:
+            _validate_port_hints(parsed_args.hint)
+            expanded_hints = _expand_port_hint_aliases(parsed_args.hint)
+            try:
+                client.find_extension('port-hints', ignore_missing=False)
+            except Exception as e:
+                msg = _('Not supported by Network API: %(e)s') % {'e': e}
+                raise exceptions.CommandError(msg)
+            if (
+                'openvswitch' in expanded_hints
+                and 'other_config' in expanded_hints['openvswitch']
+                and 'tx-steering'
+                in expanded_hints['openvswitch']['other_config']
+            ):
+                try:
+                    client.find_extension(
+                        'port-hint-ovs-tx-steering', ignore_missing=False
+                    )
+                except Exception as e:
+                    msg = _('Not supported by Network API: %(e)s') % {'e': e}
+                    raise exceptions.CommandError(msg)
+            attrs['hints'] = expanded_hints
+
        set_tags_in_post = bool(
            client.find_extension('tag-ports-during-bulk-creation')
        )
@ -972,6 +1039,29 @@ class SetPort(common.NeutronCommandWithExtraArgs):
        if parsed_args.data_plane_status:
            attrs['data_plane_status'] = parsed_args.data_plane_status

+        if parsed_args.hint:
+            _validate_port_hints(parsed_args.hint)
+            expanded_hints = _expand_port_hint_aliases(parsed_args.hint)
+            try:
+                client.find_extension('port-hints', ignore_missing=False)
+            except Exception as e:
+                msg = _('Not supported by Network API: %(e)s') % {'e': e}
+                raise exceptions.CommandError(msg)
+            if (
+                'openvswitch' in expanded_hints
+                and 'other_config' in expanded_hints['openvswitch']
+                and 'tx-steering'
+                in expanded_hints['openvswitch']['other_config']
+            ):
+                try:
+                    client.find_extension(
+                        'port-hint-ovs-tx-steering', ignore_missing=False
+                    )
+                except Exception as e:
+                    msg = _('Not supported by Network API: %(e)s') % {'e': e}
+                    raise exceptions.CommandError(msg)
+            attrs['hints'] = expanded_hints
+
        attrs.update(
            self._parse_extra_properties(parsed_args.extra_properties)
        )
@ -1083,6 +1173,12 @@ class UnsetPort(common.NeutronUnsetCommandWithExtraArgs):
            default=False,
            help=_("Clear host binding for the port."),
        )
+        parser.add_argument(
+            '--hints',
+            action='store_true',
+            default=False,
+            help=_("Clear hints for the port."),
+        )

        _tag.add_tag_option_to_parser_for_unset(parser, _('port'))

@ -1143,6 +1239,8 @@ class UnsetPort(common.NeutronUnsetCommandWithExtraArgs):
            attrs['numa_affinity_policy'] = None
        if parsed_args.host:
            attrs['binding:host_id'] = None
+        if parsed_args.hints:
+            attrs['hints'] = None

        attrs.update(
            self._parse_extra_properties(parsed_args.extra_properties)
--- a/openstackclient/tests/unit/network/v2/fakes.py
+++ b/openstackclient/tests/unit/network/v2/fakes.py
@ -1778,6 +1778,7 @@ def create_one_port(attrs=None):
                'subnet_id': 'subnet-id-' + uuid.uuid4().hex,
            }
        ],
+        'hints': {},
        'id': 'port-id-' + uuid.uuid4().hex,
        'mac_address': 'fa:16:3e:a9:4e:72',
        'name': 'port-name-' + uuid.uuid4().hex,
--- a/openstackclient/tests/unit/network/v2/test_port.py
+++ b/openstackclient/tests/unit/network/v2/test_port.py
@ -60,6 +60,7 @@ class TestPort(network_fakes.TestNetworkV2):
            'dns_name',
            'extra_dhcp_opts',
            'fixed_ips',
+            'hints',
            'id',
            'ip_allocation',
            'mac_address',
@ -99,6 +100,7 @@ class TestPort(network_fakes.TestNetworkV2):
            fake_port.dns_name,
            format_columns.ListDictColumn(fake_port.extra_dhcp_opts),
            format_columns.ListDictColumn(fake_port.fixed_ips),
+            fake_port.hints,
            fake_port.id,
            fake_port.ip_allocation,
            fake_port.mac_address,
@ -931,6 +933,133 @@ class TestCreatePort(TestPort):
        self.assertEqual(set(self.columns), set(columns))
        self.assertCountEqual(self.data, data)

+    def test_create_hints_invalid_json(self):
+        arglist = [
+            '--network',
+            self._port.network_id,
+            '--hint',
+            'invalid json',
+            'test-port',
+        ]
+        self.assertRaises(
+            argparse.ArgumentTypeError,
+            self.check_parser,
+            self.cmd,
+            arglist,
+            None,
+        )
+
+    def test_create_hints_invalid_alias(self):
+        arglist = [
+            '--network',
+            self._port.network_id,
+            '--hint',
+            'invalid-alias=value',
+            'test-port',
+        ]
+        verifylist = [
+            ('network', self._port.network_id),
+            ('enable', True),
+            ('hint', {'invalid-alias': 'value'}),
+            ('name', 'test-port'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(
+            argparse.ArgumentTypeError,
+            self.cmd.take_action,
+            parsed_args,
+        )
+
+    def test_create_hints_invalid_value(self):
+        arglist = [
+            '--network',
+            self._port.network_id,
+            '--hint',
+            'ovs-tx-steering=invalid-value',
+            'test-port',
+        ]
+        verifylist = [
+            ('network', self._port.network_id),
+            ('enable', True),
+            ('hint', {'ovs-tx-steering': 'invalid-value'}),
+            ('name', 'test-port'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(
+            argparse.ArgumentTypeError,
+            self.cmd.take_action,
+            parsed_args,
+        )
+
+    def test_create_hints_valid_alias_value(self):
+        arglist = [
+            '--network',
+            self._port.network_id,
+            '--hint',
+            'ovs-tx-steering=hash',
+            'test-port',
+        ]
+        verifylist = [
+            ('network', self._port.network_id),
+            ('enable', True),
+            ('hint', {'ovs-tx-steering': 'hash'}),
+            ('name', 'test-port'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.network.create_port.assert_called_once_with(
+            **{
+                'admin_state_up': True,
+                'network_id': self._port.network_id,
+                'hints': {
+                    'openvswitch': {'other_config': {'tx-steering': 'hash'}}
+                },
+                'name': 'test-port',
+            }
+        )
+
+        self.assertEqual(set(self.columns), set(columns))
+        self.assertCountEqual(self.data, data)
+
+    def test_create_hints_valid_json(self):
+        arglist = [
+            '--network',
+            self._port.network_id,
+            '--hint',
+            '{"openvswitch": {"other_config": {"tx-steering": "hash"}}}',
+            'test-port',
+        ]
+        verifylist = [
+            ('network', self._port.network_id),
+            ('enable', True),
+            (
+                'hint',
+                {"openvswitch": {"other_config": {"tx-steering": "hash"}}},
+            ),
+            ('name', 'test-port'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.network.create_port.assert_called_once_with(
+            **{
+                'admin_state_up': True,
+                'network_id': self._port.network_id,
+                'hints': {
+                    'openvswitch': {'other_config': {'tx-steering': 'hash'}}
+                },
+                'name': 'test-port',
+            }
+        )
+
+        self.assertEqual(set(self.columns), set(columns))
+        self.assertCountEqual(self.data, data)
+

 class TestDeletePort(TestPort):
    # Ports to delete.
@ -2010,7 +2139,7 @@ class TestSetPort(TestPort):
            self._port,
            **{
                'port_security_enabled': True,
-            }
+            },
        )

    def test_set_port_security_disabled(self):
@ -2034,7 +2163,7 @@ class TestSetPort(TestPort):
            self._port,
            **{
                'port_security_enabled': False,
-            }
+            },
        )

    def test_set_port_with_qos(self):
@ -2155,6 +2284,115 @@ class TestSetPort(TestPort):
    def test_create_with_numa_affinity_policy_legacy(self):
        self._test_create_with_numa_affinity_policy('legacy')

+    def test_set_hints_invalid_json(self):
+        arglist = [
+            '--network',
+            self._port.network_id,
+            '--hint',
+            'invalid json',
+            'test-port',
+        ]
+        self.assertRaises(
+            argparse.ArgumentTypeError,
+            self.check_parser,
+            self.cmd,
+            arglist,
+            None,
+        )
+
+    def test_set_hints_invalid_alias(self):
+        arglist = [
+            '--hint',
+            'invalid-alias=value',
+            'test-port',
+        ]
+        verifylist = [
+            ('hint', {'invalid-alias': 'value'}),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(
+            argparse.ArgumentTypeError,
+            self.cmd.take_action,
+            parsed_args,
+        )
+
+    def test_set_hints_invalid_value(self):
+        arglist = [
+            '--hint',
+            'ovs-tx-steering=invalid-value',
+            'test-port',
+        ]
+        verifylist = [
+            ('hint', {'ovs-tx-steering': 'invalid-value'}),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(
+            argparse.ArgumentTypeError,
+            self.cmd.take_action,
+            parsed_args,
+        )
+
+    def test_set_hints_valid_alias_value(self):
+        testport = network_fakes.create_one_port()
+        self.network.find_port = mock.Mock(return_value=testport)
+        self.network.find_extension = mock.Mock(
+            return_value=['port-hints', 'port-hint-ovs-tx-steering']
+        )
+        arglist = [
+            '--hint',
+            'ovs-tx-steering=hash',
+            testport.name,
+        ]
+        verifylist = [
+            ('hint', {'ovs-tx-steering': 'hash'}),
+            ('port', testport.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        self.network.update_port.assert_called_once_with(
+            testport,
+            **{
+                'hints': {
+                    'openvswitch': {'other_config': {'tx-steering': 'hash'}}
+                }
+            },
+        )
+        self.assertIsNone(result)
+
+    def test_set_hints_valid_json(self):
+        testport = network_fakes.create_one_port()
+        self.network.find_port = mock.Mock(return_value=testport)
+        self.network.find_extension = mock.Mock(
+            return_value=['port-hints', 'port-hint-ovs-tx-steering']
+        )
+        arglist = [
+            '--hint',
+            '{"openvswitch": {"other_config": {"tx-steering": "hash"}}}',
+            testport.name,
+        ]
+        verifylist = [
+            (
+                'hint',
+                {"openvswitch": {"other_config": {"tx-steering": "hash"}}},
+            ),
+            ('port', testport.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        self.network.update_port.assert_called_once_with(
+            testport,
+            **{
+                'hints': {
+                    'openvswitch': {'other_config': {'tx-steering': 'hash'}}
+                }
+            },
+        )
+        self.assertIsNone(result)
+

 class TestShowPort(TestPort):
    # The port to show.
@ -2474,3 +2712,23 @@ class TestUnsetPort(TestPort):

        self.network.update_port.assert_called_once_with(_fake_port, **attrs)
        self.assertIsNone(result)
+
+    def test_unset_hints(self):
+        testport = network_fakes.create_one_port()
+        self.network.find_port = mock.Mock(return_value=testport)
+        arglist = [
+            '--hints',
+            testport.name,
+        ]
+        verifylist = [
+            ('hints', True),
+            ('port', testport.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        self.network.update_port.assert_called_once_with(
+            testport,
+            **{'hints': None},
+        )
+        self.assertIsNone(result)
--- a/releasenotes/notes/add-port-hints-attribute-be1779e640a47d0d.yaml
+++ b/releasenotes/notes/add-port-hints-attribute-be1779e640a47d0d.yaml
@ -0,0 +1,7 @@
+---
+features:
+  - |
+    Enable management of Neutron port hints: ``port create --hint HINT``,
+    ``set port --hint HINT and ``unset port --hint``. Port hints allow
+    passing backend specific hints to Neutron mainly to tune backend
+    performance. The first hint controls Open vSwitch Tx steering.