[RFC net-next 0/4] Marvell Prestera add policer support

[RFC net-next 0/4] Marvell Prestera add policer support

[Vadym Kochan]

From: Vadym Kochan <vkochan@marvell.com>

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. Because there are some FW ABI
differences in ACL rule messages between 3.0 and 3.1 so added separate
"_ext" struct version with separate HW helper.

Also added new __tc_classid_to_hwtc() helper which calculates hw tc
without need of netdev but specifying the num of tc instead, because
ingress HW queues are globally and statically per ASIC not per port.

Serhiy Boiko (1):
  net: marvell: prestera: Offload FLOW_ACTION_POLICE

Vadym Kochan (3):
  net: marvell: prestera: do not fail if FW reply is bigger
  net: marvell: prestera: turn FW supported versions into an array
  net: sched: introduce __tc_classid_to_hwtc() helper

 .../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  |  63 ++++-----
 include/net/sch_generic.h                     |   9 +-
 6 files changed, 197 insertions(+), 43 deletions(-)

-- 
2.17.1

[RFC net-next 1/4] net: marvell: prestera: do not fail if FW reply is bigger

[Vadym Kochan]

From: Vadym Kochan <vkochan@marvell.com>

There might be a case when driver talks to the newer FW version
which has extended message packets with extra fields, in that case
lets just copy minimum what we need/can.

Signed-off-by: Vadym Kochan <vkochan@marvell.com>
---
 drivers/net/ethernet/marvell/prestera/prestera_pci.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_pci.c b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
index a250d394da38..58642b540322 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_pci.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
@@ -359,12 +359,7 @@ static int prestera_fw_cmd_send(struct prestera_fw *fw,
 	}
 
 	ret_size = prestera_fw_read(fw, PRESTERA_CMD_RCV_LEN_REG);
-	if (ret_size > out_size) {
-		dev_err(fw->dev.dev, "ret_size (%u) > out_len(%zu)\n",
-			ret_size, out_size);
-		err = -EMSGSIZE;
-		goto cmd_exit;
-	}
+	ret_size = min_t(u32, ret_size, out_size);
 
 	memcpy_fromio(out_msg, fw->cmd_mbox + in_size, ret_size);
 
-- 
2.17.1

[RFC net-next 2/4] net: marvell: prestera: turn FW supported versions into an array

[Vadym Kochan]

From: Vadym Kochan <vkochan@marvell.com>

In case of supporting more than 2 FW versions it is more flexible to
have them defined as array.

Signed-off-by: Vadym Kochan <vkochan@marvell.com>
---
 .../ethernet/marvell/prestera/prestera_pci.c  | 55 ++++++++-----------
 1 file changed, 24 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_pci.c b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
index 58642b540322..ce4cf51dba5a 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_pci.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
@@ -14,11 +14,10 @@
 
 #define PRESTERA_MSG_MAX_SIZE 1500
 
-#define PRESTERA_SUPP_FW_MAJ_VER	3
-#define PRESTERA_SUPP_FW_MIN_VER	0
-
-#define PRESTERA_PREV_FW_MAJ_VER	2
-#define PRESTERA_PREV_FW_MIN_VER	0
+static struct prestera_fw_rev prestera_fw_supp[] = {
+	{ 3, 0 },
+	{ 2, 0 }
+};
 
 #define PRESTERA_FW_PATH_FMT	"mrvl/prestera/mvsw_prestera_fw-v%u.%u.img"
 
