"AVX2-based lookup implementation" has broken ebtables --among-src

"AVX2-based lookup implementation" has broken ebtables --among-src

[Nikita Yushchenko]

Hello Stefano.

I've found that nftables rule added by

# ebtables -A INPUT --among-src 8:0:27:40:f7:9=192.168.56.10 -j log

does not match packets on kernel 5.14 and on current mainline.
Although it matched correctly on kernel 4.18

I've bisected this issue. It was introduced by your commit 7400b063969b ("nft_set_pipapo: Introduce 
AVX2-based lookup implementation") from 5.7 development cycle.

The nftables rule created by the above command uses concatenation:

# nft list chain bridge filter INPUT
table bridge filter {
         chain INPUT {
                 type filter hook input priority filter; policy accept;
                 ether saddr . ip saddr { 08:00:27:40:f7:09 . 192.168.56.10 } counter packets 0 bytes 0 
log level notice flags ether
         }
}

Looks like the AVX2-based lookup does not process this correctly.


Nikita

Re: "AVX2-based lookup implementation" has broken ebtables --among-src

[Stefano Brivio]

[Adding netfilter-devel]

Hi Nikita,

On Tue, 16 Nov 2021 11:51:01 +0300
Nikita Yushchenko <nikita.yushchenko@virtuozzo.com> wrote:

> Hello Stefano.
> 
> I've found that nftables rule added by
> 
> # ebtables -A INPUT --among-src 8:0:27:40:f7:9=192.168.56.10 -j log
> 
> does not match packets on kernel 5.14 and on current mainline.
> Although it matched correctly on kernel 4.18
> 
> I've bisected this issue. It was introduced by your commit 7400b063969b ("nft_set_pipapo: Introduce 
> AVX2-based lookup implementation") from 5.7 development cycle.
> 
> The nftables rule created by the above command uses concatenation:
> 
> # nft list chain bridge filter INPUT
> table bridge filter {
>          chain INPUT {
>                  type filter hook input priority filter; policy accept;
>                  ether saddr . ip saddr { 08:00:27:40:f7:09 . 192.168.56.10 } counter packets 0 bytes 0 
> log level notice flags ether
>          }
> }
> 
> Looks like the AVX2-based lookup does not process this correctly.

Thanks for bisecting and reporting this! I'm looking into it now, I
might be a bit slow as I'm currently traveling.

