[PATCH v3] util: support PCI passthrough net device stats collection

[Daniel Henrique Barboza]

On 9/24/20 5:18 AM, zhenwei pi wrote:
> Collect PCI passthrough net device stats from kernel by netlink
> API.
> 
> Currently, libvirt can not get PCI passthrough net device stats,
> run command:
>   #virsh domifstat instance --interface=52:54:00:2d:b2:35
>   error: Failed to get interface stats instance 52:54:00:2d:b2:35
>   error: internal error: Interface name not provided
> 
> The PCI device(usually SR-IOV virtual function device) is detached
> while it's used in PCI passthrough mode. And we can not parse this
> device from /proc/net/dev any more.
> 
> In this patch, libvirt check net device is VF of not firstly, then
> query virNetDevVFInterfaceStats(new API).
> virNetDevVFInterfaceStats parses VFs info of all PFs, compares MAC
> address until the two MAC addresses match.
> '#ip -s link show' can get the same result. Instead of parsing the
> output result, implement this feature by libnl API.
> 
> Notice that this feature deponds on driver of PF.

s/deponds/depends

> Test on Mellanox ConnectX-4 Lx, it works well.
> Also test on Intel Corporation 82599ES, it works, but only get 0.
> (ip-link command get the same result).
> 
> IFLA_VF_STATS is supported since Linux-4.2, suggested by Laine,
> just using defined(__linux__) && WITH_LIBNL is enough.
> 
> Signed-off-by: zhenwei pi <pizhenwei at bytedance.com>
> ---
>   src/libvirt_private.syms |   1 +
>   src/qemu/qemu_driver.c   |   3 ++
>   src/util/virnetdev.c     | 121 +++++++++++++++++++++++++++++++++++++++++++++++
>   src/util/virnetdev.h     |   5 ++
>   4 files changed, 130 insertions(+)
> 

[...]

> +
> +/**
> + * virNetDevVFInterfaceStats:
> + * @mac: MAC address of the VF interface
> + * @stats: returns stats of the VF interface
> + *
> + * Get the VF interface from kernel by netlink.
> + * Returns 0 on success, -1 on failure.
> + */
> +int
> +virNetDevVFInterfaceStats(virMacAddrPtr mac,
> +                          virDomainInterfaceStatsPtr stats)
> +{
> +    int rc = -1;
> +    void *nlData = NULL;
> +    struct nlattr *tb[IFLA_MAX + 1] = {NULL, };
> +    char *sysfsDevicePath = NULL;

This variable is used only in the while() loop down there and you're freeing
it after each loop iteration, and then you need to clean it in the function wide
'cleanup' label as well.

Yon can move it with autocleanup enabled down there ....


> +    DIR *dirp = NULL;
> +    struct dirent *dp;
> +
> +    if (virDirOpen(&dirp, SYSFS_NET_DIR) < 0)
> +        return -1;
> +
> +    /* get all PCI net devices, and parse VFs list from netlink API.
> +     * compare MAC address, collect device stats if matching.
> +     */
> +    while (virDirRead(dirp, &dp, SYSFS_NET_DIR) > 0) {

Here:
            g_autofree char *sysfsDevicePath = NULL;
               

> +        if (virNetDevSysfsFile(&sysfsDevicePath, dp->d_name, "device") < 0)
> +            break;
> +
> +        if (virNetDevIsPCIDevice(sysfsDevicePath)) {
> +            rc = virNetlinkDumpLink(dp->d_name, -1, &nlData, tb, 0, 0);
> +            if (rc < 0) {
> +                rc = -1;
> +                goto cleanup;
> +            }
> +
> +            rc = virNetDevParseVfStats(tb, mac, stats);
> +            VIR_FREE(nlData);
> +            if (rc == 0)
> +                goto cleanup;
> +        }
> +        VIR_FREE(sysfsDevicePath);

And then you don't need this VIR_FREE() call because sysfsDevicePath will be auto-freed
after each iteration ....


> +    }
> +
> + cleanup:
> +    VIR_FREE(sysfsDevicePath);

And this VIR_FREE() call becomes obsolete because the variable is not on function
scope any longer.



Thanks,


DHB


> +    VIR_DIR_CLOSE(dirp);
> +    return rc;
> +}
>   
>   #else /* defined(__linux__) && defined(WITH_LIBNL)  && defined(IFLA_VF_MAX) */
>   
> @@ -2309,6 +2420,16 @@ virNetDevSetNetConfig(const char *linkdev G_GNUC_UNUSED,
>   }
>   
>   
> +int
> +virNetDevVFInterfaceStats(virMacAddrPtr mac G_GNUC_UNUSED,
> +                     virDomainInterfaceStatsPtr stats G_GNUC_UNUSED)
> +{
> +    virReportSystemError(ENOSYS, "%s",
> +                         _("Unable to get VF net device stats on this platform"));
> +    return -1;
> +}
> +
> +
>   #endif /* defined(__linux__) && defined(WITH_LIBNL) && defined(IFLA_VF_MAX) */
>   
>   VIR_ENUM_IMPL(virNetDevIfState,
> diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
> index 5f581323ed..ff59d9d341 100644
> --- a/src/util/virnetdev.h
> +++ b/src/util/virnetdev.h
> @@ -312,4 +312,9 @@ int virNetDevSysfsFile(char **pf_sysfs_device_link,
>   int virNetDevRunEthernetScript(const char *ifname, const char *script)
>       G_GNUC_NO_INLINE;
>   
> +int virNetDevVFInterfaceStats(virMacAddrPtr mac,
> +                              virDomainInterfaceStatsPtr stats)
> +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> +
> +
>   G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetDevRxFilter, virNetDevRxFilterFree);
>