diff --git a/neutron/plugins/ml2/drivers/mech_sriov/agent/eswitch_manager.py b/neutron/plugins/ml2/drivers/mech_sriov/agent/eswitch_manager.py index 42f2a5b3a39..7b242cc4a2a 100644 --- a/neutron/plugins/ml2/drivers/mech_sriov/agent/eswitch_manager.py +++ b/neutron/plugins/ml2/drivers/mech_sriov/agent/eswitch_manager.py @@ -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_ 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 diff --git a/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_eswitch_manager.py b/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_eswitch_manager.py index 703493745f3..aa8643aa635 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_eswitch_manager.py +++ b/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_eswitch_manager.py @@ -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): diff --git a/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_sriov_nic_agent.py b/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_sriov_nic_agent.py index 1571ba45254..33de9e617fe 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_sriov_nic_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/mech_sriov/agent/test_sriov_nic_agent.py @@ -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)]