Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-22 Thread Roopa Prabhu
On Mon, Jun 20, 2016 at 5:28 AM, Jamal Hadi Salim  wrote:
> On 16-06-19 11:14 PM, Roopa Prabhu wrote:
>>
>> On Fri, Jun 17, 2016 at 10:12 AM, Florian Fainelli 
>> wrote:
>
>
>
>>
>> I have also mentioned this before, the default api must provide
>> accumulated (hw and sw) stats...,
>> because this is the api that the user queries on an interface.
>
>
> Sorry - I missed those discussions.
> What is current practise? Do people request for one via ip link
> stats and the other via ethtool?
> What do you guys do in your implementation?

for us the standard netlink api that returns netdev stats includes all
stats hw and sw.
When i say hw and sw, I mean some of the error counters can also include errors
counted by sw.

ethtool stats has always provided drivers/users with additional stats
that the hw or driver
can expose.


> Yes, it would be more accurate to provide aggregated stats but
> it may break backward compat if expectation is both are read
> separately today.

I don't think people see netdev stats as sw and ethtool as hw stats.
The latter just provides more granularity for debugging.
Thats the way i have looked at it forever.


> Maybe it makes sense to have a brand new TLV for these aggregated
> stats as Jiri was suggesting.That means two new TLVs not one.
> 1) TLV for aggregated stats - which cant be current one
> 2) TLV for h/w stats
>
> The existing stat implies s/ware only.
>

I don't think existing stat implies s/ware stats only. so, I think we
should be careful
about changing the meaning of existing stats.

logical devices like bridge stats have always been software only...but
with switchdev
the way we see these or implement these is to also include hardware stats when
they are hw offloaded. For us bridge vlan stats, vxlan stats and so on
will follow the
same model. You cannot introduce separate sw and hw stats for these.
All stats will have to follow a consistent model.

Thanks,
Roopa


Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-20 Thread Jiri Pirko
Mon, Jun 20, 2016 at 02:28:31PM CEST, j...@mojatatu.com wrote:
>On 16-06-19 11:14 PM, Roopa Prabhu wrote:
>>On Fri, Jun 17, 2016 at 10:12 AM, Florian Fainelli  
>>wrote:
>
>
>>
>>I have also mentioned this before, the default api must provide
>>accumulated (hw and sw) stats...,
>>because this is the api that the user queries on an interface.
>
>Sorry - I missed those discussions.
>What is current practise? Do people request for one via ip link
>stats and the other via ethtool?

Yes.


>What do you guys do in your implementation?

Currently we do what you described. This patchset changes it.


>Yes, it would be more accurate to provide aggregated stats but
>it may break backward compat if expectation is both are read
>separately today.
>Maybe it makes sense to have a brand new TLV for these aggregated
>stats as Jiri was suggesting.That means two new TLVs not one.
>1) TLV for aggregated stats - which cant be current one
>2) TLV for h/w stats
>
>The existing stat implies s/ware only.


What is "aggregated"? if hw counts it, sw counts it as well.
HW stats are in fact aggregated. It includes all.

Apps should see HW stats as they don't care if the forwarding is
offloaded to HW or not. And if someone is aware there is a forward
offload, he can query sw-only stats via iface proposed by this patchset.


Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-20 Thread Jamal Hadi Salim

On 16-06-19 11:14 PM, Roopa Prabhu wrote:

On Fri, Jun 17, 2016 at 10:12 AM, Florian Fainelli  wrote:





I have also mentioned this before, the default api must provide
accumulated (hw and sw) stats...,
because this is the api that the user queries on an interface.


Sorry - I missed those discussions.
What is current practise? Do people request for one via ip link
stats and the other via ethtool?
What do you guys do in your implementation?
Yes, it would be more accurate to provide aggregated stats but
it may break backward compat if expectation is both are read
separately today.
Maybe it makes sense to have a brand new TLV for these aggregated
stats as Jiri was suggesting.That means two new TLVs not one.
1) TLV for aggregated stats - which cant be current one
2) TLV for h/w stats

The existing stat implies s/ware only.

cheers,
jamal



Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-19 Thread Roopa Prabhu
On Fri, Jun 17, 2016 at 10:12 AM, Florian Fainelli  wrote:
> On 06/17/2016 08:42 AM, Jiri Pirko wrote:
>> Fri, Jun 17, 2016 at 05:35:53PM CEST, d...@cumulusnetworks.com wrote:
>>> On 6/17/16 8:54 AM, Jamal Hadi Salim wrote:
 On 16-06-17 10:05 AM, Jiri Pirko wrote:
> Fri, Jun 17, 2016 at 03:48:35PM CEST, d...@cumulusnetworks.com wrote:
>> On 6/17/16 2:24 AM, Jiri Pirko wrote:
>>>

>
> That is problematic. Existing apps depend on rtnetlink stats. But if we
> don't count offloaded forwarded packets, the apps don't see anything.
> Therefore I believe that this patchset approach is better. The existing
> apps continue to work and future apps can use newly introduces sw_stats
> to query slowpath traffic. Makes sense to me.
>

 I agree with Jiri. It is a bad idea to depend on ethtool for any of
 this stuff. Is there a way we can tag netlink stats instead
 to indicate they are hardware or software?
>>>
>>> Right, old API but the key here is that low level h/w stats are returned by 
>>> a
>>> different API.
>>>
>>> By default ip, ifconfig, snmpd, etc all continue to get traditional S/W 
>>> stats
>>> - counters as seen by the CPU.
>>
>> Yep. And I believe that for offloaded forwarding, this tools should see
>> hw counters, as they show what is going on in real.
>
> If your NIC is offloading packets today, these tools typically won't see
> these stats, but ethtool -S likely will report what is going on under
> the hood.
>
> Do we actually need to tell apart SW maintained from HW maintained
> stats, or at the end all that matters is just, as DaveM pointed out,
> getting the information, and in the case of an Ethernet switch, return
> HW stats by default and supplement with SW stats whenever we have them,
> all in the same namespace?
> --

I have also mentioned this before, the default api must provide
accumulated (hw and sw) stats...,
because this is the api that the user queries on an interface.
For advanced debugging, people do want a break down and thats what
traditionally ethtool has provided
and the new stats api should eventually include support for ethtool like stats.


Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-19 Thread Roopa Prabhu
On Fri, Jun 17, 2016 at 7:54 AM, Jamal Hadi Salim  wrote:
> On 16-06-17 10:05 AM, Jiri Pirko wrote:
>>
>> Fri, Jun 17, 2016 at 03:48:35PM CEST, d...@cumulusnetworks.com wrote:
>>>
>>> On 6/17/16 2:24 AM, Jiri Pirko wrote:


>
>>
>> That is problematic. Existing apps depend on rtnetlink stats. But if we
>> don't count offloaded forwarded packets, the apps don't see anything.
>> Therefore I believe that this patchset approach is better. The existing
>> apps continue to work and future apps can use newly introduces sw_stats
>> to query slowpath traffic. Makes sense to me.
>>
>
> I agree with Jiri. It is a bad idea to depend on ethtool for any of
> this stuff.

The concern should not be that it is an ethtool api.
In all previous discussions on this patchset and also my
stats api patches, i have indicated that we have to move all stats
in one place, so naturally, ethtool stats should move eventually to the
stats api as a new nested netlink attribute. I think i called it
IFLA_STATS_LINK_HW  (or something like that)...
and this nested attribute should provide the flexibility and extensibility
of the current ethtool stats api.


