Re: [PATCH net-next] net: lan966x: Add support for TC flower filter statistics

[Simon Horman]

On Mon, Feb 06, 2023 at 10:52:27AM +0100, Horatiu Vultur wrote:
> The 02/04/2023 18:12, Simon Horman wrote:
> > 
> > On Fri, Feb 03, 2023 at 02:53:49PM +0100, Horatiu Vultur wrote:
> > > Add flower filter packet statistics. This will just read the TCAM
> > > counter of the rule, which mention how many packages were hit by this
> > > rule.
> > 
> > I am curious to know how HW stats only updating the packet count
> > interacts with SW stats also incrementing other values, such as the byte
> > count.
> 
> First, our HW can count only the packages and not also the bytes,
> unfortunately. Also we use the flag 'skip_sw' when we add the rules in
> this case the statistics look OK.
> If the user doesn't use the skip_sw then the statistics will look
> something like this (using command: tc -s filter show dev eth0 ingress):
> 
>         Action statistics:
>         Sent 92 bytes 4 pkt (dropped 0, overlimits 0 requeues 0)
>         Sent software 92 bytes 2 pkt
>         Sent hardware 0 bytes 2 pkt
>         backlog 0b 0p requeues 0
>         used_hw_stats immediate
> 
> As you see there are different counters for SW and Hw statistics.

Thanks, that answers my question.
I appreciate that hw has limitations, and this does seem
to be an appropriate solution in this case.

> > > Signed-off-by: Horatiu Vultur <horatiu.vultur@xxxxxxxxxxxxx>

Reviewed-by: Simon Horman <simon.horman@xxxxxxxxxxxx>

...

> > Also, not strictly related, but could you consider, as a favour to
> > reviewers, fixing the driver so that the following doesn't fail:
> > 
> > $ make drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.o
> >   DESCEND objtool
> >   CALL    scripts/checksyscalls.sh
> >   CC      drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.o
> > In file included from drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c:3:
> > drivers/net/ethernet/microchip/lan966x/lan966x_main.h:18:10: fatal error: vcap_api.h: No such file or directory
> >    18 | #include <vcap_api.h>
> >       |          ^~~~~~~~~~~~
> > compilation terminated.
> 
> I will try to have a look at this.

Thanks, much appreciated.