Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> On Wed, Jun 19, 2019 at 07:10:10AM +0200, Florian Westphal wrote:
> > Simon Kirby <sim@xxxxxxxxxx> wrote:
> >
> > [ moving to nf-devel ]
> >
> > > I accidentally wrote "ct original" instead of "ct direction original",
> > > and this broke "nft list ruleset":
> > >
> > > # nft add set filter myset '{ type ipv4_addr; }'
> > > # nft insert rule filter input ct original ip daddr @myset
> > > # nft list ruleset
> > > nft: netlink_delinearize.c:124: netlink_parse_concat_expr: Assertion `consumed > 0' failed.
> > > Abort
> >
> > Indeed.
> >
> > This will fix the immediate problem:
> >
> > diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> > --- a/src/netlink_delinearize.c
> > +++ b/src/netlink_delinearize.c
> > @@ -329,7 +329,7 @@ static void netlink_parse_lookup(struct netlink_parse_ctx *ctx,
> > return netlink_error(ctx, loc,
> > "Lookup expression has no left hand side");
> >
> > - if (left->len < set->key->len) {
> > + if (left->len && left->len < set->key->len) {
> > expr_free(left);
> > left = netlink_parse_concat_expr(ctx, loc, sreg, set->key->len);
> > if (left == NULL)
> >
> > Pablo, the problem is that ct->key is NFT_CT_SRC, so expr->len is 0, so
> > we try to parse a concat expression. Its not until the evaluation step
> > before we will figure out from context that SRC is asking for an ipv4
> > address and update the type and expression length.
> >
> > AFAICS the plan was to stop using NFT_CT_SRC and use NFT_CT_SRC_IP(6)
> > instead so we have type and length info available directly.
> >
> > Was there a problem with it (inet family)?
>
> Right. In general, we need keys with fixed lengths, the NFT_CT_SRC
> approach where key is variable length is a problem for the set
> infrastructure, as you're describing.
>
> There's a fix for this here:
>
> https://patchwork.ozlabs.org/patch/883575/
>
> It requires kernel >= 4.17.
>
> I wanted to have netlink descriptions to deal with this case, but so
> far only probing is possible, and I would like not to open up for that
> path.
>
> I can just rebase, resubmit and merge it if you are fine with it.
I'm fine with this, it follows the same approach we use in other similar
cases, i.e. use of "ip" and "ip6" in grammar to figure out the desired
ip protocol version.
I would still push the above left->len fix, ok?