@@ -629,40 +628,34 @@ static int prestera_fw_hdr_parse(struct prestera_fw *fw)
 
 static int prestera_fw_get(struct prestera_fw *fw)
 {
-	int ver_maj = PRESTERA_SUPP_FW_MAJ_VER;
-	int ver_min = PRESTERA_SUPP_FW_MIN_VER;
 	char fw_path[128];
 	int err;
+	int i;
 
-pick_fw_ver:
-	snprintf(fw_path, sizeof(fw_path), PRESTERA_FW_PATH_FMT,
-		 ver_maj, ver_min);
-
-	err = request_firmware_direct(&fw->bin, fw_path, fw->dev.dev);
-	if (err) {
-		if (ver_maj == PRESTERA_SUPP_FW_MAJ_VER) {
-			ver_maj = PRESTERA_PREV_FW_MAJ_VER;
-			ver_min = PRESTERA_PREV_FW_MIN_VER;
+	for (i = 0; i < ARRAY_SIZE(prestera_fw_supp); i++) {
+		struct prestera_fw_rev *ver = &prestera_fw_supp[i];
 
-			dev_warn(fw->dev.dev,
-				 "missing latest %s firmware, fall-back to previous %u.%u version\n",
-				 fw_path, ver_maj, ver_min);
+		snprintf(fw_path, sizeof(fw_path), PRESTERA_FW_PATH_FMT,
+			 ver->maj, ver->min);
 
-			goto pick_fw_ver;
-		} else {
-			dev_err(fw->dev.dev, "failed to request previous firmware: %s\n",
-				fw_path);
-			return err;
+		err = request_firmware_direct(&fw->bin, fw_path, fw->dev.dev);
+		if (!err) {
+			dev_info(fw->dev.dev, "Loading %s ...", fw_path);
+			fw->rev_supp = *ver;
+			return 0;
 		}
-	}
-
-	dev_info(fw->dev.dev, "Loading %s ...", fw_path);
 
-	fw->rev_supp.maj = ver_maj;
-	fw->rev_supp.min = ver_min;
-	fw->rev_supp.sub = 0;
+		if (i == 0)
+			dev_warn(fw->dev.dev,
+				 "missing latest %s firmware, fall-back to previous version\n",
+				 fw_path);
+		else
+			dev_warn(fw->dev.dev, "failed to request previous firmware: %s\n",
+				 fw_path);
+	}
 
-	return 0;
+	dev_err(fw->dev.dev, "could not find any of the supported firmware versions\n");
+	return -ENOENT;
 }
 
 static void prestera_fw_put(struct prestera_fw *fw)
-- 
2.17.1

[RFC net-next 3/4] net: sched: introduce __tc_classid_to_hwtc() helper

[Vadym Kochan]

From: Vadym Kochan <vkochan@marvell.com>

There might be a case when the ingress HW queues are globally shared in
ASIC and are not per port (netdev). So add a __tc_classid_to_hwtc()
version which accepts number of tc instead of netdev.

Signed-off-by: Vadym Kochan <vkochan@marvell.com>
---
 include/net/sch_generic.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 9ed33e6840bd..b6e65658b0d8 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -668,11 +668,16 @@ qdisc_class_find(const struct Qdisc_class_hash *hash, u32 id)
 	return NULL;
 }
 
-static inline int tc_classid_to_hwtc(struct net_device *dev, u32 classid)
+static inline int __tc_classid_to_hwtc(u32 tc_num, u32 classid)
 {
 	u32 hwtc = TC_H_MIN(classid) - TC_H_MIN_PRIORITY;
 
-	return (hwtc < netdev_get_num_tc(dev)) ? hwtc : -EINVAL;
+	return (hwtc < tc_num) ? hwtc : -EINVAL;
+}
+
+static inline int tc_classid_to_hwtc(struct net_device *dev, u32 classid)
+{
+	return __tc_classid_to_hwtc(netdev_get_num_tc(dev), classid);
 }
 
 int qdisc_class_hash_init(struct Qdisc_class_hash *);
-- 
2.17.1

[RFC net-next 4/4] net: marvell: prestera: Offload FLOW_ACTION_POLICE

[Vadym Kochan]

From: Serhiy Boiko <serhiy.boiko@plvision.eu>

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 <vmytnyk@marvell.com>
Signed-off-by: Volodymyr Mytnyk <vmytnyk@marvell.com>
Signed-off-by: Serhiy Boiko <serhiy.boiko@plvision.eu>
Co-developed-by: Vadym Kochan <vkochan@marvell.com>
Signed-off-by: Vadym Kochan <vkochan@marvell.com>
---
 .../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..2d1dfb52aca4 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)
+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;
 }
 
+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

Re: [RFC net-next 0/4] Marvell Prestera add policer support

[Andrew Lunn]

On Fri, Jul 02, 2021 at 09:29:11PM +0300, Vadym Kochan wrote:
> From: Vadym Kochan <vkochan@marvell.com>
> 
> 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. Because there are some FW ABI
> differences in ACL rule messages between 3.0 and 3.1 so added separate
> "_ext" struct version with separate HW helper.

This driver is less than a year old, and it is on its third ABI break?
It is accumulating more and more cruft as you need to handle old and
new messages. Maybe you should take a harder look into your crystal
ball and try to figure out an ABI which you can use for 12 months or
more? Or just directly address the hardware, and skip the firmware?

      Andrew

Re: [RFC net-next 0/4] Marvell Prestera add policer support

[Vadym Kochan]

Hi Andrew,

On Fri, Jul 02, 2021 at 08:52:43PM +0200, Andrew Lunn wrote:
> On Fri, Jul 02, 2021 at 09:29:11PM +0300, Vadym Kochan wrote:
> > From: Vadym Kochan <vkochan@marvell.com>
> > 
> > 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. Because there are some FW ABI
> > differences in ACL rule messages between 3.0 and 3.1 so added separate
> > "_ext" struct version with separate HW helper.
> 
> This driver is less than a year old, and it is on its third ABI break?
> It is accumulating more and more cruft as you need to handle old and
> new messages. Maybe you should take a harder look into your crystal
> ball and try to figure out an ABI which you can use for 12 months or
> more? Or just directly address the hardware, and skip the firmware?
> 
>       Andrew

I thought (considering the latest discussion about latest FW PULL
REQUEST) it will be not a problem to update FW (and adapt the driver)
quite often during the initial feature bring-up (actually the older
supported FW code will be removed in the driver after some time). If it
is the problem, then probably it makes sense to first add new FW 4.x
with much more features and after add support of these features in the
driver step-by-step.