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 <[email protected]>
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 <[email protected]>
---
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
Hi.
On Wed, Jun 19, 2019 at 12:23 PM Stephen Rothwell <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> ---
> 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
--
Best Regards
Masahiro Yamada
On Wed, Jun 19, 2019 at 1:02 PM Masahiro Yamada
<[email protected]> wrote:
>
> Hi.
>
>
> On Wed, Jun 19, 2019 at 12:23 PM Stephen Rothwell <[email protected]> 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.
--
Best Regards
Masahiro Yamada
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 <[email protected]> wrote:
>
> On Wed, Jun 19, 2019 at 1:02 PM Masahiro Yamada
> <[email protected]> wrote:
>>
>> Hi.
>>
>>
>> On Wed, Jun 19, 2019 at 12:23 PM Stephen Rothwell <[email protected]> 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 <[email protected]>
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" <[email protected]>
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 <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
---
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
> On 19 Jun 2019, at 15:13, David Miller <[email protected]> wrote:
>
>
> I've fixed this as follows, thanks:
>
> ====================
> From 23cdf8752b26d4edbd60a6293bca492d83192d4d Mon Sep 17 00:00:00 2001
> From: "David S. Miller" <[email protected]>
> 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 <[email protected]>
> Signed-off-by: David S. Miller <[email protected]>
> ---
> 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 <[email protected]> wrote:
>
>
>
>> On 19 Jun 2019, at 15:13, David Miller <[email protected]> wrote:
>>
>>
>> I've fixed this as follows, thanks:
>>
>> ====================
>> From 23cdf8752b26d4edbd60a6293bca492d83192d4d Mon Sep 17 00:00:00 2001
>> From: "David S. Miller" <[email protected]>
>> 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 <[email protected]>
>> Signed-off-by: David S. Miller <[email protected]>
>> ---
>> 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