Re: [libnftnl PATCH 0/6] Attribute policies for expressions

[Phil Sutter <phil@nwl.cc>]

On Fri, Dec 15, 2023 at 10:53:44PM +0100, Phil Sutter wrote:
> This is the former RFC turned into a complete implementation including
> Florian's suggested improvements.
> 
> Patch 1 is fallout, took me a while debugging the segfaulting test case
> until I noticed it wasn't my fault! :)
> 
> Patch 2 is the same as in the RFC.
> 
> Patch 3 separates the type value checking from patch 2 and drops
> expressions' default switch cases where all possible values are handled.
> 
> Patch 4 is prep work for patch 5.
> 
> Patch 5 adds the new struct expr_ops field and defines policies for all
> expressions.
> 
> Patch 6 then enables policy checking.
> 
> Some remarks for consideration:
> 
> * This adds kernel-internal knowledge to libnftnl, namely in max name
>   lengths. Maybe not ideal, but I found it more sensible than Florian's
>   suggested alternative of using 65528 to just not exceed netlink
>   limits.
> 
> * nftnl_expr_set_u*() setters start failing when they would happily
>   overstep boundaries before. This is intentional, but getting the
>   policy values right (at first I thought 'sizeof(enum nft_registers)'
>   was a good idea) showed how hard to diagnose bugs in that area are. I
>   think we should make the setters return success/fail like
>   nftnl_expr_set_str does already, even if that breaks ABI (does it?).
>   nftables probably benefits from setter wrappers which call
>   netlink_abi_error() if the setter fails.
> 
> Phil Sutter (6):
>   tests: Fix objref test case
>   expr: Repurpose struct expr_ops::max_attr field
>   expr: Call expr_ops::set with legal types only
>   include: Sync nf_log.h with kernel headers
>   expr: Introduce struct expr_ops::attr_policy
>   expr: Enforce attr_policy compliance in nftnl_expr_set()

Series applied after checking theres no effect on nftables' py testsuite
results in different releases (v0.9.9, v1.0.6, v1.0.9).