[libvirt] [PATCHv2] nodedev: add switchdev to NIC capabilities

[Ján Tomko]

On Thu, Sep 14, 2017 at 11:23:02AM -0400, Laine Stump wrote:
>(Almost all of my comments result in "ok, no action needed". There are
>just three items I would like to see changed (2 trivial, 1 also small
>but Edan or John may think it prudent to re-test with the change before
>pushing) - I marked those comments with [**].
>
>On 08/21/2017 05:19 AM, Edan David wrote:
>> Adding functionality to libvirt that will allow querying the interface
>> for the availability of switchdev Offloading NIC capabilities.
>> The switchdev mode was introduced in kernel 4.8, the iproute2-devlink
>> command to retrieve the swtichdev NIC feature,
>> Command example:  devlink dev eswitch show pci/0000:03:00.0
>> This feature is needed for Openstack so we can do a scheduling decision
>> if the NIC is in Hardware Offload (switchdev) or regular SR-IOV (legacy) mode.
>> And select the appropriate hypervisors with the requested capability see [1].
>>
>> [1] - https://specs.openstack.org/openstack/nova-specs/specs/pike/approved/enable-sriov-nic-features.html
>> ---
>>  configure.ac                                      |  13 ++
>>  docs/formatnode.html.in                           |   1 +
>>  src/util/virnetdev.c                              | 187 +++++++++++++++++++++-
>>  src/util/virnetdev.h                              |   1 +
>>  tests/nodedevschemadata/net_00_13_02_b9_f9_d3.xml |   1 +
>>  tests/nodedevschemadata/net_00_15_58_2f_e9_55.xml |   1 +
>>  6 files changed, 203 insertions(+), 1 deletion(-)
>>

This patch fails to compile on a Gentoo machine with
sys-kernel/linux-headers-4.8:

util/virnetdev.c:3248:18: error: use of undeclared identifier 'DEVLINK_CMD_ESWITCH_GET'; did you mean 'DEVLINK_CMD_ESWITCH_MODE_GET'?
    gmsgh->cmd = DEVLINK_CMD_ESWITCH_GET;
                 ^~~~~~~~~~~~~~~~~~~~~~~
                 DEVLINK_CMD_ESWITCH_MODE_GET
/usr/include/linux/devlink.h:60:2: note: 'DEVLINK_CMD_ESWITCH_MODE_GET' declared here
        DEVLINK_CMD_ESWITCH_MODE_GET,
        ^
1 error generated.

>> diff --git a/configure.ac b/configure.ac
>> index b12b7fa..c089798 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -627,6 +627,19 @@ if test "$with_linux" = "yes"; then
>>      AC_CHECK_HEADERS([linux/btrfs.h])
>>  fi
>>
>> +dnl
>> +dnl check for kernel headers required by devlink
>> +dnl
>> +if test "$with_linux" = "yes"; then
>> +    AC_CHECK_HEADERS([linux/devlink.h])
>> +    AC_CHECK_DECLS([DEVLINK_GENL_VERSION, DEVLINK_GENL_NAME, DEVLINK_ATTR_MAX, DEVLINK_CMD_ESWITCH_GET, DEVLINK_ATTR_BUS_NAME, DEVLINK_ATTR_DEV_NAME, DEVLINK_ATTR_ESWITCH_MODE, DEVLINK_ESWITCH_MODE_SWITCHDEV],
>
>That's very ..... thorough, and potentially misleading if someone ever
>wanted to use devlink to check for something other than switchdev (e.g.
>[mythical feature X]) on some system that didn't have the proper stuff
>defined for switchdev, but did have it for [other mythical feature X].
>But that's all just hypothetical, so this is fine with me.
>
>
>> +                   [AC_DEFINE([HAVE_DECL_DEVLINK],
>> +                              [1],
>> +                              [whether devlink declarations is available])],
>
>[**]
>s/is/are/
>

It seems the [action-if-found] is executed for every found symbol:

#define HAVE_DECL_DEVLINK_GENL_VERSION 1
#define HAVE_DECL_DEVLINK 1
#define HAVE_DECL_DEVLINK_GENL_NAME 1
#define HAVE_DECL_DEVLINK 1
#define HAVE_DECL_DEVLINK_ATTR_MAX 1
#define HAVE_DECL_DEVLINK 1
#define HAVE_DECL_DEVLINK_CMD_ESWITCH_GET 0
#define HAVE_DECL_DEVLINK_ATTR_BUS_NAME 1
#define HAVE_DECL_DEVLINK 1
#define HAVE_DECL_DEVLINK_ATTR_DEV_NAME 1
#define HAVE_DECL_DEVLINK 1
#define HAVE_DECL_DEVLINK_ATTR_ESWITCH_MODE 1
#define HAVE_DECL_DEVLINK 1
#define HAVE_DECL_DEVLINK_ESWITCH_MODE_SWITCHDEV 1
#define HAVE_DECL_DEVLINK 1

so this usage of AC_CHECK_DECLS is bogus.

Jan

>> +                   [],
>> +                   [[#include <linux/devlink.h>]])
>> +fi
>> +
>>  dnl Allow perl/python overrides
>>  AC_PATH_PROGS([PYTHON], [python2 python])
>>  if test -z "$PYTHON"; then
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170918/44cd568c/attachment-0001.sig>