Re: linux-next: Tree for Jul 15 (HEADERS_TEST w/ netfilter tables offload)

Re: linux-next: Tree for Jul 15 (HEADERS_TEST w/ netfilter tables offload)

[Laura Garcia]

CC'ing netfilter.

On Mon, Jul 15, 2019 at 6:45 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 7/14/19 9:48 PM, Stephen Rothwell wrote:
> > Hi all,
> >
> > Please do not add v5.4 material to your linux-next included branches
> > until after v5.3-rc1 has been released.
> >
> > Changes since 20190712:
> >
>
> Hi,
>
> I am seeing these build errors from HEADERS_TEST (or KERNEL_HEADERS_TEST)
> for include/net/netfilter/nf_tables_offload.h.s:
>
>   CC      include/net/netfilter/nf_tables_offload.h.s
> In file included from ./../include/net/netfilter/nf_tables_offload.h:5:0,
>                  from <command-line>:0:
> ../include/net/netfilter/nf_tables.h: In function ‘nft_gencursor_next’:
> ../include/net/netfilter/nf_tables.h:1223:14: error: ‘const struct net’ has no member named ‘nft’; did you mean ‘nf’?
>   return net->nft.gencursor + 1 == 1 ? 1 : 0;
>               ^~~
>               nf
> In file included from ../include/linux/kernel.h:11:0,
>                  from ../include/net/flow_offload.h:4,
>                  from ./../include/net/netfilter/nf_tables_offload.h:4,
>                  from <command-line>:0:
> ../include/net/netfilter/nf_tables.h: In function ‘nft_genmask_cur’:
> ../include/net/netfilter/nf_tables.h:1234:29: error: ‘const struct net’ has no member named ‘nft’; did you mean ‘nf’?
>   return 1 << READ_ONCE(net->nft.gencursor);
>                              ^
> ../include/linux/compiler.h:261:17: note: in definition of macro ‘__READ_ONCE’
>   union { typeof(x) __val; char __c[1]; } __u;   \
>                  ^
> ../include/net/netfilter/nf_tables.h:1234:14: note: in expansion of macro ‘READ_ONCE’
>   return 1 << READ_ONCE(net->nft.gencursor);
>               ^~~~~~~~~
> ../include/net/netfilter/nf_tables.h:1234:29: error: ‘const struct net’ has no member named ‘nft’; did you mean ‘nf’?
>   return 1 << READ_ONCE(net->nft.gencursor);
>                              ^
> ../include/linux/compiler.h:263:22: note: in definition of macro ‘__READ_ONCE’
>    __read_once_size(&(x), __u.__c, sizeof(x));  \
>                       ^
> ../include/net/netfilter/nf_tables.h:1234:14: note: in expansion of macro ‘READ_ONCE’
>   return 1 << READ_ONCE(net->nft.gencursor);
>               ^~~~~~~~~
> ../include/net/netfilter/nf_tables.h:1234:29: error: ‘const struct net’ has no member named ‘nft’; did you mean ‘nf’?
>   return 1 << READ_ONCE(net->nft.gencursor);
>                              ^
> ../include/linux/compiler.h:263:42: note: in definition of macro ‘__READ_ONCE’
>    __read_once_size(&(x), __u.__c, sizeof(x));  \
>                                           ^
> ../include/net/netfilter/nf_tables.h:1234:14: note: in expansion of macro ‘READ_ONCE’
>   return 1 << READ_ONCE(net->nft.gencursor);
>               ^~~~~~~~~
> ../include/net/netfilter/nf_tables.h:1234:29: error: ‘const struct net’ has no member named ‘nft’; did you mean ‘nf’?
>   return 1 << READ_ONCE(net->nft.gencursor);
>                              ^
> ../include/linux/compiler.h:265:30: note: in definition of macro ‘__READ_ONCE’
>    __read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \
>                               ^
> ../include/net/netfilter/nf_tables.h:1234:14: note: in expansion of macro ‘READ_ONCE’
>   return 1 << READ_ONCE(net->nft.gencursor);
>               ^~~~~~~~~
> ../include/net/netfilter/nf_tables.h:1234:29: error: ‘const struct net’ has no member named ‘nft’; did you mean ‘nf’?
>   return 1 << READ_ONCE(net->nft.gencursor);
>                              ^
> ../include/linux/compiler.h:265:50: note: in definition of macro ‘__READ_ONCE’
>    __read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \
>                                                   ^
> ../include/net/netfilter/nf_tables.h:1234:14: note: in expansion of macro ‘READ_ONCE’
>   return 1 << READ_ONCE(net->nft.gencursor);
>               ^~~~~~~~~
> make[2]: *** [../scripts/Makefile.build:304: include/net/netfilter/nf_tables_offload.h.s] Error 1
>
>
> Should this header file not be tested?
>
> thanks.
> --
> ~Randy

