configure.ac | 8 ++++++-- src/util/virnetdev.c | 3 +++ 2 files changed, 9 insertions(+), 2 deletions(-)
Turns out the mechanism of providing multiple definitions to check via
AC_CHECK_DECLS in order to determine whether HAVE_DECL_DEVLINK should
be set resulted in a false positive since as long as one of the defs
was true, then the HAVE_DECL_DEVLINK got defined.
But we cannot just check for the symbol we most care about due to
a complication in a kernel 4.11 change that altered the name of the
symbol from DEVLINK_CMD_ESWITCH_MODE_GET to DEVLINK_CMD_ESWITCH_GET
in the newer kernel and added a #define for the former name with an
ominous comment/caveat that the old format is obsolete should never
be used. However, if we only looked for the new symbol, that would
leave kernels 4.8 through 4.10 unable to use the command even though
it exists under an old name.
So, rather than checking for whether the devlink.h on the system has
multiple symbols, let's only check for whether the command we want
is defined. This patch checks for both symbol names in order to
allow usage and then adds a check to see if the new name doesn't
exist after linux/devlink.h is include, then to use #define of
the new name to the old name - essentially the reverse logic that
the 4.11 header file has.
Note that we do not need to check for other symbols as well since
they already existed prior to when the various ESWITCH symbols were
added. Other defintions include DEVLINK_GENL_VERSION, DEVLINK_GENL_NAME,
DEVLINK_ATTR_MAX, DEVLINK_ATTR_BUS_NAME, and DEVLINK_ATTR_DEV_NAME.
The DEVLINK_ATTR_ESWITCH_MODE and DEVLINK_ESWITCH_MODE_SWITCHDEV
were added at the same time as DEVLINK_CMD_ESWITCH_MODE_GET, but
it's only DEVLINK_CMD_ESWITCH_MODE_GET that changed in 4.11.
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
v1: https://www.redhat.com/archives/libvir-list/2017-September/msg00543.html
Just to make it official - here's what I described in the v1 followup
which got the ACK if comment from Andrea:
https://www.redhat.com/archives/libvir-list/2017-September/msg00591.html
Still, I won't push this yet until it too gets an official blessing.
configure.ac | 8 ++++++--
src/util/virnetdev.c | 3 +++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac
index c9509c7f9..73ae7fdd5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -630,12 +630,16 @@ fi
dnl
dnl check for kernel headers required by devlink
dnl
+dnl kernel 4.8 introduced DEVLINK_CMD_ESWITCH_MODE_GET, but that was
+dnl later changed in kernel 4.11 to be just DEVLINK_CMD_ESWITCH_GET, so
+dnl for "completeness" let's allow HAVE_DECL_DEVLINK to be define if
+dnl either is defined.
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],
+ AC_CHECK_DECLS([DEVLINK_CMD_ESWITCH_GET, DEVLINK_CMD_ESWITCH_MODE_GET],
[AC_DEFINE([HAVE_DECL_DEVLINK],
[1],
- [whether devlink declarations are available])],
+ [whether devlink CMD_ESWITCH_GET is available])],
[],
[[#include <linux/devlink.h>]])
fi
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 040693925..435ca5562 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -61,6 +61,9 @@
#if HAVE_DECL_DEVLINK
# include <linux/devlink.h>
+# ifndef DEVLINK_CMD_ESWITCH_GET
+# define DEVLINK_CMD_ESWITCH_GET DEVLINK_CMD_ESWITCH_MODE_GET
+# endif
#endif
#ifndef IFNAMSIZ
--
2.13.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, 2017-09-19 at 11:15 -0400, John Ferlan wrote: > Just to make it official - here's what I described in the v1 followup > which got the ACK if comment from Andrea: > > https://www.redhat.com/archives/libvir-list/2017-September/msg00591.html My "ACK if" was to Jano's patch, which he already pushed a few hours ago. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 09/19/2017 12:05 PM, Andrea Bolognani wrote: > On Tue, 2017-09-19 at 11:15 -0400, John Ferlan wrote: >> Just to make it official - here's what I described in the v1 followup >> which got the ACK if comment from Andrea: >> >> https://www.redhat.com/archives/libvir-list/2017-September/msg00591.html > > My "ACK if" was to Jano's patch, which he already pushed a few > hours ago. > Oh - I misread - great that's fine. Guess I hadn't updated the repo in a bit.... I'm away from the normal place and things are much slower here... Tks - John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Sep 19, 2017 at 11:15:28AM -0400, John Ferlan wrote: >Turns out the mechanism of providing multiple definitions to check via >AC_CHECK_DECLS in order to determine whether HAVE_DECL_DEVLINK should >be set resulted in a false positive since as long as one of the defs >was true, then the HAVE_DECL_DEVLINK got defined. > >But we cannot just check for the symbol we most care about due to >a complication in a kernel 4.11 change that altered the name of the >symbol from DEVLINK_CMD_ESWITCH_MODE_GET to DEVLINK_CMD_ESWITCH_GET >in the newer kernel and added a #define for the former name with an >ominous comment/caveat that the old format is obsolete should never >be used. However, if we only looked for the new symbol, that would >leave kernels 4.8 through 4.10 unable to use the command even though >it exists under an old name. > >So, rather than checking for whether the devlink.h on the system has >multiple symbols, let's only check for whether the command we want >is defined. This patch checks for both symbol names in order to >allow usage and then adds a check to see if the new name doesn't >exist after linux/devlink.h is include, then to use #define of >the new name to the old name - essentially the reverse logic that >the 4.11 header file has. > >Note that we do not need to check for other symbols as well since >they already existed prior to when the various ESWITCH symbols were >added. Other defintions include DEVLINK_GENL_VERSION, DEVLINK_GENL_NAME, >DEVLINK_ATTR_MAX, DEVLINK_ATTR_BUS_NAME, and DEVLINK_ATTR_DEV_NAME. >The DEVLINK_ATTR_ESWITCH_MODE and DEVLINK_ESWITCH_MODE_SWITCHDEV >were added at the same time as DEVLINK_CMD_ESWITCH_MODE_GET, but >it's only DEVLINK_CMD_ESWITCH_MODE_GET that changed in 4.11. > >Signed-off-by: John Ferlan <jferlan@redhat.com> >--- > > v1: https://www.redhat.com/archives/libvir-list/2017-September/msg00543.html > > Just to make it official - here's what I described in the v1 followup > which got the ACK if comment from Andrea: > > https://www.redhat.com/archives/libvir-list/2017-September/msg00591.html > > Still, I won't push this yet until it too gets an official blessing. > > configure.ac | 8 ++++++-- > src/util/virnetdev.c | 3 +++ > 2 files changed, 9 insertions(+), 2 deletions(-) > This looks nicer than Andrea's version that checked them separately at configure-time. Still, I hope we won't need to push this and whoever needs it on an older system can backport the fix to their kernel headers. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.