diff mbox series

ethtool build failure

Message ID CAEyMn7a5SwQtMxrrJ-C0Jy6THZcCCPXp5ouC+jRLH4ySK-8p_A@mail.gmail.com
State Not Applicable
Headers show
Series ethtool build failure | expand

Commit Message

Heiko Thiery June 6, 2020, 1:24 p.m. UTC 
Hi Michael et all,

I'm digging in the reason for a failure when building ethtool with
buildroot [1].

I see the following error:
---
data/buildroot/buildroot-test/instance-0/output/host/bin/i686-linux-gcc
-DHAVE_CONFIG_H -I.  -I./uapi  -D_LARGEFILE_SOURCE
-D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -D_LARGEFILE_SOURCE
-D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -Os   -static -c -o
netlink/desc-rtnl.o netlink/desc-rtnl.c
In file included from ./uapi/linux/ethtool_netlink.h:12,
                 from netlink/desc-ethtool.c:7:
./uapi/linux/ethtool.h:1294:19: warning: implicit declaration of
function '__KERNEL_DIV_ROUND_UP' [-Wimplicit-function-declaration]
  __u32 queue_mask[__KERNEL_DIV_ROUND_UP(MAX_NUM_QUEUE, 32)];
                   ^~~~~~~~~~~~~~~~~~~~~
./uapi/linux/ethtool.h:1294:8: error: variably modified 'queue_mask'
at file scope
  __u32 queue_mask[__KERNEL_DIV_ROUND_UP(MAX_NUM_QUEUE, 32)];
        ^~~~~~~~~~
---

The problems seems to be injected by the "warning: implicit
declaration of function".

When I move the __KERNEL_DIV_ROUND_UP macro right beside usage in
"uapi/linux/ethtool.h" the failure is gone.

---
        __u32   sub_command;
---

Has anyone an idea what goes wrong?

[1] http://autobuild.buildroot.net/results/23dffe2f9766c12badb6e42bf3f0356c6df0cbfb/build-end.log

Comments

Michal Kubecek June 6, 2020, 1:43 p.m. UTC | #1 
On Sat, Jun 06, 2020 at 03:24:22PM +0200, Heiko Thiery wrote:
> Hi Michael et all,
> 
> I'm digging in the reason for a failure when building ethtool with
> buildroot [1].
> 
> I see the following error:
> ---
> data/buildroot/buildroot-test/instance-0/output/host/bin/i686-linux-gcc
> -DHAVE_CONFIG_H -I.  -I./uapi  -D_LARGEFILE_SOURCE
> -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -D_LARGEFILE_SOURCE
> -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -Os   -static -c -o
> netlink/desc-rtnl.o netlink/desc-rtnl.c
> In file included from ./uapi/linux/ethtool_netlink.h:12,
>                  from netlink/desc-ethtool.c:7:
> ./uapi/linux/ethtool.h:1294:19: warning: implicit declaration of
> function '__KERNEL_DIV_ROUND_UP' [-Wimplicit-function-declaration]
>   __u32 queue_mask[__KERNEL_DIV_ROUND_UP(MAX_NUM_QUEUE, 32)];
>                    ^~~~~~~~~~~~~~~~~~~~~
> ./uapi/linux/ethtool.h:1294:8: error: variably modified 'queue_mask'
> at file scope
>   __u32 queue_mask[__KERNEL_DIV_ROUND_UP(MAX_NUM_QUEUE, 32)];
>         ^~~~~~~~~~
> ---

Thank you for the report. This is fixed by first part of this patch:

  https://patchwork.ozlabs.org/project/netdev/patch/bb60cbfe99071fca4b0ea9e62d67a2341d8dd652.1590707335.git.mkubecek@suse.cz/

I'm going to apply it (with the rest of the series) this weekend.

