Merge ~fnordahl/ubuntu/+source/libvirt:bug/1892132-hiruste into ubuntu/+source/libvirt:ubuntu/hirsute-devel

[Frode Nordahl]

[Christian Ehrhardt  (paelzer)]

We've just completed the former libvirt SRU - well actually it still is in phasing but it can be considered complete. So it is the right time to do this.

* Changelog:
  - [+] changelog entry correct version and targeted codename
  - [+] changelog entries correct
  - [+] bug references correct

* New Delta:
  - [+] patches match what was proposed/committed upstream
  - [-] patches correctly included in debian/patches/series
  - [-] patches have correct DEP3 metadata

It isn't strictly required, but libvirt has that many patches that Ubuntu and started to structure them in directories.
You'll have (a subset of) directories in debian/patches for
- debian (custom patches for Debian)
- stable (pre release stable updates)
- revert (plain reverts of upstream)
- forwarded (a patch created for U/D and forwarded to upstream)
- ubuntu (custom patches for Ubuntu)
- ubuntu-aa (custom patches for Ubuntu in regard to apparmor)

Not all are sorted yet, but at least those we add we try to keep in line, therefore I'd ask to put them into debian/patches/ubuntu if that isn't too much to ask for.

In regard to metadata you have origin - thanks, that probably is the most important one.
There are many that can be added, but of the somewhat required fields you miss out on adding
Bug-Ubuntu: https://bugs.launchpad.net/bugs/<todo-bug>
Last-Update: 20xx-mm-dd

* Git/Maintenance
  - [+] testcases added or not needed for this
  - [-] commits are properly split (more important on -dev than on SRUs)

It is not strictly needed for SRUs, but still helps later maintenance (e.g. to cherry pick the changes but not the CL which always is different) if code-changes and changelog would be in individual commits. If you don't mind changing that would be appreciated.

* Build/Test:
  - not checked, is there a PPA with this somewhere already?

P.S. no need to change this but the branch being called "hiruste" made me fail quite some commands :-)

review: Needs Fixing

[Frode Nordahl (fnordahl)]

Thank you for the review, Christian.

Will propose new MP shortly that addresses your insightful feedback.

PS: Sorry about the misnamed branch, will re-propose to new MP to fix that.

Cheers!

[Christian Ehrhardt  (paelzer)]

Superseded by
https://code.launchpad.net/~fnordahl/ubuntu/+source/libvirt/+git/libvirt/+merge/40706

Unmerged commits

d098fc9... by Frode Nordahl on 2021-07-16

7.0.0-2ubuntu2.1 (patches unapplied)

