On 23 Feb 2024, at 14:32, Felix Huettner via dev wrote:
> Extract checking for a given kernel version to a separate function. > It will be used also in the next patch. > > Signed-off-by: Felix Huettner <felix.huettner@mail.schwarz> Just a quick observation on this patch, see comments below. //Eelco > --- > v4: > - extract function to check kernel version > > lib/netdev-linux.c | 14 +++----------- > lib/util.c | 24 ++++++++++++++++++++++++ > lib/util.h | 4 ++++ > 3 files changed, 31 insertions(+), 11 deletions(-) > > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > index bf91ef462..51bd71ae3 100644 > --- a/lib/netdev-linux.c > +++ b/lib/netdev-linux.c > @@ -6427,18 +6427,10 @@ getqdisc_is_safe(void) > static bool safe = false; > > if (ovsthread_once_start(&once)) { > - struct utsname utsname; > - int major, minor; > - > - if (uname(&utsname) == -1) { > - VLOG_WARN("uname failed (%s)", ovs_strerror(errno)); > - } else if (!ovs_scan(utsname.release, "%d.%d", &major, &minor)) { > - VLOG_WARN("uname reported bad OS release (%s)", utsname.release); > - } else if (major < 2 || (major == 2 && minor < 35)) { > - VLOG_INFO("disabling unsafe RTM_GETQDISC in Linux kernel %s", > - utsname.release); > - } else { > + if (ovs_kernel_is_version_or_newer(2, 35)) { > safe = true; > + } else { > + VLOG_INFO("disabling unsafe RTM_GETQDISC in Linux kernel"); > } > ovsthread_once_done(&once); > } > diff --git a/lib/util.c b/lib/util.c > index 3fb3a4b40..95c605af8 100644 > --- a/lib/util.c > +++ b/lib/util.c > @@ -27,6 +27,7 @@ > #include <string.h> > #ifdef __linux__ > #include <sys/prctl.h> > +#include <sys/utsname.h> > #endif > #include <sys/stat.h> > #include <unistd.h> > @@ -2500,3 +2501,26 @@ OVS_CONSTRUCTOR(winsock_start) { > } > } > #endif > + > +#ifndef _WIN32 > +bool > +ovs_kernel_is_version_or_newer(int target_major, int target_minor) > +{ > + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; > + static int current_major, current_minor = -1; > + > + if (ovsthread_once_start(&once)) { > + struct utsname utsname; > + > + if (uname(&utsname) == -1) { > + VLOG_WARN("uname failed (%s)", ovs_strerror(errno)); > + } else if (!ovs_scan(utsname.release, "%d.%d", > + ¤t_major, ¤t_minor)) { > + VLOG_WARN("uname reported bad OS release (%s)", utsname.release); > + } > + ovsthread_once_done(&once); > + } > + return current_major < target_major || ( If I’m running kernel 1.0, this function will return true for the above invocation, this sounds wrong to me. > + current_major == target_major && current_minor < target_minor); If I’m running 2.34 it will also return true, and false on 2.35 and above. In other words, this function works in reverse. In addition, if there was a failure in the ovsthread_once_start() section, we should always return false. > +} > +#endif > diff --git a/lib/util.h b/lib/util.h > index f2d45bcac..57d8a2072 100644 > --- a/lib/util.h > +++ b/lib/util.h > @@ -611,4 +611,8 @@ int ftruncate(int fd, off_t length); > } > #endif > > +#ifndef _WIN32 > +bool ovs_kernel_is_version_or_newer(int target_major, int target_minor); > +#endif > + > #endif /* util.h */ > > base-commit: 5f2af0b7a30e7de84de97556223f892ef63ec14b > -- > 2.43.0 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev