| Message ID | 20220217082803.3881-1-jianbol@nvidia.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | flow_offload: add tc police parameters | expand |
On Thu, Feb 17, 2022 at 08:28:01AM +0000, Jianbo Liu wrote: > As a preparation for more advanced police offload in mlx5 (e.g., > jumping to another chain when bandwidth is not exceeded), extend the > flow offload API with more tc-police parameters. Adjust existing > drivers to reject unsupported configurations. Hi, I have a concern that a) patch 1 introduces a facility that may break existing drivers; and b) patch 2 then fixes this I'd slightly prefer if the series was rearranged to avoid this problem. ...
On 2022-02-17 1:34 PM, Simon Horman wrote: > On Thu, Feb 17, 2022 at 08:28:01AM +0000, Jianbo Liu wrote: >> As a preparation for more advanced police offload in mlx5 (e.g., >> jumping to another chain when bandwidth is not exceeded), extend the >> flow offload API with more tc-police parameters. Adjust existing >> drivers to reject unsupported configurations. > > Hi, > > I have a concern that > a) patch 1 introduces a facility that may break existing drivers; and > b) patch 2 then fixes this > > I'd slightly prefer if the series was rearranged to avoid this problem. > > ... Hi Simon, It can't be rearranged as patch 2 can't compile without patch 1. Patch 1 only adds more information passing to the driver. The drivers functionality doesn't change. drivers today ignore police information, like actions, and still being ignored after patch 1. patch 2 updates the drivers to use that information instead of ignoring it. so it fixes the drivers that without patch 1 can't be fixed. Thanks, Roi
On Thu, Feb 17, 2022 at 12:34:39PM +0100, Simon Horman wrote: > On Thu, Feb 17, 2022 at 08:28:01AM +0000, Jianbo Liu wrote: > > As a preparation for more advanced police offload in mlx5 (e.g., > > jumping to another chain when bandwidth is not exceeded), extend the > > flow offload API with more tc-police parameters. Adjust existing > > drivers to reject unsupported configurations. > > Hi, > > I have a concern that > a) patch 1 introduces a facility that may break existing drivers; and > b) patch 2 then fixes this > > I'd slightly prefer if the series was rearranged to avoid this problem. Not sure what you mean by the above. Patch #1 extends the flow offload API with tc-police parameters that weren't communicated to drivers until now. Drivers still ignore the new parameters after this patch. It is only in patch #2 that these drivers reject configurations where the parameters are set. Therefore, the only breakage I see is the one that can happen after patch #2: unaware user space that was installing actions that weren't fully reflected to hardware. If we want to be on the safe side, it is possible to remove the errors, but keep the extack messages so that user space is at least somewhat aware.
As a preparation for more advanced police offload in mlx5 (e.g., jumping to another chain when bandwidth is not exceeded), extend the flow offload API with more tc-police parameters. Adjust existing drivers to reject unsupported configurations. Changes since v1: * Add one more strict validation for the control of drop/ok. Jianbo Liu (2): net: flow_offload: add tc police action parameters flow_offload: reject offload for all drivers with invalid police parameters drivers/net/dsa/sja1105/sja1105_flower.c | 27 +++++++++ .../chelsio/cxgb4/cxgb4_tc_matchall.c | 55 +++++++++++++++++++ .../net/ethernet/freescale/enetc/enetc_qos.c | 31 +++++++++++ .../ethernet/marvell/octeontx2/nic/otx2_tc.c | 54 ++++++++++++++++++ .../net/ethernet/mellanox/mlx5/core/en_tc.c | 27 +++++++++ .../ethernet/mellanox/mlxsw/spectrum_flower.c | 27 +++++++++ drivers/net/ethernet/mscc/ocelot_flower.c | 28 ++++++++++ drivers/net/ethernet/mscc/ocelot_net.c | 27 +++++++++ .../ethernet/netronome/nfp/flower/qos_conf.c | 28 ++++++++++ include/net/flow_offload.h | 19 +++++++ include/net/tc_act/tc_police.h | 30 ++++++++++ net/sched/act_police.c | 46 ++++++++++++++++ 12 files changed, 399 insertions(+)