Re: linux-next: Tree for Jul 15 (HEADERS_TEST w/ netfilter tables offload)

[Pablo Neira Ayuso]

On Mon, Jul 15, 2019 at 07:28:04PM +0200, Laura Garcia wrote:
> CC'ing netfilter.
> 
> On Mon, Jul 15, 2019 at 6:45 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> >
> > On 7/14/19 9:48 PM, Stephen Rothwell wrote:
> > > Hi all,
> > >
> > > Please do not add v5.4 material to your linux-next included branches
> > > until after v5.3-rc1 has been released.
> > >
> > > Changes since 20190712:
> > >
> >
> > Hi,
> >
> > I am seeing these build errors from HEADERS_TEST (or KERNEL_HEADERS_TEST)
> > for include/net/netfilter/nf_tables_offload.h.s:
> >
> >   CC      include/net/netfilter/nf_tables_offload.h.s
[...]
> > Should this header file not be tested?

Yes, it should indeed be added.

Re: linux-next: Tree for Jul 15 (HEADERS_TEST w/ netfilter tables offload)

[Masahiro Yamada]

On Tue, Jul 16, 2019 at 2:33 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> On Mon, Jul 15, 2019 at 07:28:04PM +0200, Laura Garcia wrote:
> > CC'ing netfilter.
> >
> > On Mon, Jul 15, 2019 at 6:45 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> > >
> > > On 7/14/19 9:48 PM, Stephen Rothwell wrote:
> > > > Hi all,
> > > >
> > > > Please do not add v5.4 material to your linux-next included branches
> > > > until after v5.3-rc1 has been released.
> > > >
> > > > Changes since 20190712:
> > > >
> > >
> > > Hi,
> > >
> > > I am seeing these build errors from HEADERS_TEST (or KERNEL_HEADERS_TEST)
> > > for include/net/netfilter/nf_tables_offload.h.s:
> > >
> > >   CC      include/net/netfilter/nf_tables_offload.h.s
> [...]
> > > Should this header file not be tested?

This means you must endlessly exclude
headers that include nf_tables.h


> Yes, it should indeed be added.

Adding 'header-test-' is the last resort.


I had already queued a patch:

git clone -b build-test
git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git

commit 9dae5c5fc798e0e970cca4cd1b224dece3ad4766
Author: Masahiro Yamada <yamada.masahiro@socionext.com>
Date:   Mon Jul 15 00:42:56 2019 +0900

    netfilter: nf_tables: split local helpers out to nft_internals.h



If it is OK, I will send it out.



-- 
Best Regards
Masahiro Yamada

Re: linux-next: Tree for Jul 15 (HEADERS_TEST w/ netfilter tables offload)

[Pablo Neira Ayuso]

On Tue, Jul 16, 2019 at 02:56:09AM +0900, Masahiro Yamada wrote:
> On Tue, Jul 16, 2019 at 2:33 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >
> > On Mon, Jul 15, 2019 at 07:28:04PM +0200, Laura Garcia wrote:
> > > CC'ing netfilter.
> > >
> > > On Mon, Jul 15, 2019 at 6:45 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> > > >
> > > > On 7/14/19 9:48 PM, Stephen Rothwell wrote:
> > > > > Hi all,
> > > > >
> > > > > Please do not add v5.4 material to your linux-next included branches
> > > > > until after v5.3-rc1 has been released.
> > > > >
> > > > > Changes since 20190712:
> > > > >
> > > >
> > > > Hi,
> > > >
> > > > I am seeing these build errors from HEADERS_TEST (or KERNEL_HEADERS_TEST)
> > > > for include/net/netfilter/nf_tables_offload.h.s:
> > > >
> > > >   CC      include/net/netfilter/nf_tables_offload.h.s
> > [...]
> > > > Should this header file not be tested?
> 
> This means you must endlessly exclude
> headers that include nf_tables.h
> 
> 
> > Yes, it should indeed be added.
> 
> Adding 'header-test-' is the last resort.

