Re: Linux 5.17-rc5

Re: Linux 5.17-rc5

[Linus Torvalds]

On Mon, Feb 21, 2022 at 6:23 AM Woody Suwalski <wsuwalski@gmail.com> wrote:
>
> Compile failed like reported by Robert Gadson in
> https://lkml.org/lkml/2022/2/20/341
>
> As a workaround:
> nf_defrag_ipv6.patch
> --- a/net/netfilter/xt_socket.c    2022-02-21 07:29:21.938263397 -0500
> +++ b/net/netfilter/xt_socket.c    2022-02-21 07:40:16.730022272 -0500
> @@ -17,11 +17,11 @@
>   #include <net/inet_sock.h>
>   #include <net/netfilter/ipv4/nf_defrag_ipv4.h>
>
> -#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
> +//#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
>   #include <linux/netfilter_ipv6/ip6_tables.h>
...

Hmm. That may fix the compile failure, but it looks somewhat broken to me.

Other cases of nf_defrag_ipv6_disable() end up being protected by

   #if IS_ENABLED(CONFIG_NF_TABLES_IPV6)

at the use-point, not by just assuming it always exists even when
CONFIG_NF_TABLES_IPV6 is off.

So I think the proper fix is something along the lines of

  --- a/net/netfilter/xt_socket.c
  +++ b/net/netfilter/xt_socket.c
  @@ -220,8 +220,10 @@ static void socket_mt_destroy(const struct
xt_mtdtor_param *par)
   {
        if (par->family == NFPROTO_IPV4)
                nf_defrag_ipv4_disable(par->net);
  +#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
        else if (par->family == NFPROTO_IPV6)
                nf_defrag_ipv6_disable(par->net);
  +#endif
   }

instead. Entirely untested, because that's how I roll, but I suspect
the netfilter people will know what to do.

Added guilty parties and mailing list to the participants.

                Linus

Re: Linux 5.17-rc5

[Eric Dumazet]

On Mon, Feb 21, 2022 at 9:56 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Mon, Feb 21, 2022 at 6:23 AM Woody Suwalski <wsuwalski@gmail.com> wrote:
> >
> > Compile failed like reported by Robert Gadson in
> > https://lkml.org/lkml/2022/2/20/341
> >
> > As a workaround:
> > nf_defrag_ipv6.patch
> > --- a/net/netfilter/xt_socket.c    2022-02-21 07:29:21.938263397 -0500
> > +++ b/net/netfilter/xt_socket.c    2022-02-21 07:40:16.730022272 -0500
> > @@ -17,11 +17,11 @@
> >   #include <net/inet_sock.h>
> >   #include <net/netfilter/ipv4/nf_defrag_ipv4.h>
> >
> > -#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
> > +//#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
> >   #include <linux/netfilter_ipv6/ip6_tables.h>
> ...
>
> Hmm. That may fix the compile failure, but it looks somewhat broken to me.
>
> Other cases of nf_defrag_ipv6_disable() end up being protected by
>
>    #if IS_ENABLED(CONFIG_NF_TABLES_IPV6)
>
> at the use-point, not by just assuming it always exists even when
> CONFIG_NF_TABLES_IPV6 is off.
>
> So I think the proper fix is something along the lines of
>
>   --- a/net/netfilter/xt_socket.c
>   +++ b/net/netfilter/xt_socket.c
>   @@ -220,8 +220,10 @@ static void socket_mt_destroy(const struct
> xt_mtdtor_param *par)
>    {
>         if (par->family == NFPROTO_IPV4)
>                 nf_defrag_ipv4_disable(par->net);
>   +#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
>         else if (par->family == NFPROTO_IPV6)
>                 nf_defrag_ipv6_disable(par->net);
>   +#endif
>    }
>
> instead. Entirely untested, because that's how I roll, but I suspect
> the netfilter people will know what to do.
>
> Added guilty parties and mailing list to the participants.
>
>                 Linus

I am pretty sure Pablo fixed this one week ago.

https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git/commit/?id=2874b7911132f6975e668f6849c8ac93bc4e1f35

Re: Linux 5.17-rc5

[Linus Torvalds]

On Mon, Feb 21, 2022 at 10:02 AM Eric Dumazet <edumazet@google.com> wrote:
>
> I am pretty sure Pablo fixed this one week ago.

.. looks about right. Apart from the "it was never sent to me, so -rc5
ended up showing the problem" part.

             Linus

Re: Linux 5.17-rc5

[Eric Dumazet]

On Mon, Feb 21, 2022 at 10:08 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Mon, Feb 21, 2022 at 10:02 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > I am pretty sure Pablo fixed this one week ago.
>
> .. looks about right. Apart from the "it was never sent to me, so -rc5
> ended up showing the problem" part.
>

Indeed, I personally these kinds of trivial fixes should be sent right away,
especially considering two bots complained about it.

Re: Linux 5.17-rc5

[Woody Suwalski]

Linus Torvalds wrote:
> On Mon, Feb 21, 2022 at 10:02 AM Eric Dumazet <edumazet@google.com> wrote:
>> I am pretty sure Pablo fixed this one week ago.
> .. looks about right. Apart from the "it was never sent to me, so -rc5
> ended up showing the problem" part.
>
>               Linus
Patch works-for-me. Thanks...
Woody

Re: Linux 5.17-rc5

[Pablo Neira Ayuso]

On Mon, Feb 21, 2022 at 10:07:57AM -0800, Linus Torvalds wrote:
> On Mon, Feb 21, 2022 at 10:02 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > I am pretty sure Pablo fixed this one week ago.
> 
> .. looks about right. Apart from the "it was never sent to me, so -rc5
> ended up showing the problem" part.

That's my fault, I did not catch up to reach -rc5, I am sorry about that.

Re: Linux 5.17-rc5

[Pablo Neira Ayuso]

On Mon, Feb 21, 2022 at 10:21:16AM -0800, Eric Dumazet wrote:
> On Mon, Feb 21, 2022 at 10:08 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > On Mon, Feb 21, 2022 at 10:02 AM Eric Dumazet <edumazet@google.com> wrote:
> > >
> > > I am pretty sure Pablo fixed this one week ago.
> >
> > .. looks about right. Apart from the "it was never sent to me, so -rc5
> > ended up showing the problem" part.
> >
> 
> Indeed, I personally these kinds of trivial fixes should be sent right away,
> especially considering two bots complained about it.

I did not consider this so important, that was my fault.

Re: Linux 5.17-rc5

[Eric Dumazet]

On Mon, Feb 21, 2022 at 10:25 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> On Mon, Feb 21, 2022 at 10:21:16AM -0800, Eric Dumazet wrote:
> > On Mon, Feb 21, 2022 at 10:08 AM Linus Torvalds
> > <torvalds@linux-foundation.org> wrote:
> > >
> > > On Mon, Feb 21, 2022 at 10:02 AM Eric Dumazet <edumazet@google.com> wrote:
> > > >
> > > > I am pretty sure Pablo fixed this one week ago.
> > >
> > > .. looks about right. Apart from the "it was never sent to me, so -rc5
> > > ended up showing the problem" part.
> > >
> >
> > Indeed, I personally these kinds of trivial fixes should be sent right away,
> > especially considering two bots complained about it.
>
> I did not consider this so important, that was my fault.

Well, I was the one adding this compile error ;)

Testing CONFIG_IPV6=n builds is not my top priority.

Re: Linux 5.17-rc5

[Pablo Neira Ayuso]

On Mon, Feb 21, 2022 at 10:28:46AM -0800, Eric Dumazet wrote:
> On Mon, Feb 21, 2022 at 10:25 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >
> > On Mon, Feb 21, 2022 at 10:21:16AM -0800, Eric Dumazet wrote:
> > > On Mon, Feb 21, 2022 at 10:08 AM Linus Torvalds
> > > <torvalds@linux-foundation.org> wrote:
> > > >
> > > > On Mon, Feb 21, 2022 at 10:02 AM Eric Dumazet <edumazet@google.com> wrote:
> > > > >
> > > > > I am pretty sure Pablo fixed this one week ago.
> > > >
> > > > .. looks about right. Apart from the "it was never sent to me, so -rc5
> > > > ended up showing the problem" part.
> > > >
> > >
> > > Indeed, I personally these kinds of trivial fixes should be sent right away,
> > > especially considering two bots complained about it.
> >
> > I did not consider this so important, that was my fault.
> 
> Well, I was the one adding this compile error ;)

I poorly performed as a duly reviewer :)

> Testing CONFIG_IPV6=n builds is not my top priority.

For the record, this patch is now flying to the netdev tree via pull request:

https://patchwork.kernel.org/project/netdevbpf/patch/20220221161757.250801-1-pablo@netfilter.org/

Thanks.

Re: Linux 5.17-rc5

[Robert Gadsdon]

On 2/21/22 10:01, Eric Dumazet wrote:
> I am pretty sure Pablo fixed this one week ago.
> https://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf.git/commit/?id=2874b7911132f6975e668f6849c8ac93bc4e1f35

Applied this patch, and the problem is fixed, now..

Thanks..

Robert Gadsdon.
February 21st 2022.