openstack
/
os-vif
Code Issues Proposed changes

Refactor code of linux_net to more cleaner and increase performace 

The patch adds new functions '_get_phys_port_name' for reading physical
port name of the SR-IOV port and '_get_phys_switch_id' for reading
physical port switch ID of the SR-IOV port, in addition to refactoring
'get_representor_port' to use the new functions and decrease calls for
"_get_pf_func" and netdevs associated with the PF will now be processed
in the loop, however it will not be matching 'phys_port_name' which
ensures the correct behaviour.

In addition to updating the unit test for linux_net and remove not
needed mocks

Related-Bug: #1892132
Change-Id: I3fdbea4f48cb79ebfd03a4da21e2232ccafb7a76
(cherry picked from commit 76f7565b99)
This commit is contained in:
Mamduh 2020-08-18 12:40:10 +03:00 committed by Moshe Levi
parent 37260a25ac
commit 167bb030f1
2 changed files with 137 additions and 185 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

72
vif_plug_ovs/linux_net.py
View File

@ -239,46 +239,35 @@ def get_representor_port(pf_ifname, vf_num):
VF number in the phys_port_name. That interface is the representor for
the requested VF.
"""
pf_path = "/sys/class/net/%s" % pf_ifname
pf_sw_id_file = os.path.join(pf_path, "phys_switch_id")
pf_sw_id = None
try:
with open(pf_sw_id_file, 'r') as fd:
pf_sw_id = fd.readline().rstrip()
pf_sw_id = _get_phys_switch_id(pf_ifname)
except (OSError, IOError):
raise exception.RepresentorNotFound(ifname=pf_ifname, vf_num=vf_num)
pf_subsystem_file = os.path.join(pf_path, "subsystem")
pf_subsystem_file = "/sys/class/net/%s/subsystem" % pf_ifname
try:
devices = os.listdir(pf_subsystem_file)
except (OSError, IOError):
raise exception.RepresentorNotFound(ifname=pf_ifname, vf_num=vf_num)
for device in devices:
address_str, pf = get_function_by_ifname(device)
if pf:
continue
ifname_pf_func = _get_pf_func(pf_ifname)
if ifname_pf_func is None:
raise exception.RepresentorNotFound(ifname=pf_ifname, vf_num=vf_num)
device_path = "/sys/class/net/%s" % device
device_sw_id_file = os.path.join(device_path, "phys_switch_id")
for device in devices:
try:
with open(device_sw_id_file, 'r') as fd:
device_sw_id = fd.readline().rstrip()
device_sw_id = _get_phys_switch_id(device)
if not device_sw_id or device_sw_id != pf_sw_id:
continue
except (OSError, IOError):
continue
if device_sw_id != pf_sw_id:
continue
device_port_name_file = (
os.path.join(device_path, 'phys_port_name'))
if not os.path.isfile(device_port_name_file):
continue
try:
with open(device_port_name_file, 'r') as fd:
phys_port_name = fd.readline().rstrip()
phys_port_name = _get_phys_port_name(device)
if phys_port_name is None:
continue
except (OSError, IOError):
continue
@ -287,9 +276,6 @@ def get_representor_port(pf_ifname, vf_num):
# the PCI func number of pf_ifname.
rep_parent_pf_func = _parse_pf_number(phys_port_name)
if rep_parent_pf_func is not None:
ifname_pf_func = _get_pf_func(pf_ifname)
if ifname_pf_func is None:
continue
if int(rep_parent_pf_func) != int(ifname_pf_func):
continue
@ -321,9 +307,7 @@ def _get_sysfs_netdev_path(pci_addr, pf_interface):
def _is_switchdev(netdev):
"""Returns True if a netdev has a readable phys_switch_id"""
try:
sw_id_file = "/sys/class/net/%s/phys_switch_id" % netdev
with open(sw_id_file, 'r') as fd:
phys_switch_id = fd.readline().rstrip()
phys_switch_id = _get_phys_switch_id(netdev)
if phys_switch_id != "" and phys_switch_id is not None:
return True
except (OSError, IOError):
@ -389,3 +373,33 @@ def get_pf_pci_from_vf(vf_pci):
"""
physfn_path = os.readlink("/sys/bus/pci/devices/%s/physfn" % vf_pci)
return os.path.basename(physfn_path)
def _get_phys_port_name(ifname):
"""Get the interface name and return its phys_port_name
:param ifname: The interface name
:return: The phys_port_name of the given ifname
"""
phys_port_name_path = "/sys/class/net/%s/phys_port_name" % ifname
if not os.path.isfile(phys_port_name_path):
return None
with open(phys_port_name_path, 'r') as fd:
return fd.readline().strip()
def _get_phys_switch_id(ifname):
"""Get the interface name and return its phys_switch_id
:param ifname: The interface name
:return: The phys_switch_id of the given ifname
"""
phys_port_name_path = "/sys/class/net/%s/phys_switch_id" % ifname
if not os.path.isfile(phys_port_name_path):
return None
with open(phys_port_name_path, 'r') as fd:
return fd.readline().strip()

