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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
