Re: [patch net-next v6_repost 2/3] net: core: add SW stats to if_stats_msg

[Roopa Prabhu <roopa@cumulusnetworks.com>]

On 8/19/16, 6:17 AM, Jiri Pirko wrote:
> From: Nogah Frankel <nogahf@mellanox.com>
>
> Add a nested attribute of SW stats to if_stats_msg
> under IFLA_STATS_LINK_SW_64.
>
> Signed-off-by: Nogah Frankel <nogahf@mellanox.com>
> Reviewed-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  include/uapi/linux/if_link.h |  1 +
>  net/core/rtnetlink.c         | 21 +++++++++++++++++++++
>  2 files changed, 22 insertions(+)
>
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index a1b5202..1c9b808 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -825,6 +825,7 @@ enum {
>  	IFLA_STATS_LINK_64,
>  	IFLA_STATS_LINK_XSTATS,
>  	IFLA_STATS_LINK_XSTATS_SLAVE,
> +	IFLA_STATS_LINK_SW_64,
>  
hate to sound like a broken record here...

sorry, but like I have been saying throughout this patch series,
i don't think I can ack a new attribute for so called 'software stats' at the
same level as existing IFLA_STATS_LINK_64. It just adds ambiguity for existing stats and
confusion for future stats.

Today's IFLA_STATS64 or IFLA_STATS_LINK_64 provides aggregate stats
and historically its been 'HW only' or 'SW only' or 'HW + SW'. It depends on the driver. logical devices
provide pure 'SW only' stats here. There is no real reason to qualify them now. The user/app
has only cared about and will continue to care about only aggregate stats. That requirement
has never changed.

Everything else is breakdown for debug-ability.., hence should be in this second bucket I
have been talking about (these are traditionally available via ethtool today).

I am not arguing against the value of the stats this patch series provides. But, since the beginning
of the stats api, I have always talked about a nested extensible attribute for drivers who wish to
break their stats down. so, I have only been requesting to put this so called 'software stats' attribute
in a new nested attribute which can be extended for other such specially qualified stats.

And, I thought we had agreed on such an attribute. I mention such a nested attribute
here again on your previous post: http://marc.info/?l=linux-netdev&m=147085641703885&w=2

Thanks,
Roopa