| Message ID | d13ef7c00b60a50a5e8ddbb7ff138399689d3483.1707474099.git.aclaudi@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Stephen Hemminger |
| Headers | show |
| Series | iproute2: fix build failure on ppc64le | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Fri, Feb 09, 2024 at 11:24:47AM +0100, Andrea Claudi wrote: > ppc64le build fails with error on ifstat.c when > -Wincompatible-pointer-types is enabled: > > ifstat.c: In function ‘dump_raw_db’: > ifstat.c:323:44: error: initialization of ‘long long unsigned int *’ from incompatible pointer type ‘__u64 *’ {aka ‘long unsigned int *’} [-Wincompatible-pointer-types] > 323 | unsigned long long *vals = n->val; > > Several other warnings are produced when -Wformat= is set, for example: > > ss.c:3244:34: warning: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘__u64’ {aka ‘long unsigned int’} [-Wformat=] > 3244 | out(" rcv_nxt:%llu", s->mptcpi_rcv_nxt); > | ~~~^ ~~~~~~~~~~~~~~~~~ > | | | > | | __u64 {aka long unsigned int} > | long long unsigned int > | %lu > > This happens because __u64 is defined as long unsigned on ppc64le. As > pointed out by Florian Weimar, we should use -D__SANE_USERSPACE_TYPES__ > if we really want to use long long unsigned in iproute2. > > This fix the build failure and all the warnings without any change on > the code itself. > > Suggested-by: Florian Weimer <fweimer@redhat.com> > Signed-off-by: Andrea Claudi <aclaudi@redhat.com> Sorry, I forgot to add: Fixes: 5a52102b7c8f ("ifstat: Add extended statistics to ifstat") even after I recommended it to Stephen G... Seems I need more coffee today :) > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 8024d45e..3b9daede 100644 > --- a/Makefile > +++ b/Makefile > @@ -60,7 +60,7 @@ CC := gcc > HOSTCC ?= $(CC) > DEFINES += -D_GNU_SOURCE > # Turn on transparent support for LFS > -DEFINES += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE > +DEFINES += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D__SANE_USERSPACE_TYPES__ > CCOPTS = -O2 -pipe > WFLAGS := -Wall -Wstrict-prototypes -Wmissing-prototypes > WFLAGS += -Wmissing-declarations -Wold-style-definition -Wformat=2 > -- > 2.43.0 >
On Fri, 9 Feb 2024 11:24:47 +0100 Andrea Claudi <aclaudi@redhat.com> wrote: > ss.c:3244:34: warning: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘__u64’ {aka ‘long unsigned int’} [-Wformat=] > 3244 | out(" rcv_nxt:%llu", s->mptcpi_rcv_nxt); > | ~~~^ ~~~~~~~~~~~~~~~~~ > | | | > | | __u64 {aka long unsigned int} > | long long unsigned int > | %lu > > This happens because __u64 is defined as long unsigned on ppc64le. As > pointed out by Florian Weimar, we should use -D__SANE_USERSPACE_TYPES__ > if we really want to use long long unsigned in iproute2. Ok, this looks good. Another way to fix would be to use the macros defined in inttypes.h out(" rcv_nxt:"PRIu64, s->mptcpi_rcv_nxt);
On 2/9/24 9:35 AM, Stephen Hemminger wrote: > On Fri, 9 Feb 2024 11:24:47 +0100 > Andrea Claudi <aclaudi@redhat.com> wrote: > >> ss.c:3244:34: warning: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘__u64’ {aka ‘long unsigned int’} [-Wformat=] >> 3244 | out(" rcv_nxt:%llu", s->mptcpi_rcv_nxt); >> | ~~~^ ~~~~~~~~~~~~~~~~~ >> | | | >> | | __u64 {aka long unsigned int} >> | long long unsigned int >> | %lu >> >> This happens because __u64 is defined as long unsigned on ppc64le. As >> pointed out by Florian Weimar, we should use -D__SANE_USERSPACE_TYPES__ >> if we really want to use long long unsigned in iproute2. > > Ok, this looks good. > Another way to fix would be to use the macros defined in inttypes.h > > out(" rcv_nxt:"PRIu64, s->mptcpi_rcv_nxt); > since the uapi is __u64, I think this is the better approach.
On Fri, 9 Feb 2024 15:14:28 -0700 David Ahern <dsahern@gmail.com> wrote: > On 2/9/24 9:35 AM, Stephen Hemminger wrote: > > On Fri, 9 Feb 2024 11:24:47 +0100 > > Andrea Claudi <aclaudi@redhat.com> wrote: > > > >> ss.c:3244:34: warning: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘__u64’ {aka ‘long unsigned int’} [-Wformat=] > >> 3244 | out(" rcv_nxt:%llu", s->mptcpi_rcv_nxt); > >> | ~~~^ ~~~~~~~~~~~~~~~~~ > >> | | | > >> | | __u64 {aka long unsigned int} > >> | long long unsigned int > >> | %lu > >> > >> This happens because __u64 is defined as long unsigned on ppc64le. As > >> pointed out by Florian Weimar, we should use -D__SANE_USERSPACE_TYPES__ > >> if we really want to use long long unsigned in iproute2. > > > > Ok, this looks good. > > Another way to fix would be to use the macros defined in inttypes.h > > > > out(" rcv_nxt:"PRIu64, s->mptcpi_rcv_nxt); > > > > since the uapi is __u64, I think this is the better approach. NVM Tried it, but __u64 is not the same as uint64_t even on x86. __u64 is long long unsigned int uint64_t is long unsigned int
On Fri, Feb 09, 2024 at 04:45:42PM -0800, Stephen Hemminger wrote: > On Fri, 9 Feb 2024 15:14:28 -0700 > David Ahern <dsahern@gmail.com> wrote: > > > On 2/9/24 9:35 AM, Stephen Hemminger wrote: > > > On Fri, 9 Feb 2024 11:24:47 +0100 > > > Andrea Claudi <aclaudi@redhat.com> wrote: > > > > > >> ss.c:3244:34: warning: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘__u64’ {aka ‘long unsigned int’} [-Wformat=] > > >> 3244 | out(" rcv_nxt:%llu", s->mptcpi_rcv_nxt); > > >> | ~~~^ ~~~~~~~~~~~~~~~~~ > > >> | | | > > >> | | __u64 {aka long unsigned int} > > >> | long long unsigned int > > >> | %lu > > >> > > >> This happens because __u64 is defined as long unsigned on ppc64le. As > > >> pointed out by Florian Weimar, we should use -D__SANE_USERSPACE_TYPES__ > > >> if we really want to use long long unsigned in iproute2. > > > > > > Ok, this looks good. > > > Another way to fix would be to use the macros defined in inttypes.h > > > > > > out(" rcv_nxt:"PRIu64, s->mptcpi_rcv_nxt); > > > > > > > since the uapi is __u64, I think this is the better approach. > > NVM > Tried it, but __u64 is not the same as uint64_t even on x86. > __u64 is long long unsigned int > uint64_t is long unsigned int > Is there anything more I can do about this?
On 2/14/24 8:30 AM, Andrea Claudi wrote: > On Fri, Feb 09, 2024 at 04:45:42PM -0800, Stephen Hemminger wrote: >> On Fri, 9 Feb 2024 15:14:28 -0700 >> David Ahern <dsahern@gmail.com> wrote: >> >>> On 2/9/24 9:35 AM, Stephen Hemminger wrote: >>>> On Fri, 9 Feb 2024 11:24:47 +0100 >>>> Andrea Claudi <aclaudi@redhat.com> wrote: >>>> >>>>> ss.c:3244:34: warning: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘__u64’ {aka ‘long unsigned int’} [-Wformat=] >>>>> 3244 | out(" rcv_nxt:%llu", s->mptcpi_rcv_nxt); >>>>> | ~~~^ ~~~~~~~~~~~~~~~~~ >>>>> | | | >>>>> | | __u64 {aka long unsigned int} >>>>> | long long unsigned int >>>>> | %lu >>>>> >>>>> This happens because __u64 is defined as long unsigned on ppc64le. As >>>>> pointed out by Florian Weimar, we should use -D__SANE_USERSPACE_TYPES__ >>>>> if we really want to use long long unsigned in iproute2. >>>> >>>> Ok, this looks good. >>>> Another way to fix would be to use the macros defined in inttypes.h >>>> >>>> out(" rcv_nxt:"PRIu64, s->mptcpi_rcv_nxt); >>>> >>> >>> since the uapi is __u64, I think this is the better approach. >> >> NVM >> Tried it, but __u64 is not the same as uint64_t even on x86. >> __u64 is long long unsigned int >> uint64_t is long unsigned int >> > > Is there anything more I can do about this? > where does the uint64_t come in? include/uapi/linux/mptcp.h has mptcpi_rcv_nxt as __u64 and PRIu64 macros should be working without a problem - this is what perf tool uses consistently.
On Wed, 14 Feb 2024 08:49:02 -0700 David Ahern <dsahern@gmail.com> wrote: > On 2/14/24 8:30 AM, Andrea Claudi wrote: > > On Fri, Feb 09, 2024 at 04:45:42PM -0800, Stephen Hemminger wrote: > >> On Fri, 9 Feb 2024 15:14:28 -0700 > >> David Ahern <dsahern@gmail.com> wrote: > >> > >>> On 2/9/24 9:35 AM, Stephen Hemminger wrote: > >>>> On Fri, 9 Feb 2024 11:24:47 +0100 > >>>> Andrea Claudi <aclaudi@redhat.com> wrote: > >>>> > >>>>> ss.c:3244:34: warning: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘__u64’ {aka ‘long unsigned int’} [-Wformat=] > >>>>> 3244 | out(" rcv_nxt:%llu", s->mptcpi_rcv_nxt); > >>>>> | ~~~^ ~~~~~~~~~~~~~~~~~ > >>>>> | | | > >>>>> | | __u64 {aka long unsigned int} > >>>>> | long long unsigned int > >>>>> | %lu > >>>>> > >>>>> This happens because __u64 is defined as long unsigned on ppc64le. As > >>>>> pointed out by Florian Weimar, we should use -D__SANE_USERSPACE_TYPES__ > >>>>> if we really want to use long long unsigned in iproute2. > >>>> > >>>> Ok, this looks good. > >>>> Another way to fix would be to use the macros defined in inttypes.h > >>>> > >>>> out(" rcv_nxt:"PRIu64, s->mptcpi_rcv_nxt); > >>>> > >>> > >>> since the uapi is __u64, I think this is the better approach. > >> > >> NVM > >> Tried it, but __u64 is not the same as uint64_t even on x86. > >> __u64 is long long unsigned int > >> uint64_t is long unsigned int > >> > > > > Is there anything more I can do about this? > > > > where does the uint64_t come in? include/uapi/linux/mptcp.h has > mptcpi_rcv_nxt as __u64 and PRIu64 macros should be working without a > problem - this is what perf tool uses consistently. I just did this: diff --git a/misc/ss.c b/misc/ss.c index 5296cabe9982..679d50b8fef6 100644 --- a/misc/ss.c +++ b/misc/ss.c @@ -8,6 +8,7 @@ #include <stdio.h> #include <stdlib.h> #include <unistd.h> +#include <inttypes.h> #include <fcntl.h> #include <sys/ioctl.h> #include <sys/socket.h> @@ -3241,7 +3242,7 @@ static void mptcp_stats_print(struct mptcp_info *s) if (s->mptcpi_snd_una) out(" snd_una:%llu", s->mptcpi_snd_una); if (s->mptcpi_rcv_nxt) - out(" rcv_nxt:%llu", s->mptcpi_rcv_nxt); + out(" rcv_nxt:%" PRIu64, s->mptcpi_rcv_nxt); if (s->mptcpi_local_addr_used) out(" local_addr_used:%u", s->mptcpi_local_addr_used); if (s->mptcpi_local_addr_max) And got this: CC ss.o ss.c: In function ‘mptcp_stats_print’: ss.c:3245:21: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘__u64’ {aka ‘long long unsigned int’} [-Wformat=] 3245 | out(" rcv_nxt:%" PRIu64, s->mptcpi_rcv_nxt); | ^~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~ | | | __u64 {aka long long unsigned int} In file included from ss.c:11: /usr/include/inttypes.h:105:41: note: format string is defined here 105 | # define PRIu64 __PRI64_PREFIX "u"
On 2/14/24 8:05 PM, Stephen Hemminger wrote: > > I just did this: > > diff --git a/misc/ss.c b/misc/ss.c > index 5296cabe9982..679d50b8fef6 100644 > --- a/misc/ss.c > +++ b/misc/ss.c > @@ -8,6 +8,7 @@ > #include <stdio.h> > #include <stdlib.h> > #include <unistd.h> > +#include <inttypes.h> > #include <fcntl.h> > #include <sys/ioctl.h> > #include <sys/socket.h> > @@ -3241,7 +3242,7 @@ static void mptcp_stats_print(struct mptcp_info *s) > if (s->mptcpi_snd_una) > out(" snd_una:%llu", s->mptcpi_snd_una); > if (s->mptcpi_rcv_nxt) > - out(" rcv_nxt:%llu", s->mptcpi_rcv_nxt); > + out(" rcv_nxt:%" PRIu64, s->mptcpi_rcv_nxt); > if (s->mptcpi_local_addr_used) > out(" local_addr_used:%u", s->mptcpi_local_addr_used); > if (s->mptcpi_local_addr_max) > > > And got this: > CC ss.o > ss.c: In function ‘mptcp_stats_print’: > ss.c:3245:21: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘__u64’ {aka ‘long long unsigned int’} [-Wformat=] > 3245 | out(" rcv_nxt:%" PRIu64, s->mptcpi_rcv_nxt); > | ^~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~ > | | > | __u64 {aka long long unsigned int} > In file included from ss.c:11: > /usr/include/inttypes.h:105:41: note: format string is defined here > 105 | # define PRIu64 __PRI64_PREFIX "u" > Andrea: can you check on how perf (kernel utils) compiles on ppc64le? I thought Arnaldo had build "farms" for all of the architectures; maybe not for this one. __u64 is used a lot in the perf_events UAPI and PRIu64 is used extensively in the userspace code.
On Wed, Feb 14, 2024 at 08:10:54PM -0700, David Ahern wrote: > On 2/14/24 8:05 PM, Stephen Hemminger wrote: > > > > I just did this: > > > > diff --git a/misc/ss.c b/misc/ss.c > > index 5296cabe9982..679d50b8fef6 100644 > > --- a/misc/ss.c > > +++ b/misc/ss.c > > @@ -8,6 +8,7 @@ > > #include <stdio.h> > > #include <stdlib.h> > > #include <unistd.h> > > +#include <inttypes.h> > > #include <fcntl.h> > > #include <sys/ioctl.h> > > #include <sys/socket.h> > > @@ -3241,7 +3242,7 @@ static void mptcp_stats_print(struct mptcp_info *s) > > if (s->mptcpi_snd_una) > > out(" snd_una:%llu", s->mptcpi_snd_una); > > if (s->mptcpi_rcv_nxt) > > - out(" rcv_nxt:%llu", s->mptcpi_rcv_nxt); > > + out(" rcv_nxt:%" PRIu64, s->mptcpi_rcv_nxt); > > if (s->mptcpi_local_addr_used) > > out(" local_addr_used:%u", s->mptcpi_local_addr_used); > > if (s->mptcpi_local_addr_max) > > > > > > And got this: > > CC ss.o > > ss.c: In function ‘mptcp_stats_print’: > > ss.c:3245:21: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘__u64’ {aka ‘long long unsigned int’} [-Wformat=] > > 3245 | out(" rcv_nxt:%" PRIu64, s->mptcpi_rcv_nxt); > > | ^~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~ > > | | > > | __u64 {aka long long unsigned int} > > In file included from ss.c:11: > > /usr/include/inttypes.h:105:41: note: format string is defined here > > 105 | # define PRIu64 __PRI64_PREFIX "u" > > > > Andrea: can you check on how perf (kernel utils) compiles on ppc64le? I > thought Arnaldo had build "farms" for all of the architectures; maybe > not for this one. __u64 is used a lot in the perf_events UAPI and PRIu64 > is used extensively in the userspace code. Sure, I'll look into perf and let you know. Thanks, Andrea
diff --git a/Makefile b/Makefile index 8024d45e..3b9daede 100644 --- a/Makefile +++ b/Makefile @@ -60,7 +60,7 @@ CC := gcc HOSTCC ?= $(CC) DEFINES += -D_GNU_SOURCE # Turn on transparent support for LFS -DEFINES += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE +DEFINES += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D__SANE_USERSPACE_TYPES__ CCOPTS = -O2 -pipe WFLAGS := -Wall -Wstrict-prototypes -Wmissing-prototypes WFLAGS += -Wmissing-declarations -Wold-style-definition -Wformat=2
ppc64le build fails with error on ifstat.c when -Wincompatible-pointer-types is enabled: ifstat.c: In function ‘dump_raw_db’: ifstat.c:323:44: error: initialization of ‘long long unsigned int *’ from incompatible pointer type ‘__u64 *’ {aka ‘long unsigned int *’} [-Wincompatible-pointer-types] 323 | unsigned long long *vals = n->val; Several other warnings are produced when -Wformat= is set, for example: ss.c:3244:34: warning: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘__u64’ {aka ‘long unsigned int’} [-Wformat=] 3244 | out(" rcv_nxt:%llu", s->mptcpi_rcv_nxt); | ~~~^ ~~~~~~~~~~~~~~~~~ | | | | | __u64 {aka long unsigned int} | long long unsigned int | %lu This happens because __u64 is defined as long unsigned on ppc64le. As pointed out by Florian Weimar, we should use -D__SANE_USERSPACE_TYPES__ if we really want to use long long unsigned in iproute2. This fix the build failure and all the warnings without any change on the code itself. Suggested-by: Florian Weimer <fweimer@redhat.com> Signed-off-by: Andrea Claudi <aclaudi@redhat.com> --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)