If you need a quick workaround, by the way, defining a "ether . ip"
set without the 'interval' flag and using a reference to it from the
nft rule will cause a switch to the nft_hash back-end (which presumably
doesn't have the same issue), see also:

	https://wiki.nftables.org/wiki-nftables/index.php/Portal:DeveloperDocs/set_internals

-- 
Stefano

Re: "AVX2-based lookup implementation" has broken ebtables --among-src

[Florian Westphal]

Stefano Brivio <sbrivio@redhat.com> wrote:
> [Adding netfilter-devel]
> 
> Hi Nikita,
> 
> On Tue, 16 Nov 2021 11:51:01 +0300
> Nikita Yushchenko <nikita.yushchenko@virtuozzo.com> wrote:
> 
> > Hello Stefano.
> > 
> > I've found that nftables rule added by
> > 
> > # ebtables -A INPUT --among-src 8:0:27:40:f7:9=192.168.56.10 -j log
> > 
> > does not match packets on kernel 5.14 and on current mainline.
> > Although it matched correctly on kernel 4.18
> > 
> > I've bisected this issue. It was introduced by your commit 7400b063969b ("nft_set_pipapo: Introduce 
> > AVX2-based lookup implementation") from 5.7 development cycle.
> > 
> > The nftables rule created by the above command uses concatenation:
> > 
> > # nft list chain bridge filter INPUT
> > table bridge filter {
> >          chain INPUT {
> >                  type filter hook input priority filter; policy accept;
> >                  ether saddr . ip saddr { 08:00:27:40:f7:09 . 192.168.56.10 } counter packets 0 bytes 0 
> > log level notice flags ether
> >          }
> > }
> > 
> > Looks like the AVX2-based lookup does not process this correctly.
> 
> Thanks for bisecting and reporting this! I'm looking into it now, I
> might be a bit slow as I'm currently traveling.

Might be a bug in ebtables.  This is what nft monitor shows:

add chain bridge filter INPUT { type filter hook input priority filter;
	policy accept; }
	add rule bridge filter INPUT ether saddr . ip saddr {
08:00:27:40:f7:09 .
	   192.168.56.10-0x1297286e2b2 [..]

I can have a look at ebtables-nft side.

Re: "AVX2-based lookup implementation" has broken ebtables --among-src

[Nikita Yushchenko]

>>> Looks like the AVX2-based lookup does not process this correctly.
>>
>> Thanks for bisecting and reporting this! I'm looking into it now, I
>> might be a bit slow as I'm currently traveling.
> 
> Might be a bug in ebtables....

Exactly same ebtables binary (and exactly same rule) works with kernel 4.18 and all kernels up to the 
mentioned patch applied.


Nikita

Re: "AVX2-based lookup implementation" has broken ebtables --among-src

[Stefano Brivio]

On Wed, 17 Nov 2021 15:08:54 +0300
Nikita Yushchenko <nikita.yushchenko@virtuozzo.com> wrote:

> >>> Looks like the AVX2-based lookup does not process this correctly.
> >>
> >> Thanks for bisecting and reporting this! I'm looking into it now, I
> >> might be a bit slow as I'm currently traveling.  
> > 
> > Might be a bug in ebtables....  
> 
> Exactly same ebtables binary (and exactly same rule) works with
> kernel 4.18 and all kernels up to the mentioned patch applied.

Sorry for the delay, I've been offline the past days, I'll restart
looking into this now.

-- 
Stefano

Re: "AVX2-based lookup implementation" has broken ebtables --among-src

[Stefano Brivio]

On Mon, 22 Nov 2021 14:29:33 +0100
Stefano Brivio <sbrivio@redhat.com> wrote:

> On Wed, 17 Nov 2021 15:08:54 +0300
> Nikita Yushchenko <nikita.yushchenko@virtuozzo.com> wrote:
> 
> > >>> Looks like the AVX2-based lookup does not process this correctly.  
> > >>
> > >> Thanks for bisecting and reporting this! I'm looking into it now, I
> > >> might be a bit slow as I'm currently traveling.    
> > > 
> > > Might be a bug in ebtables....    
> > 
> > Exactly same ebtables binary (and exactly same rule) works with
> > kernel 4.18 and all kernels up to the mentioned patch applied.  
> 
> Sorry for the delay, I've been offline the past days, I'll restart
> looking into this now.

I'm still debugging this but, if it helps, I found another workaround
while checking: swapping the order of IP address and MAC address
"fixes" it -- unfortunately I didn't think of this while writing the
selftests, so that's what nft_concat_range.sh checks, a set with type
"net, mac", and not "mac, net". E.g.:

table ip t {
	set s {
		type ipv4_addr . ether_addr
		flags interval
		elements = { 192.168.122.1 . 52:54:00:04:9e:00 }
	}

	chain c {
		type filter hook input priority filter; policy accept;
		ip saddr . ether saddr @s counter packets 19 bytes 1284
	}
}

...of course this is due to an implementation detail (and the bug I'm
chasing), functionally it's expected to be the same.

-- 
Stefano

Re: "AVX2-based lookup implementation" has broken ebtables --among-src

[Florian Westphal]

Florian Westphal <fw@strlen.de> wrote:
> Might be a bug in ebtables.  This is what nft monitor shows:
> 
> add chain bridge filter INPUT { type filter hook input priority filter;
> 	policy accept; }
> 	add rule bridge filter INPUT ether saddr . ip saddr {
> 08:00:27:40:f7:09 .
> 	   192.168.56.10-0x1297286e2b2 [..]

nft monitor calls interval_map_decompose() even though it should not
in this case.  After fix this shows:

add rule bridge filter INPUT ether saddr . ip saddr { 08:00:27:40:f7:09 . 192.168.56.10, 08:00:27:40:f7:09 . 192.168.56.10 } counter

... instead, which looks correct (even though
the concat range is technically not required in this case).