linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented
@ 2021-05-08  6:49 Heiko Thiery
  2021-05-08  8:59 ` Petr Vorel
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Heiko Thiery @ 2021-05-08  6:49 UTC (permalink / raw)
  To: netdev
  Cc: petr.vorel, dsahern, heiko.thiery, linux-kernel, stephen, Dmitry Yakunin

With commit d5e6ee0dac64 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.

Fixes: d5e6ee0dac64 ("ss: introduce cgroup2 cache and helper functions")
Cc: Dmitry Yakunin <zeil@yandex-team.ru>
Cc: Petr Vorel <petr.vorel@gmail.com>
Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
---
v3:
 - use correct syscall number (thanks to Petr Vorel)
 - add #include <sys/syscall.h> (thanks to Petr Vorel)
 - remove bogus parameters (thanks to Petr Vorel)
 - fix #ifdef (thanks to Petr Vorel)
 - added Fixes tag (thanks to David Ahern)
 - build test with buildroot 2020.08.3 using uclibc 1.0.34

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 Petr Vorel)

 configure | 28 ++++++++++++++++++++++++++++
 lib/fs.c  | 25 +++++++++++++++++++++++++
 2 files changed, 53 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 f161d888..05697a7e 100644
--- a/lib/fs.c
+++ b/lib/fs.c
@@ -25,11 +25,36 @@
 
 #include "utils.h"
 
+#ifndef HAVE_HANDLE_AT
+# include <sys/syscall.h>
+#endif
+
 #define CGROUP2_FS_NAME "cgroup2"
 
 /* if not already mounted cgroup2 is mounted here for iproute2's use */
 #define MNT_CGRP2_PATH  "/var/run/cgroup2"
 
+
+#ifndef HAVE_HANDLE_AT
+struct file_handle {
+	unsigned handle_bytes;
+	int handle_type;
+	unsigned char f_handle[];
+};
+
+static int name_to_handle_at(int dirfd, const char *pathname,
+	struct file_handle *handle, int *mount_id, int flags)
+{
+	return syscall(__NR_name_to_handle_at, dirfd, pathname, handle,
+	               mount_id, flags);
+}
+
+static int open_by_handle_at(int mount_fd, struct file_handle *handle, int flags)
+{
+	return syscall(__NR_open_by_handle_at, 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.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented
  2021-05-08  6:49 [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented Heiko Thiery
@ 2021-05-08  8:59 ` Petr Vorel
  2021-05-09 22:20 ` David Ahern
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Petr Vorel @ 2021-05-08  8:59 UTC (permalink / raw)
  To: Heiko Thiery; +Cc: netdev, dsahern, linux-kernel, stephen, Dmitry Yakunin

Hi Heiko,

Reviewed-by: Petr Vorel <petr.vorel@gmail.com>
Thanks!

It should be ok, but I'll try to test it during the weekend.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented
  2021-05-08  6:49 [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented Heiko Thiery
  2021-05-08  8:59 ` Petr Vorel
@ 2021-05-09 22:20 ` David Ahern
  2021-05-14 10:22   ` Heiko Thiery
  2021-05-15 16:58 ` Petr Vorel
  2021-05-17 14:45 ` David Ahern
  3 siblings, 1 reply; 13+ messages in thread
From: David Ahern @ 2021-05-09 22:20 UTC (permalink / raw)
  To: Heiko Thiery, netdev; +Cc: petr.vorel, linux-kernel, stephen, Dmitry Yakunin