* Add support for switchdev NICs that link representor ports to parent PCI
  device. (LP: #1892132)
  - d/p/Add-phys_port_name-support-on-virPCIGetNetName.patch
  - d/p/add-virNetDevGetPhysPortName.patch

Signed-off-by: Frode Nordahl <email address hidden>

diff --git a/debian/changelog b/debian/changelog
index ce374d0..473e888 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+libvirt (7.0.0-2ubuntu2.1) hirsute; urgency=medium
+
+  * Add support for switchdev NICs that link representor ports to parent PCI
+    device. (LP: #1892132)
+    - d/p/Add-phys_port_name-support-on-virPCIGetNetName.patch
+    - d/p/add-virNetDevGetPhysPortName.patch
+
+ -- Frode Nordahl <[e-mail]>  Fri, 16 Jul 2021 05:16:36 +0000
+
 libvirt (7.0.0-2ubuntu2) hirsute; urgency=medium
 
   * d/p/u/lp-1921754*: add EPYC-Rome-v2 as v1 missed IBRS and thereby fails
diff --git a/debian/patches/Add-phys_port_name-support-on-virPCIGetNetName.patch b/debian/patches/Add-phys_port_name-support-on-virPCIGetNetName.patch
new file mode 100644
index 0000000..0fe40f5
--- /dev/null
+++ b/debian/patches/Add-phys_port_name-support-on-virPCIGetNetName.patch
@@ -0,0 +1,131 @@
+Description: To support kernels or dkms builds of mlx5_core driver with
+ https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=123f0f53dd64b67e34142485fe866a8a581f12f1
+Origin: upstream, https://github.com/libvirt/libvirt/commit/5b1c525b1f3608156884aed0dc5e925306c1e260
+
+diff --git a/src/util/virpci.c b/src/util/virpci.c
+index 50fd5ef7ea..c685927b60 100644
+--- a/src/util/virpci.c
++++ b/src/util/virpci.c
+@@ -2467,9 +2467,9 @@ virPCIDeviceAddressGetSysfsFile(virPCIDeviceAddressPtr addr,
+  * virPCIGetNetName:
+  * @device_link_sysfs_path: sysfs path to the PCI device
+  * @idx: used to choose which netdev when there are several
+- *       (ignored if physPortID is set)
++ *       (ignored if physPortID is set or physPortName is available)
+  * @physPortID: match this string in the netdev's phys_port_id
+- *       (or NULL to ignore and use idx instead)
++ *       (or NULL to ignore and use phys_port_name or idx instead)
+  * @netname: used to return the name of the netdev
+  *       (set to NULL (but returns success) if there is no netdev)
+  *
+@@ -2501,6 +2501,14 @@ virPCIGetNetName(const char *device_link_sysfs_path,
+     }
+ 
+     while (virDirRead(dir, &entry, pcidev_sysfs_net_path) > 0) {
++        /* save the first entry we find to use as a failsafe
++         * in case we don't match the phys_port_id. This is
++         * needed because some NIC drivers (e.g. i40e)
++         * implement phys_port_id for PFs, but not for VFs
++         */
++        if (!firstEntryName)
++            firstEntryName = g_strdup(entry->d_name);
++
+         /* if the caller sent a physPortID, compare it to the
+          * physportID of this netdev. If not, look for entry[idx].
+          */
+@@ -2511,33 +2519,49 @@ virPCIGetNetName(const char *device_link_sysfs_path,
+                 return -1;
+ 
+             /* if this one doesn't match, keep looking */
+-            if (STRNEQ_NULLABLE(physPortID, thisPhysPortID)) {
+-                /* save the first entry we find to use as a failsafe
+-                 * in case we don't match the phys_port_id. This is
+-                 * needed because some NIC drivers (e.g. i40e)
+-                 * implement phys_port_id for PFs, but not for VFs
+-                 */
+-                if (!firstEntryName)
+-                    firstEntryName = g_strdup(entry->d_name);
+-
++            if (STRNEQ_NULLABLE(physPortID, thisPhysPortID))
+                 continue;
+-            }
++
+         } else {
+-            if (i++ < idx)
+-                continue;
++            /* Most switch devices use phys_port_name instead of
++             * phys_port_id.
++             * NOTE: VFs' representors net devices can be linked to PF's PCI
++             * device, which mean that there'll be multiple net devices
++             * instances and to get a proper net device need to match on
++             * specific regex.
++             * To get PF netdev, for ex., used following regex:
++             * "(p[0-9]+$)|(p[0-9]+s[0-9]+$)"
++             * or to get exact VF's netdev next regex is used:
++             * "pf0vf1$"
++             */
++            g_autofree char *thisPhysPortName = NULL;
++
++            if (virNetDevGetPhysPortName(entry->d_name, &thisPhysPortName) < 0)
++                return -1;
++
++            if (thisPhysPortName) {
++
++                /* if this one doesn't match, keep looking */
++                if (!virStringMatch(thisPhysPortName, VIR_PF_PHYS_PORT_NAME_REGEX))
++                    continue;
++
++            } else {
++
++                if (i++ < idx)
++                    continue;
++            }
+         }
+ 
+         *netname = g_strdup(entry->d_name);
+         return 0;
+     }
+ 
+-    if (!physPortID)
+-        return 0;
+-
+     if (firstEntryName) {
+-        /* we didn't match the provided phys_port_id, but this
+-         * is probably because phys_port_id isn't implemented
+-         * for this NIC driver, so just return the first
++        /* we didn't match the provided phys_port_id / find a
++         * phys_port_name matching VIR_PF_PHYS_PORT_NAME_REGEX / find
++         * as many net devices as the value of idx, but this is
++         * probably because phys_port_id / phys_port_name isn't
++         * implemented for this NIC driver, so just return the first
+          * (probably only) netname we found.
+          */
+         *netname = g_steal_pointer(&firstEntryName);
+@@ -2545,9 +2569,8 @@ virPCIGetNetName(const char *device_link_sysfs_path,
+     }
+ 
+     virReportError(VIR_ERR_INTERNAL_ERROR,
+-                   _("Could not find network device with "
+-                     "phys_port_id '%s' under PCI device at %s"),
+-                   physPortID, device_link_sysfs_path);
++                   _("Could not find any network device under PCI device at %s"),
++                   device_link_sysfs_path);
+     return -1;
+ }
+ 
+diff --git a/src/util/virpci.h b/src/util/virpci.h
+index 43828b0a8a..1a4bf1d751 100644
+--- a/src/util/virpci.h
++++ b/src/util/virpci.h
+@@ -55,6 +55,11 @@ struct _virZPCIDeviceAddress {
+ 
+ #define VIR_PCI_DEVICE_ADDRESS_FMT "%04x:%02x:%02x.%d"
+ 
++/* Represents format of PF's phys_port_name in switchdev mode:
++ * 'p%u' or 'p%us%u'. New line checked since value is readed from sysfs file.
++ */
++#define VIR_PF_PHYS_PORT_NAME_REGEX  "(p[0-9]+$)|(p[0-9]+s[0-9]+$)"
++
+ struct _virPCIDeviceAddress {
+     unsigned int domain;
+     unsigned int bus;
diff --git a/debian/patches/add-virNetDevGetPhysPortName.patch b/debian/patches/add-virNetDevGetPhysPortName.patch
new file mode 100644
index 0000000..03ee48b
--- /dev/null
+++ b/debian/patches/add-virNetDevGetPhysPortName.patch
@@ -0,0 +1,113 @@
+Description: To support kernels or dkms builds of mlx5_core driver with
+ https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=123f0f53dd64b67e34142485fe866a8a581f12f1
+Origin: upstream, https://github.com/libvirt/libvirt/commit/97ebb982453bc23759c5f180799d6f2207b81c80
+
+diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
+index a73e5f72f1..2485718b48 100644
+--- a/src/util/virnetdev.c
++++ b/src/util/virnetdev.c
+@@ -1147,6 +1147,29 @@ virNetDevGetPCIDevice(const char *devName)
+ # endif
+ 
+ 
++/* A wrapper to get content of file from ifname SYSFS_NET_DIR
++ */
++static int
++virNetDevGetSysfsFileValue(const char *ifname,
++                           const char *fileName,
++                           char **sysfsFileData)
++{
++    g_autofree char *sysfsFile = NULL;
++
++    *sysfsFileData = NULL;
++
++    if (virNetDevSysfsFile(&sysfsFile, ifname, fileName) < 0)
++        return -1;
++
++    /* a failure to read just means the driver doesn't support
++     * <fileName>, so set success now and ignore the return from
++     * virFileReadAllQuiet().
++     */
++
++    ignore_value(virFileReadAllQuiet(sysfsFile, 1024, sysfsFileData));
++    return 0;
++}
++
+ /**
+  * virNetDevGetPhysPortID:
+  *
+@@ -1165,20 +1188,29 @@ int
+ virNetDevGetPhysPortID(const char *ifname,
+                        char **physPortID)
+ {
+-    g_autofree char *physPortIDFile = NULL;
+-
+-    *physPortID = NULL;
+-
+-    if (virNetDevSysfsFile(&physPortIDFile, ifname, "phys_port_id") < 0)
+-        return -1;
++    return virNetDevGetSysfsFileValue(ifname, "phys_port_id", physPortID);
++}
+ 
+-    /* a failure to read just means the driver doesn't support
+-     * phys_port_id, so set success now and ignore the return from
+-     * virFileReadAllQuiet().
+-     */
+ 
+-    ignore_value(virFileReadAllQuiet(physPortIDFile, 1024, physPortID));
+-    return 0;
++/**
++ * virNetDevGetPhysPortName:
++ *
++ * @ifname: name of a netdev
++ *
++ * @physPortName: pointer to char* that will receive @ifname's
++ *                phys_port_name from sysfs (null terminated
++ *                string). Could be NULL if @ifname's net driver doesn't
++ *                support phys_port_name (most netdev drivers
++ *                don't). Caller is responsible for freeing the string
++ *                when finished.
++ *
++ * Returns 0 on success or -1 on failure.
++ */
++int
++virNetDevGetPhysPortName(const char *ifname,
++                         char **physPortName)
++{
++    return virNetDevGetSysfsFileValue(ifname, "phys_port_name", physPortName);
+ }
+ 
+ 
+@@ -1433,6 +1465,17 @@ virNetDevGetPhysPortID(const char *ifname G_GNUC_UNUSED,
+     return 0;
+ }
+ 
++int
++virNetDevGetPhysPortName(const char *ifname G_GNUC_UNUSED,
++                       char **physPortName)
++{
++    /* this actually should never be called, and is just here to
++     * satisfy the linker.
++     */
++    *physPortName = NULL;
++    return 0;
++}
++
+ int
+ virNetDevGetVirtualFunctions(const char *pfname G_GNUC_UNUSED,
+                              char ***vfname G_GNUC_UNUSED,
+diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
+index f016012718..e9349e7f59 100644
+--- a/src/util/virnetdev.h
++++ b/src/util/virnetdev.h
+@@ -250,6 +250,10 @@ int virNetDevGetPhysPortID(const char *ifname,
+                            char **physPortID)
+     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
+     G_GNUC_WARN_UNUSED_RESULT;
++int virNetDevGetPhysPortName(const char *ifname,
++                           char **physPortName)
++    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
++    G_GNUC_WARN_UNUSED_RESULT;
+ 
+ int virNetDevGetVirtualFunctions(const char *pfname,
+                                  char ***vfname,
diff --git a/debian/patches/series b/debian/patches/series
index eedc4d2..00de36e 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -40,3 +40,5 @@ ubuntu/lp-1921880-cpu_map-Add-EPYC-Milan-x86-CPU-model.patch
 ubuntu/lp-1921880-cpu_map-Install-x86_EPYC-Milan.xml.patch
 ubuntu/lp-1921880-cpu_map-sync_qemu_i386.py-Add-mapping-for-svme-addr-.patch
 ubuntu/lp-1921880-cpu_map-Fix-spelling-of-svme-addr-chk-feature.patch
+Add-phys_port_name-support-on-virPCIGetNetName.patch
+add-virNetDevGetPhysPortName.patch