[PATCH iproute2-next v2] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented

With commit d5e6ee0dac64b64e the usage of functions name_to_handle_at() and
open_by_handle_at() are introduced. But these function are not available
e.g. in uclibc-ng < 1.0.35. To have a backward compatibility check for the
availability in the configure script and in case of absence do a direct
syscall.

Signed-off-by: Heiko Thiery <redacted>
---
v2:
 - small correction to subject
 - removed IP_CONFIG_HANDLE_AT:=y option since it is not required
 - fix indentation in check function
 - removed empty lines (thanks to Petr Vorel)
 - add #define _GNU_SOURCE in check (thanks to Petr Vorel)
 - check only for name_to_handle_at (thanks to Peter Vorel)

 configure | 28 ++++++++++++++++++++++++++++
 lib/fs.c  | 21 +++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/configure b/configure
index 2c363d3b..179eae08 100755
--- a/configure
+++ b/configure

@@ -202,6 +202,31 @@ EOF
     rm -f $TMPDIR/setnstest.c $TMPDIR/setnstest
 }
 
+check_name_to_handle_at()
+{
+    cat >$TMPDIR/name_to_handle_at_test.c <<EOF
+#define _GNU_SOURCE
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+int main(int argc, char **argv)
+{
+	struct file_handle *fhp;
+	int mount_id, flags, dirfd;
+	char *pathname;
+	name_to_handle_at(dirfd, pathname, fhp, &mount_id, flags);
+	return 0;
+}
+EOF
+    if $CC -I$INCLUDE -o $TMPDIR/name_to_handle_at_test $TMPDIR/name_to_handle_at_test.c >/dev/null 2>&1; then
+        echo "yes"
+        echo "CFLAGS += -DHAVE_HANDLE_AT" >>$CONFIG
+    else
+        echo "no"
+    fi
+    rm -f $TMPDIR/name_to_handle_at_test.c $TMPDIR/name_to_handle_at_test
+}
+
 check_ipset()
 {
     cat >$TMPDIR/ipsettest.c <<EOF

@@ -492,6 +517,9 @@ fi
 echo -n "libc has setns: "
 check_setns
 
+echo -n "libc has name_to_handle_at: "
+check_name_to_handle_at
+
 echo -n "SELinux support: "
 check_selinux
 

diff --git a/lib/fs.c b/lib/fs.c
index ee0b130b..feb12864 100644
--- a/lib/fs.c
+++ b/lib/fs.c

@@ -30,6 +30,27 @@
 /* if not already mounted cgroup2 is mounted here for iproute2's use */
 #define MNT_CGRP2_PATH  "/var/run/cgroup2"
 
+
+#ifndef defined HAVE_HANDLE_AT
+struct file_handle {
+	unsigned handle_bytes;
+	int handle_type;
+	unsigned char f_handle[];
+};
+
+int name_to_handle_at(int dirfd, const char *pathname,
+	struct file_handle *handle, int *mount_id, int flags)
+{
+	return syscall(name_to_handle_at, 5, dirfd, pathname, handle,
+	               mount_id, flags);
+}
+
+int open_by_handle_at(int mount_fd, struct file_handle *handle, int flags)
+{
+	return syscall(open_by_handle_at, 3, mount_fd, handle, flags);
+}
+#endif
+
 /* return mount path of first occurrence of given fstype */
 static char *find_fs_mount(const char *fs_to_find)
 {

-- 
2.30.0

Hi Heiko,

...

I overlooked this in v1. name_to_handle_at must be replaced by __NR_name_to_handle_at:
(name_to_handle_at is the function name, not a syscall number).
It also requires to include <sys/syscall.h>:

#include <sys/syscall.h>
...
	return syscall(__NR_name_to_handle_at, 5, dirfd, pathname, handle,
	               mount_id, flags);

+	               mount_id, flags);
+}
+
+int open_by_handle_at(int mount_fd, struct file_handle *handle, int flags)
+{
+	return syscall(open_by_handle_at, 3, mount_fd, handle, flags);

And here needs to be __NR_open_by_handle_at

Kind regards,
Petr

+}
+#endif
+
 /* return mount path of first occurrence of given fstype */
 static char *find_fs_mount(const char *fs_to_find)
 {

Hi,

+
+#ifndef defined HAVE_HANDLE_AT

This is also wrong, it must be:
#ifndef HAVE_HANDLE_AT

+struct file_handle {
+	unsigned handle_bytes;
+	int handle_type;
+	unsigned char f_handle[];
+};
+
+int name_to_handle_at(int dirfd, const char *pathname,
+	struct file_handle *handle, int *mount_id, int flags)
+{
+	return syscall(name_to_handle_at, 5, dirfd, pathname, handle,
+	               mount_id, flags);

Also I overlooked bogus 5 parameter, why is here? Correct is:

	return syscall(__NR_name_to_handle_at, dfd, pathname, handle,
			   mount_id, flags);

+}
+
+int open_by_handle_at(int mount_fd, struct file_handle *handle, int flags)
+{
+	return syscall(open_by_handle_at, 3, mount_fd, handle, flags);

And here 3, correct version is is:
	return syscall(__NR_open_by_handle_at, mount_fd, handle, flags);


+ adding at the top:

#ifndef HAVE_HANDLE_AT
# include <sys/syscall.h>
#endif

Kind regards,
Petr

Hi,

quoted

+++ b/lib/fs.c

@@ -30,6 +30,27 @@
 /* if not already mounted cgroup2 is mounted here for iproute2's use */
 #define MNT_CGRP2_PATH  "/var/run/cgroup2"

quoted

+
+#ifndef defined HAVE_HANDLE_AT

This is also wrong, it must be:
#ifndef HAVE_HANDLE_AT

quoted

+struct file_handle {
+	unsigned handle_bytes;
+	int handle_type;
+	unsigned char f_handle[];
+};
+
+int name_to_handle_at(int dirfd, const char *pathname,
+	struct file_handle *handle, int *mount_id, int flags)
+{
+	return syscall(name_to_handle_at, 5, dirfd, pathname, handle,
+	               mount_id, flags);

Also I overlooked bogus 5 parameter, why is here? Correct is:

	return syscall(__NR_name_to_handle_at, dfd, pathname, handle,
			   mount_id, flags);

Uh, one more typo on my side, sorry (dfd => dirfd):
	return syscall(__NR_name_to_handle_at, dirfd, pathname, handle,
 			   mount_id, flags);


Kind regards,
Petr

quoted

+}
+
+int open_by_handle_at(int mount_fd, struct file_handle *handle, int flags)
+{
+	return syscall(open_by_handle_at, 3, mount_fd, handle, flags);

And here 3, correct version is is:
	return syscall(__NR_open_by_handle_at, mount_fd, handle, flags);

+ adding at the top:

#ifndef HAVE_HANDLE_AT
# include <sys/syscall.h>
#endif

Kind regards,
Petr

On 4/30/21 12:26 AM, Heiko Thiery wrote:

With commit d5e6ee0dac64b64e the usage of functions name_to_handle_at() and

that is a change to ss:

d5e6ee0dac64 ss: introduce cgroup2 cache and helper functions

open_by_handle_at() are introduced. But these function are not available
e.g. in uclibc-ng < 1.0.35. To have a backward compatibility check for the
availability in the configure script and in case of absence do a direct
syscall.

When you find the proper commit add a Fixes line before the Signed-off-by:

Fixes: <id> ("subject line")

Signed-off-by: Heiko Thiery <redacted>
---

make sure you cc the author of the original commit.

v2:
 - small correction to subject
 - removed IP_CONFIG_HANDLE_AT:=y option since it is not required
 - fix indentation in check function
 - removed empty lines (thanks to Petr Vorel)
 - add #define _GNU_SOURCE in check (thanks to Petr Vorel)
 - check only for name_to_handle_at (thanks to Peter Vorel)

you have 3 responses to this commit. Please send an updated patch with
all the changes and the the above comments addressed.

Thanks,

Hi Petr,

Am Fr., 30. Apr. 2021 um 21:29 Uhr schrieb Petr Vorel [off-list ref]:

Hi,

quoted

quoted

+++ b/lib/fs.c

@@ -30,6 +30,27 @@
 /* if not already mounted cgroup2 is mounted here for iproute2's use */
 #define MNT_CGRP2_PATH  "/var/run/cgroup2"

quoted

quoted

+
+#ifndef defined HAVE_HANDLE_AT

This is also wrong, it must be:
#ifndef HAVE_HANDLE_AT

quoted

quoted

+struct file_handle {
+   unsigned handle_bytes;
+   int handle_type;
+   unsigned char f_handle[];
+};
+
+int name_to_handle_at(int dirfd, const char *pathname,
+   struct file_handle *handle, int *mount_id, int flags)
+{
+   return syscall(name_to_handle_at, 5, dirfd, pathname, handle,
+                  mount_id, flags);

Also I overlooked bogus 5 parameter, why is here? Correct is:

quoted

      return syscall(__NR_name_to_handle_at, dfd, pathname, handle,
                         mount_id, flags);

Uh, one more typo on my side, sorry (dfd => dirfd):
        return syscall(__NR_name_to_handle_at, dirfd, pathname, handle,
                           mount_id, flags);

Thanks for the review and finding the sloppiness. I really should test
the changes before. Nevertheless, I will prepare a new version and
test it this time.

BR,
-- 
Heiko

Hi David,

Am Sa., 1. Mai 2021 um 17:03 Uhr schrieb David Ahern [off-list ref]:

On 4/30/21 12:26 AM, Heiko Thiery wrote:

quoted

With commit d5e6ee0dac64b64e the usage of functions name_to_handle_at() and

that is a change to ss:

d5e6ee0dac64 ss: introduce cgroup2 cache and helper functions

quoted

open_by_handle_at() are introduced. But these function are not available
e.g. in uclibc-ng < 1.0.35. To have a backward compatibility check for the
availability in the configure script and in case of absence do a direct
syscall.

When you find the proper commit add a Fixes line before the Signed-off-by:

What do you mean with finding the right commit? This (d5e6ee0dac64) is
the commit that introduced the usage of the missing functions. Or have
I overlooked something?

Fixes: <id> ("subject line")

quoted

Signed-off-by: Heiko Thiery <redacted>
---

make sure you cc the author of the original commit.

Ok.

quoted

v2:
 - small correction to subject
 - removed IP_CONFIG_HANDLE_AT:=y option since it is not required
 - fix indentation in check function
 - removed empty lines (thanks to Petr Vorel)
 - add #define _GNU_SOURCE in check (thanks to Petr Vorel)
 - check only for name_to_handle_at (thanks to Peter Vorel)

you have 3 responses to this commit. Please send an updated patch with
all the changes and the the above comments addressed.

I will implement the comments and send a new version.

Thanks,

Thanks,
-- 
Heiko

Hi Petr,

Am Fr., 30. Apr. 2021 um 21:29 Uhr schrieb Petr Vorel [off-list ref]:

quoted

Hi,

quoted

quoted

quoted

+++ b/lib/fs.c

@@ -30,6 +30,27 @@
 /* if not already mounted cgroup2 is mounted here for iproute2's use */
 #define MNT_CGRP2_PATH  "/var/run/cgroup2"

quoted

quoted

quoted

+
+#ifndef defined HAVE_HANDLE_AT

This is also wrong, it must be:
#ifndef HAVE_HANDLE_AT

quoted

quoted

quoted

+struct file_handle {
+   unsigned handle_bytes;
+   int handle_type;
+   unsigned char f_handle[];
+};
+
+int name_to_handle_at(int dirfd, const char *pathname,
+   struct file_handle *handle, int *mount_id, int flags)
+{
+   return syscall(name_to_handle_at, 5, dirfd, pathname, handle,
+                  mount_id, flags);

Also I overlooked bogus 5 parameter, why is here? Correct is:

quoted

quoted

      return syscall(__NR_name_to_handle_at, dfd, pathname, handle,
                         mount_id, flags);

Uh, one more typo on my side, sorry (dfd => dirfd):
        return syscall(__NR_name_to_handle_at, dirfd, pathname, handle,
                           mount_id, flags);

Thanks for the review and finding the sloppiness. I really should test
the changes before. Nevertheless, I will prepare a new version and
test it this time.

I tested ss with changed I proposed and it looks like it's ok.
But I run ss on qemu without any daemon running => I'll retest your v3 once you
post it with some daemons running so that the code is really triggered.

Kind regards,
Petr

BR,

Hi David,

Am Sa., 1. Mai 2021 um 17:03 Uhr schrieb David Ahern [off-list ref]:

quoted

On 4/30/21 12:26 AM, Heiko Thiery wrote:

quoted

With commit d5e6ee0dac64b64e the usage of functions name_to_handle_at() and

quoted

that is a change to ss:

quoted

d5e6ee0dac64 ss: introduce cgroup2 cache and helper functions

quoted

quoted

open_by_handle_at() are introduced. But these function are not available
e.g. in uclibc-ng < 1.0.35. To have a backward compatibility check for the
availability in the configure script and in case of absence do a direct
syscall.

quoted

When you find the proper commit add a Fixes line before the Signed-off-by:

What do you mean with finding the right commit? This (d5e6ee0dac64) is
the commit that introduced the usage of the missing functions. Or have
I overlooked something?

Just put into commit message before your Signed-off-by tag this:

Fixes: d5e6ee0d ("ss: introduce cgroup2 cache and helper functions")

quoted

Fixes: <id> ("subject line")

quoted

Signed-off-by: Heiko Thiery <redacted>
---

Kind regards,
Petr

On 5/2/21 5:20 AM, Petr Vorel wrote:

quoted

What do you mean with finding the right commit? This (d5e6ee0dac64) is
the commit that introduced the usage of the missing functions. Or have
I overlooked something?

your right; d5e6ee0dac64 is the commit.

Just put into commit message before your Signed-off-by tag this:

Fixes: d5e6ee0d ("ss: introduce cgroup2 cache and helper functions")