Re: [PATCH 2/2] ip: ipstats: Do not hide HW statistics when hw_stats_info is missing

[Petr Machata <me@pmachata.org>]

Alexander Zubkov <green@qrator.net> writes:

> ipstats_show_hw_stats() returns as soon as the
> IFLA_OFFLOAD_XSTATS_HW_S_INFO attribute is absent, silently dropping the
> counters that the message does carry. __ipstats_show_hw_stats() already
> copes with a NULL info attribute, so let it, and skip the record only
> when neither attribute is present.
>
> There is no known way to reach this through "ip stats": the l3_stats
> descriptor always requests HW_S_INFO next to the counters, and since the
> previous patch the two are reassembled even when the kernel splits them
> across messages. This is a hardening measure so that a message carrying
> just the counters is displayed rather than silently discarded.
>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Alexander Zubkov <green@qrator.net>
> ---
>  ip/ipstats.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/ip/ipstats.c b/ip/ipstats.c
> index c531885f..54b8267d 100644
> --- a/ip/ipstats.c
> +++ b/ip/ipstats.c
> @@ -472,13 +472,16 @@ static int ipstats_show_hw_stats(struct ipstats_stat_show_attrs *attrs,
>  	int err = 0;
>  
>  	at_hwsi = ipstats_stat_show_get_attr(attrs, group, hw_s_info, &err);
> -	if (at_hwsi == NULL)
> +	if (at_hwsi == NULL && err != 0)
>  		return err;

I am pretty sure this is deliberate. The logic is, if we do not even get
hw_s_info, then surely we will not get hw_stats either, because the
kernel is supposed to provide information about the statistics together
with the statistics themselves.

Now this did not take into account that messages might be split. But
with your patch, this is will not be an issue anymore either.

On second though, it is not the task of iproute2 to policy the kernel,
and when life gives you hw_stats but no hw_s_info, you make hw_stats'ade
or whatever. So I actually think the patch is OK.

So,
Reviewed-by: Petr Machata <me@pmachata.org>

>  
>  	at_stats = ipstats_stat_show_get_attr(attrs, group, hw_stats, &err);
>  	if (at_stats == NULL && err != 0)
>  		return err;
>  
> +	if (at_hwsi == NULL && at_stats == NULL)
> +		return 0;
> +
>  	return __ipstats_show_hw_stats(at_hwsi, at_stats, idx);
>  }