From: Serhiy Boiko <[email protected]>
Offload action police when keyed to a flower classifier.
Only rate and burst is supported for now. The conform-exceed
drop is assumed as a default value.
Policer support requires FW 3.1 version. Still to make a backward
compatibility with ACL of FW 3.0 introduced separate FW msg structs for
ACL calls which have different field layout.
Co-developed-by: Volodymyr Mytnyk <[email protected]>
Signed-off-by: Volodymyr Mytnyk <[email protected]>
Signed-off-by: Serhiy Boiko <[email protected]>
Co-developed-by: Vadym Kochan <[email protected]>
Signed-off-by: Vadym Kochan <[email protected]>
---
v2:
1) Added missing "static" in prestera_hw.c:
- __prestera_hw_acl_rule_ext_add
- __prestera_hw_acl_rule_add
.../ethernet/marvell/prestera/prestera_acl.c | 14 ++
.../ethernet/marvell/prestera/prestera_acl.h | 11 +-
.../marvell/prestera/prestera_flower.c | 18 +++
.../ethernet/marvell/prestera/prestera_hw.c | 125 +++++++++++++++++-
.../ethernet/marvell/prestera/prestera_pci.c | 1 +
5 files changed, 165 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_acl.c b/drivers/net/ethernet/marvell/prestera/prestera_acl.c
index 83c75ffb1a1c..9a473f94fab0 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_acl.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_acl.c
@@ -8,6 +8,8 @@
#include "prestera_acl.h"
#include "prestera_span.h"
+#define PRESTERA_ACL_DEF_HW_TC 3
+
struct prestera_acl {
struct prestera_switch *sw;
struct list_head rules;
@@ -29,6 +31,7 @@ struct prestera_acl_rule {
u32 priority;
u8 n_actions;
u8 n_matches;
+ u8 hw_tc;
u32 id;
};
@@ -203,6 +206,7 @@ prestera_acl_rule_create(struct prestera_flow_block *block,
INIT_LIST_HEAD(&rule->action_list);
rule->cookie = cookie;
rule->block = block;
+ rule->hw_tc = PRESTERA_ACL_DEF_HW_TC;
return rule;
}
@@ -251,6 +255,16 @@ void prestera_acl_rule_priority_set(struct prestera_acl_rule *rule,
rule->priority = priority;
}
+u8 prestera_acl_rule_hw_tc_get(struct prestera_acl_rule *rule)
+{
+ return rule->hw_tc;
+}
+
+void prestera_acl_rule_hw_tc_set(struct prestera_acl_rule *rule, u8 hw_tc)
+{
+ rule->hw_tc = hw_tc;
+}
+
int prestera_acl_rule_match_add(struct prestera_acl_rule *rule,
struct prestera_acl_rule_match_entry *entry)
{
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_acl.h b/drivers/net/ethernet/marvell/prestera/prestera_acl.h
index 39b7869be659..2a2fbae1432a 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_acl.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera_acl.h
@@ -25,7 +25,8 @@ enum prestera_acl_rule_match_entry_type {
enum prestera_acl_rule_action {
PRESTERA_ACL_RULE_ACTION_ACCEPT,
PRESTERA_ACL_RULE_ACTION_DROP,
- PRESTERA_ACL_RULE_ACTION_TRAP
+ PRESTERA_ACL_RULE_ACTION_TRAP,
+ PRESTERA_ACL_RULE_ACTION_POLICE,
};
struct prestera_switch;
@@ -50,6 +51,12 @@ struct prestera_flow_block {
struct prestera_acl_rule_action_entry {
struct list_head list;
enum prestera_acl_rule_action id;
+ union {
+ struct {
+ u64 rate;
+ u64 burst;
+ } police;
+ };
};
struct prestera_acl_rule_match_entry {
@@ -120,5 +127,7 @@ void prestera_acl_rule_del(struct prestera_switch *sw,
int prestera_acl_rule_get_stats(struct prestera_switch *sw,
struct prestera_acl_rule *rule,
u64 *packets, u64 *bytes, u64 *last_use);
+u8 prestera_acl_rule_hw_tc_get(struct prestera_acl_rule *rule);
+void prestera_acl_rule_hw_tc_set(struct prestera_acl_rule *rule, u8 hw_tc);
#endif /* _PRESTERA_ACL_H_ */
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_flower.c b/drivers/net/ethernet/marvell/prestera/prestera_flower.c
index e571ba09ec08..76f30856ac98 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_flower.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_flower.c
@@ -5,6 +5,8 @@
#include "prestera_acl.h"
#include "prestera_flower.h"
+#define PRESTERA_HW_TC_NUM 8
+
static int prestera_flower_parse_actions(struct prestera_flow_block *block,
struct prestera_acl_rule *rule,
struct flow_action *flow_action,
@@ -30,6 +32,11 @@ static int prestera_flower_parse_actions(struct prestera_flow_block *block,
case FLOW_ACTION_TRAP:
a_entry.id = PRESTERA_ACL_RULE_ACTION_TRAP;
break;
+ case FLOW_ACTION_POLICE:
+ a_entry.id = PRESTERA_ACL_RULE_ACTION_POLICE;
+ a_entry.police.rate = act->police.rate_bytes_ps;
+ a_entry.police.burst = act->police.burst;
+ 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);
+ }
+
prestera_acl_rule_priority_set(rule, f->common.prio);
if (flow_rule_match_key(f_rule, FLOW_DISSECTOR_KEY_META)) {
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
index c1297859e471..918cdfbfed37 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_hw.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
@@ -91,6 +91,7 @@ enum {
enum {
PRESTERA_CMD_SWITCH_ATTR_MAC = 1,
PRESTERA_CMD_SWITCH_ATTR_AGEING = 2,
+ PRESTERA_SWITCH_ATTR_TRAP_POLICER = 3,
};
enum {
@@ -319,6 +320,19 @@ struct prestera_msg_acl_action {
u32 id;
};
+struct prestera_msg_acl_action_ext {
+ u32 id;
+ union {
+ struct {
+ u64 rate;
+ u64 burst;
+ } police;
+ struct {
+ u64 res[3];
+ } reserv;
+ } __packed;
+};
+
struct prestera_msg_acl_match {
u32 type;
union {
@@ -354,6 +368,16 @@ struct prestera_msg_acl_rule_req {
u8 n_matches;
};
+struct prestera_msg_acl_rule_ext_req {
+ struct prestera_msg_cmd cmd;
+ u32 id;
+ u32 priority;
+ u16 ruleset_id;
+ u8 n_actions;
+ u8 n_matches;
+ u8 hw_tc;
+};
+
struct prestera_msg_acl_rule_resp {
struct prestera_msg_ret ret;
u32 id;
@@ -908,6 +932,36 @@ static int prestera_hw_acl_actions_put(struct prestera_msg_acl_action *action,
return 0;
}
+static int prestera_hw_acl_actions_ext_put(struct prestera_msg_acl_action_ext *action,
+ struct prestera_acl_rule *rule)
+{
+ struct list_head *a_list = prestera_acl_rule_action_list_get(rule);
+ struct prestera_acl_rule_action_entry *a_entry;
+ int i = 0;
+
+ list_for_each_entry(a_entry, a_list, list) {
+ action[i].id = a_entry->id;
+
+ switch (a_entry->id) {
+ case PRESTERA_ACL_RULE_ACTION_ACCEPT:
+ case PRESTERA_ACL_RULE_ACTION_DROP:
+ case PRESTERA_ACL_RULE_ACTION_TRAP:
+ /* just rule action id, no specific data */
+ break;
+ case PRESTERA_ACL_RULE_ACTION_POLICE:
+ action[i].police.rate = a_entry->police.rate;
+ action[i].police.burst = a_entry->police.burst;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ i++;
+ }
+
+ return 0;
+}
+
static int prestera_hw_acl_matches_put(struct prestera_msg_acl_match *match,
struct prestera_acl_rule *rule)
{
@@ -963,9 +1017,9 @@ static int prestera_hw_acl_matches_put(struct prestera_msg_acl_match *match,
return 0;
}
-int prestera_hw_acl_rule_add(struct prestera_switch *sw,
- struct prestera_acl_rule *rule,
- u32 *rule_id)
+static int __prestera_hw_acl_rule_add(struct prestera_switch *sw,
+ struct prestera_acl_rule *rule,
+ u32 *rule_id)
{
struct prestera_msg_acl_action *actions;
struct prestera_msg_acl_match *matches;
@@ -1017,6 +1071,71 @@ int prestera_hw_acl_rule_add(struct prestera_switch *sw,
return err;
}
+static int __prestera_hw_acl_rule_ext_add(struct prestera_switch *sw,
+ struct prestera_acl_rule *rule,
+ u32 *rule_id)
+{
+ struct prestera_msg_acl_action_ext *actions;
+ struct prestera_msg_acl_rule_ext_req *req;
+ struct prestera_msg_acl_match *matches;
+ struct prestera_msg_acl_rule_resp resp;
+ u8 n_actions;
+ u8 n_matches;
+ void *buff;
+ u32 size;
+ int err;
+
+ n_actions = prestera_acl_rule_action_len(rule);
+ n_matches = prestera_acl_rule_match_len(rule);
+
+ size = sizeof(*req) + sizeof(*actions) * n_actions +
+ sizeof(*matches) * n_matches;
+
+ buff = kzalloc(size, GFP_KERNEL);
+ if (!buff)
+ return -ENOMEM;
+
+ req = buff;
+ actions = buff + sizeof(*req);
+ matches = buff + sizeof(*req) + sizeof(*actions) * n_actions;
+
+ /* put acl actions into the message */
+ err = prestera_hw_acl_actions_ext_put(actions, rule);
+ if (err)
+ goto free_buff;
+
+ /* put acl matches into the message */
+ err = prestera_hw_acl_matches_put(matches, rule);
+ if (err)
+ goto free_buff;
+
+ req->ruleset_id = prestera_acl_rule_ruleset_id_get(rule);
+ req->priority = prestera_acl_rule_priority_get(rule);
+ req->n_actions = prestera_acl_rule_action_len(rule);
+ req->n_matches = prestera_acl_rule_match_len(rule);
+ req->hw_tc = prestera_acl_rule_hw_tc_get(rule);
+
+ err = prestera_cmd_ret(sw, PRESTERA_CMD_TYPE_ACL_RULE_ADD,
+ &req->cmd, size, &resp.ret, sizeof(resp));
+ if (err)
+ goto free_buff;
+
+ *rule_id = resp.id;
+free_buff:
+ kfree(buff);
+ return err;
+}
+
+int prestera_hw_acl_rule_add(struct prestera_switch *sw,
+ struct prestera_acl_rule *rule,
+ u32 *rule_id)
+{
+ if (sw->dev->fw_rev.maj == 3 && sw->dev->fw_rev.min == 0)
+ return __prestera_hw_acl_rule_add(sw, rule, rule_id);
+
+ return __prestera_hw_acl_rule_ext_add(sw, rule, rule_id);
+};
+
int prestera_hw_acl_rule_del(struct prestera_switch *sw, u32 rule_id)
{
struct prestera_msg_acl_rule_req req = {
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_pci.c b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
index ce4cf51dba5a..f988603af1b6 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_pci.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
@@ -15,6 +15,7 @@
#define PRESTERA_MSG_MAX_SIZE 1500
static struct prestera_fw_rev prestera_fw_supp[] = {
+ { 3, 1 },
{ 3, 0 },
{ 2, 0 }
};
--
2.17.1
On Mon, Aug 02, 2021 at 05:08:49PM +0300, Vadym Kochan wrote:
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_flower.c b/drivers/net/ethernet/marvell/prestera/prestera_flower.c
> index e571ba09ec08..76f30856ac98 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_flower.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_flower.c
> @@ -5,6 +5,8 @@
> #include "prestera_acl.h"
> #include "prestera_flower.h"
>
> +#define PRESTERA_HW_TC_NUM 8
> +
> static int prestera_flower_parse_actions(struct prestera_flow_block *block,
> struct prestera_acl_rule *rule,
> struct flow_action *flow_action,
> @@ -30,6 +32,11 @@ static int prestera_flower_parse_actions(struct prestera_flow_block *block,
> case FLOW_ACTION_TRAP:
> a_entry.id = PRESTERA_ACL_RULE_ACTION_TRAP;
> break;
> + case FLOW_ACTION_POLICE:
> + a_entry.id = PRESTERA_ACL_RULE_ACTION_POLICE;
> + a_entry.police.rate = act->police.rate_bytes_ps;
> + a_entry.police.burst = act->police.burst;
If packet rate based policing is not supported, an error should be
returned here with extack.
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.
> + 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?
What about visibility regarding number of packets that were dropped by
the policer?
Hi Ido,
Thanks for the review. Pls see the comments inline.
> On Mon, Aug 02, 2021 at 05:08:49PM +0300, Vadym Kochan wrote:
> > diff --git a/drivers/net/ethernet/marvell/prestera/prestera_flower.c b/drivers/net/ethernet/marvell/prestera/prestera_flower.c
> > index e571ba09ec08..76f30856ac98 100644
> > --- a/drivers/net/ethernet/marvell/prestera/prestera_flower.c
> > +++ b/drivers/net/ethernet/marvell/prestera/prestera_flower.c
> > @@ -5,6 +5,8 @@
> > ?#include "prestera_acl.h"
> > ?#include "prestera_flower.h"
> >
> > +#define PRESTERA_HW_TC_NUM ? 8
> > +
> > ?static int prestera_flower_parse_actions(struct prestera_flow_block *block,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct prestera_acl_rule *rule,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct flow_action *flow_action,
> > @@ -30,6 +32,11 @@ static int prestera_flower_parse_actions(struct prestera_flow_block *block,
> > ? ? ? ? ? ? ? ?case FLOW_ACTION_TRAP:
> > ? ? ? ? ? ? ? ? ? ? ? ?a_entry.id = PRESTERA_ACL_RULE_ACTION_TRAP;
> > ? ? ? ? ? ? ? ? ? ? ? ?break;
> > + ? ? ? ? ? ? case FLOW_ACTION_POLICE:
> > + ? ? ? ? ? ? ? ? ? ? a_entry.id = PRESTERA_ACL_RULE_ACTION_POLICE;
> > + ? ? ? ? ? ? ? ? ? ? a_entry.police.rate = act->police.rate_bytes_ps;
> > + ? ? ? ? ? ? ? ? ? ? a_entry.police.burst = act->police.burst;
>
> If packet rate based policing is not supported, an error should be
> returned here with extack.
Agree, it makes sense.
>
> 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 ?
>
> > + ? ? ? ? ? ? ? ? ? ? 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
>
> 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.
On Tue, Aug 03, 2021 at 04:19:03PM +0000, Volodymyr Mytnyk [C] wrote:
> > 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.
>
> >
> > > + ? ? ? ? ? ? ? ? ? ? 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.
>
> >
> > 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.