Re: [PATCH libnftnl] expr: fix new header name printing of payload expr

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Tue, Mar 18, 2025 at 04:02:04PM +0000, Zhongqiu Duan wrote:
> The debug printing of the payload expr shows the tunnel header name as
> unknown. Since after the first version we added two new payload bases
> inner and tunnel, I prefer to make this change to meet possible future
> extensions rather than setting NFT_PAYLOAD_TUN_HEADER as the new bound.

Thanks for catching this.

> Reproduce:
> 
> nft --debug netlink add rule inet t c meta l4proto udp vxlan vni 0x123456
> 
> Before patch:
>   ...
>   [ inner type 1 hdrsize 8 flags f [ payload load 3b @ unknown header + 4 => reg 1 ] ]

I can see a tests/py is already displaying unknown.

>   ...
> 
> After patch:
>   ...
>   [ inner type 1 hdrsize 8 flags f [ payload load 3b @ tunnel header + 4 => reg 1 ] ]
>   ...
> 
> Fixes: da49c1241474 ("src: expr: use the function base2str in payload")
> Fixes: 3f3909afd76d ("expr: add inner support")
> Signed-off-by: Zhongqiu Duan <dzq.aishenghu0@gmail.com>
> ---
>  src/expr/payload.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/expr/payload.c b/src/expr/payload.c
> index c3ac0c345aec..992c353423ef 100644
> --- a/src/expr/payload.c
> +++ b/src/expr/payload.c
> @@ -207,7 +207,7 @@ static const char *base2str_array[NFT_PAYLOAD_TUN_HEADER + 1] = {
>  
>  static const char *base2str(enum nft_payload_bases base)
>  {
> -	if (base > NFT_PAYLOAD_INNER_HEADER)
> +	if (base >= array_size(base2str_array) || !base2str_array[base])

Too defensive. Should be sufficient with:

	if (base >= array_size(base2str_array))

I don't expect array to be have an empty (unset) slot.

I can mangle and apply here, no need to resend.

Thanks.

>  		return "unknown";
>  
>  	return base2str_array[base];
> -- 
> 2.43.0
> 
>