add support for phys_port_name attribute in Xenial/16.04LTS

[Eric Desrochers]

[Impact]
In Xenial/16.04LTS, one can't generate network interface name from "phys_port_name" attribute.

"phys_port_name" indicates the interface physical port name within the NIC.

[Test Case]

Check that udev (systemd-udevd) provides the phys_port_name property
Tests should be done on kernel versions: v4.15 (HWE)

# cat /sys/class/net/<INTERFACE>/phys_port_name
yyy

Look if interface name contains the 'phys_port_name':

$ ip link show
....
3: ens3nyyy: <BROADCAST,MULTICAST,UP,LOWER_UP> ...
....

[Regression Potential]

* This piece of code is already in place in Bionic (systemd) and late.
AFAICT, nothing has been reported since then with regards to this feature.

* phys_port_name kernel support has been introduced in v4.1, but none of the current v4.4 Xenial kernel drivers uses it (minus rocker which is a test bed software switch for devel work on switchdev, it has no real function)

# Xenial - kernel v4.4
config-4.4.0-142-generic:# CONFIG_NET_SWITCHDEV is not set

No drivers uses the phys_port_name method at all + NET_SWITCHDEV is not even turned on.

# Xenial HWE - kernel v4.15
config-4.15.0-99-generic:CONFIG_NET_SWITCHDEV=y

Meaning if a regression arise, it will be limited to the HWE kernel (v4.15) and to the "Ethernet switch device driver model (switchdev)":
mlx5
mlxsw
bnxt
sfc (solarflar)

--
drivers/net/ethernet:
--
broadcom/bnxt/bnxt_vfr.c: .ndo_get_phys_port_name = bnxt_vf_rep_get_phys_port_name
broadcom/bnxt/bnxt.c: .ndo_get_phys_port_name = bnxt_get_phys_port_name
cavium/liquidio/lio_vf_rep.c: .ndo_get_phys_port_name = lio_vf_rep_phys_port_name,
mellanox/mlx5/core/en_rep.c: .ndo_get_phys_port_name = mlx5e_rep_get_phys_port_name,
mellanox/mlxsw/switchx2.c: .ndo_get_phys_port_name = mlxsw_sx_port_get_phys_port_name,
mellanox/mlxsw/spectrum.c: .ndo_get_phys_port_name = mlxsw_sp_port_get_phys_port_name,
netronome/nfp/nfp_net_repr.c: .ndo_get_phys_port_name = nfp_port_get_phys_port_name,
netronome/nfp/nfp_net_common.c: .ndo_get_phys_port_name = nfp_port_get_phys_port_name,
rocker/rocker_main.c: .ndo_get_phys_port_name = rocker_port_get_phys_port_name,
sfc/efx.c: .ndo_get_phys_port_name = efx_get_phys_port_name,
--

# On other more specific kernels the regression potential can be expanded to virtio-net driver as well.

# cloud-init may taint the result as it renames switchdev(s) after systemd at first initialization (even without the phys_port_name support) as follows:

(Testing on a sfc card / server deployed by MAAS)

syslog:May 20 22:12:43 OBFUSCATED_HOST kernel: [ 36.199674] sfc 0000:08:00.1 ens1f1: renamed from eth1 ## systemd
syslog:May 20 22:12:43 OBFUSCATED_HOST kernel: [ 37.128236] sfc 0000:08:00.0 ens1f0: renamed from eth0 ## systemd

syslog:May 20 22:12:43 OBFUSCATED_HOST kernel: [ 84.653460] sfc 0000:08:00.0 ens1f0np0: renamed from ens1f0 ## cloud-init
syslog:May 20 22:12:43 OBFUSCATED_HOST kernel: [ 84.697177] sfc 0000:08:00.1 ens1f1np1: renamed from ens1f1 ## cloud-init

cloud-init.log:2020-05-20 22:04:53,980 - util.py[DEBUG]: Running command ['ip', 'link', 'set', 'ens1f0', 'name', 'ens1f0np0'] with allowed return codes [0] (shell=False, capture=True)
cloud-init.log:2020-05-20 22:04:54,016 - util.py[DEBUG]: Running command ['ip', 'link', 'set', 'ens1f1', 'name', 'ens1f1np1'] with allowed return codes [0] (shell=False, capture=True)