On 5/8/21 12:49 AM, Heiko Thiery wrote:
> With commit d5e6ee0dac64 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.
> 
> Fixes: d5e6ee0dac64 ("ss: introduce cgroup2 cache and helper functions")
> Cc: Dmitry Yakunin <zeil@yandex-team.ru>
> Cc: Petr Vorel <petr.vorel@gmail.com>
> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
> ---
> v3:
>  - use correct syscall number (thanks to Petr Vorel)
>  - add #include <sys/syscall.h> (thanks to Petr Vorel)
>  - remove bogus parameters (thanks to Petr Vorel)
>  - fix #ifdef (thanks to Petr Vorel)
>  - added Fixes tag (thanks to David Ahern)
>  - build test with buildroot 2020.08.3 using uclibc 1.0.34
> 
> 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 Petr Vorel)
> 
>  configure | 28 ++++++++++++++++++++++++++++
>  lib/fs.c  | 25 +++++++++++++++++++++++++
>  2 files changed, 53 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 f161d888..05697a7e 100644
> --- a/lib/fs.c
> +++ b/lib/fs.c
> @@ -25,11 +25,36 @@
>  
>  #include "utils.h"
>  
> +#ifndef HAVE_HANDLE_AT
> +# include <sys/syscall.h>
> +#endif
> +
>  #define CGROUP2_FS_NAME "cgroup2"
>  
>  /* if not already mounted cgroup2 is mounted here for iproute2's use */
>  #define MNT_CGRP2_PATH  "/var/run/cgroup2"
>  
> +
> +#ifndef HAVE_HANDLE_AT
> +struct file_handle {
> +	unsigned handle_bytes;
> +	int handle_type;
> +	unsigned char f_handle[];
> +};
> +
> +static int name_to_handle_at(int dirfd, const char *pathname,
> +	struct file_handle *handle, int *mount_id, int flags)
> +{
> +	return syscall(__NR_name_to_handle_at, dirfd, pathname, handle,
> +	               mount_id, flags);
> +}
> +
> +static int open_by_handle_at(int mount_fd, struct file_handle *handle, int flags)
> +{
> +	return syscall(__NR_open_by_handle_at, mount_fd, handle, flags);
> +}
> +#endif
> +
>  /* return mount path of first occurrence of given fstype */
>  static char *find_fs_mount(const char *fs_to_find)
>  {
> 

This causes compile failures if anyone is reusing a tree. It would be
good to require config.mk to be updated if configure is newer.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented
  2021-05-09 22:20 ` David Ahern
@ 2021-05-14 10:22   ` Heiko Thiery
  2021-05-14 12:20     ` Petr Vorel
  0 siblings, 1 reply; 13+ messages in thread
From: Heiko Thiery @ 2021-05-14 10:22 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, petr.vorel, linux-kernel, stephen, Dmitry Yakunin

Hi David,

Am Mo., 10. Mai 2021 um 00:20 Uhr schrieb David Ahern <dsahern@gmail.com>:
>
> On 5/8/21 12:49 AM, Heiko Thiery wrote:
> > With commit d5e6ee0dac64 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.
> >
> > Fixes: d5e6ee0dac64 ("ss: introduce cgroup2 cache and helper functions")
> > Cc: Dmitry Yakunin <zeil@yandex-team.ru>
> > Cc: Petr Vorel <petr.vorel@gmail.com>
> > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
> > ---
> > v3:
> >  - use correct syscall number (thanks to Petr Vorel)
> >  - add #include <sys/syscall.h> (thanks to Petr Vorel)
> >  - remove bogus parameters (thanks to Petr Vorel)
> >  - fix #ifdef (thanks to Petr Vorel)
> >  - added Fixes tag (thanks to David Ahern)
> >  - build test with buildroot 2020.08.3 using uclibc 1.0.34
> >
> > 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 Petr Vorel)
> >
> >  configure | 28 ++++++++++++++++++++++++++++
> >  lib/fs.c  | 25 +++++++++++++++++++++++++
> >  2 files changed, 53 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 f161d888..05697a7e 100644
> > --- a/lib/fs.c
> > +++ b/lib/fs.c
> > @@ -25,11 +25,36 @@
> >
> >  #include "utils.h"
> >
> > +#ifndef HAVE_HANDLE_AT
> > +# include <sys/syscall.h>
> > +#endif
> > +
> >  #define CGROUP2_FS_NAME "cgroup2"
> >
> >  /* if not already mounted cgroup2 is mounted here for iproute2's use */
> >  #define MNT_CGRP2_PATH  "/var/run/cgroup2"
> >
> > +
> > +#ifndef HAVE_HANDLE_AT
> > +struct file_handle {
> > +     unsigned handle_bytes;
> > +     int handle_type;
> > +     unsigned char f_handle[];
> > +};
> > +
> > +static int name_to_handle_at(int dirfd, const char *pathname,
> > +     struct file_handle *handle, int *mount_id, int flags)
> > +{
> > +     return syscall(__NR_name_to_handle_at, dirfd, pathname, handle,
> > +                    mount_id, flags);
> > +}
> > +
> > +static int open_by_handle_at(int mount_fd, struct file_handle *handle, int flags)
> > +{
> > +     return syscall(__NR_open_by_handle_at, mount_fd, handle, flags);
> > +}
> > +#endif
> > +
> >  /* return mount path of first occurrence of given fstype */
> >  static char *find_fs_mount(const char *fs_to_find)
> >  {
> >
>
> This causes compile failures if anyone is reusing a tree. It would be
> good to require config.mk to be updated if configure is newer.

Do you mean the config.mk should have a dependency to configure in the
Makefile? Wouldn't that be better as a separate patch?

Thanks
-- 
Heiko

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented
  2021-05-14 10:22   ` Heiko Thiery
@ 2021-05-14 12:20     ` Petr Vorel
  2021-05-14 14:10       ` David Ahern
  0 siblings, 1 reply; 13+ messages in thread
From: Petr Vorel @ 2021-05-14 12:20 UTC (permalink / raw)
  To: Heiko Thiery; +Cc: David Ahern, netdev, linux-kernel, stephen, Dmitry Yakunin

> Hi David,

> Am Mo., 10. Mai 2021 um 00:20 Uhr schrieb David Ahern <dsahern@gmail.com>:

> > On 5/8/21 12:49 AM, Heiko Thiery wrote:
> > > With commit d5e6ee0dac64 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.

> > > Fixes: d5e6ee0dac64 ("ss: introduce cgroup2 cache and helper functions")
> > > Cc: Dmitry Yakunin <zeil@yandex-team.ru>
> > > Cc: Petr Vorel <petr.vorel@gmail.com>
> > > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
> > > ---
> > > v3:
> > >  - use correct syscall number (thanks to Petr Vorel)
> > >  - add #include <sys/syscall.h> (thanks to Petr Vorel)
> > >  - remove bogus parameters (thanks to Petr Vorel)
> > >  - fix #ifdef (thanks to Petr Vorel)
> > >  - added Fixes tag (thanks to David Ahern)
> > >  - build test with buildroot 2020.08.3 using uclibc 1.0.34

> > > 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 Petr Vorel)

