| Message ID | 20190713210306.30815-1-michael-dev@fami-braun.de |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | David Miller |
| Headers | show |
| Series | Fix dumping vlan rules | expand |
CC'ing netfilter. On Sat, Jul 13, 2019 at 11:10 PM <michael-dev@fami-braun.de> wrote: > > From: "M. Braun" <michael-dev@fami-braun.de> > > Given the following bridge rules: > 1. ip protocol icmp accept > 2. ether type vlan vlan type ip ip protocol icmp accept > > 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 > > 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@fami-braun.de> > --- > src/payload.c | 12 ++++++++++++ > 1 file changed, 12 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 > + * as this lacks ether type vlan. > + * More generally speaking, do not kill protocol type > + * for stacked protocols if we only have protcol type matches. > + */ > + if (dep->left->etype == EXPR_PAYLOAD && dep->op == OP_EQ && > + expr->flags & EXPR_F_PROTOCOL && > + expr->payload.base == dep->left->payload.base) > + return false; > break; > } > > -- > 2.20.1 >
michael-dev@fami-braun.de <michael-dev@fami-braun.de> wrote: > From: "M. Braun" <michael-dev@fami-braun.de> > > Given the following bridge rules: > 1. ip protocol icmp accept > 2. ether type vlan vlan type ip ip protocol icmp accept > > The are currently both dumped by "nft list ruleset" as > 1. ip protocol icmp accept > 2. ip protocol icmp accept Yes, thats a bug, the dependency removal is incorrect. > +++ 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 > + * as this lacks ether type vlan. > + * More generally speaking, do not kill protocol type > + * for stacked protocols if we only have protcol type matches. > + */ > + if (dep->left->etype == EXPR_PAYLOAD && dep->op == OP_EQ && > + expr->flags & EXPR_F_PROTOCOL && > + expr->payload.base == dep->left->payload.base) > + return false; Can you please add a test case for this problem to tests/py/bridge/vlan.t so we catch this when messing with dependency handling in the future? Also, please submit v2 directly to netfilter-devel@. Thanks!
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 + * as this lacks ether type vlan. + * More generally speaking, do not kill protocol type + * for stacked protocols if we only have protcol type matches. + */ + if (dep->left->etype == EXPR_PAYLOAD && dep->op == OP_EQ && + expr->flags & EXPR_F_PROTOCOL && + expr->payload.base == dep->left->payload.base) + return false; break; }