> The problems seems to be injected by the "warning: implicit
> declaration of function".
> 
> When I move the __KERNEL_DIV_ROUND_UP macro right beside usage in
> "uapi/linux/ethtool.h" the failure is gone.
> 
> ---
> diff --git a/uapi/linux/ethtool.h b/uapi/linux/ethtool.h
> index d3dcb45..6710fa0 100644
> --- a/uapi/linux/ethtool.h
> +++ b/uapi/linux/ethtool.h
> @@ -1288,6 +1288,11 @@ enum ethtool_sfeatures_retval_bits {
>   * @queue_mask: Bitmap of the queues which sub command apply to
>   * @data: A complete command structure following for each of the
> queues addressed
>   */
> +/* ethtool.h epxects __KERNEL_DIV_ROUND_UP to be defined by <linux/kernel.h> */
> +#include <linux/kernel.h>
> +#ifndef __KERNEL_DIV_ROUND_UP
> +#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
> +#endif
>  struct ethtool_per_queue_op {
>         __u32   cmd;
>         __u32   sub_command;
> ---

This would fix the warning and error too but uapi/linux/ethtool.h is
a sanitized copy of a kernel header which we import and do not apply
further changes. Moreover, there is no need to have multiple definitions
of the same macro and there is already one in internal.h.

Michal
Heiko Thiery June 6, 2020, 1:47 p.m. UTC | #2 
Hi Michal,

Am Sa., 6. Juni 2020 um 15:43 Uhr schrieb Michal Kubecek <mkubecek@suse.cz>:
>
> On Sat, Jun 06, 2020 at 03:24:22PM +0200, Heiko Thiery wrote:
> > Hi Michael et all,
> >
> > I'm digging in the reason for a failure when building ethtool with
> > buildroot [1].
> >
> > I see the following error:
> > ---
> > data/buildroot/buildroot-test/instance-0/output/host/bin/i686-linux-gcc
> > -DHAVE_CONFIG_H -I.  -I./uapi  -D_LARGEFILE_SOURCE
> > -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -D_LARGEFILE_SOURCE
> > -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -Os   -static -c -o
> > netlink/desc-rtnl.o netlink/desc-rtnl.c
> > In file included from ./uapi/linux/ethtool_netlink.h:12,
> >                  from netlink/desc-ethtool.c:7:
> > ./uapi/linux/ethtool.h:1294:19: warning: implicit declaration of
> > function '__KERNEL_DIV_ROUND_UP' [-Wimplicit-function-declaration]
> >   __u32 queue_mask[__KERNEL_DIV_ROUND_UP(MAX_NUM_QUEUE, 32)];
> >                    ^~~~~~~~~~~~~~~~~~~~~
> > ./uapi/linux/ethtool.h:1294:8: error: variably modified 'queue_mask'
> > at file scope
> >   __u32 queue_mask[__KERNEL_DIV_ROUND_UP(MAX_NUM_QUEUE, 32)];
> >         ^~~~~~~~~~
> > ---
>
> Thank you for the report. This is fixed by first part of this patch:
>
>   https://patchwork.ozlabs.org/project/netdev/patch/bb60cbfe99071fca4b0ea9e62d67a2341d8dd652.1590707335.git.mkubecek@suse.cz/
>
> I'm going to apply it (with the rest of the series) this weekend.

I will try to apply this patch and check if the failure is gone.

>
> > The problems seems to be injected by the "warning: implicit
> > declaration of function".
> >
> > When I move the __KERNEL_DIV_ROUND_UP macro right beside usage in
> > "uapi/linux/ethtool.h" the failure is gone.
> >
> > ---
> > diff --git a/uapi/linux/ethtool.h b/uapi/linux/ethtool.h
> > index d3dcb45..6710fa0 100644
> > --- a/uapi/linux/ethtool.h
> > +++ b/uapi/linux/ethtool.h
> > @@ -1288,6 +1288,11 @@ enum ethtool_sfeatures_retval_bits {
> >   * @queue_mask: Bitmap of the queues which sub command apply to
> >   * @data: A complete command structure following for each of the
> > queues addressed
> >   */
> > +/* ethtool.h epxects __KERNEL_DIV_ROUND_UP to be defined by <linux/kernel.h> */
> > +#include <linux/kernel.h>
> > +#ifndef __KERNEL_DIV_ROUND_UP
> > +#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
> > +#endif
> >  struct ethtool_per_queue_op {
> >         __u32   cmd;
> >         __u32   sub_command;
> > ---
>
> This would fix the warning and error too but uapi/linux/ethtool.h is
> a sanitized copy of a kernel header which we import and do not apply
> further changes. Moreover, there is no need to have multiple definitions
> of the same macro and there is already one in internal.h.

I saw that the definition is already done in internal.h. I just did a
quick hackaround to check if the failure is gone.

Thanks!
Heiko Thiery June 6, 2020, 1:51 p.m. UTC | #3 
Hi Michal,

Am Sa., 6. Juni 2020 um 15:47 Uhr schrieb Heiko Thiery <heiko.thiery@gmail.com>:
>
> Hi Michal,
>
> Am Sa., 6. Juni 2020 um 15:43 Uhr schrieb Michal Kubecek <mkubecek@suse.cz>:
> >
> > On Sat, Jun 06, 2020 at 03:24:22PM +0200, Heiko Thiery wrote:
> > > Hi Michael et all,
> > >
> > > I'm digging in the reason for a failure when building ethtool with
> > > buildroot [1].
> > >
> > > I see the following error:
> > > ---
> > > data/buildroot/buildroot-test/instance-0/output/host/bin/i686-linux-gcc
> > > -DHAVE_CONFIG_H -I.  -I./uapi  -D_LARGEFILE_SOURCE
> > > -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -D_LARGEFILE_SOURCE
> > > -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -Os   -static -c -o
> > > netlink/desc-rtnl.o netlink/desc-rtnl.c
> > > In file included from ./uapi/linux/ethtool_netlink.h:12,
> > >                  from netlink/desc-ethtool.c:7:
> > > ./uapi/linux/ethtool.h:1294:19: warning: implicit declaration of
> > > function '__KERNEL_DIV_ROUND_UP' [-Wimplicit-function-declaration]
> > >   __u32 queue_mask[__KERNEL_DIV_ROUND_UP(MAX_NUM_QUEUE, 32)];
> > >                    ^~~~~~~~~~~~~~~~~~~~~
> > > ./uapi/linux/ethtool.h:1294:8: error: variably modified 'queue_mask'
> > > at file scope
> > >   __u32 queue_mask[__KERNEL_DIV_ROUND_UP(MAX_NUM_QUEUE, 32)];
> > >         ^~~~~~~~~~
> > > ---
> >
> > Thank you for the report. This is fixed by first part of this patch:
> >
> >   https://patchwork.ozlabs.org/project/netdev/patch/bb60cbfe99071fca4b0ea9e62d67a2341d8dd652.1590707335.git.mkubecek@suse.cz/
> >
> > I'm going to apply it (with the rest of the series) this weekend.
>
> I will try to apply this patch and check if the failure is gone.

I can confirm with the patch the failure is gone.

Many thanks for the fast help.
diff mbox series

Patch

diff --git a/uapi/linux/ethtool.h b/uapi/linux/ethtool.h
index d3dcb45..6710fa0 100644
--- a/uapi/linux/ethtool.h
+++ b/uapi/linux/ethtool.h
@@ -1288,6 +1288,11 @@  enum ethtool_sfeatures_retval_bits {
  * @queue_mask: Bitmap of the queues which sub command apply to
  * @data: A complete command structure following for each of the
queues addressed
  */
+/* ethtool.h epxects __KERNEL_DIV_ROUND_UP to be defined by <linux/kernel.h> */
+#include <linux/kernel.h>
+#ifndef __KERNEL_DIV_ROUND_UP
+#define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
+#endif
 struct ethtool_per_queue_op {
        __u32   cmd;