| Message ID | 20190619132326.1846345b@canb.auug.org.au |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
| Series | linux-next: build failure after merge of the net-next tree | expand |
Hi. On Wed, Jun 19, 2019 at 12:23 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote: > > Hi all, > > After merging the net-next tree, today's linux-next build (x86_64 > allmodconfig) failed like this: > > In file included from usr/include/linux/tc_act/tc_ctinfo.hdrtest.c:1: > ./usr/include/linux/tc_act/tc_ctinfo.h:30:21: error: implicit declaration of function 'BIT' [-Werror=implicit-function-declaration] > CTINFO_MODE_DSCP = BIT(0), > ^~~ > ./usr/include/linux/tc_act/tc_ctinfo.h:30:2: error: enumerator value for 'CTINFO_MODE_DSCP' is not an integer constant > CTINFO_MODE_DSCP = BIT(0), > ^~~~~~~~~~~~~~~~ > ./usr/include/linux/tc_act/tc_ctinfo.h:32:1: error: enumerator value for 'CTINFO_MODE_CPMARK' is not an integer constant > }; > ^ > > Caused by commit > > 24ec483cec98 ("net: sched: Introduce act_ctinfo action") > > Presumably exposed by commit > > b91976b7c0e3 ("kbuild: compile-test UAPI headers to ensure they are self-contained") > > from the kbuild tree. My commit correctly blocked the broken UAPI header, Hooray! People export more and more headers that are never able to compile in user-space. We must block new breakages from coming in. BIT() is not exported to user-space since it is not prefixed with underscore. You can use _BITUL() in user-space, which is available in include/uapi/linux/const.h Thanks. > I have applied the following (obvious) patch for today. > > From: Stephen Rothwell <sfr@canb.auug.org.au> > Date: Wed, 19 Jun 2019 13:15:22 +1000 > Subject: [PATCH] net: sched: don't use BIT() in uapi headers > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > --- > include/uapi/linux/tc_act/tc_ctinfo.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/uapi/linux/tc_act/tc_ctinfo.h b/include/uapi/linux/tc_act/tc_ctinfo.h > index da803e05a89b..6166c62dd7dd 100644 > --- a/include/uapi/linux/tc_act/tc_ctinfo.h > +++ b/include/uapi/linux/tc_act/tc_ctinfo.h > @@ -27,8 +27,8 @@ enum { > #define TCA_CTINFO_MAX (__TCA_CTINFO_MAX - 1) > > enum { > - CTINFO_MODE_DSCP = BIT(0), > - CTINFO_MODE_CPMARK = BIT(1) > + CTINFO_MODE_DSCP = (1UL << 0), > + CTINFO_MODE_CPMARK = (1UL << 1) > }; > > #endif > -- > 2.20.1 > > -- > Cheers, > Stephen Rothwell
On Wed, Jun 19, 2019 at 1:02 PM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > Hi. > > > On Wed, Jun 19, 2019 at 12:23 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote: > > > > Hi all, > > > > After merging the net-next tree, today's linux-next build (x86_64 > > allmodconfig) failed like this: > > > > In file included from usr/include/linux/tc_act/tc_ctinfo.hdrtest.c:1: > > ./usr/include/linux/tc_act/tc_ctinfo.h:30:21: error: implicit declaration of function 'BIT' [-Werror=implicit-function-declaration] > > CTINFO_MODE_DSCP = BIT(0), > > ^~~ > > ./usr/include/linux/tc_act/tc_ctinfo.h:30:2: error: enumerator value for 'CTINFO_MODE_DSCP' is not an integer constant > > CTINFO_MODE_DSCP = BIT(0), > > ^~~~~~~~~~~~~~~~ > > ./usr/include/linux/tc_act/tc_ctinfo.h:32:1: error: enumerator value for 'CTINFO_MODE_CPMARK' is not an integer constant > > }; > > ^ > > > > Caused by commit > > > > 24ec483cec98 ("net: sched: Introduce act_ctinfo action") > > > > Presumably exposed by commit > > > > b91976b7c0e3 ("kbuild: compile-test UAPI headers to ensure they are self-contained") > > > > from the kbuild tree. > > > My commit correctly blocked the broken UAPI header, Hooray! > > People export more and more headers that > are never able to compile in user-space. > > We must block new breakages from coming in. > > > BIT() is not exported to user-space > since it is not prefixed with underscore. > > > You can use _BITUL() in user-space, > which is available in include/uapi/linux/const.h > > I just took a look at include/uapi/linux/tc_act/tc_ctinfo.h I just wondered why the following can be compiled: struct tc_ctinfo { tc_gen; }; Then, I found 'tc_gen' is a macro. #define tc_gen \ __u32 index; \ __u32 capab; \ int action; \ int refcnt; \ int bindcnt What a hell.
Greetings! As the guilty party in authoring this, and also pretty new around here I’m wondering what I need to do to help clean it up? > On 19 Jun 2019, at 05:14, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > On Wed, Jun 19, 2019 at 1:02 PM Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> >> Hi. >> >> >> On Wed, Jun 19, 2019 at 12:23 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote: >>> >>> Hi all, >>> >>> After merging the net-next tree, today's linux-next build (x86_64 >>> allmodconfig) failed like this: >>> >>> In file included from usr/include/linux/tc_act/tc_ctinfo.hdrtest.c:1: >>> ./usr/include/linux/tc_act/tc_ctinfo.h:30:21: error: implicit declaration of function 'BIT' [-Werror=implicit-function-declaration] >>> CTINFO_MODE_DSCP = BIT(0), >>> ^~~ >>> ./usr/include/linux/tc_act/tc_ctinfo.h:30:2: error: enumerator value for 'CTINFO_MODE_DSCP' is not an integer constant >>> CTINFO_MODE_DSCP = BIT(0), >>> ^~~~~~~~~~~~~~~~ >>> ./usr/include/linux/tc_act/tc_ctinfo.h:32:1: error: enumerator value for 'CTINFO_MODE_CPMARK' is not an integer constant >>> }; >>> ^ >>> >>> Caused by commit >>> >>> 24ec483cec98 ("net: sched: Introduce act_ctinfo action") >>> >>> Presumably exposed by commit >>> >>> b91976b7c0e3 ("kbuild: compile-test UAPI headers to ensure they are self-contained") >>> >>> from the kbuild tree. Stephen, thanks for the fixup - is that now in the tree or do I need to submit a fix via the normal net-next channel so it gets picked up by the iproute2 people who maintain a local copy of the uapi includes? >> >> >> My commit correctly blocked the broken UAPI header, Hooray! >> >> People export more and more headers that >> are never able to compile in user-space. >> >> We must block new breakages from coming in. >> >> >> BIT() is not exported to user-space >> since it is not prefixed with underscore. >> >> >> You can use _BITUL() in user-space, >> which is available in include/uapi/linux/const.h Thanks for the pointers. I am confused as to why I didn’t hit this issue when I built & run tested locally off the net-next tree. >> >> > > > I just took a look at > include/uapi/linux/tc_act/tc_ctinfo.h > > > I just wondered why the following can be compiled: > > struct tc_ctinfo { > tc_gen; > }; > > > Then, I found 'tc_gen' is a macro. > > #define tc_gen \ > __u32 index; \ > __u32 capab; \ > int action; \ > int refcnt; \ > int bindcnt > > > > What a hell. This is what other actions do e.g. tc_skbedit. Can you advise what I should do instead? > -- > Best Regards > Masahiro Yamada Many thanks to all for your valuable time & advice. Cheers, Kevin D-B gpg: 012C ACB2 28C6 C53E 9775 9123 B3A2 389B 9DE2 334A
From: Masahiro Yamada <yamada.masahiro@socionext.com> Date: Wed, 19 Jun 2019 13:14:06 +0900 > What a hell. I know, some serious bush league coding going on in the networking right?
I've fixed this as follows, thanks: ==================== From 23cdf8752b26d4edbd60a6293bca492d83192d4d Mon Sep 17 00:00:00 2001 From: "David S. Miller" <davem@davemloft.net> Date: Wed, 19 Jun 2019 10:12:58 -0400 Subject: [PATCH] act_ctinfo: Don't use BIT() in UAPI headers. Use _BITUL() instead. Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Signed-off-by: David S. Miller <davem@davemloft.net> --- include/uapi/linux/tc_act/tc_ctinfo.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/uapi/linux/tc_act/tc_ctinfo.h b/include/uapi/linux/tc_act/tc_ctinfo.h index da803e05a89b..32337304fbe5 100644 --- a/include/uapi/linux/tc_act/tc_ctinfo.h +++ b/include/uapi/linux/tc_act/tc_ctinfo.h @@ -27,8 +27,8 @@ enum { #define TCA_CTINFO_MAX (__TCA_CTINFO_MAX - 1) enum { - CTINFO_MODE_DSCP = BIT(0), - CTINFO_MODE_CPMARK = BIT(1) + CTINFO_MODE_DSCP = _BITUL(0), + CTINFO_MODE_CPMARK = _BITUL(1) }; #endif
> On 19 Jun 2019, at 15:13, David Miller <davem@davemloft.net> wrote: > > > I've fixed this as follows, thanks: > > ==================== > From 23cdf8752b26d4edbd60a6293bca492d83192d4d Mon Sep 17 00:00:00 2001 > From: "David S. Miller" <davem@davemloft.net> > Date: Wed, 19 Jun 2019 10:12:58 -0400 > Subject: [PATCH] act_ctinfo: Don't use BIT() in UAPI headers. > > Use _BITUL() instead. > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: David S. Miller <davem@davemloft.net> > --- > include/uapi/linux/tc_act/tc_ctinfo.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/uapi/linux/tc_act/tc_ctinfo.h b/include/uapi/linux/tc_act/tc_ctinfo.h > index da803e05a89b..32337304fbe5 100644 > --- a/include/uapi/linux/tc_act/tc_ctinfo.h > +++ b/include/uapi/linux/tc_act/tc_ctinfo.h > @@ -27,8 +27,8 @@ enum { > #define TCA_CTINFO_MAX (__TCA_CTINFO_MAX - 1) > > enum { > - CTINFO_MODE_DSCP = BIT(0), > - CTINFO_MODE_CPMARK = BIT(1) > + CTINFO_MODE_DSCP = _BITUL(0), > + CTINFO_MODE_CPMARK = _BITUL(1) > }; > > #endif > -- > 2.20.1 > Hi David, Thanks for that. Owe you a beer! Thinking out loud, doesn’t this also require: #include <linux/const.h> Or at least iproute2 would need to know about _BITUL as it doesn’t at present. Which also means iproute2’s Linux uapi shadow would also have to import linux/const.h. Or have I got wrong end of stick? Cheers, Kevin
> On 19 Jun 2019, at 16:09, Kevin 'ldir' Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> wrote: > > > >> On 19 Jun 2019, at 15:13, David Miller <davem@davemloft.net> wrote: >> >> >> I've fixed this as follows, thanks: >> >> ==================== >> From 23cdf8752b26d4edbd60a6293bca492d83192d4d Mon Sep 17 00:00:00 2001 >> From: "David S. Miller" <davem@davemloft.net> >> Date: Wed, 19 Jun 2019 10:12:58 -0400 >> Subject: [PATCH] act_ctinfo: Don't use BIT() in UAPI headers. >> >> Use _BITUL() instead. >> >> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> >> Signed-off-by: David S. Miller <davem@davemloft.net> >> --- >> include/uapi/linux/tc_act/tc_ctinfo.h | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/include/uapi/linux/tc_act/tc_ctinfo.h b/include/uapi/linux/tc_act/tc_ctinfo.h >> index da803e05a89b..32337304fbe5 100644 >> --- a/include/uapi/linux/tc_act/tc_ctinfo.h >> +++ b/include/uapi/linux/tc_act/tc_ctinfo.h >> @@ -27,8 +27,8 @@ enum { >> #define TCA_CTINFO_MAX (__TCA_CTINFO_MAX - 1) >> >> enum { >> - CTINFO_MODE_DSCP = BIT(0), >> - CTINFO_MODE_CPMARK = BIT(1) >> + CTINFO_MODE_DSCP = _BITUL(0), >> + CTINFO_MODE_CPMARK = _BITUL(1) >> }; >> >> #endif >> -- >> 2.20.1 >> > > Hi David, > > Thanks for that. Owe you a beer! > > Thinking out loud, doesn’t this also require: > > #include <linux/const.h> > > > Or at least iproute2 would need to know about _BITUL as it doesn’t at present. > Which also means iproute2’s Linux uapi shadow would also have to import > linux/const.h. Or have I got wrong end of stick? > > Cheers, > > Kevin > Whilst out walking the dog I realised I’m a moron. The CTINFO_MODE_FOO is only used within module, it shouldn’t even be exposed to user space. I’ll send a patch shortly. Sorry, I’m pretty embarrassed at how something so apparently simple on the surface has had so many issues. Cheers, Kevin D-B gpg: 012C ACB2 28C6 C53E 9775 9123 B3A2 389B 9DE2 334A
diff --git a/include/uapi/linux/tc_act/tc_ctinfo.h b/include/uapi/linux/tc_act/tc_ctinfo.h index da803e05a89b..6166c62dd7dd 100644 --- a/include/uapi/linux/tc_act/tc_ctinfo.h +++ b/include/uapi/linux/tc_act/tc_ctinfo.h @@ -27,8 +27,8 @@ enum { #define TCA_CTINFO_MAX (__TCA_CTINFO_MAX - 1) enum { - CTINFO_MODE_DSCP = BIT(0), - CTINFO_MODE_CPMARK = BIT(1) + CTINFO_MODE_DSCP = (1UL << 0), + CTINFO_MODE_CPMARK = (1UL << 1) }; #endif
Hi all, After merging the net-next tree, today's linux-next build (x86_64 allmodconfig) failed like this: In file included from usr/include/linux/tc_act/tc_ctinfo.hdrtest.c:1: ./usr/include/linux/tc_act/tc_ctinfo.h:30:21: error: implicit declaration of function 'BIT' [-Werror=implicit-function-declaration] CTINFO_MODE_DSCP = BIT(0), ^~~ ./usr/include/linux/tc_act/tc_ctinfo.h:30:2: error: enumerator value for 'CTINFO_MODE_DSCP' is not an integer constant CTINFO_MODE_DSCP = BIT(0), ^~~~~~~~~~~~~~~~ ./usr/include/linux/tc_act/tc_ctinfo.h:32:1: error: enumerator value for 'CTINFO_MODE_CPMARK' is not an integer constant }; ^ Caused by commit 24ec483cec98 ("net: sched: Introduce act_ctinfo action") Presumably exposed by commit b91976b7c0e3 ("kbuild: compile-test UAPI headers to ensure they are self-contained") from the kbuild tree. I have applied the following (obvious) patch for today. From: Stephen Rothwell <sfr@canb.auug.org.au> Date: Wed, 19 Jun 2019 13:15:22 +1000 Subject: [PATCH] net: sched: don't use BIT() in uapi headers Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> --- include/uapi/linux/tc_act/tc_ctinfo.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)