> Is there a way we can tag netlink stats instead
> to indicate they are hardware or software?
> We already have a use case with the tc where someone could get/set
> hardware and/or software.
>
> cheers,
> jamal


Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-19 Thread Roopa Prabhu
On Fri, Jun 17, 2016 at 7:05 AM, Jiri Pirko  wrote:
> Fri, Jun 17, 2016 at 03:48:35PM CEST, d...@cumulusnetworks.com wrote:
>>On 6/17/16 2:24 AM, Jiri Pirko wrote:
>>>
>>>The problem we try to handle is different, it's about offloaded
>>>forwarded packets which are not seen by kernel. Let me try to draw it :)
>>>
>>>port1   port2 (HW stats are counted here)
>>>  \  /
>>>   \/
>>>\  /
>>> --(A) ASIC --(B)--
>>>|
>>>   (C)
>>>|
>>>   CPU (SW stats are counted here)
>>>
>>>
>>>Now we have couple of flows for TX and RX (direction does not matter here):
>>>
>>>1) port1->A->ASIC->C->CPU
>>>
>>>   For this flow, HW and SW stats are equal.
>>>
>>>2) port1->A->ASIC->C->CPU->C->ASIC->B->port2
>>>
>>>   For this flow, HW and SW stats are equal.
>>>
>>>3) port1->A->ASIC->B->port2
>>>
>>>   For this flow, SW stats are 0.
>>>
>>>The purpose of this patchset is to provide facility for user to
>>>find out the difference between flows 1+2 and 3. In other words, user
>>>will be able to see the statistics for his slow-path (through kernel).
>>>
>>>Also, as a default the accumulated stats (HW) will be exposed to user
>>>so the userspace apps can react properly.
>>>
>>
>>You no longer agree with this discussion?
>>  http://comments.gmane.org/gmane.linux.network/346740
>>
>>Essentially netdevice stats show counters for packets punted to the cpu and
>>ethool -S shows h/w stats. This patch set seems to invert that.
>
> That is problematic. Existing apps depend on rtnetlink stats. But if we
> don't count offloaded forwarded packets, the apps don't see anything.
> Therefore I believe that this patchset approach is better. The existing
> apps continue to work and future apps can use newly introduces sw_stats
> to query slowpath traffic. Makes sense to me.
>

Apps only care about stats. they don't care about sw vs hardware
stats. what apps are these ?.
For debugging, I agree it would be useful, but thats why we have
always had ethtool stats which
the driver can break down and display. And in all my patches about the
new stats api, i have indicated
that we will migrate the existing ethtool stats to a new netlink
attribute in the new stats api.


Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-19 Thread Jiri Pirko
Sat, Jun 18, 2016 at 03:58:56PM CEST, j...@mojatatu.com wrote:
>On 16-06-18 04:00 AM, Jiri Pirko wrote:
>>Fri, Jun 17, 2016 at 07:12:22PM CEST, f.faine...@gmail.com wrote:
>
Yep. And I believe that for offloaded forwarding, this tools should see
hw counters, as they show what is going on in real.
>>>
>>>If your NIC is offloading packets today, these tools typically won't see
>>>these stats, but ethtool -S likely will report what is going on under
>>>the hood.
>>>
>>>Do we actually need to tell apart SW maintained from HW maintained
>>>stats, or at the end all that matters is just, as DaveM pointed out,
>>>getting the information, and in the case of an Ethernet switch, return
>>>HW stats by default and supplement with SW stats whenever we have them,
>>>all in the same namespace?
>>
>
>In general it is extremely useful for debugging to be able to see them
>separately. One API to unify them (and that API being netlink) is
>the way to go. I dont know if you can ever obsolete ethtool if lots
>of other utils are using it - but would be nice.
>It is also useful to just get the sum of them - but user space can
>take care of that. David A., whatever user space tools that depended
>on ethtool should now be able to retrieve them via netlink, no?
>
>>I believe it is valuable for user to know stats for slow path
>>(non-forwarded by ASIC). Also, it's just another rtnl attr. Easy.
>>
>
>So Jiri, I see:
>IFLA_SW_STATS64 should that be: IFLA_HW_STATS_LINK_64?
>I think IFLA_STATS_LINK_64 should continue to send s/ware stats.

Well, we spent a lot of time to think about this. The problem with your
approach is that existing apps don't see "real-stats" - hw stats. For
example snmp daemon takes IFLA_STATS_LINK_64i, so it has to see HW stats
there. In order to not break existing apps, we expose HW stats as default.


Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-18 Thread Jamal Hadi Salim

On 16-06-18 04:00 AM, Jiri Pirko wrote:

Fri, Jun 17, 2016 at 07:12:22PM CEST, f.faine...@gmail.com wrote:



