openstack
/
kuryr-kubernetes
Code Issues Proposed changes

Set MAC address for VF via netlink message to PF 

SR-IOV binding driver uses pyroute2 library to set MAC addresses
to VFs. This is internally implemented via ioctl(SIOCSIFHWADDR)
giving it the name of that device. This is equal to calling
'ip link set dev $VFDEV address $MAC'. However, there is another
way to set MAC address for VF. It works via netlink RTM_SETLINK
message to the PF. This is equal to calling
'ip link set dev $PFDEV vf $VFID mac $MAC'.

How it works:
* ioctl(SIOCSIFHWADDR) asks the VF driver to set the MAC
  --> VF driver asks PF to set MAC for it
  --> PF sets the MAC for VF.
* RTM_SETLINK message asks the PF to set MAC for VF
  --> PF sets the MAC for VF.

In case of setting directly via PF, PF additionally sets an
"administratively changed MAC" flag for that VF in the PF's
driver, and from that point on (until the PF's driver is
reloaded) that VF's MAC address can't be changed using the
method #1.

It's a security feature designed to forbid MAC changing by the
guest OS/app inside the container.

Above leads to the issue where SR-IOV CNI is not able to set MAC
address for VF if its MAC was previously administratively set at
least once (by hands or other software):

  ioctl SIOCSIFHWADDR: Cannot assign requested address

  kernel: igb 0000:05:00.0:
    VF 0 attempted to override administratively set MAC address
    Reload the VF driver to resume operations

After that CNI fails the whole transaction, i.e. fails to change
the interface name as well and subsequently fails the binding.

Netlink PF method to change MAC addresses should be used always.
This will additionally forbid the MAC changing from the inside
of container.

Change-Id: Ic47672e4ce645d9d37b520b6a412a44ae61036e1
Closes-Bug: 1825383
Co-authored-by: Danil Golov <d.golov@samsung.com>
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
This commit is contained in:
Ilya Maximets 2019-04-18 16:27:49 +03:00 committed by Danil Golov
parent 1a32ae5de8
commit d2b223ffca
2 changed files with 22 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
kuryr_kubernetes/cni/binding/sriov.py
View File

@ -65,12 +65,13 @@ class VIFSriovDriver(object):
vlan_id = vif.network.vlan
self._set_vf_vlan(pf, vf_index, vlan_id)
self._set_vf_mac(pf, vf_index, vif.address)
with h_ipdb.interfaces[vf_name] as host_iface:
host_iface.net_ns_fd = utils.convert_netns(netns)
with c_ipdb.interfaces[vf_name] as iface:
iface.ifname = ifname
iface.address = vif.address
iface.mtu = vif.network.mtu
iface.up()
@ -156,6 +157,21 @@ class VIFSriovDriver(object):
LOG.debug("PF %s has %s VFs", pf, nvfs)
return nvfs
def _set_vf_mac(self, pf, vf_index, mac):
"""Call `ip link set enp2s0f1 vf 3 mac fa:16:3e:87:b2:ac`"""
LOG.debug("Seting VF MAC: pf = %s, vf_index = %s, mac = %s",
pf, vf_index, mac)
cmd = [
'ip', 'link',
'set', pf, 'vf', vf_index, 'mac', mac
]
try:
return processutils.execute(*cmd, run_as_root=True)
except Exception:
LOG.exception("Unable to execute %s", cmd)
raise
def _set_vf_vlan(self, pf, vf_index, vlan_id):
"""Call `ip link set enp1s0f0 vf 5 vlan 10`"""
cmd = [

7
kuryr_kubernetes/tests/unit/cni/test_binding.py
View File

@ -219,15 +219,18 @@ class TestSriovDriver(TestDriverMixin, test_base.TestCase):
'_get_host_pf_names')
@mock.patch('kuryr_kubernetes.cni.binding.sriov.VIFSriovDriver.'
'_get_available_vf_info')
def test_connect(self, m_avail_vf_info, m_host_pf_names):
@mock.patch('kuryr_kubernetes.cni.binding.sriov.VIFSriovDriver.'
'_set_vf_mac')
def test_connect(self, m_set_vf_mac, m_avail_vf_info, m_host_pf_names):
m_avail_vf_info.return_value = [self.ifname, 1, 'h_interface']
m_host_pf_names.return_value = 'h_interface'
self._test_connect()
self.assertEqual(self.ifname, self.m_c_iface.ifname)
self.assertEqual(1, self.m_c_iface.mtu)
self.assertEqual(str(self.vif.address), self.m_c_iface.address)
self.m_c_iface.up.assert_called_once_with()
m_set_vf_mac.assert_called_once_with('h_interface', 1,
str(self.vif.address))
def test_disconnect(self):
self._test_disconnect()