| Message ID | 20210312140831.23346-1-simon.horman@netronome.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | net/sched: act_police: add support for packet-per-second policing | expand |
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. ... Sorry, I missed CCing a number of interested parties when posting this patch-set. I've added them to this email. Ref: https://lore.kernel.org/netdev/20210312140831.23346-1-simon.horman@netronome.com/
Hello: This series was applied to netdev/net-next.git (refs/heads/master): On Fri, 12 Mar 2021 15:08:28 +0100 you 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. > > [...] Here is the summary with links: - [v3,net-next,1/3] flow_offload: add support for packet-per-second policing https://git.kernel.org/netdev/net-next/c/25660156f4cc - [v3,net-next,2/3] flow_offload: reject configuration of packet-per-second policing in offload drivers https://git.kernel.org/netdev/net-next/c/6a56e19902af - [v3,net-next,3/3] net/sched: act_police: add support for packet-per-second policing https://git.kernel.org/netdev/net-next/c/2ffe0395288a You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
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 > * 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!
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!
Sorry for the delay. Was AFK yesterday On Mon, Mar 15, 2021 at 03:41:56PM +0100, Simon Horman wrote: > 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? IIUC, police.json only performs configuration tests. tc_police.sh on the other hand, configures a topology, injects traffic and validates that the bandwidth after the police action is according to user configuration. You can test the software data path by using veth pairs or the hardware data path by using physical ports looped to each other. So I think that extending both tests is worthwhile. > > 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? Personally, I prefer selftests to be posted together with the implementation, regardless if they depend on new iproute2 functionality. In the unlikely case that the kernel patches were accepted, but changes were requested for the command line interface, you can always patch the selftests later. Jakub recently added this section: https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html#how-do-i-post-corresponding-changes-to-user-space-components He writes "User space code exercising kernel features should be posted alongside kernel patches." And you can see that in the example the last patch is a selftest: ``` [PATCH net-next 0/3] net: some feature cover letter └─ [PATCH net-next 1/3] net: some feature prep └─ [PATCH net-next 2/3] net: some feature do it └─ [PATCH net-next 3/3] selftest: net: some feature [PATCH iproute2-next] ip: add support for some feature ``` > > In any case, I'll get moving on posting the iproute2 changes. Thanks! > > > > * 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!
Hi Ido, On Tue, Mar 16, 2021 at 10:35:35AM +0200, Ido Schimmel wrote: > Sorry for the delay. Was AFK yesterday No problem at all. ... > > > > 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? > > IIUC, police.json only performs configuration tests. tc_police.sh on the > other hand, configures a topology, injects traffic and validates that > the bandwidth after the police action is according to user > configuration. You can test the software data path by using veth pairs > or the hardware data path by using physical ports looped to each other. > > So I think that extending both tests is worthwhile. Thanks, we'll see about making it so. > > 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? > > Personally, I prefer selftests to be posted together with the > implementation, regardless if they depend on new iproute2 functionality. > In the unlikely case that the kernel patches were accepted, but changes > were requested for the command line interface, you can always patch the > selftests later. > > Jakub recently added this section: > https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html#how-do-i-post-corresponding-changes-to-user-space-components > > He writes "User space code exercising kernel features should be posted > alongside kernel patches." > > And you can see that in the example the last patch is a selftest: > > ``` > [PATCH net-next 0/3] net: some feature cover letter > └─ [PATCH net-next 1/3] net: some feature prep > └─ [PATCH net-next 2/3] net: some feature do it > └─ [PATCH net-next 3/3] selftest: net: some feature > > [PATCH iproute2-next] ip: add support for some feature > ``` Thanks, we'll try to follow this in our next feature submission. ...
On 2021-03-12 9:22 a.m., Simon Horman 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. > > ... > > Sorry, I missed CCing a number of interested parties when posting > this patch-set. I've added them to this email. > > Ref: https://lore.kernel.org/netdev/20210312140831.23346-1-simon.horman@netronome.com/ > For 2/3: Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com> If you submit one or two testcases to tools/testing/selftests/tc-testing you'll be both a hero and someone committing an act of kindness. cheers, jamal
On Tue, Mar 16, 2021 at 09:53:49AM -0400, Jamal Hadi Salim wrote: > On 2021-03-12 9:22 a.m., Simon Horman 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. > > > > ... > > > > Sorry, I missed CCing a number of interested parties when posting > > this patch-set. I've added them to this email. > > > > Ref: https://lore.kernel.org/netdev/20210312140831.23346-1-simon.horman@netronome.com/ > > > > For 2/3: > Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com> > > If you submit one or two testcases to tools/testing/selftests/tc-testing > you'll be both a hero and someone committing an act of kindness. Thanks Jamal, my team are heroes and are working on testcases.
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) * 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 Baowen Zheng (2): flow_offload: reject configuration of packet-per-second policing in offload drivers net/sched: act_police: add support for packet-per-second policing Xingfeng Hu (1): flow_offload: add support for packet-per-second policing drivers/net/dsa/sja1105/sja1105_flower.c | 6 ++ .../chelsio/cxgb4/cxgb4_tc_matchall.c | 11 ++- .../net/ethernet/freescale/enetc/enetc_qos.c | 5 ++ .../net/ethernet/mellanox/mlx5/core/en_tc.c | 4 + .../ethernet/mellanox/mlxsw/spectrum_flower.c | 5 ++ drivers/net/ethernet/mscc/ocelot_flower.c | 5 ++ drivers/net/ethernet/mscc/ocelot_net.c | 6 ++ .../ethernet/netronome/nfp/flower/qos_conf.c | 5 ++ include/net/flow_offload.h | 2 + include/net/sch_generic.h | 14 ++++ include/net/tc_act/tc_police.h | 52 +++++++++++++ include/uapi/linux/pkt_cls.h | 2 + net/sched/act_police.c | 59 +++++++++++++-- net/sched/cls_api.c | 3 + net/sched/sch_generic.c | 75 ++++++++++++------- 15 files changed, 221 insertions(+), 33 deletions(-)