> > >  configure | 28 ++++++++++++++++++++++++++++
> > >  lib/fs.c  | 25 +++++++++++++++++++++++++
> > >  2 files changed, 53 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 f161d888..05697a7e 100644
> > > --- a/lib/fs.c
> > > +++ b/lib/fs.c
> > > @@ -25,11 +25,36 @@

> > >  #include "utils.h"

> > > +#ifndef HAVE_HANDLE_AT
> > > +# include <sys/syscall.h>
> > > +#endif
> > > +
> > >  #define CGROUP2_FS_NAME "cgroup2"

> > >  /* if not already mounted cgroup2 is mounted here for iproute2's use */
> > >  #define MNT_CGRP2_PATH  "/var/run/cgroup2"

> > > +
> > > +#ifndef HAVE_HANDLE_AT
> > > +struct file_handle {
> > > +     unsigned handle_bytes;
> > > +     int handle_type;
> > > +     unsigned char f_handle[];
> > > +};
> > > +
> > > +static int name_to_handle_at(int dirfd, const char *pathname,
> > > +     struct file_handle *handle, int *mount_id, int flags)
> > > +{
> > > +     return syscall(__NR_name_to_handle_at, dirfd, pathname, handle,
> > > +                    mount_id, flags);
> > > +}
> > > +
> > > +static int open_by_handle_at(int mount_fd, struct file_handle *handle, int flags)
> > > +{
> > > +     return syscall(__NR_open_by_handle_at, mount_fd, handle, flags);
> > > +}
> > > +#endif
> > > +
> > >  /* return mount path of first occurrence of given fstype */
> > >  static char *find_fs_mount(const char *fs_to_find)
> > >  {


> > This causes compile failures if anyone is reusing a tree. It would be
> > good to require config.mk to be updated if configure is newer.

> Do you mean the config.mk should have a dependency to configure in the
> Makefile? Wouldn't that be better as a separate patch?

I guess it should be a separate patch. I'm surprised it wasn't needed before.

Kind regards,
Petr

> Thanks

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented
  2021-05-14 12:20     ` Petr Vorel
@ 2021-05-14 14:10       ` David Ahern
  2021-05-15 16:59         ` Petr Vorel
  0 siblings, 1 reply; 13+ messages in thread
From: David Ahern @ 2021-05-14 14:10 UTC (permalink / raw)
  To: Petr Vorel, Heiko Thiery; +Cc: netdev, linux-kernel, stephen, Dmitry Yakunin

On 5/14/21 6:20 AM, Petr Vorel wrote:
> 
>>> This causes compile failures if anyone is reusing a tree. It would be
>>> good to require config.mk to be updated if configure is newer.
>> Do you mean the config.mk should have a dependency to configure in the
>> Makefile? Wouldn't that be better as a separate patch?
> I guess it should be a separate patch. I'm surprised it wasn't needed before.
> 


yes, it should be a separate patch, but it needs to precede this one.

This worked for me last weekend; I'll send it when I get a chance.

diff --git a/Makefile b/Makefile
index 19bd163e2e04..5bc11477ab7a 100644
--- a/Makefile
+++ b/Makefile
@@ -60,7 +60,7 @@ SUBDIRS=lib ip tc bridge misc netem genl tipc devlink
rdma dcb man vdpa
 LIBNETLINK=../lib/libutil.a ../lib/libnetlink.a
 LDLIBS += $(LIBNETLINK)

-all: config.mk
+all: config
        @set -e; \
        for i in $(SUBDIRS); \
        do echo; echo $$i; $(MAKE) -C $$i; done
@@ -80,8 +80,10 @@ all: config.mk
        @echo "Make Arguments:"
        @echo " V=[0|1]             - set build verbosity level"

-config.mk:
-       sh configure $(KERNEL_INCLUDE)
+config:
+       @if [ ! -f config.mk -o configure -nt config.mk ]; then \
+               sh configure $(KERNEL_INCLUDE); \
+       fi

 install: all
        install -m 0755 -d $(DESTDIR)$(SBINDIR)


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented
  2021-05-08  6:49 [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented Heiko Thiery
  2021-05-08  8:59 ` Petr Vorel
  2021-05-09 22:20 ` David Ahern
@ 2021-05-15 16:58 ` Petr Vorel
  2021-05-17 14:45 ` David Ahern
  3 siblings, 0 replies; 13+ messages in thread
From: Petr Vorel @ 2021-05-15 16:58 UTC (permalink / raw)
  To: Heiko Thiery
  Cc: netdev, dsahern, linux-kernel, stephen, Dmitry Yakunin, Peter Korsgaard

Hi,

[ Cc Petr (Buildroot maintainer) ]
> With commit d5e6ee0dac64 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.

> Fixes: d5e6ee0dac64 ("ss: introduce cgroup2 cache and helper functions")
> Cc: Dmitry Yakunin <zeil@yandex-team.ru>
> Cc: Petr Vorel <petr.vorel@gmail.com>
> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
> ---
> v3:
>  - use correct syscall number (thanks to Petr Vorel)
>  - add #include <sys/syscall.h> (thanks to Petr Vorel)
>  - remove bogus parameters (thanks to Petr Vorel)
>  - fix #ifdef (thanks to Petr Vorel)
>  - added Fixes tag (thanks to David Ahern)
>  - build test with buildroot 2020.08.3 using uclibc 1.0.34
I tested it to some extent. I was not able to test it on buildroot uclibc:
$ ss -a --cgroup # I put debugging printf
ss.c:3336 inet_show_sock(): tb[INET_DIAG_CGROUP_ID]: (nil), INET_DIAG_CGROUP_ID: 21

I tried mount both cgroup (with cgroupfs-mount) and cgroup2 (using mount).

But it's hard to trigger this code also on regular linux distro with glibc:

$ ss --cgroup -a >/dev/null
Failed to open cgroup2 by ID
Failed to open cgroup2 by ID
Failed to open cgroup2 by ID
Failed to open cgroup2 by ID
Failed to open cgroup2 by ID
Failed to open cgroup2 by ID

Debugging when replacing glibc wrapper with these functions calling raw syscall
it works the same (i.e. "Failed to open cgroup2 by ID")

Thus:
Tested-by: Petr Vorel <petr.vorel@gmail.com>
(to my previous Reviewed-by: tag).

Hope David Ahern send his patch for config.mk dependency to configure,
as his fragment [1] LGTM.

Kind regards,
Petr

[1] https://lore.kernel.org/netdev/82c9159f-0644-40af-fb4c-cc8507456719@gmail.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented
  2021-05-14 14:10       ` David Ahern
@ 2021-05-15 16:59         ` Petr Vorel
  2021-05-15 18:26           ` David Ahern
  0 siblings, 1 reply; 13+ messages in thread
From: Petr Vorel @ 2021-05-15 16:59 UTC (permalink / raw)
  To: David Ahern; +Cc: Heiko Thiery, netdev, linux-kernel, stephen, Dmitry Yakunin

Hi David,
> On 5/14/21 6:20 AM, Petr Vorel wrote:

> >>> This causes compile failures if anyone is reusing a tree. It would be
> >>> good to require config.mk to be updated if configure is newer.
> >> Do you mean the config.mk should have a dependency to configure in the
> >> Makefile? Wouldn't that be better as a separate patch?
> > I guess it should be a separate patch. I'm surprised it wasn't needed before.



> yes, it should be a separate patch, but it needs to precede this one.

> This worked for me last weekend; I'll send it when I get a chance.

> diff --git a/Makefile b/Makefile
> index 19bd163e2e04..5bc11477ab7a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -60,7 +60,7 @@ SUBDIRS=lib ip tc bridge misc netem genl tipc devlink
> rdma dcb man vdpa
>  LIBNETLINK=../lib/libutil.a ../lib/libnetlink.a
>  LDLIBS += $(LIBNETLINK)

> -all: config.mk
> +all: config
>         @set -e; \
>         for i in $(SUBDIRS); \
>         do echo; echo $$i; $(MAKE) -C $$i; done
> @@ -80,8 +80,10 @@ all: config.mk
>         @echo "Make Arguments:"
>         @echo " V=[0|1]             - set build verbosity level"

> -config.mk:
> -       sh configure $(KERNEL_INCLUDE)
> +config:
> +       @if [ ! -f config.mk -o configure -nt config.mk ]; then \
> +               sh configure $(KERNEL_INCLUDE); \
> +       fi

>  install: all
>         install -m 0755 -d $(DESTDIR)$(SBINDIR)

Thanks a lot, please send it.

I know this is only a fragment, but:
Reviewed-by: Petr Vorel <petr.vorel@gmail.com>

-nt is supported by dash and busybox sh.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented
  2021-05-15 16:59         ` Petr Vorel
@ 2021-05-15 18:26           ` David Ahern
  0 siblings, 0 replies; 13+ messages in thread
From: David Ahern @ 2021-05-15 18:26 UTC (permalink / raw)
  To: Petr Vorel; +Cc: Heiko Thiery, netdev, linux-kernel, stephen, Dmitry Yakunin

On 5/15/21 10:59 AM, Petr Vorel wrote:
> Hi David,
>> On 5/14/21 6:20 AM, Petr Vorel wrote:
> 
>>>>> This causes compile failures if anyone is reusing a tree. It would be
>>>>> good to require config.mk to be updated if configure is newer.
>>>> Do you mean the config.mk should have a dependency to configure in the
>>>> Makefile? Wouldn't that be better as a separate patch?
>>> I guess it should be a separate patch. I'm surprised it wasn't needed before.
> 
> 
> 
>> yes, it should be a separate patch, but it needs to precede this one.
> 
>> This worked for me last weekend; I'll send it when I get a chance.
> 
>> diff --git a/Makefile b/Makefile
>> index 19bd163e2e04..5bc11477ab7a 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -60,7 +60,7 @@ SUBDIRS=lib ip tc bridge misc netem genl tipc devlink
>> rdma dcb man vdpa
>>  LIBNETLINK=../lib/libutil.a ../lib/libnetlink.a
>>  LDLIBS += $(LIBNETLINK)
> 
>> -all: config.mk
>> +all: config
>>         @set -e; \
>>         for i in $(SUBDIRS); \
>>         do echo; echo $$i; $(MAKE) -C $$i; done
>> @@ -80,8 +80,10 @@ all: config.mk
>>         @echo "Make Arguments:"
>>         @echo " V=[0|1]             - set build verbosity level"
> 
>> -config.mk:
>> -       sh configure $(KERNEL_INCLUDE)
>> +config:
>> +       @if [ ! -f config.mk -o configure -nt config.mk ]; then \
>> +               sh configure $(KERNEL_INCLUDE); \
>> +       fi
> 
>>  install: all
>>         install -m 0755 -d $(DESTDIR)$(SBINDIR)
> 
> Thanks a lot, please send it.
> 
> I know this is only a fragment, but:
> Reviewed-by: Petr Vorel <petr.vorel@gmail.com>
> 
> -nt is supported by dash and busybox sh.
> 

That helps. My concern was all the sh variants.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented
  2021-05-08  6:49 [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented Heiko Thiery
                   ` (2 preceding siblings ...)
  2021-05-15 16:58 ` Petr Vorel
@ 2021-05-17 14:45 ` David Ahern
  2021-05-17 17:36   ` Petr Vorel
  3 siblings, 1 reply; 13+ messages in thread
From: David Ahern @ 2021-05-17 14:45 UTC (permalink / raw)
  To: Heiko Thiery, netdev; +Cc: petr.vorel, linux-kernel, stephen, Dmitry Yakunin

On 5/8/21 12:49 AM, Heiko Thiery wrote:
> With commit d5e6ee0dac64 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.
> 
> Fixes: d5e6ee0dac64 ("ss: introduce cgroup2 cache and helper functions")
> Cc: Dmitry Yakunin <zeil@yandex-team.ru>
> Cc: Petr Vorel <petr.vorel@gmail.com>
> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
> ---
> v3:
>  - use correct syscall number (thanks to Petr Vorel)
>  - add #include <sys/syscall.h> (thanks to Petr Vorel)
>  - remove bogus parameters (thanks to Petr Vorel)
>  - fix #ifdef (thanks to Petr Vorel)
>  - added Fixes tag (thanks to David Ahern)
>  - build test with buildroot 2020.08.3 using uclibc 1.0.34
> 
> 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 Petr Vorel)
> 
>  configure | 28 ++++++++++++++++++++++++++++
>  lib/fs.c  | 25 +++++++++++++++++++++++++
>  2 files changed, 53 insertions(+)
> 

applied to iproute2-next.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented
  2021-05-17 14:45 ` David Ahern
@ 2021-05-17 17:36   ` Petr Vorel
  2021-05-18  1:51     ` David Ahern
  0 siblings, 1 reply; 13+ messages in thread
From: Petr Vorel @ 2021-05-17 17:36 UTC (permalink / raw)
  To: David Ahern; +Cc: Heiko Thiery, netdev, linux-kernel, stephen, Dmitry Yakunin

> On 5/8/21 12:49 AM, Heiko Thiery wrote:
> > With commit d5e6ee0dac64 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.

> > Fixes: d5e6ee0dac64 ("ss: introduce cgroup2 cache and helper functions")
> > Cc: Dmitry Yakunin <zeil@yandex-team.ru>
> > Cc: Petr Vorel <petr.vorel@gmail.com>
> > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
> > ---
> > v3:
> >  - use correct syscall number (thanks to Petr Vorel)
> >  - add #include <sys/syscall.h> (thanks to Petr Vorel)
> >  - remove bogus parameters (thanks to Petr Vorel)
> >  - fix #ifdef (thanks to Petr Vorel)
> >  - added Fixes tag (thanks to David Ahern)
> >  - build test with buildroot 2020.08.3 using uclibc 1.0.34

> > 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 Petr Vorel)

> >  configure | 28 ++++++++++++++++++++++++++++
> >  lib/fs.c  | 25 +++++++++++++++++++++++++
> >  2 files changed, 53 insertions(+)


> applied to iproute2-next.

Thanks a lot!

I guess, it'll be merged to regular iproute2 in next merge window (for 5.14).

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented
  2021-05-17 17:36   ` Petr Vorel
@ 2021-05-18  1:51     ` David Ahern
  2021-05-18  7:24       ` Heiko Thiery
  0 siblings, 1 reply; 13+ messages in thread
From: David Ahern @ 2021-05-18  1:51 UTC (permalink / raw)
  To: Petr Vorel; +Cc: Heiko Thiery, netdev, linux-kernel, stephen, Dmitry Yakunin

On 5/17/21 11:36 AM, Petr Vorel wrote:
> I guess, it'll be merged to regular iproute2 in next merge window (for 5.14).
> 

Let's see how things go over the 5.13 dev cycle. If no problems, maybe
Stephen can merge this one and the config change to main before the release.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented
  2021-05-18  1:51     ` David Ahern
@ 2021-05-18  7:24       ` Heiko Thiery
  0 siblings, 0 replies; 13+ messages in thread
From: Heiko Thiery @ 2021-05-18  7:24 UTC (permalink / raw)
  To: David Ahern; +Cc: Petr Vorel, netdev, linux-kernel, stephen, Dmitry Yakunin

Hi all,

Am Di., 18. Mai 2021 um 03:51 Uhr schrieb David Ahern <dsahern@gmail.com>:
>
> On 5/17/21 11:36 AM, Petr Vorel wrote:
> > I guess, it'll be merged to regular iproute2 in next merge window (for 5.14).
> >
>
> Let's see how things go over the 5.13 dev cycle. If no problems, maybe
> Stephen can merge this one and the config change to main before the release.

Thanks

-- 
Heiko

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-05-18  7:25 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-08  6:49 [PATCH iproute2-next v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented Heiko Thiery
2021-05-08  8:59 ` Petr Vorel
2021-05-09 22:20 ` David Ahern
2021-05-14 10:22   ` Heiko Thiery
2021-05-14 12:20     ` Petr Vorel
2021-05-14 14:10       ` David Ahern
2021-05-15 16:59         ` Petr Vorel
2021-05-15 18:26           ` David Ahern
2021-05-15 16:58 ` Petr Vorel
2021-05-17 14:45 ` David Ahern
2021-05-17 17:36   ` Petr Vorel
2021-05-18  1:51     ` David Ahern
2021-05-18  7:24       ` Heiko Thiery

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).