Yep. And I believe that for offloaded forwarding, this tools should see
hw counters, as they show what is going on in real.


If your NIC is offloading packets today, these tools typically won't see
these stats, but ethtool -S likely will report what is going on under
the hood.

Do we actually need to tell apart SW maintained from HW maintained
stats, or at the end all that matters is just, as DaveM pointed out,
getting the information, and in the case of an Ethernet switch, return
HW stats by default and supplement with SW stats whenever we have them,
all in the same namespace?




In general it is extremely useful for debugging to be able to see them
separately. One API to unify them (and that API being netlink) is
the way to go. I dont know if you can ever obsolete ethtool if lots
of other utils are using it - but would be nice.
It is also useful to just get the sum of them - but user space can
take care of that. David A., whatever user space tools that depended
on ethtool should now be able to retrieve them via netlink, no?


I believe it is valuable for user to know stats for slow path
(non-forwarded by ASIC). Also, it's just another rtnl attr. Easy.



So Jiri, I see:
IFLA_SW_STATS64 should that be: IFLA_HW_STATS_LINK_64?
I think IFLA_STATS_LINK_64 should continue to send s/ware stats.

cheers,
jamal


Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-18 Thread Jiri Pirko
Fri, Jun 17, 2016 at 07:12:22PM CEST, f.faine...@gmail.com wrote:
>On 06/17/2016 08:42 AM, Jiri Pirko wrote:
>> Fri, Jun 17, 2016 at 05:35:53PM CEST, d...@cumulusnetworks.com wrote:
>>> On 6/17/16 8:54 AM, Jamal Hadi Salim wrote:
 On 16-06-17 10:05 AM, Jiri Pirko wrote:
> Fri, Jun 17, 2016 at 03:48:35PM CEST, d...@cumulusnetworks.com wrote:
>> On 6/17/16 2:24 AM, Jiri Pirko wrote:
>>>

>
> That is problematic. Existing apps depend on rtnetlink stats. But if we
> don't count offloaded forwarded packets, the apps don't see anything.
> Therefore I believe that this patchset approach is better. The existing
> apps continue to work and future apps can use newly introduces sw_stats
> to query slowpath traffic. Makes sense to me.
>

 I agree with Jiri. It is a bad idea to depend on ethtool for any of
 this stuff. Is there a way we can tag netlink stats instead
 to indicate they are hardware or software?
>>>
>>> Right, old API but the key here is that low level h/w stats are returned by 
>>> a
>>> different API.
>>>
>>> By default ip, ifconfig, snmpd, etc all continue to get traditional S/W 
>>> stats
>>> - counters as seen by the CPU.
>> 
>> Yep. And I believe that for offloaded forwarding, this tools should see
>> hw counters, as they show what is going on in real.
>
>If your NIC is offloading packets today, these tools typically won't see
>these stats, but ethtool -S likely will report what is going on under
>the hood.
>
>Do we actually need to tell apart SW maintained from HW maintained
>stats, or at the end all that matters is just, as DaveM pointed out,
>getting the information, and in the case of an Ethernet switch, return
>HW stats by default and supplement with SW stats whenever we have them,
>all in the same namespace?

I believe it is valuable for user to know stats for slow path
(non-forwarded by ASIC). Also, it's just another rtnl attr. Easy.


Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-17 Thread Florian Fainelli
On 06/17/2016 08:42 AM, Jiri Pirko wrote:
> Fri, Jun 17, 2016 at 05:35:53PM CEST, d...@cumulusnetworks.com wrote:
>> On 6/17/16 8:54 AM, Jamal Hadi Salim wrote:
>>> On 16-06-17 10:05 AM, Jiri Pirko wrote:
 Fri, Jun 17, 2016 at 03:48:35PM CEST, d...@cumulusnetworks.com wrote:
> On 6/17/16 2:24 AM, Jiri Pirko wrote:
>>
>>>

 That is problematic. Existing apps depend on rtnetlink stats. But if we
 don't count offloaded forwarded packets, the apps don't see anything.
 Therefore I believe that this patchset approach is better. The existing
 apps continue to work and future apps can use newly introduces sw_stats
 to query slowpath traffic. Makes sense to me.

