openstack
/
os-vif
Code Issues Proposed changes

Fix VF-rep lookup routine to use parent PF number 

The current logic in get_representor_port() matches the VF number with
the one in 'phys_port_name' file. If and when multiple PFs on the same
card are configured in switchdev mode, we could end up returning a wrong
representor device. Fix this by including the parent PF number also in
this logic, when 'pfXvfY' (or vfY@pfX) format is used in 'phys_port_name'
file.

This patch also provides corresponding changes in unit tests. The tests
have been updated to validate 'phys_port_name' in 'pfXvfY' format (in
addition to the simple format - vfX or X). The test case for 2 PFs
specifically utilizes the 'pfXvfY' format, since otherwise it won't
be possible to disambiguate VFs with the same VF-ID under 2 PFs on the
same card (switch).

Change-Id: I5727878b63026a19fafcf7e71dda3ba480432fbf
Closes-Bug: #1743494
Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
This commit is contained in:
Sriharsha Basavapatna 2018-01-03 18:33:28 +05:30 committed by sean mooney
parent 87e0edde4d
commit 84688a22db
2 changed files with 110 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
vif_plug_ovs/linux_net.py
View File

@ -43,6 +43,10 @@ INT_RE = re.compile("^(\d+)$")
VF_RE = re.compile("vf(\d+)", re.IGNORECASE)
# phys_port_name contains PF## or pf##
PF_RE = re.compile("pf(\d+)", re.IGNORECASE)
# bus_info (bdf) contains <bus>:<dev>.<func>
PF_FUNC_RE = re.compile("\.(\d+)", 0)
_SRIOV_TOTALVFS = "sriov_totalvfs"
def _ovs_vsctl(args, timeout=None):
@ -267,6 +271,40 @@ def _parse_pf_number(phys_port_name):
return None
# This function is taken from nova/pci/utils.py
def get_function_by_ifname(ifname):
"""Given the device name, returns the PCI address of a device
and returns True if the address is in a physical function.
"""
dev_path = "/sys/class/net/%s/device" % ifname
sriov_totalvfs = 0
if os.path.isdir(dev_path):
try:
# sriov_totalvfs contains the maximum possible VFs for this PF
dev_path_file = os.path.join(dev_path, _SRIOV_TOTALVFS)
with open(dev_path_file, 'r') as fd:
sriov_totalvfs = int(fd.readline().rstrip())
return (os.readlink(dev_path).strip("./"),
sriov_totalvfs > 0)
except (IOError, ValueError):
return os.readlink(dev_path).strip("./"), False
return None, False
def _get_pf_func(pf_ifname):
"""Gets PF function number using pf_ifname and returns function
number or None.
"""
address_str, pf = get_function_by_ifname(pf_ifname)
if not address_str:
return None
match = PF_FUNC_RE.search(address_str)
if match:
return match.group(1)
return None
def get_representor_port(pf_ifname, vf_num):
"""Get the representor netdevice which is corresponding to the VF.
@ -292,7 +330,8 @@ def get_representor_port(pf_ifname, vf_num):
raise exception.RepresentorNotFound(ifname=pf_ifname, vf_num=vf_num)
for device in devices:
if device == pf_ifname:
address_str, pf = get_function_by_ifname(device)
if pf:
continue
device_path = "/sys/class/net/%s" % device
@ -317,6 +356,17 @@ def get_representor_port(pf_ifname, vf_num):
except (OSError, IOError):
continue
# If the phys_port_name of the VF-rep is of the format pfXvfY
# (or vfY@pfX), then match "X" (parent PF's func number) with
# 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
representor_num = _parse_vf_number(phys_port_name)
# Note: representor_num can be 0, referring to VF0
if representor_num is None:

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

@ -401,7 +401,9 @@ class LinuxNetTest(testtools.TestCase):
@mock.patch('six.moves.builtins.open')
@mock.patch.object(os.path, 'isfile')
@mock.patch.object(os, 'listdir')
def test_get_representor_port(self, mock_listdir, mock_isfile, mock_open):
@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_listdir.return_value = [
'pf_ifname', 'rep_vf_1', 'rep_vf_2'
]
@ -409,7 +411,14 @@ class LinuxNetTest(testtools.TestCase):
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'])
['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(),
@ -433,8 +442,10 @@ class LinuxNetTest(testtools.TestCase):
@mock.patch('six.moves.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_2_pfs(
self, mock_listdir, mock_isfile, mock_open):
self, mock_get_function_by_ifname, mock_listdir, mock_isfile,
mock_open):
mock_listdir.return_value = [
'pf_ifname1', 'pf_ifname2', 'rep_pf1_vf_1', 'rep_pf1_vf_2',
'rep_pf2_vf_1', 'rep_pf2_vf_2',
@ -443,15 +454,29 @@ class LinuxNetTest(testtools.TestCase):
mock_open.return_value.__enter__ = lambda s: s
readline_mock = mock_open.return_value.readline
readline_mock.side_effect = (
['pf1_sw_id', 'pf1_sw_id', 'pf2_sw_id', '1', 'pf1_sw_id', '2'])
ifname = linux_net.get_representor_port('pf_ifname1', '2')
self.assertEqual('rep_pf1_vf_2', ifname)
['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)])
ifname = linux_net.get_representor_port('pf_ifname2', '2')
self.assertEqual('rep_pf2_vf_2', ifname)
@mock.patch('six.moves.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_not_found(
self, mock_listdir, mock_isfile, mock_open):
self, mock_get_function_by_ifname, mock_listdir, mock_isfile,
mock_open):
mock_listdir.return_value = [
'pf_ifname', 'rep_vf_1', 'rep_vf_2'
]
@ -460,6 +485,13 @@ class LinuxNetTest(testtools.TestCase):
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)])
self.assertRaises(
exception.RepresentorNotFound,
linux_net.get_representor_port,
@ -468,8 +500,10 @@ class LinuxNetTest(testtools.TestCase):
@mock.patch('six.moves.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_exception_io_error(
self, mock_listdir, mock_isfile, mock_open):
self, mock_get_function_by_ifname, mock_listdir, mock_isfile,
mock_open):
mock_listdir.return_value = [
'pf_ifname', 'rep_vf_1', 'rep_vf_2'
]
@ -478,6 +512,13 @@ class LinuxNetTest(testtools.TestCase):
readline_mock = mock_open.return_value.readline
readline_mock.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)])
self.assertRaises(
exception.RepresentorNotFound,
linux_net.get_representor_port,
@ -486,8 +527,10 @@ class LinuxNetTest(testtools.TestCase):
@mock.patch('six.moves.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_exception_value_error(
self, mock_listdir, mock_isfile, mock_open):
self, mock_get_function_by_ifname, mock_listdir, mock_isfile,
mock_open):
mock_listdir.return_value = [
'pf_ifname', 'rep_vf_1', 'rep_vf_2'
]
@ -496,6 +539,13 @@ class LinuxNetTest(testtools.TestCase):
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)])
self.assertRaises(
exception.RepresentorNotFound,
linux_net.get_representor_port,