[OVN] Fix remote-managed binding profile validation

When binding profile parameter validation was extended in
change I8493cba28e92b9b36bdb952c9737c0fea6fb7b75, a subtle bug
was introduced.

Namely, if a binding profile specification specified ``None`` for
expected capability, the code would literally look for an instance
of ``None`` in the list of capabilities, as can be read from the
documentation [0] this was not the intended behavior.

Update unit tests that asserted the incorrect behavior.

Add new unit test specifically for the case of a remote-managed
vnic that also has extra capabilities.

0: https://opendev.org/openstack/neutron/src/commit/71962255dee06d75258984b39d3a5c6c6293534d/neutron/common/ovn/constants.py#L340-L353
Closes-Bug: #2116087
Change-Id: I69d49cd3737b3bee8c8b4221149085375e9ac1e2
Signed-off-by: Frode Nordahl <[email protected]>
(cherry picked from commit 1e01e6a)

Author: fnordahl

neutron/common/ovn/utils.py

@@ -463,7 +463,9 @@ def validate_and_get_data_from_binding_profile(port):
         if pbp_param_set.vnic_type:
             if pbp_param_set.vnic_type != vnic_type:
                 continue
-            if capabilities and pbp_param_set.capability not in capabilities:
+            if (capabilities and
+                    pbp_param_set.capability is not None and
+                    pbp_param_set.capability not in capabilities):
                 continue
         param_set = pbp_param_set.param_set
         param_keys = param_set.keys()

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

@@ -606,23 +606,24 @@ def setUp(self):
             'neutron_lib.plugins.directory.get_plugin').start()
         self.VNIC_FAKE_NORMAL = 'fake-vnic-normal'
         self.VNIC_FAKE_OTHER = 'fake-vnic-other'
+        self.VNIC_FAKE_THIRD = 'fake-vnic-third'
 
         # Replace constants.OVN_PORT_BINDING_PROFILE_PARAMS to allow synthesis
         _params = constants.OVN_PORT_BINDING_PROFILE_PARAMS.copy()
         _params.extend([
             constants.OVNPortBindingProfileParamSet(
                 {'key': [str, type(None)]},
                 self.VNIC_FAKE_NORMAL, None),
-            constants.OVNPortBindingProfileParamSet(
-                {'key': [str], 'other_key': [str]},
-                self.VNIC_FAKE_OTHER, None),
             constants.OVNPortBindingProfileParamSet(
                 {
                     'key': [str],
                     'other_key': [int],
                     'third_key': [str]
                 },
                 self.VNIC_FAKE_OTHER, constants.PORT_CAP_SWITCHDEV),
+            constants.OVNPortBindingProfileParamSet(
+                {'key': [str], 'other_key': [str]},
+                self.VNIC_FAKE_THIRD, None),
         ])
         self.OVN_PORT_BINDING_PROFILE_PARAMS = mock.patch.object(
             constants,
@@ -737,6 +738,27 @@ def test_valid_input(self):
                 {portbindings.VNIC_TYPE: portbindings.VNIC_REMOTE_MANAGED,
                  constants.OVN_PORT_BINDING_PROFILE: expect}))
 
+    def test_valid_input_surplus_capabilities(self):
+        capabilities = ['rx', 'tx', 'sg', 'tso', 'gso', 'gro', 'rxvlan',
+                        'txvlan', 'rxhash', 'rdma', 'txudptnl']
+        binding_profile = {
+            'pci_vendor_info': 'dead:beef',
+            'pci_slot': '0000:ca:fe.42',
+            'physical_network': 'physnet1',
+            'card_serial_number': 'AB2000X00042',
+            'pf_mac_address': '00:53:00:00:00:42',
+            'vf_num': 42,
+            constants.PORT_CAP_PARAM: capabilities
+        }
+        expect = binding_profile.copy()
+        del(expect[constants.PORT_CAP_PARAM])
+        self.assertEqual(
+            utils.BPInfo(expect, portbindings.VNIC_REMOTE_MANAGED,
+                         capabilities),
+            utils.validate_and_get_data_from_binding_profile(
+                {portbindings.VNIC_TYPE: portbindings.VNIC_REMOTE_MANAGED,
+                 constants.OVN_PORT_BINDING_PROFILE: binding_profile}))
+
     def test_valid_input_surplus_keys(self):
         # Confirm that extra keys are allowed
         binding_profile = {
@@ -810,12 +832,12 @@ def test_overlapping_param_set_different_vnic_type(self):
             utils.validate_and_get_data_from_binding_profile(
                 {portbindings.VNIC_TYPE: self.VNIC_FAKE_NORMAL,
                  constants.OVN_PORT_BINDING_PROFILE: binding_profile}))
-        # It is valid for VNIC_FAKE_OTHER
+        # It is valid for VNIC_FAKE_THIRD
         expected_bp = binding_profile.copy()
         self.assertEqual(
-            utils.BPInfo(expected_bp, self.VNIC_FAKE_OTHER, []),
+            utils.BPInfo(expected_bp, self.VNIC_FAKE_THIRD, []),
             utils.validate_and_get_data_from_binding_profile(
-                {portbindings.VNIC_TYPE: self.VNIC_FAKE_OTHER,
+                {portbindings.VNIC_TYPE: self.VNIC_FAKE_THIRD,
                  constants.OVN_PORT_BINDING_PROFILE: binding_profile}))
 
     def test_overlapping_param_set_different_vnic_type_and_capability(self):
@@ -825,13 +847,13 @@ def test_overlapping_param_set_different_vnic_type_and_capability(self):
             'other_key': 42,
             'third_key': 'value',
         }
-        # This param set is not valid for VNIC_FAKE_OTHER without capability
+        # This param set is not valid for VNIC_FAKE_THIRD without capability
         expect = binding_profile.copy()
         del(expect['third_key'])
         self.assertRaises(
             neutron_lib.exceptions.InvalidInput,
             utils.validate_and_get_data_from_binding_profile,
-            {portbindings.VNIC_TYPE: self.VNIC_FAKE_OTHER,
+            {portbindings.VNIC_TYPE: self.VNIC_FAKE_THIRD,
              constants.OVN_PORT_BINDING_PROFILE: binding_profile})
         # This param set is also not valid as the capabilities do not match
         binding_profile = {