On Mon, Jul 15, 2019 at 06:59:01PM +0200, michael-dev@xxxxxxxxxxxxx wrote:
> From: "M. Braun" <michael-dev@xxxxxxxxxxxxx>
>
> Given the following bridge rules:
> 1. ip protocol icmp accept
> 2. ether type vlan vlan type ip ip protocol icmp accept
No testcase for #2?
> The are currently both dumped by "nft list ruleset" as
> 1. ip protocol icmp accept
> 2. ip protocol icmp accept
>
> Though, the netlink code actually is different
Good catch.
> bridge filter FORWARD 4
> [ payload load 2b @ link header + 12 => reg 1 ]
> [ cmp eq reg 1 0x00000008 ]
> [ payload load 1b @ network header + 9 => reg 1 ]
> [ cmp eq reg 1 0x00000001 ]
> [ immediate reg 0 accept ]
>
> bridge filter FORWARD 5 4
> [ payload load 2b @ link header + 12 => reg 1 ]
> [ cmp eq reg 1 0x00000081 ]
> [ payload load 2b @ link header + 16 => reg 1 ]
> [ cmp eq reg 1 0x00000008 ]
> [ payload load 1b @ network header + 9 => reg 1 ]
> [ cmp eq reg 1 0x00000001 ]
> [ immediate reg 0 accept ]
>
> Fix this by avoiding the removal of all vlan statements
> in the given example.
>
> Signed-off-by: Michael Braun <michael-dev@xxxxxxxxxxxxx>
> ---
> src/payload.c | 12 ++++++++++++
> tests/py/bridge/vlan.t | 2 ++
> tests/py/bridge/vlan.t.payload | 10 ++++++++++
> tests/py/bridge/vlan.t.payload.netdev | 12 ++++++++++++
> 4 files changed, 36 insertions(+)
>
> diff --git a/src/payload.c b/src/payload.c
> index 3bf1ecc..905422a 100644
> --- a/src/payload.c
> +++ b/src/payload.c
> @@ -506,6 +506,18 @@ static bool payload_may_dependency_kill(struct payload_dep_ctx *ctx,
> dep->left->payload.desc == &proto_ip6) &&
> expr->payload.base == PROTO_BASE_TRANSPORT_HDR)
> return false;
> + /* Do not kill
> + * ether type vlan and vlan type ip and ip protocol icmp
> + * into
> + * ip protocol icmp
So, what happens here is that:
#1 vlan type ip kills ether type vlan
#2 ip protocol icmp kills vlan type ip
right?
> + * as this lacks ether type vlan.
> + * More generally speaking, do not kill protocol type
> + * for stacked protocols if we only have protcol type matches.
s/protcol/protocol
> + */
> + if (dep->left->etype == EXPR_PAYLOAD && dep->op == OP_EQ &&
> + expr->flags & EXPR_F_PROTOCOL &&
> + expr->payload.base == dep->left->payload.base)
If the current expression is a key (EXPR_F_PROTOCOL expressions tells
us what it comes in the upper layer) and base of such expression is
the same as the dependency.
I'd prefer this rule is restricted to vlan, and wait for more similar
usecases before this rule can be generalized.
OK?
Thanks.
