openstack
/
os-vif
Code Issues Proposed changes

Improve OVS Representor Lookup 

This patch set addresses some of the concerns outlined in:
https://bugs.launchpad.net/os-vif/+bug/1704485

* Determining the PF switchdev from the VF PCI address:
  /sys/bus/pci/devices/<PCI_ADDR>/physfn/net/* is scanned and the _last_
  netdev is assumed to be the PF switchdev netdev.

  It's more general to assume that there might be multiple netdevs, and
  one would be the desired switchdev. This change iterates through all the
  netdevs and pick the first with a valid phys_switch_id. (A sysfs file
  that returns a non-null ID.)

* Note: in stead of returning the _last_ netdev in the physfn
  directory, the behaviour now returns the first netdev that has a valid
  switchdev id.

* Added test cases to verify the above behaviour.

Change-Id: Ia8d4609746df38c7072e535fb92b00b903d05042
Partial-Bug: #1704485
Signed-off-by: Jan Gutter <jan.gutter@netronome.com>
This commit is contained in:
Jan Gutter 2017-07-14 23:25:33 +02:00
parent 4a8e07f6c5
commit c9facf2dd3
4 changed files with 96 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
vif_plug_ovs/linux_net.py
View File

@ -293,18 +293,41 @@ def _get_sysfs_netdev_path(pci_addr, pf_interface):
return "/sys/bus/pci/devices/%s/net" % (pci_addr)
def get_ifname_by_pci_address(pci_addr, pf_interface=False):
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()
if phys_switch_id != "" and phys_switch_id is not None:
return True
except (OSError, IOError):
return False
return False
def get_ifname_by_pci_address(pci_addr, pf_interface=False, switchdev=False):
"""Get the interface name based on a VF's pci address
:param pci_addr: the PCI address of the VF
:param pf_interface: if True, look for the netdev of the parent PF
:param switchdev: if True, ensure that phys_switch_id is valid
:returns: netdev interface name
The returned interface name is either the parent PF or that of the VF
itself based on the argument of pf_interface.
"""
dev_path = _get_sysfs_netdev_path(pci_addr, pf_interface)
# make the if statement later more readable
ignore_switchdev = not switchdev
try:
dev_info = os.listdir(dev_path)
return dev_info.pop()
for netdev in os.listdir(dev_path):
if ignore_switchdev or _is_switchdev(netdev):
return netdev
except Exception:
raise exception.PciDeviceNotFoundById(id=pci_addr)
raise exception.PciDeviceNotFoundById(id=pci_addr)
def get_vf_num_by_pci_address(pci_addr):

4
vif_plug_ovs/ovs.py
View File

@ -160,7 +160,7 @@ class OvsPlugin(plugin.PluginBase):
vif.network.bridge, constants.OVS_DATAPATH_SYSTEM)
pci_slot = vif.dev_address
pf_ifname = linux_net.get_ifname_by_pci_address(
pci_slot, pf_interface=True)
pci_slot, pf_interface=True, switchdev=True)
vf_num = linux_net.get_vf_num_by_pci_address(pci_slot)
representor = linux_net.get_representor_port(pf_ifname, vf_num)
linux_net.set_interface_state(representor, 'up')
@ -221,7 +221,7 @@ class OvsPlugin(plugin.PluginBase):
"""Remove port from OVS."""
pci_slot = vif.dev_address
pf_ifname = linux_net.get_ifname_by_pci_address(pci_slot,
pf_interface=True)
pf_interface=True, switchdev=True)
vf_num = linux_net.get_vf_num_by_pci_address(pci_slot)
representor = linux_net.get_representor_port(pf_ifname, vf_num)
linux_net.delete_ovs_vif_port(vif.network.bridge, representor)

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

@ -291,6 +291,49 @@ class LinuxNetTest(testtools.TestCase):
mock_vsctl.assert_called_with(args, timeout=timeout)
self.assertTrue(result)
@mock.patch('six.moves.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()])
test_switchdev = linux_net._is_switchdev('pf_ifname')
self.assertEqual(test_switchdev, False)
@mock.patch('six.moves.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)])
test_switchdev = linux_net._is_switchdev('pf_ifname')
mock_open.assert_has_calls(open_calls)
self.assertEqual(test_switchdev, False)
@mock.patch('six.moves.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)])
test_switchdev = linux_net._is_switchdev('pf_ifname')
mock_open.assert_has_calls(open_calls)
self.assertEqual(test_switchdev, True)
@mock.patch('six.moves.builtins.open')
@mock.patch.object(os.path, 'isfile')
@mock.patch.object(os, 'listdir')
@ -394,18 +437,34 @@ class LinuxNetTest(testtools.TestCase):
linux_net.get_representor_port,
'pf_ifname', '3')
@mock.patch('six.moves.builtins.open')
@mock.patch.object(os.path, 'isfile')
@mock.patch.object(os, 'listdir')
def test_physical_function_inferface_name(self, mock_listdir):
def test_physical_function_inferface_name(
self, mock_listdir, mock_isfile, mock_open):
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 = (
['', 'valid_switch'])
ifname = linux_net.get_ifname_by_pci_address(
'0000:00:00.1', pf_interface=True)
self.assertEqual(ifname, 'bar')
'0000:00:00.1', pf_interface=True, switchdev=False)
self.assertEqual(ifname, 'foo')
@mock.patch('six.moves.builtins.open')
@mock.patch.object(os.path, 'isfile')
@mock.patch.object(os, 'listdir')
def test_virtual_function_inferface_name(self, mock_listdir):
def test_physical_function_inferface_name_with_switchdev(
self, mock_listdir, mock_isfile, mock_open):
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 = (
['', 'valid_switch'])
ifname = linux_net.get_ifname_by_pci_address(
'0000:00:00.1', pf_interface=False)
'0000:00:00.1', pf_interface=True, switchdev=True)
self.assertEqual(ifname, 'bar')
@mock.patch.object(os, 'listdir')

6
vif_plug_ovs/tests/unit/test_plugin.py
View File

@ -345,7 +345,8 @@ class PluginTest(testtools.TestCase):
'ensure_ovs_bridge': [mock.call('br0',
constants.OVS_DATAPATH_SYSTEM)],
'get_ifname_by_pci_address': [mock.call('0002:24:12.3',
pf_interface=True)],
pf_interface=True,
switchdev=True)],
'get_vf_num_by_pci_address': [mock.call('0002:24:12.3')],
'get_representor_port': [mock.call('eth0', '2')],
'set_interface_state': [mock.call('eth0_2', 'up')],
@ -379,7 +380,8 @@ class PluginTest(testtools.TestCase):
calls = {
'get_ifname_by_pci_address': [mock.call('0002:24:12.3',
pf_interface=True)],
pf_interface=True,
switchdev=True)],
'get_vf_num_by_pci_address': [mock.call('0002:24:12.3')],
'get_representor_port': [mock.call('eth0', '2')],
'set_interface_state': [mock.call('eth0_2', 'down')],