mbox series

[net,0/2] Fix police 'continue' action offload

Message ID 20220704204405.2563457-1-vladbu@nvidia.com (mailing list archive)
Headers show
Series Fix police 'continue' action offload | expand

Message

Vlad Buslov July 4, 2022, 8:44 p.m. UTC
TC act_police with 'continue' action had been supported by mlx5 matchall
classifier offload implementation for some time. However, 'continue' was
assumed implicitly and recently got broken in multiple places. Fix it in
both TC hardware offload validation code and mlx5 driver.

Vlad Buslov (2):
  net/sched: act_police: allow 'continue' action offload
  net/mlx5e: Fix matchall police parameters validation

 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 13 ++++++-------
 include/net/flow_offload.h                      |  1 +
 net/sched/act_police.c                          |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

Comments

Jakub Kicinski July 6, 2022, 12:11 a.m. UTC | #1
On Mon, 4 Jul 2022 22:44:03 +0200 Vlad Buslov wrote:
> TC act_police with 'continue' action had been supported by mlx5 matchall
> classifier offload implementation for some time. However, 'continue' was
> assumed implicitly and recently got broken in multiple places. Fix it in
> both TC hardware offload validation code and mlx5 driver.

Saeed, ack or should they go via your tree? 

The signals are a little mixed since the tree in the subject is "net"
which makes sense but there's no ack / sob from Saeed.
Saeed Mahameed July 6, 2022, 5:46 a.m. UTC | #2
On 05 Jul 17:11, Jakub Kicinski wrote:
>On Mon, 4 Jul 2022 22:44:03 +0200 Vlad Buslov wrote:
>> TC act_police with 'continue' action had been supported by mlx5 matchall
>> classifier offload implementation for some time. However, 'continue' was
>> assumed implicitly and recently got broken in multiple places. Fix it in
>> both TC hardware offload validation code and mlx5 driver.
>
>Saeed, ack or should they go via your tree?
>
>The signals are a little mixed since the tree in the subject is "net"
>which makes sense but there's no ack / sob from Saeed.

Acked, vlad was under pressure to submit this I told him to go ahead and I
will ack on the list.
Vlad Buslov July 6, 2022, 9:57 a.m. UTC | #3
Adding maintainers of other drivers that might have been impacted by the
validation change. If your driver supports 'continue' police action
offload with matchall classifier, then you may want to also relax the
validation restrictions in similar manner as I do in patch 2 for mlx5,
as I also submitted OvS fix to restore matchall police notexceed action
type to 'continue' here:
https://mail.openvswitch.org/pipermail/ovs-dev/2022-July/395561.html

On Mon 04 Jul 2022 at 22:44, Vlad Buslov <vladbu@nvidia.com> wrote:
> TC act_police with 'continue' action had been supported by mlx5 matchall
> classifier offload implementation for some time. However, 'continue' was
> assumed implicitly and recently got broken in multiple places. Fix it in
> both TC hardware offload validation code and mlx5 driver.
>
> Vlad Buslov (2):
>   net/sched: act_police: allow 'continue' action offload
>   net/mlx5e: Fix matchall police parameters validation
>
>  drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 13 ++++++-------
>  include/net/flow_offload.h                      |  1 +
>  net/sched/act_police.c                          |  2 +-
>  3 files changed, 8 insertions(+), 8 deletions(-)
patchwork-bot+netdevbpf@kernel.org July 6, 2022, 11:50 a.m. UTC | #4
Hello:

This series was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Mon, 4 Jul 2022 22:44:03 +0200 you wrote:
> TC act_police with 'continue' action had been supported by mlx5 matchall
> classifier offload implementation for some time. However, 'continue' was
> assumed implicitly and recently got broken in multiple places. Fix it in
> both TC hardware offload validation code and mlx5 driver.
> 
> Vlad Buslov (2):
>   net/sched: act_police: allow 'continue' action offload
>   net/mlx5e: Fix matchall police parameters validation
> 
> [...]

Here is the summary with links:
  - [net,1/2] net/sched: act_police: allow 'continue' action offload
    https://git.kernel.org/netdev/net/c/052f744f4446
  - [net,2/2] net/mlx5e: Fix matchall police parameters validation
    https://git.kernel.org/netdev/net/c/4d1e07d83ccc

You are awesome, thank you!
Simon Horman July 7, 2022, 9:33 a.m. UTC | #5
+ oss-drivers@corigine.com

On Wed, Jul 06, 2022 at 12:57:48PM +0300, Vlad Buslov wrote:
> On Mon 04 Jul 2022 at 22:44, Vlad Buslov <vladbu@nvidia.com> wrote:
> > TC act_police with 'continue' action had been supported by mlx5 matchall
> > classifier offload implementation for some time. However, 'continue' was
> > assumed implicitly and recently got broken in multiple places. Fix it in
> > both TC hardware offload validation code and mlx5 driver.
> >
> > Vlad Buslov (2):
> >   net/sched: act_police: allow 'continue' action offload
> >   net/mlx5e: Fix matchall police parameters validation
> >
> >  drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 13 ++++++-------
> >  include/net/flow_offload.h                      |  1 +
> >  net/sched/act_police.c                          |  2 +-
> >  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> Adding maintainers of other drivers that might have been impacted by the
> validation change. If your driver supports 'continue' police action
> offload with matchall classifier, then you may want to also relax the
> validation restrictions in similar manner as I do in patch 2 for mlx5,
> as I also submitted OvS fix to restore matchall police notexceed action
> type to 'continue' here:
> https://mail.openvswitch.org/pipermail/ovs-dev/2022-July/395561.html

Thanks Vlad,

I've looked through the nfp driver and it appears that
nfp_policer_validate() rejects policer offload if the
conform/exceed action (act_id) is not FLOW_ACTION_DROP.

So I think it will not be affected by any new valid values for act_id.
And thus this patchset, which adds FLOW_ACTION_CONTINUE as a valid
value for act_id should not affect the nfp driver.

For patch 1/2:

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