systemd:
... OBFUSCATED_HOST systemd[1]: sys-subsystem-net-devices-ens1f1.device: Dev sys-subsystem-net-devices-ens1f1.device appeared twice with different sysfs paths /sys/devices/pci0000:00/0000:00:03.0/0000:08:00.1/net/ens1f1 and /sys/devices/pci0000:00/0000:00:03.0/0000:08:00.1/net/ens1f1np1

# 1 concern was raise here by ddstreet:
https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1875927/comments/1

Here's the result of the test I have performed:
https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1875927/comments/5

This change indeed rename existing switchdev interfaces to add the 'phys_port_name' (if any) into it. I'm afraid this will be a non-acceptable behaviour change in stable release and won't be SRU'able.

More discussion on this topic to come ....

[Other informations]
https://github.com/systemd/systemd/commit/4887b656c22af059d4e833de7b56544f24951184
https://github.com/systemd/systemd/pull/4506

[Original Description]

It has been brought to my attention that systemd in Xenial/16.04LTS doesn't have support for phys_port_name[0] attribute.

The support has been first introduced in systemd version "232" via:
https://github.com/systemd/systemd/commit/4887b656c22af059d4e833de7b56544f24951184
https://github.com/systemd/systemd/pull/4506

Bionic and late have the necessary bits ( systemd >232), but not Xenial (229)[1]

Support for "phys_port_name" has been first introduced in the kernel with v4.1[2]

[0]
- https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-class-net
- https://www.freedesktop.org/software/systemd/man/systemd.net-naming-scheme.html
- https://www.kernel.org/doc/Documentation/networking/switchdev.txt

[1]
# git systemd/systemd
git describe --contains 4887b656c22af059d4e833de7b56544f24951184
v232~15

# rmadison
 => systemd | 229-4ubuntu21.27 | xenial-updates
 systemd | 237-3ubuntu10.39 | bionic-updates
 systemd | 240-6ubuntu5.8 | disco-updates
 systemd | 242-7ubuntu3.7 | eoan-updates
 systemd | 245.4-4ubuntu3 | focal
 systemd | 245.4-4ubuntu3 | groovy

[2]
https://github.com/torvalds/linux/commit/db24a9044ee1

$ git describe --contains db24a9044ee1
v4.1-rc1

[Dan Streetman (ddstreet)]

I think this would change the device name for existing devices, right? If so, that doesn't seem appropriate for an SRU, as it could break existing users that expect their device names to be consistent...

[Eric Desrochers (slashd)]

Right, @ddstreet it's a very good point to validate/verify.
Definitely worth looking at that aspect pre-SRU.

Bionic and late has the fix since release time ... meaning that patch has never been introduced via SRU yet.

- Eric

[Eric Desrochers (slashd)]

Centos had to revert the patch in their systemd package via:
commit 83b94d10951d79445f7975bff835a88261c11a4e

SOURCES/0499-Revert-udev-net_id-add-support-for-phys_port_name-at.patch

They workaround (differing from upstream) the regression installing script and udev rule to add phys_port_name for mlxsw and rocker drivers and put them in dracut.

[Eric Desrochers (slashd)]

https://git.centos.org/rpms/systemd/c/83b94d10951d79445f7975bff835a88261c11a4e?branch=83b94d10951d79445f7975bff835a88261c11a4e

[Eric Desrochers (slashd)]

ddstreet's concern were right.

[Bionic w/ v4.15 kernel w/o cloud-init installed/configured]
** cloud-init rename the interface for some reasons after systemd that is why I uninstalled it for this test on my MAAS server deployment **

6: ens1f0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop portid 000f5331c120 state DOWN mode DEFAULT group default qlen 1000
link/ether 00:0f:53:31:c1:20 brd ff:ff:ff:ff:ff:ff

$ cat /sys/class/net/ens1f0/phys_port_name
p0

[Same thing as above with a systemd test packages + its derived packages]

4: ens1f0np0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop portid 000f5331c120 state DOWN mode DEFAULT group default qlen 1000
link/ether 00:0f:53:31:c1:20 brd ff:ff:ff:ff:ff:ff

cat /sys/class/net/ens1f0np0/phys_port_name
p0

Result:
The switchdev interface changes name to introduce the phys_port_name in the naming scheme.