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] [day] [month] [year] [list]

Date:   Tue, 23 Aug 2016 19:46:43 -0700
From:   Roopa Prabhu <roopa@...ulusnetworks.com>
To:     Jiri Pirko <jiri@...nulli.us>
CC:     David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
        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/23/16, 7:52 AM, Jiri Pirko wrote:
> Tue, Aug 23, 2016 at 04:46:37PM CEST, roopa@...ulusnetworks.com wrote:
>> On 8/23/16, 12:26 AM, Jiri Pirko wrote:
>>> Tue, Aug 23, 2016 at 09:04:15AM CEST, davem@...emloft.net wrote:
>>>> From: Jiri Pirko <jiri@...nulli.us>
>>>> Date: Tue, 23 Aug 2016 08:53:18 +0200
>>>>
>>>>> Anyway I think that next level of nesting is not necessary. On
>>>>> contrary, it is wrong. The current level is extensible, mixed and
>>>>> flagged already. I don't see any reason why not to add whatever kind of
>>>>> stats here. What makes IFLA_STATS_LINK_SW_64 or for example
>>>>> IFLA_STATS_LINK_HW_ACL so special it has to be nested in some other
>>>>> attr? I would understand it it would be values of one family, but that
>>>>> is not the case.
>>>> First, I agree with Roopa.  If we want to put this stuff out
>>>> there is should be bucketed together in a nested attribute with
>>>> other similar stats specifications.
>>> Well I still don't think that IFLA_STATS_LINK_SW_64 and
>>> IFLA_STATS_LINK_HW_ACL are related. You cannot put it under *DRIVER*
>>> nest as IFLA_STATS_LINK_SW_64 are core stats.
>> not sure i understand, why is this core stats ?.
>> should a new logical device implement  IFLA_STATS_LINK_64 or IFLA_STATS_LINK_SW_64 ?
>> any other users ?.
>>
>>
>>>  So we can put them under
>>> *MISC* nest attr. But that is exactly purpose of the top-level here.
>>> /me confused
>> By design top level is for higher level grouping of stats (that also helps us maintain a lean higher
>> level filter space). They are mainly categories of stats. for example we have a nested link
>> XSTATS attribute..which are again a break down of stats already counted in IFLA_STATS_LINK_64.
>> That's why I think we can group this into another kind of breakdown stats.
> I give up. What name do you suggest for the nested attribute?

how about the below ?
IFLA_STATS_LINK_OFFLOAD_XSTATS [
        IFLA_OFFLOAD_XSTATS_SW_HIT or IFLA_OFFLOAD_XSTATS_CPU_HIT or whatever you want to call it ?
]

reasons: we need an offload driver stats bucket anyways (for the hw driver extended stats.
similar to ethtool stats). keeping the name generic will help cover switchdev and other hw offload uses.
If there is a need for a second level filter in the future, we can always
add a IFLA_OFFLOAD_XSTATS_FILTER attribute when that becomes necessary.

I agree with dave that this is essentially a filter on the existing stats and can be implemented as
a flag someplace. But, the best way I can see that filter implemented in the current code/api
is with a nested attribute like above.

Thanks Jiri!