Re: [nft PATCH 3/4] netlink: Keep going after set element parsing failures

[Phil Sutter <phil@nwl.cc>]

On Wed, May 21, 2025 at 04:17:49PM +0200, Pablo Neira Ayuso wrote:
> On Wed, May 21, 2025 at 03:12:41PM +0200, Phil Sutter wrote:
> > Print an error message and try to deserialize the remaining elements
> > instead of calling BUG().
> > 
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >  src/netlink.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/netlink.c b/src/netlink.c
> > index 1222919458bae..3221d9f8ffc93 100644
> > --- a/src/netlink.c
> > +++ b/src/netlink.c
> > @@ -1475,7 +1475,9 @@ int netlink_delinearize_setelem(struct netlink_ctx *ctx,
> >  		key->byteorder = set->key->byteorder;
> >  		key->len = set->key->len;
> >  	} else {
> > -		BUG("Unexpected set element with no key\n");
> > +		netlink_io_error(ctx, NULL,
> > +			         "Unexpected set element with no key\n");
> > +		return 0;
> 
> If set element has no key, then something is very wrong. There is
> already one exception that is the catch-all element (which has no
> key).

Yes, in these cases the ruleset parser fails and the output is very
likely broken or at least incomplete. This series merely aligns error
handling: Take netlink_parse_cmp() for instance: If NFTNL_EXPR_CMP_SREG
attribute is missing or bogus, netlink_error() is called and the
function returns (void). No error status propagation happens (which we
could change easily), but most importantly the parser continues to
deserialize as much as possible.

> This is enqueuing an error record, but 0 is returned, I am not sure if
> this is ever going to be printed.

It does: Forcing the code to enter that third branch, listing a set with three
elements looks like this:

% sudo ./src/nft list ruleset
table ip t {
	set s {
		type inet_service
	}
}
netlink: Error: Unexpected set element with no key

netlink: Error: Unexpected set element with no key

netlink: Error: Unexpected set element with no key

> I am not sure this patch works.

Well, that extra newline is indeed a bug. :)

Cheers, Phil