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

(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/enab... > --- > 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(-) >

> 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/

> + [], > + [[#include <linux/devlink.h>]]) > +fi > + > dnl Allow perl/python overrides > AC_PATH_PROGS([PYTHON], [python2 python]) > if test -z "$PYTHON"; then