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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <57BBE47D.3060805@cumulusnetworks.com>
Date:   Mon, 22 Aug 2016 22:51:57 -0700
From:   Roopa Prabhu <roopa@...ulusnetworks.com>
To:     Jiri Pirko <jiri@...nulli.us>
CC:     netdev@...r.kernel.org, davem@...emloft.net, nogahf@...lanox.com,
        idosch@...lanox.com, eladr@...lanox.com, yotamg@...lanox.com,
        ogerlitz@...lanox.com, nikolay@...ulusnetworks.com,
        linville@...driver.com, tgraf@...g.ch, gospo@...ulusnetworks.com,
        sfeldma@...il.com, sd@...asysnail.net, eranbe@...lanox.com,
        ast@...mgrid.com, edumazet@...gle.com, hannes@...essinduktion.org,
        f.fainelli@...il.com, dsa@...ulusnetworks.com
Subject: Re: [patch net-next v6_repost 2/3] net: core: add SW stats to if_stats_msg

On 8/19/16, 6:17 AM, Jiri Pirko wrote:
> From: Nogah Frankel <nogahf@...lanox.com>
>
> Add a nested attribute of SW stats to if_stats_msg
> under IFLA_STATS_LINK_SW_64.
>
> Signed-off-by: Nogah Frankel <nogahf@...lanox.com>
> Reviewed-by: Ido Schimmel <idosch@...lanox.com>
> Signed-off-by: Jiri Pirko <jiri@...lanox.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