Re: [ovs-dev] [PATCH v4 1/2] util: Support checking for kernel versions.

2024-02-25 Thread Eelco Chaudron


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 

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()) {
> -struct utsname utsname;
> -int major, minor;
> -
> -if (uname() == -1) {
> -VLOG_WARN("uname failed (%s)", ovs_strerror(errno));
> -} else if (!ovs_scan(utsname.release, "%d.%d", , )) {
> -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();
>  }
> 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 
>  #ifdef __linux__
>  #include 
> +#include 
>  #endif
>  #include 
>  #include 
> @@ -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()) {
> +struct utsname utsname;
> +
> +if (uname() == -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);
> +}
> +ovsthread_once_done();
> +}
> +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


[ovs-dev] [PATCH v4 1/2] util: Support checking for kernel versions.

2024-02-23 Thread Felix Huettner via dev
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 
---
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()) {
-struct utsname utsname;
-int major, minor;
-
-if (uname() == -1) {
-VLOG_WARN("uname failed (%s)", ovs_strerror(errno));
-} else if (!ovs_scan(utsname.release, "%d.%d", , )) {
-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();
 }
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 
 #ifdef __linux__
 #include 
+#include 
 #endif
 #include 
 #include 
@@ -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()) {
+struct utsname utsname;
+
+if (uname() == -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);
+}
+ovsthread_once_done();
+}
+return current_major < target_major || (
+current_major == target_major && current_minor < target_minor);
+}
+#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