Subject: Re: [PATCH v3 net-next 0/3] net/sched: act_police: add support for packet-per-second policing

From: Simon Horman <[email-masked]>
Date: Mon, 15 Mar 2021 15:41:56 +0100

On Sun, Mar 14, 2021 at 10:17:37AM +0200, Ido Schimmel wrote:
> On Fri, Mar 12, 2021 at 03:08:28PM +0100, Simon Horman wrote:
> > This series enhances the TC policer action implementation to allow a
> > policer action instance to enforce a rate-limit based on
> > packets-per-second, configurable using a packet-per-second rate and burst
> > parameters.
> > 
> > In the hope of aiding review this is broken up into three patches.
> > 
> > * [PATCH 1/3] flow_offload: add support for packet-per-second policing
> > 
> >   Add support for this feature to the flow_offload API that is used to allow
> >   programming flows, including TC rules and their actions, into hardware.
> > 
> > * [PATCH 2/3] flow_offload: reject configuration of packet-per-second policing in offload drivers
> > 
> >   Teach all exiting users of the flow_offload API that allow offload of
> >   policer action instances to reject offload if packet-per-second rate
> >   limiting is configured: none support it at this time
> > 
> > * [PATCH 3/3] net/sched: act_police: add support for packet-per-second policing
> > 
> >   With the above ground-work in place add the new feature to the TC policer
> >   action itself
> > 
> > With the above in place the feature may be used.
> > 
> > As follow-ups we plan to provide:
> > * Corresponding updates to iproute2
> > * Corresponding self tests (which depend on the iproute2 changes)
> 
> I was about to ask :)
> 
> FYI, there is this selftest:
> tools/testing/selftests/net/forwarding/tc_police.sh
> 
> Which can be extended to also test packet rate policing

Thanks Ido,

The approach we have taken is to add tests to
tools/testing/selftests/tc-testing/tc-tests/actions/police.json

Do you think adding a test to tc_police.sh is also worthwhile? Or should be
done instead of updating police.json?

Lastly, my assumption is that the tests should be posted once iproute2
changes they depend on have been accepted. Is this correct in your opinion?

In any case, I'll get moving on posting the iproute2 changes.

> > * Hardware offload support for the NFP driver
> > 
> > Key changes since v2:
> > * Added patches 1 and 2, which makes adding patch 3 safe for existing
> >   hardware offload of the policer action
> > * Re-worked patch 3 so that a TC policer action instance may be configured
> >   for packet-per-second or byte-per-second rate limiting, but not both.
> > * Corrected kdoc usage
> 
> Thanks!