On Tue, Sep 26, 2023 at 05:49:10PM +0200, Phil Sutter wrote:
> On Tue, Sep 26, 2023 at 05:25:00PM +0200, Pablo Neira Ayuso wrote:
> > This fix is similar to 22d201010919 ("netlink_linearize: skip set element
> > expression in set statement key") to fix map statement.
> >
> > netlink_gen_map_stmt() relies on the map key, that is expressed as a set
> > element. Use the set element key instead to skip the set element wrap,
> > otherwise get_register() abort execution:
> >
> > nft: netlink_linearize.c:650: netlink_gen_expr: Assertion `dreg < ctx->reg_low' failed.
> >
> > Reported-by: Luci Stanescu <luci@xxxxxxx>
> > Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
>
> This patch adds map stmt printing and parsing support to JSON, but not a
> word about that in the commit message. Did this slip in by accident
> (e.g. 'git commit -a')? Anyway, I think it should go into a separate
> patch.
Not accidental.
I just added complete JSON, it has been time consuming, I initially
thought about fixing this issue only and leave JSON alone unfixed, but
looking at recent stuff, that will backfire sooner or later.
I took the existing set statement support and extend it to have a
"data" field to store the data mapping.
> [...]
> > diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
> > index 53a318aa2e62..99ed9f387a81 100644
> > --- a/src/netlink_linearize.c
> > +++ b/src/netlink_linearize.c
> > @@ -1585,13 +1585,13 @@ static void netlink_gen_map_stmt(struct netlink_linearize_ctx *ctx,
> > int num_stmts = 0;
> > struct stmt *this;
> >
> > - sreg_key = get_register(ctx, stmt->map.key);
> > - netlink_gen_expr(ctx, stmt->map.key, sreg_key);
> > + sreg_key = get_register(ctx, stmt->set.key->key);
> > + netlink_gen_expr(ctx, stmt->set.key->key, sreg_key);
> >
> > sreg_data = get_register(ctx, stmt->map.data);
> > netlink_gen_expr(ctx, stmt->map.data, sreg_data);
> >
> > - release_register(ctx, stmt->map.key);
> > + release_register(ctx, stmt->set.key->key);
> > release_register(ctx, stmt->map.data);
> >
> > nle = alloc_nft_expr("dynset");
>
> Any particular reason why this doesn't just use stmt->map.key->key? The
It should use map.key->key, yes.
> first two fields in structs set_stmt and map_stmt are identical, so the
> above works "by accident".
Exactly.
I will fix and send a v2.
