From: Roopa Prabhu <roopa@cumulusnetworks•com>
To: Jiri Pirko <jiri@resnulli•us>
Cc: netdev@vger•kernel.org, davem@davemloft•net, nogahf@mellanox•com,
idosch@mellanox•com, eladr@mellanox•com, yotamg@mellanox•com,
ogerlitz@mellanox•com, nikolay@cumulusnetworks•com,
linville@tuxdriver•com, tgraf@suug•ch, gospo@cumulusnetworks•com,
sfeldma@gmail•com, sd@queasysnail•net, eranbe@mellanox•com,
ast@plumgrid•com, edumazet@google•com,
hannes@stressinduktion•org
Subject: Re: [patch net-next 1/4] netdevice: add SW statistics ndo
Date: Sat, 14 May 2016 08:47:41 -0700 [thread overview]
Message-ID: <5737489D.4090800@cumulusnetworks.com> (raw)
In-Reply-To: <20160514124917.GA1957@nanopsycho.orion>
On 5/14/16, 5:49 AM, Jiri Pirko wrote:
> Fri, May 13, 2016 at 08:47:48PM CEST, roopa@cumulusnetworks•com wrote:
>> On 5/12/16, 11:03 PM, Jiri Pirko wrote:
>>> Thu, May 12, 2016 at 11:10:08PM CEST, roopa@cumulusnetworks•com wrote:
>>>> On 5/12/16, 4:48 AM, Jiri Pirko wrote:
>>>>> From: Nogah Frankel <nogahf@mellanox•com>
>>>>>
>>>>> Till now we had a ndo statistics function that returned SW statistics.
>>>>> We want to change the "basic" statistics to return HW statistics if
>>>>> available.
>>>>> In this case we need to expose a new ndo to return the SW statistics.
>>>>> Add a new ndo declaration to get SW statistics
>>>>> Add a function that gets SW statistics if a competible ndo exist
>>>>>
>>>>> Signed-off-by: Nogah Frankel <nogahf@mellanox•com>
>>>>> Reviewed-by: Ido Schimmel <idosch@mellanox•com>
>>>>> Signed-off-by: Jiri Pirko <jiri@mellanox•com>
>>>>> ---
>>>>>
>>>> To me netdev stats is combined 'SW + HW' stats for that netdev.
>>>> ndo_get_stats64 callback into the drivers does the magic of adding HW stats
>>>> to SW (netdev) stats and returning (see enic_get_stats). HW stats is available for netdevs
>>>> that are offloaded or are backed by hardware. SW stats is the stats that the driver maintains
>>>> (logical or physical). HW stats is queried and added to the SW stats.
>>> I'm not sure I follow. HW stats already contain SW stats. Because on
>>> slow path every packet that is not offloaded and goes through kernel is
>>> counted into HW stats as well (because it goes through HW port).
>> yes, correct... we don't want to double count those. But since these stats are
>> generally queried from hw, I am calling them HW stats.
>> you will not really maintain a software counter for this. But, the driver can maintain its own
>> counters for rx and tx errors etc and I call these SW stats. They are counted at the driver.
>>
>>> If you
>>> do HW stats + SW stats, what you get makes no sense. Am I missing something?
>> If you go by my definition of HW and SW stats above, on a ndo_get_stats64() call,
>> you will add the SW counters + HW counters and return. In my definition, the pkts
>> that was rx'ed or tx'ed successfully are always in the HW count.
>>
>>> Btw, looking at enic_get_stats, looks exactly what we introduce for
>>> mlxsw in this patchset.
>> In enic_get_stats, the ones counted in software are the ones taken from 'enic->'
>> net_stats->rx_over_errors = enic->rq_truncated_pkts;
>> net_stats->rx_crc_errors = enic->rq_bad_fcs;
>>
>>> With this patchset, we only allow user to se the actual stats for
>>> slow-path aka SW stats.
>> hmm...ok. But i am not sure how many will use this new attribute.
>> When you do 'ip -s link show' you really want all counters on that port
>> hardware or software does not matter at that point.
>>
>> My suggestion to move this to ethtool like attribute is because that is an existing
>> way to break down your stats which ever way you want. And the best part is it can be
>> customized (say rx_pkts_cpu_saw)
> I bevieve that ethtool is really not a place to expose sw stats. Does
> not make sense.
2 things:
- i was surprised you don't want your ndo_get_stats64 to be a unified view of HW and SW stats
- by bringing up ethtool like stats (IFLA_STATS_LINK_HW_EXTENDED) I am just saying
it has always been a way to breakdown stats. If you don't want to show explicit SW stats there,
there is always a way to show HW only stats....and now you know the delta between the unified stats
and the HW only stats is your SW stats.
next prev parent reply other threads:[~2016-05-14 15:47 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-12 11:48 [patch net-next 0/4] return offloaded stats as default and expose original sw start Jiri Pirko
2016-05-12 11:48 ` [patch net-next 1/4] netdevice: add SW statistics ndo Jiri Pirko
2016-05-12 21:10 ` Roopa Prabhu
2016-05-12 22:23 ` Florian Fainelli
2016-05-13 6:06 ` Jiri Pirko
2016-05-13 6:03 ` Jiri Pirko
2016-05-13 18:47 ` Roopa Prabhu
2016-05-14 12:49 ` Jiri Pirko
2016-05-14 15:47 ` Roopa Prabhu [this message]
2016-05-14 18:46 ` Jiri Pirko
2016-05-15 4:11 ` Roopa Prabhu
2016-05-15 6:47 ` Jiri Pirko
2016-05-15 15:44 ` Andrew Lunn
2016-05-16 9:00 ` Jiri Pirko
2016-05-17 8:39 ` Thomas Graf
2016-05-17 8:41 ` Jiri Pirko
2016-05-17 14:42 ` Roopa Prabhu
2016-05-12 11:48 ` [patch net-next 2/4] rtnetlink: add HW/SW stats distinction in rtnl_fill_stats Jiri Pirko
2016-05-12 15:51 ` David Miller
2016-05-12 11:48 ` [patch net-next 3/4] net: core: add SW stats to if_stats_msg Jiri Pirko
2016-05-12 11:48 ` [patch net-next 4/4] mlxsw: spectrum: Implement SW stats ndo and expose HW stats by default Jiri Pirko
-- strict thread matches above, loose matches on Subject: below --
2016-05-12 9:59 [patch net-next 0/4] return offloaded stats as default and expose original sw start Jiri Pirko
2016-05-12 9:59 ` [patch net-next 1/4] netdevice: add SW statistics ndo Jiri Pirko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5737489D.4090800@cumulusnetworks.com \
--to=roopa@cumulusnetworks$(echo .)com \
--cc=ast@plumgrid$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=eladr@mellanox$(echo .)com \
--cc=eranbe@mellanox$(echo .)com \
--cc=gospo@cumulusnetworks$(echo .)com \
--cc=hannes@stressinduktion$(echo .)org \
--cc=idosch@mellanox$(echo .)com \
--cc=jiri@resnulli$(echo .)us \
--cc=linville@tuxdriver$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=nikolay@cumulusnetworks$(echo .)com \
--cc=nogahf@mellanox$(echo .)com \
--cc=ogerlitz@mellanox$(echo .)com \
--cc=sd@queasysnail$(echo .)net \
--cc=sfeldma@gmail$(echo .)com \
--cc=tgraf@suug$(echo .)ch \
--cc=yotamg@mellanox$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox