Re: bug report: MAC src + protocol optiomization failing with 802.1Q frames

[Florian Westphal <fw@strlen.de>]

Antoine C. <acalando@free.fr> wrote:
> Following the mails I sent on the user mailing list, it seems that
> there is a bug occurring with the first rule below (the second is
> fine):
> 
> # nft list table netdev t
> table netdev t {
>         chain c {
>               ether saddr aa:bb:cc:dd:00:38 ip saddr 192.168.140.56 \
> log prefix "--tests 1&2 --"
>               ip saddr 192.168.140.56 ether saddr aa:bb:cc:dd:00:38 \
> log prefix "--tests 2&1 --"
>         }
> }
> 
> It is translated this way:
> netdev t c
>   [ meta load iiftype => reg 1 ]
>   [ cmp eq reg 1 0x00000001 ]
>   [ payload load 8b @ link header + 6 => reg 1 ]
>   [ cmp eq reg 1 0xddccbbaa 0x00083800 ]
>   [ payload load 4b @ network header + 12 => reg 1 ]
>   [ cmp eq reg 1 0x388ca8c0 ]
>   [ log prefix --tests 1&2 -- ]
> 
> The MAC source and the protocol are loaded at the same time
> then checked... but with an 802.1Q packet, it is actually 
> wrong: the ethertype will be 0x8100 and the protocol (here 
> IPv4, 0x0800), will be 4 bytes further. And it that case,
> the second test above will succeed because the protocol 
> is loaded independently.
> 
> I just tested with latest versions of libmnl/libnftnl/nft 
> and I get the same behavior.

The question is what these rules should actually match, there
are no consistent semantics in nftables for bridge and
netdev families: The existing behaviour is undefined resp.
random.

Should "ip saddr 1.2.3.4" match:

Only in classic ethernet case?
In VLAN?
In QinQ?

What about IP packet in a PPPOE frame?
What about other L2 protocols?

Pablo, I can't come up with any good answer for this; I think
an explicit dissector expression is needed to populate l3 and l4
information into nft_pktinfo structure for bridge/netdev families so
"ip saddr" would only ever match plain ethernet (no vlans, no pppoe).

This also means the existing skb->protocol based dependencies
need to die resp. check for offloaded vlan headers.

Whats your take?

This is also related to Eric Woudstras work to add qinq+pppoe
automatching.