Merge "Use effective MAC address for macvtap assigned VFs"

This commit is contained in:
Zuul 2020-01-18 08:31:43 +00:00 committed by Gerrit Code Review
commit 2edb29d5d2
3 changed files with 133 additions and 82 deletions

View File

@ -69,29 +69,46 @@ class PciOsWrapper(object):
return os.path.isdir(cls.DEVICE_PATH % dev_name)
@classmethod
def is_assigned_vf(cls, dev_name, vf_index):
def is_assigned_vf_direct(cls, dev_name, vf_index):
"""Check if VF is assigned.
Checks if a given vf index of a given device name is assigned
by checking the relevant path in the system:
as PCI passthrough by checking the relevant path in the system:
VF is assigned if:
Direct VF: PCI_PATH does not exist.
@param dev_name: pf network device name
@param vf_index: vf index
@return: True if VF is assigned, False otherwise
"""
path = cls.PCI_PATH % (dev_name, vf_index)
return not os.path.isdir(path)
@classmethod
def get_vf_macvtap_upper_devs(cls, dev_name, vf_index):
"""Retrieve VF netdev upper (macvtap) devices.
@param dev_name: pf network device name
@param vf_index: vf index
@return: list of upper net devices associated with the VF
"""
path = cls.PCI_PATH % (dev_name, vf_index)
upper_macvtap_path = os.path.join(path, "*", cls.MAC_VTAP_PREFIX)
devs = [os.path.basename(dev) for dev in glob.glob(upper_macvtap_path)]
# file name is in the format of upper_<netdev_name> extract netdev name
return [dev.split('_')[1] for dev in devs]
@classmethod
def is_assigned_vf_macvtap(cls, dev_name, vf_index):
"""Check if VF is assigned.
Checks if a given vf index of a given device name is assigned
as macvtap by checking the relevant path in the system:
Macvtap VF: upper_macvtap path exists.
@param dev_name: pf network device name
@param vf_index: vf index
@return: True if VF is assigned, False otherwise
"""
if not cls.pf_device_exists(dev_name):
# If the root PCI path does not exist, then the VF cannot
# actually have been allocated and there is no way we can
# manage it.
return False
path = cls.PCI_PATH % (dev_name, vf_index)
if not os.path.isdir(path):
return True
upper_macvtap_path = os.path.join(path, "*", cls.MAC_VTAP_PREFIX)
return bool(glob.glob(upper_macvtap_path))
return bool(cls.get_vf_macvtap_upper_devs(dev_name, vf_index))
@classmethod
def get_numvfs(cls, dev_name):
@ -157,17 +174,10 @@ class EmbSwitch(object):
@return: list of VF pair (mac address, pci slot)
"""
vf_to_pci_slot_mapping = {}
assigned_devices_info = []
for pci_slot, vf_index in self.pci_slot_map.items():
if not PciOsWrapper.is_assigned_vf(self.dev_name, vf_index):
continue
vf_to_pci_slot_mapping[vf_index] = pci_slot
if vf_to_pci_slot_mapping:
vf_to_mac_mapping = self.pci_dev_wrapper.get_assigned_macs(
list(vf_to_pci_slot_mapping.keys()))
for vf_index, mac in vf_to_mac_mapping.items():
pci_slot = vf_to_pci_slot_mapping[vf_index]
mac = self.get_pci_device(pci_slot)
if mac:
assigned_devices_info.append((mac, pci_slot))
return assigned_devices_info
@ -243,18 +253,45 @@ class EmbSwitch(object):
raise exc.InvalidPciSlotError(pci_slot=pci_slot)
return self.pci_dev_wrapper.set_vf_spoofcheck(vf_index, enabled)
def _get_macvtap_mac(self, vf_index):
upperdevs = PciOsWrapper.get_vf_macvtap_upper_devs(
self.dev_name, vf_index)
# NOTE(adrianc) although there can be many macvtap upper
# devices, we expect to have excatly one.
if len(upperdevs) > 1:
LOG.warning("Found more than one macvtap upper device for PF "
"%(pf)s with VF index %(vf_index)s.",
{"pf": self.dev_name, "vf_index": vf_index})
upperdev = upperdevs[0]
return pci_lib.PciDeviceIPWrapper(
upperdev).device(upperdev).link.address
def get_pci_device(self, pci_slot):
"""Get mac address for given Virtual Function address
@param pci_slot: pci slot
@return: MAC address of virtual function
"""
if not PciOsWrapper.pf_device_exists(self.dev_name):
# If the root PCI path does not exist, then the VF cannot
# actually have been allocated and there is no way we can
# manage it.
return None
vf_index = self.pci_slot_map.get(pci_slot)
mac = None
if vf_index is not None:
if PciOsWrapper.is_assigned_vf(self.dev_name, vf_index):
# NOTE(adrianc) for VF passthrough take administrative mac from PF
# netdevice, for macvtap take mac directly from macvtap interface.
# This is done to avoid relying on hypervisor [lack of] logic to
# keep effective and administrative mac in sync.
if PciOsWrapper.is_assigned_vf_direct(self.dev_name, vf_index):
macs = self.pci_dev_wrapper.get_assigned_macs([vf_index])
mac = macs.get(vf_index)
elif PciOsWrapper.is_assigned_vf_macvtap(
self.dev_name, vf_index):
mac = self._get_macvtap_mac(vf_index)
return mac

View File

@ -43,9 +43,6 @@ class TestCreateESwitchManager(base.BaseTestCase):
dev_name="p6p1", reason="device" " not found")),\
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper.pf_device_exists",
return_value=True), \
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper.is_assigned_vf",
return_value=True):
eswitch_mgr = esm.ESwitchManager()
self.addCleanup(self.cleanup)
@ -60,9 +57,6 @@ class TestCreateESwitchManager(base.BaseTestCase):
return_value=self.SCANNED_DEVICES),\
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper.pf_device_exists",
return_value=True), \
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper.is_assigned_vf",
return_value=True):
eswitch_mgr = esm.ESwitchManager()
self.addCleanup(self.cleanup)
@ -99,9 +93,6 @@ class TestESwitchManagerApi(base.BaseTestCase):
return_value=self.SCANNED_DEVICES), \
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper.pf_device_exists",
return_value=True), \
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper.is_assigned_vf",
return_value=True):
eswitch_mgr.discover_devices(device_mappings, None)
@ -450,8 +441,11 @@ class TestEmbSwitch(base.BaseTestCase):
"PciDeviceIPWrapper.get_assigned_macs",
return_value={0: self.ASSIGNED_MAC}),\
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper.is_assigned_vf",
return_value=True):
"eswitch_manager.PciOsWrapper.pf_device_exists",
return_value=True), \
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper."
"is_assigned_vf_direct", return_value=True):
result = emb_switch.get_assigned_devices_info()
self.assertIn(self.ASSIGNED_MAC, list(result)[0])
self.assertIn(self.PCI_SLOT, list(result)[0])
@ -465,8 +459,8 @@ class TestEmbSwitch(base.BaseTestCase):
"PciDeviceIPWrapper.get_assigned_macs",
return_value=self.VF_TO_MAC_MAPPING),\
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper.is_assigned_vf",
return_value=True):
"eswitch_manager.PciOsWrapper."
"is_assigned_vf_direct", return_value=True):
devices_info = emb_switch.get_assigned_devices_info()
for device_info in devices_info:
mac = device_info[0]
@ -476,8 +470,11 @@ class TestEmbSwitch(base.BaseTestCase):
def test_get_assigned_devices_empty(self):
with mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper.is_assigned_vf",
return_value=False):
"eswitch_manager.PciOsWrapper.is_assigned_vf_direct",
return_value=False), \
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper."
"is_assigned_vf_macvtap", return_value=False):
result = self.emb_switch.get_assigned_devices_info()
self.assertFalse(result)
@ -593,8 +590,11 @@ class TestEmbSwitch(base.BaseTestCase):
"PciDeviceIPWrapper.get_assigned_macs",
return_value={0: self.ASSIGNED_MAC}),\
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper.is_assigned_vf",
return_value=True):
"eswitch_manager.PciOsWrapper."
"is_assigned_vf_direct", return_value=True), \
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper.pf_device_exists",
return_value=True):
result = self.emb_switch.get_pci_device(self.PCI_SLOT)
self.assertEqual(self.ASSIGNED_MAC, result)
@ -603,8 +603,11 @@ class TestEmbSwitch(base.BaseTestCase):
"PciDeviceIPWrapper.get_assigned_macs",
return_value=[self.ASSIGNED_MAC]),\
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper.is_assigned_vf",
return_value=True):
"eswitch_manager.PciOsWrapper.pf_device_exists",
return_value=True), \
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper."
"is_assigned_vf_direct", return_value=True):
result = self.emb_switch.get_pci_device(self.WRONG_PCI_SLOT)
self.assertIsNone(result)
@ -613,6 +616,32 @@ class TestEmbSwitch(base.BaseTestCase):
self.assertEqual([tup[0] for tup in self.SCANNED_DEVICES],
sorted(result))
def _test__get_macvtap_mac(self, upper_devs):
ip_wrapper_mock_inst = mock.MagicMock()
with mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent.pci_lib."
"PciDeviceIPWrapper",
return_value=ip_wrapper_mock_inst), \
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper."
"get_vf_macvtap_upper_devs",
return_value=upper_devs), \
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.LOG.warning") as log_mock:
self.emb_switch._get_macvtap_mac(0)
ip_wrapper_mock_inst.device.assert_called_with(upper_devs[0])
if len(upper_devs) > 1:
self.assertTrue(log_mock.called)
else:
self.assertFalse(log_mock.called)
def test__get_macvtap_mac_single_upper_dev(self):
upper_devs = ["macvtap0"]
self._test__get_macvtap_mac(upper_devs)
def test__get_macvtap_mac_multiple_upper_devs(self):
upper_devs = ["macvtap0", "macvtap1"]
self._test__get_macvtap_mac(upper_devs)
class TestPciOsWrapper(base.BaseTestCase):
DEV_NAME = "p7p1"
@ -669,30 +698,26 @@ class TestPciOsWrapper(base.BaseTestCase):
self.assertEqual([],
esm.PciOsWrapper.scan_vf_devices(self.DEV_NAME))
def _mock_assign_vf(self, dir_exists):
def _mock_assign_vf_direct(self, dir_exists):
with mock.patch("os.path.isdir",
return_value=dir_exists), \
mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper.pf_device_exists",
return_value=True):
result = esm.PciOsWrapper.is_assigned_vf(self.DEV_NAME,
self.VF_INDEX)
return_value=dir_exists):
result = esm.PciOsWrapper.is_assigned_vf_direct(self.DEV_NAME,
self.VF_INDEX)
self.assertEqual(not dir_exists, result)
def test_is_assigned_vf_true(self):
self._mock_assign_vf(True)
def test_is_assigned_vf_direct_true(self):
self._mock_assign_vf_direct(True)
def test_is_assigned_vf_false(self):
self._mock_assign_vf(False)
def test_is_assigned_vf_direct_false(self):
self._mock_assign_vf_direct(False)
def _mock_assign_vf_macvtap(self, macvtap_exists):
def _glob(file_path):
return ["upper_macvtap0"] if macvtap_exists else []
with mock.patch("os.path.isdir", return_value=True),\
mock.patch("glob.glob", side_effect=_glob):
result = esm.PciOsWrapper.is_assigned_vf(self.DEV_NAME,
self.VF_INDEX)
with mock.patch("glob.glob", side_effect=_glob):
result = esm.PciOsWrapper.is_assigned_vf_macvtap(self.DEV_NAME,
self.VF_INDEX)
self.assertEqual(macvtap_exists, result)
def test_is_assigned_vf_macvtap_true(self):
@ -701,12 +726,19 @@ class TestPciOsWrapper(base.BaseTestCase):
def test_is_assigned_vf_macvtap_false(self):
self._mock_assign_vf_macvtap(False)
@mock.patch("os.path.exists", return_value=False)
@mock.patch("os.listdir", return_value=["eth0", "eth1"])
def test_is_assigned_vf_pf_disappeared(self, list_dir_mock, *args):
self.assertFalse(esm.PciOsWrapper.is_assigned_vf(
self.DEV_NAME, self.VF_INDEX))
self.assertFalse(list_dir_mock.called)
def _test_get_vf_macvtap_upper_devs(self, upper_devs):
with mock.patch("glob.glob", return_value=upper_devs):
result = esm.PciOsWrapper.get_vf_macvtap_upper_devs(self.DEV_NAME,
self.VF_INDEX)
self.assertEqual([dev.split("_")[1] for dev in upper_devs], result)
def test_get_vf_macvtap_upper_devs(self):
upper_devs = ["upper_macvtap0", "upper_macvtap1"]
self._test_get_vf_macvtap_upper_devs(upper_devs)
def test_get_vf_macvtap_upper_devs_no_devs(self):
upper_devs = []
self._test_get_vf_macvtap_upper_devs(upper_devs)
def test_pf_device_exists_with_no_dir(self):
with mock.patch("os.path.isdir", return_value=False):

View File

@ -75,12 +75,6 @@ class TestSriovAgent(base.BaseTestCase):
self.agent.scan_devices(set(), set())
self.assertEqual(2, agent_conf['devices'])
@mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent.pci_lib."
"PciDeviceIPWrapper.get_assigned_macs",
return_value=[(DEVICE_MAC, PCI_SLOT)])
@mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper.is_assigned_vf",
return_value=True)
def test_treat_devices_removed_with_existed_device(self, *args):
agent = sriov_nic_agent.SriovNicSwitchAgent({}, {}, 0, {}, {}, {})
devices = [(DEVICE_MAC, PCI_SLOT)]
@ -92,12 +86,6 @@ class TestSriovAgent(base.BaseTestCase):
self.assertFalse(resync)
self.assertTrue(fn_udd.called)
@mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent.pci_lib."
"PciDeviceIPWrapper.get_assigned_macs",
return_value=[(DEVICE_MAC, PCI_SLOT)])
@mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper.is_assigned_vf",
return_value=True)
def test_treat_devices_removed_with_not_existed_device(self, *args):
agent = sriov_nic_agent.SriovNicSwitchAgent({}, {}, 0, {}, {}, {})
devices = [(DEVICE_MAC, PCI_SLOT)]
@ -112,12 +100,6 @@ class TestSriovAgent(base.BaseTestCase):
self.assertFalse(resync)
self.assertTrue(fn_udd.called)
@mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent.pci_lib."
"PciDeviceIPWrapper.get_assigned_macs",
return_value=[(DEVICE_MAC, PCI_SLOT)])
@mock.patch("neutron.plugins.ml2.drivers.mech_sriov.agent."
"eswitch_manager.PciOsWrapper.is_assigned_vf",
return_value=True)
def test_treat_devices_removed_failed(self, *args):
agent = sriov_nic_agent.SriovNicSwitchAgent({}, {}, 0, {}, {}, {})
devices = [(DEVICE_MAC, PCI_SLOT)]