>>>
>>> I agree with Jiri. It is a bad idea to depend on ethtool for any of
>>> this stuff. Is there a way we can tag netlink stats instead
>>> to indicate they are hardware or software?
>>
>> Right, old API but the key here is that low level h/w stats are returned by a
>> different API.
>>
>> By default ip, ifconfig, snmpd, etc all continue to get traditional S/W stats
>> - counters as seen by the CPU.
> 
> Yep. And I believe that for offloaded forwarding, this tools should see
> hw counters, as they show what is going on in real.

If your NIC is offloading packets today, these tools typically won't see
these stats, but ethtool -S likely will report what is going on under
the hood.

Do we actually need to tell apart SW maintained from HW maintained
stats, or at the end all that matters is just, as DaveM pointed out,
getting the information, and in the case of an Ethernet switch, return
HW stats by default and supplement with SW stats whenever we have them,
all in the same namespace?
-- 
Florian


Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-17 Thread Jiri Pirko
Fri, Jun 17, 2016 at 05:35:53PM CEST, d...@cumulusnetworks.com wrote:
>On 6/17/16 8:54 AM, Jamal Hadi Salim wrote:
>>On 16-06-17 10:05 AM, Jiri Pirko wrote:
>>>Fri, Jun 17, 2016 at 03:48:35PM CEST, d...@cumulusnetworks.com wrote:
On 6/17/16 2:24 AM, Jiri Pirko wrote:
>
>>
>>>
>>>That is problematic. Existing apps depend on rtnetlink stats. But if we
>>>don't count offloaded forwarded packets, the apps don't see anything.
>>>Therefore I believe that this patchset approach is better. The existing
>>>apps continue to work and future apps can use newly introduces sw_stats
>>>to query slowpath traffic. Makes sense to me.
>>>
>>
>>I agree with Jiri. It is a bad idea to depend on ethtool for any of
>>this stuff. Is there a way we can tag netlink stats instead
>>to indicate they are hardware or software?
>
>Right, old API but the key here is that low level h/w stats are returned by a
>different API.
>
>By default ip, ifconfig, snmpd, etc all continue to get traditional S/W stats
>- counters as seen by the CPU.

Yep. And I believe that for offloaded forwarding, this tools should see
hw counters, as they show what is going on in real.


Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-17 Thread David Ahern

On 6/17/16 8:54 AM, Jamal Hadi Salim wrote:

On 16-06-17 10:05 AM, Jiri Pirko wrote:

Fri, Jun 17, 2016 at 03:48:35PM CEST, d...@cumulusnetworks.com wrote:

On 6/17/16 2:24 AM, Jiri Pirko wrote:






That is problematic. Existing apps depend on rtnetlink stats. But if we
don't count offloaded forwarded packets, the apps don't see anything.
Therefore I believe that this patchset approach is better. The existing
apps continue to work and future apps can use newly introduces sw_stats
to query slowpath traffic. Makes sense to me.



I agree with Jiri. It is a bad idea to depend on ethtool for any of
this stuff. Is there a way we can tag netlink stats instead
to indicate they are hardware or software?


Right, old API but the key here is that low level h/w stats are returned 
by a different API.


By default ip, ifconfig, snmpd, etc all continue to get traditional S/W 
stats - counters as seen by the CPU.


Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-17 Thread Jiri Pirko
Fri, Jun 17, 2016 at 04:54:34PM CEST, j...@mojatatu.com wrote:
>On 16-06-17 10:05 AM, Jiri Pirko wrote:
>>Fri, Jun 17, 2016 at 03:48:35PM CEST, d...@cumulusnetworks.com wrote:
>>>On 6/17/16 2:24 AM, Jiri Pirko wrote:

>
>>
>>That is problematic. Existing apps depend on rtnetlink stats. But if we
>>don't count offloaded forwarded packets, the apps don't see anything.
>>Therefore I believe that this patchset approach is better. The existing
>>apps continue to work and future apps can use newly introduces sw_stats
>>to query slowpath traffic. Makes sense to me.
>>
>
>I agree with Jiri. It is a bad idea to depend on ethtool for any of
>this stuff. Is there a way we can tag netlink stats instead
>to indicate they are hardware or software?