OK, so policy now is that all internal headers should compile
standalone, right?

I can have a look and make it compile standalone.

Re: linux-next: Tree for Jul 15 (HEADERS_TEST w/ netfilter tables offload)

[Masahiro Yamada]

On Tue, Jul 16, 2019 at 3:09 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> On Tue, Jul 16, 2019 at 02:56:09AM +0900, Masahiro Yamada wrote:
> > On Tue, Jul 16, 2019 at 2:33 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > >
> > > On Mon, Jul 15, 2019 at 07:28:04PM +0200, Laura Garcia wrote:
> > > > CC'ing netfilter.
> > > >
> > > > On Mon, Jul 15, 2019 at 6:45 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> > > > >
> > > > > On 7/14/19 9:48 PM, Stephen Rothwell wrote:
> > > > > > Hi all,
> > > > > >
> > > > > > Please do not add v5.4 material to your linux-next included branches
> > > > > > until after v5.3-rc1 has been released.
> > > > > >
> > > > > > Changes since 20190712:
> > > > > >
> > > > >
> > > > > Hi,
> > > > >
> > > > > I am seeing these build errors from HEADERS_TEST (or KERNEL_HEADERS_TEST)
> > > > > for include/net/netfilter/nf_tables_offload.h.s:
> > > > >
> > > > >   CC      include/net/netfilter/nf_tables_offload.h.s
> > > [...]
> > > > > Should this header file not be tested?
> >
> > This means you must endlessly exclude
> > headers that include nf_tables.h
> >
> >
> > > Yes, it should indeed be added.
> >
> > Adding 'header-test-' is the last resort.
>
> OK, so policy now is that all internal headers should compile
> standalone, right?

I would not say that.
I just want to put as much code as possible into the test-coverage.

If there is a good reason to opt out of the header-test, that is OK.
We should take a look at the cause of the error
before blindly adding it into the blacklist.


For this particular case, I just thought some functions
could be localized in net/netfilter/, and would be cleaner.

Having said that, I am not familiar enough with
the netfilter subsystem.
So, this should be reviewed by the experts in the area.


Anyway, CONFIG_NF_TABLES seems mandatory to compile
include/net/netfilter/nf_tables_*.h

So, I will queue the following patch
to suppress the error for now.

diff --git a/include/Kbuild b/include/Kbuild
index 7e9f1acb9dd5..e59605243bca 100644
--- a/include/Kbuild
+++ b/include/Kbuild
@@ -905,10 +905,11 @@ header-test-                      +=
net/netfilter/nf_nat_redirect.h
 header-test-                   += net/netfilter/nf_queue.h
 header-test-                   += net/netfilter/nf_reject.h
 header-test-                   += net/netfilter/nf_synproxy.h
-header-test-                   += net/netfilter/nf_tables.h
-header-test-                   += net/netfilter/nf_tables_core.h
-header-test-                   += net/netfilter/nf_tables_ipv4.h
+header-test-$(CONFIG_NF_TABLES)        += net/netfilter/nf_tables.h
+header-test-$(CONFIG_NF_TABLES)        += net/netfilter/nf_tables_core.h
+header-test-$(CONFIG_NF_TABLES)        += net/netfilter/nf_tables_ipv4.h
 header-test-                   += net/netfilter/nf_tables_ipv6.h
+header-test-$(CONFIG_NF_TABLES)        += net/netfilter/nf_tables_offload.h
 header-test-                   += net/netfilter/nft_fib.h
 header-test-                   += net/netfilter/nft_meta.h
 header-test-                   += net/netfilter/nft_reject.h



This test just landed in upstream,
and will take some time to iron out the issues.

If I am disturbing people too much,
I perhaps need to loosen the policy.
Sorry if this test is too annoying.


Thanks.


--
Best Regards
Masahiro Yamada