Re: [nf-next PATCH v2 1/2] netfilter: xt_recent: Reduce size of struct recent_entry::nstamps

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

Hi Phil,

On Fri, Jun 14, 2024 at 05:16:40PM +0200, Phil Sutter wrote:
> There is no point in this change besides presenting its possibility
> separate from a follow-up patch extending the size of both 'index' and
> 'nstamps' fields.
> 
> The value of 'nstamps' is initialized to 1 in recent_entry_init() and
> adjusted in recent_entry_update() to match that of 'index' if it becomes
> larger after being incremented. Since 'index' is of type u8, it will at
> max become 255 (and wrap to 0 afterwards). Therefore, 'nstamps' will
> also never exceed the value 255.

Series LGTM.

I'd suggest you collapse these two patches while keeping the
description above, because nstamps is shrinked here in 1/2 then it
gets back to original u16 in 2/2.

Maybe something like:

The value of 'nstamps' is initialized to 1 in recent_entry_init() and
adjusted in recent_entry_update() to match that of 'index' if it becomes
larger after being incremented. Since 'index' is of type u8, it will at
max become 255 (and wrap to 0 afterwards). Therefore, 'nstamps' will
also never exceed the value 255. But this patch expands 'index' to
u16 and then 'nstamps' needs to use u16 too, which exactly the
existing field size in the existing codebase.

I can do this mangling if you prefer to save you cycles, in such case
if you also like better wording, just let me know.

Thanks.

> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  net/netfilter/xt_recent.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/xt_recent.c b/net/netfilter/xt_recent.c
> index ef93e0d3bee0..60259280b2d5 100644
> --- a/net/netfilter/xt_recent.c
> +++ b/net/netfilter/xt_recent.c
> @@ -70,7 +70,7 @@ struct recent_entry {
>  	u_int16_t		family;
>  	u_int8_t		ttl;
>  	u_int8_t		index;
> -	u_int16_t		nstamps;
> +	u_int8_t		nstamps;
>  	unsigned long		stamps[];
>  };
>  
> -- 
> 2.43.0
> 
>