In this patchset, those are 2 nl attrs. And they come kernel->user at once.
So I see no need for any tagging. Also won't be appropriate.

>We already have a use case with the tc where someone could get/set
>hardware and/or software.

That is user->kernel.

>
>cheers,
>jamal


Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-17 Thread Jamal Hadi Salim

On 16-06-17 10:05 AM, Jiri Pirko wrote:

Fri, Jun 17, 2016 at 03:48:35PM CEST, d...@cumulusnetworks.com wrote:

On 6/17/16 2:24 AM, Jiri Pirko wrote:






That is problematic. Existing apps depend on rtnetlink stats. But if we
don't count offloaded forwarded packets, the apps don't see anything.
Therefore I believe that this patchset approach is better. The existing
apps continue to work and future apps can use newly introduces sw_stats
to query slowpath traffic. Makes sense to me.



I agree with Jiri. It is a bad idea to depend on ethtool for any of
this stuff. Is there a way we can tag netlink stats instead
to indicate they are hardware or software?
We already have a use case with the tc where someone could get/set
hardware and/or software.

cheers,
jamal


Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-17 Thread Jiri Pirko
Fri, Jun 17, 2016 at 03:48:35PM CEST, d...@cumulusnetworks.com wrote:
>On 6/17/16 2:24 AM, Jiri Pirko wrote:
>>
>>The problem we try to handle is different, it's about offloaded
>>forwarded packets which are not seen by kernel. Let me try to draw it :)
>>
>>port1   port2 (HW stats are counted here)
>>  \  /
>>   \/
>>\  /
>> --(A) ASIC --(B)--
>>|
>>   (C)
>>|
>>   CPU (SW stats are counted here)
>>
>>
>>Now we have couple of flows for TX and RX (direction does not matter here):
>>
>>1) port1->A->ASIC->C->CPU
>>
>>   For this flow, HW and SW stats are equal.
>>
>>2) port1->A->ASIC->C->CPU->C->ASIC->B->port2
>>
>>   For this flow, HW and SW stats are equal.
>>
>>3) port1->A->ASIC->B->port2
>>
>>   For this flow, SW stats are 0.
>>
>>The purpose of this patchset is to provide facility for user to
>>find out the difference between flows 1+2 and 3. In other words, user
>>will be able to see the statistics for his slow-path (through kernel).
>>
>>Also, as a default the accumulated stats (HW) will be exposed to user
>>so the userspace apps can react properly.
>>
>
>You no longer agree with this discussion?
>  http://comments.gmane.org/gmane.linux.network/346740
>
>Essentially netdevice stats show counters for packets punted to the cpu and
>ethool -S shows h/w stats. This patch set seems to invert that.

That is problematic. Existing apps depend on rtnetlink stats. But if we
don't count offloaded forwarded packets, the apps don't see anything.
Therefore I believe that this patchset approach is better. The existing
apps continue to work and future apps can use newly introduces sw_stats
to query slowpath traffic. Makes sense to me.



Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-17 Thread David Ahern

On 6/17/16 2:24 AM, Jiri Pirko wrote:


The problem we try to handle is different, it's about offloaded
forwarded packets which are not seen by kernel. Let me try to draw it :)

port1   port2 (HW stats are counted here)
  \  /
   \/
\  /
 --(A) ASIC --(B)--
|
   (C)
|
   CPU (SW stats are counted here)


Now we have couple of flows for TX and RX (direction does not matter here):

1) port1->A->ASIC->C->CPU

   For this flow, HW and SW stats are equal.

2) port1->A->ASIC->C->CPU->C->ASIC->B->port2

   For this flow, HW and SW stats are equal.

3) port1->A->ASIC->B->port2

   For this flow, SW stats are 0.

The purpose of this patchset is to provide facility for user to
find out the difference between flows 1+2 and 3. In other words, user
will be able to see the statistics for his slow-path (through kernel).

Also, as a default the accumulated stats (HW) will be exposed to user
so the userspace apps can react properly.