250
vif_plug_ovs/tests/unit/test_linux_net.py
View File

@ -133,47 +133,22 @@ class LinuxNetTest(testtools.TestCase):
linux_net.add_bridge_port("br0", "vnet1")
mock_set.assert_called_once_with("vnet1", master="br0")
@mock.patch('builtins.open')
@mock.patch.object(os.path, 'isfile')
def test_is_switchdev_ioerror(self, mock_isfile, mock_open):
mock_isfile.side_effect = [True]
mock_open.return_value.__enter__ = lambda s: s
readline_mock = mock_open.return_value.readline
readline_mock.side_effect = (
[IOError()])
@mock.patch.object(linux_net, '_get_phys_switch_id')
def test_is_switchdev_ioerror(self, mock__get_phys_switch_id):
mock__get_phys_switch_id.side_effect = ([IOError()])
test_switchdev = linux_net._is_switchdev('pf_ifname')
self.assertEqual(test_switchdev, False)
@mock.patch('builtins.open')
@mock.patch.object(os.path, 'isfile')
def test_is_switchdev_empty(self, mock_isfile, mock_open):
mock_isfile.side_effect = [True]
mock_open.return_value.__enter__ = lambda s: s
readline_mock = mock_open.return_value.readline
readline_mock.side_effect = (
[''])
open_calls = (
[mock.call('/sys/class/net/pf_ifname/phys_switch_id', 'r'),
mock.call().readline(),
mock.call().__exit__(None, None, None)])
@mock.patch.object(linux_net, '_get_phys_switch_id')
def test_is_switchdev_empty(self, mock__get_phys_switch_id):
mock__get_phys_switch_id.return_value = ''
test_switchdev = linux_net._is_switchdev('pf_ifname')
mock_open.assert_has_calls(open_calls)
self.assertEqual(test_switchdev, False)
@mock.patch('builtins.open')
@mock.patch.object(os.path, 'isfile')
def test_is_switchdev_positive(self, mock_isfile, mock_open):
mock_isfile.side_effect = [True]
mock_open.return_value.__enter__ = lambda s: s
readline_mock = mock_open.return_value.readline
readline_mock.side_effect = (
['pf_sw_id'])
open_calls = (
[mock.call('/sys/class/net/pf_ifname/phys_switch_id', 'r'),
mock.call().readline(),
mock.call().__exit__(None, None, None)])
@mock.patch.object(linux_net, '_get_phys_switch_id')
def test_is_switchdev_positive(self, mock__get_phys_switch_id):
mock__get_phys_switch_id.return_value = 'pf_sw_id'
test_switchdev = linux_net._is_switchdev('pf_ifname')
mock_open.assert_has_calls(open_calls)
self.assertEqual(test_switchdev, True)
def test_parse_vf_number(self):
@ -192,184 +167,115 @@ class LinuxNetTest(testtools.TestCase):
self.assertEqual(linux_net._parse_pf_number("pf31"), "31")
self.assertIsNone(linux_net._parse_pf_number("g4rbl3d"))
@mock.patch('builtins.open')
@mock.patch.object(os.path, 'isfile')
@mock.patch.object(os, 'listdir')
@mock.patch.object(linux_net, "get_function_by_ifname")
def test_get_representor_port(self, mock_get_function_by_ifname,
mock_listdir, mock_isfile, mock_open):
@mock.patch.object(linux_net, "_get_pf_func")
@mock.patch.object(linux_net, "_get_phys_port_name")
@mock.patch.object(linux_net, '_get_phys_switch_id')
def test_get_representor_port(self, mock__get_phys_switch_id,
mock__get_phys_port_name,
mock__get_pf_func,
mock_listdir):
mock_listdir.return_value = [
'pf_ifname', 'rep_vf_1', 'rep_vf_2'
]
mock_isfile.side_effect = [True, True]
mock_open.return_value.__enter__ = lambda s: s
readline_mock = mock_open.return_value.readline
readline_mock.side_effect = (
['pf_sw_id', 'pf_sw_id', '1', 'pf_sw_id', 'pf0vf2'])
# PCI IDs mocked:
# PF0: 0000:0a:00.0
# PF0VF1: 0000:0a:02.1 PF0VF2: 0000:0a:02.2
mock_get_function_by_ifname.side_effect = (
[("0000:0a:00.0", True),
("0000:0a:02.1", False),
("0000:0a:02.2", False), ("0000:0a:00.0", True)])
open_calls = (
[mock.call('/sys/class/net/pf_ifname/phys_switch_id', 'r'),
mock.call().readline(),
mock.call().__exit__(None, None, None),
mock.call('/sys/class/net/rep_vf_1/phys_switch_id', 'r'),
mock.call().readline(),
mock.call().__exit__(None, None, None),
mock.call('/sys/class/net/rep_vf_1/phys_port_name', 'r'),
mock.call().readline(),
mock.call().__exit__(None, None, None),
mock.call('/sys/class/net/rep_vf_2/phys_switch_id', 'r'),
mock.call().readline(),
mock.call().__exit__(None, None, None),
mock.call('/sys/class/net/rep_vf_2/phys_port_name', 'r'),
mock.call().readline(),
mock.call().__exit__(None, None, None)])
mock__get_phys_switch_id.return_value = 'pf_sw_id'
mock__get_pf_func.return_value = "0"
mock__get_phys_port_name.side_effect = (['1', "pf0vf1", "pf0vf2"])
ifname = linux_net.get_representor_port('pf_ifname', '2')
mock_open.assert_has_calls(open_calls)
self.assertEqual('rep_vf_2', ifname)
@mock.patch('builtins.open')
@mock.patch.object(os.path, 'isfile')
@mock.patch.object(os, 'listdir')
@mock.patch.object(linux_net, "get_function_by_ifname")
@mock.patch.object(linux_net, "_get_pf_func")
@mock.patch.object(linux_net, "_get_phys_port_name")
@mock.patch.object(linux_net, "_get_phys_switch_id")
def test_get_representor_port_2_pfs(
self, mock_get_function_by_ifname, mock_listdir, mock_isfile,
mock_open):
self, mock__get_phys_switch_id, mock__get_phys_port_name,
mock__get_pf_func, mock_listdir):
mock_listdir.return_value = [
'pf_ifname1', 'pf_ifname2', 'rep_pf1_vf_1', 'rep_pf1_vf_2',
'rep_pf2_vf_1', 'rep_pf2_vf_2',
]
mock_isfile.side_effect = [True, True, True, True]
mock_open.return_value.__enter__ = lambda s: s
readline_mock = mock_open.return_value.readline
readline_mock.side_effect = (
['pf_sw_id',
'pf_sw_id', 'VF1@PF1', 'pf_sw_id', 'vf2@pf1',
'pf_sw_id', 'pf2vf1', 'pf_sw_id', 'pf2vf2'])
# PCI IDs mocked:
# PF1: 0000:0a:00.1 PF2: 0000:0a:00.2
# PF1VF1: 0000:0a:02.1 PF1VF2: 0000:0a:02.2
# PF2VF1: 0000:0a:04.1 PF2VF2: 0000:0a:04.2
mock_get_function_by_ifname.side_effect = (
[("0000:0a:00.1", True), ("0000:0a:00.2", True),
("0000:0a:02.1", False), ("0000:0a:00.2", True),
("0000:0a:02.2", False), ("0000:0a:00.2", True),
("0000:0a:04.1", False), ("0000:0a:00.2", True),
("0000:0a:04.2", False), ("0000:0a:00.2", True)])
mock__get_phys_switch_id.return_value = 'pf_sw_id'
mock__get_pf_func.return_value = "2"
mock__get_phys_port_name.side_effect = (
["p1", "p2", "VF1@PF1", "pf2vf1", "vf2@pf1", "pf2vf2"])
ifname = linux_net.get_representor_port('pf_ifname2', '2')
self.assertEqual('rep_pf2_vf_2', ifname)
@mock.patch('builtins.open')
@mock.patch.object(os.path, 'isfile')
@mock.patch.object(os, 'listdir')
@mock.patch.object(linux_net, "get_function_by_ifname")
@mock.patch.object(linux_net, "_get_pf_func")
@mock.patch.object(linux_net, "_get_phys_switch_id")
@mock.patch.object(linux_net, "_get_phys_port_name")
def test_get_representor_port_not_found(
self, mock_get_function_by_ifname, mock_listdir, mock_isfile,
mock_open):
self, mock__get_phys_port_name, mock__get_phys_switch_id,
mock__get_pf_func, mock_listdir):
mock_listdir.return_value = [
'pf_ifname', 'rep_vf_1', 'rep_vf_2'
]
mock_isfile.side_effect = [True, True]
mock_open.return_value.__enter__ = lambda s: s
readline_mock = mock_open.return_value.readline
readline_mock.side_effect = (
['pf_sw_id', 'pf_sw_id', '1', 'pf_sw_id', '2'])
# PCI IDs mocked:
# PF0: 0000:0a:00.0
# PF0VF1: 0000:0a:02.1 PF0VF2: 0000:0a:02.2
mock_get_function_by_ifname.side_effect = (
[("0000:0a:00.0", True),
("0000:0a:02.1", False),
("0000:0a:02.2", False)])
mock__get_phys_switch_id.return_value = 'pf_sw_id'
mock__get_pf_func.return_value = "0"
mock__get_phys_port_name.side_effect = (
["p0", "1", "2"])
self.assertRaises(
exception.RepresentorNotFound,
linux_net.get_representor_port,
'pf_ifname', '3'),
@mock.patch('builtins.open')
@mock.patch.object(os.path, 'isfile')
@mock.patch.object(os, 'listdir')
@mock.patch.object(linux_net, "get_function_by_ifname")
@mock.patch.object(linux_net, "_get_pf_func")
@mock.patch.object(linux_net, "_get_phys_port_name")
@mock.patch.object(linux_net, "_get_phys_switch_id")
def test_get_representor_port_exception_io_error(
self, mock_get_function_by_ifname, mock_listdir, mock_isfile,
mock_open):
self, mock__get_phys_switch_id, mock__get_phys_port_name,
mock__get_pf_func, mock_listdir):
mock_listdir.return_value = [
'pf_ifname', 'rep_vf_1', 'rep_vf_2'
]
mock_isfile.side_effect = [True, True]
mock_open.return_value.__enter__ = lambda s: s
readline_mock = mock_open.return_value.readline
readline_mock.side_effect = (
mock__get_phys_switch_id.side_effect = (
['pf_sw_id', 'pf_sw_id', IOError(), 'pf_sw_id', '2'])
# PCI IDs mocked:
# PF0: 0000:0a:00.0
# PF0VF1: 0000:0a:02.1 PF0VF2: 0000:0a:02.2
mock_get_function_by_ifname.side_effect = (
[("0000:0a:00.0", True),
("0000:0a:02.1", False),
("0000:0a:02.2", False), ("0000:0a:00.0", True)])
mock__get_pf_func.return_value = "0"
mock__get_phys_port_name.side_effect = (
["p0", "pf0vf0", "pf0vf1"])
self.assertRaises(
exception.RepresentorNotFound,
linux_net.get_representor_port,
'pf_ifname', '3')
@mock.patch('builtins.open')
@mock.patch.object(os.path, 'isfile')
@mock.patch.object(os, 'listdir')
@mock.patch.object(linux_net, "get_function_by_ifname")
@mock.patch.object(linux_net, "_get_pf_func")
@mock.patch.object(linux_net, "_get_phys_port_name")
@mock.patch.object(linux_net, "_get_phys_switch_id")
def test_get_representor_port_exception_value_error(
self, mock_get_function_by_ifname, mock_listdir, mock_isfile,
mock_open):
self, mock__get_phys_switch_id, mock__get_phys_port_name,
mock__get_pf_func, mock_listdir):
mock_listdir.return_value = [
'pf_ifname', 'rep_vf_1', 'rep_vf_2'
]
mock_isfile.side_effect = [True, True]
mock_open.return_value.__enter__ = lambda s: s
readline_mock = mock_open.return_value.readline
readline_mock.side_effect = (
['pf_sw_id', 'pf_sw_id', '1', 'pf_sw_id', 'a'])
# PCI IDs mocked:
# PF0: 0000:0a:00.0
# PF0VF1: 0000:0a:02.1 PF0VF2: 0000:0a:02.2
mock_get_function_by_ifname.side_effect = (
[("0000:0a:00.0", True),
("0000:0a:02.1", False),
("0000:0a:02.2", False)])
mock__get_phys_switch_id.return_value = 'pf_sw_id'
mock__get_phys_port_name.side_effect = (['p0', '1', 'a'])
mock__get_pf_func.return_value = "0"
self.assertRaises(
exception.RepresentorNotFound,
linux_net.get_representor_port,
'pf_ifname', '3')
@mock.patch('builtins.open')
@mock.patch.object(os.path, 'isfile')
@mock.patch.object(os, 'listdir')
def test_physical_function_inferface_name(
self, mock_listdir, mock_isfile, mock_open):
@mock.patch.object(linux_net, '_get_phys_switch_id')
def test_physical_function_interface_name(
self, mock__get_phys_switch_id, mock_listdir):
mock_listdir.return_value = ['foo', 'bar']
mock_isfile.side_effect = [True, True]
mock_open.return_value.__enter__ = lambda s: s
readline_mock = mock_open.return_value.readline
readline_mock.side_effect = (
mock__get_phys_switch_id.side_effect = (
['', 'valid_switch'])
ifname = linux_net.get_ifname_by_pci_address(
'0000:00:00.1', pf_interface=True, switchdev=False)
self.assertEqual(ifname, 'foo')
@mock.patch('builtins.open')
@mock.patch.object(os.path, 'isfile')
@mock.patch.object(os, 'listdir')
def test_physical_function_inferface_name_with_switchdev(
self, mock_listdir, mock_isfile, mock_open):
@mock.patch.object(linux_net, '_get_phys_switch_id')
def test_physical_function_interface_name_with_switchdev(
self, mock__get_phys_switch_id, mock_listdir):
mock_listdir.return_value = ['foo', 'bar']
mock_isfile.side_effect = [True, True]
mock_open.return_value.__enter__ = lambda s: s
readline_mock = mock_open.return_value.readline
readline_mock.side_effect = (
mock__get_phys_switch_id.side_effect = (
['', 'valid_switch'])
ifname = linux_net.get_ifname_by_pci_address(
'0000:00:00.1', pf_interface=True, switchdev=True)
@ -420,3 +326,35 @@ class LinuxNetTest(testtools.TestCase):
linux_net.get_vf_num_by_pci_address,
'0000:00:00.1'
)
@mock.patch('builtins.open')
@mock.patch.object(os.path, 'isfile')
def test__get_phys_port_name(self, mock_isfile, mock_open):
mock_open.return_value.__enter__ = lambda s: s
readline_mock = mock_open.return_value.readline
readline_mock.return_value = 'pf0vf0'
mock_isfile.return_value = True
phys_port_name = linux_net._get_phys_port_name("vf_ifname")
self.assertEqual(phys_port_name, 'pf0vf0')
@mock.patch.object(os.path, 'isfile')
def test__get_phys_port_name_not_found(self, mock_isfile):
mock_isfile.return_value = False
phys_port_name = linux_net._get_phys_port_name("vf_ifname")
self.assertIsNone(phys_port_name)
@mock.patch('builtins.open')
@mock.patch.object(os.path, 'isfile')
def test__get_phys_switch_id(self, mock_isfile, mock_open):
mock_open.return_value.__enter__ = lambda s: s
readline_mock = mock_open.return_value.readline
readline_mock.return_value = '66e40000039b0398'
mock_isfile.return_value = True
phys_port_name = linux_net._get_phys_switch_id("ifname")
self.assertEqual(phys_port_name, '66e40000039b0398')
@mock.patch.object(os.path, 'isfile')
def test__get_phys_switch_id_not_found(self, mock_isfile):
mock_isfile.return_value = False
phys_port_name = linux_net._get_phys_switch_id("ifname")
self.assertIsNone(phys_port_name)