Re: [PATCH net-next v2 4/4] net: marvell: prestera: Offload FLOW_ACTION_POLICE

On Tue, Aug 03, 2021 at 04:19:03PM +0000, Volodymyr Mytnyk [C] wrote:

quoted

On Mon, Aug 02, 2021 at 05:08:49PM +0300, Vadym Kochan wrote:
It seems the implementation assumes that each rule has a different
policer, so an error should be returned in case the same policer is
shared between different rules.

Each rule has a different policer assigned by HW. Do you mean the police.index should be checked here ?

Yes. Checked to make sure each rule uses a different policer.

quoted

quoted

+                     break;
               default:
                       NL_SET_ERR_MSG_MOD(extack, "Unsupported action");
                       pr_err("Unsupported action\n");

@@ -110,6 +117,17 @@ static int prestera_flower_parse(struct prestera_flow_block *block,

               return -EOPNOTSUPP;
       }

+     if (f->classid) {
+             int hw_tc = __tc_classid_to_hwtc(PRESTERA_HW_TC_NUM, f->classid);
+
+             if (hw_tc < 0) {
+                     NL_SET_ERR_MSG_MOD(f->common.extack, "Unsupported HW TC");
+                     return hw_tc;
+             }
+
+             prestera_acl_rule_hw_tc_set(rule, hw_tc);
+     }

Not sure what this is. Can you show a command line example of how this
is used?

This is HW traffic class used for packets that are trapped to CPU port. The usage is as the following:

tc qdisc add dev DEV clsact
tc filter add dev DEV ingress flower skip_sw dst_mac 00:AA:AA:AA:AA:00 hw_tc 1 action trap

You are not using any police action in this example and the changelogs
do not say anything about trap / CPU port so I fail to understand how
this hunk is related to the submission.

quoted

What about visibility regarding number of packets that were dropped by
the policer?

This is not support at this moment by the driver, so it is always zero now.

You plan to support it? I imagine the hardware policer is able to report
the number of packets it dropped.