2021-05-08 06:52:09

by Heiko Thiery

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

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 <[email protected]>
Cc: Petr Vorel <[email protected]>
Signed-off-by: Heiko Thiery <[email protected]>
---
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


2021-05-08 09:02:55

by Petr Vorel

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

Hi Heiko,

Reviewed-by: Petr Vorel <[email protected]>
Thanks!

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

Kind regards,
Petr

2021-05-09 22:24:45

by David Ahern

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

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 <[email protected]>
> Cc: Petr Vorel <[email protected]>
> Signed-off-by: Heiko Thiery <[email protected]>
> ---
> 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.

2021-05-14 16:45:31

by Heiko Thiery

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

Hi David,

Am Mo., 10. Mai 2021 um 00:20 Uhr schrieb David Ahern <[email protected]>:
>
> 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 <[email protected]>
> > Cc: Petr Vorel <[email protected]>
> > Signed-off-by: Heiko Thiery <[email protected]>
> > ---
> > 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

2021-05-14 18:16:50

by Petr Vorel

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

> Hi David,

> Am Mo., 10. Mai 2021 um 00:20 Uhr schrieb David Ahern <[email protected]>:

> > 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 <[email protected]>
> > > Cc: Petr Vorel <[email protected]>
> > > Signed-off-by: Heiko Thiery <[email protected]>
> > > ---
> > > 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

2021-05-14 19:01:32

by David Ahern

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

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)


2021-05-15 22:57:43

by Petr Vorel

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

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 <[email protected]>
> Cc: Petr Vorel <[email protected]>
> Signed-off-by: Heiko Thiery <[email protected]>
> ---
> 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 <[email protected]>
(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/[email protected]/

2021-05-16 07:31:57

by Petr Vorel

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

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 <[email protected]>

-nt is supported by dash and busybox sh.

Kind regards,
Petr

2021-05-16 09:01:40

by David Ahern

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

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 <[email protected]>
>
> -nt is supported by dash and busybox sh.
>

That helps. My concern was all the sh variants.

2021-05-18 18:54:14

by David Ahern

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

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 <[email protected]>
> Cc: Petr Vorel <[email protected]>
> Signed-off-by: Heiko Thiery <[email protected]>
> ---
> 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.


2021-05-18 21:12:27

by Petr Vorel

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

> 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 <[email protected]>
> > Cc: Petr Vorel <[email protected]>
> > Signed-off-by: Heiko Thiery <[email protected]>
> > ---
> > 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

2021-05-19 11:17:57

by David Ahern

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

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.

2021-05-19 16:08:43

by Heiko Thiery

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

Hi all,

Am Di., 18. Mai 2021 um 03:51 Uhr schrieb David Ahern <[email protected]>:
>
> 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