diff mbox series

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

Message ID 20210508064925.8045-1-heiko.thiery@gmail.com (mailing list archive)
State Accepted
Delegated to: David Ahern
Headers show
Series [iproute2-next,v3] lib/fs: fix issue when {name,open}_to_handle_at() is not implemented | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Heiko Thiery May 8, 2021, 6:49 a.m. UTC
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(+)

Comments

Petr Vorel May 8, 2021, 8:59 a.m. UTC | #1
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
David Ahern May 9, 2021, 10:20 p.m. UTC | #2
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.
Heiko Thiery May 14, 2021, 10:22 a.m. UTC | #3
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
Petr Vorel May 14, 2021, 12:20 p.m. UTC | #4
> 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
David Ahern May 14, 2021, 2:10 p.m. UTC | #5
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)
Petr Vorel May 15, 2021, 4:58 p.m. UTC | #6
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/
Petr Vorel May 15, 2021, 4:59 p.m. UTC | #7
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
David Ahern May 15, 2021, 6:26 p.m. UTC | #8
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.
David Ahern May 17, 2021, 2:45 p.m. UTC | #9
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.
Petr Vorel May 17, 2021, 5:36 p.m. UTC | #10
> 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
David Ahern May 18, 2021, 1:51 a.m. UTC | #11
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.
Heiko Thiery May 18, 2021, 7:24 a.m. UTC | #12
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
diff mbox series

Patch

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)
 {