You no longer agree with this discussion?
  http://comments.gmane.org/gmane.linux.network/346740

Essentially netdevice stats show counters for packets punted to the cpu 
and ethool -S shows h/w stats. This patch set seems to invert that.


Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-17 Thread Jiri Pirko
Fri, Jun 17, 2016 at 02:26:32AM CEST, da...@davemloft.net wrote:
>From: Jiri Pirko 
>Date: Thu, 16 Jun 2016 10:37:13 +0200
>
>> Until now we had stats functions return SW statistics. However, it makes
>> a lot of sense to return HW stats as default. The existing apps count with
>> having the defaults stats complete, but that is not true now as the offloaded
>> forward traffic is not visible there.
>> 
>> If user wants to know real SW stats, this patchset provides way to get
>> it as well.
>
>I think we haven't been very good about defining good rules nor
>guidelines for how to handle HW vs SW stats, and I'm talking
>strictly about what we publish via rtnl_link_stats64.
>
>However, if I were working from scratch on a new driver what I would
>be inclined to do is use HW stats for everything that the chip
>provides direct and accurate support for, and fill in the gaps with SW
>stats.
>
>Because to me, stats are stats, the user wants to know (for example)
>how many broadcast packets have gone through the port and doesn't care
>how you obtain that number.
>
>If the problem being addressed is that drivers aren't reporting
>information on all the packets going through the device, then that's a
>bug.
>
>But it seems to me like mlxsw is already maintaining the software
>statistic counters, so I can't see a performance reason for not
>properly providing all of the statistics using HW vs. SW as is
>appropriate for each and every value to fix this problem.  Why
>create an entire new facility just for that?  It doesn't seem to
>be needed.
>
>Maybe you just need to describe things a bit more completely in this
>header posting.


The problem we try to handle is different, it's about offloaded
forwarded packets which are not seen by kernel. Let me try to draw it :)

port1   port2 (HW stats are counted here)
  \  /
   \/
\  /
 --(A) ASIC --(B)--
|
   (C)
|
   CPU (SW stats are counted here)


Now we have couple of flows for TX and RX (direction does not matter here):

1) port1->A->ASIC->C->CPU

   For this flow, HW and SW stats are equal.

2) port1->A->ASIC->C->CPU->C->ASIC->B->port2

   For this flow, HW and SW stats are equal.

3) port1->A->ASIC->B->port2

   For this flow, SW stats are 0.

The purpose of this patchset is to provide facility for user to
find out the difference between flows 1+2 and 3. In other words, user
will be able to see the statistics for his slow-path (through kernel).

Also, as a default the accumulated stats (HW) will be exposed to user
so the userspace apps can react properly.


Re: [patch net-next v4 0/4] return offloaded stats as default and expose original sw stats

2016-06-16 Thread David Miller
From: Jiri Pirko 
Date: Thu, 16 Jun 2016 10:37:13 +0200

> Until now we had stats functions return SW statistics. However, it makes
> a lot of sense to return HW stats as default. The existing apps count with
> having the defaults stats complete, but that is not true now as the offloaded
> forward traffic is not visible there.
> 
> If user wants to know real SW stats, this patchset provides way to get
> it as well.

I think we haven't been very good about defining good rules nor
guidelines for how to handle HW vs SW stats, and I'm talking
strictly about what we publish via rtnl_link_stats64.

However, if I were working from scratch on a new driver what I would
be inclined to do is use HW stats for everything that the chip
provides direct and accurate support for, and fill in the gaps with SW
stats.

Because to me, stats are stats, the user wants to know (for example)
how many broadcast packets have gone through the port and doesn't care
how you obtain that number.

If the problem being addressed is that drivers aren't reporting
information on all the packets going through the device, then that's a
bug.

But it seems to me like mlxsw is already maintaining the software
statistic counters, so I can't see a performance reason for not
properly providing all of the statistics using HW vs. SW as is
appropriate for each and every value to fix this problem.  Why
create an entire new facility just for that?  It doesn't seem to
be needed.

Maybe you just need to describe things a bit more completely in this
header posting.