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

This commit adds more clarity to handling of TC_CLSMATCHALL_REPLACE and
TC_CLSMATCHALL_DESTROY events by calling newly added *_mall_*() handlers
instead of directly calling SPAN API.

This also extracts matchall rules management out of SPAN API since SPAN
is a hardware module which is used to implement 'matchall egress mirred'
action only.

Signed-off-by: Taras Chornyi <tchornyi@marvell.com>
Signed-off-by: Serhiy Boiko <serhiy.boiko@plvision.eu>
Signed-off-by: Maksym Glubokiy <maksym.glubokiy@plvision.eu>
---
 .../net/ethernet/marvell/prestera/Makefile    |  2 +-
 .../ethernet/marvell/prestera/prestera_flow.c |  9 +--
 .../marvell/prestera/prestera_matchall.c      | 66 +++++++++++++++++++
 .../marvell/prestera/prestera_matchall.h      | 15 +++++
 .../ethernet/marvell/prestera/prestera_span.c | 60 +----------------
 .../ethernet/marvell/prestera/prestera_span.h | 10 +--
 6 files changed, 96 insertions(+), 66 deletions(-)
 create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_matchall.c
 create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_matchall.h

diff --git a/drivers/net/ethernet/marvell/prestera/Makefile b/drivers/net/ethernet/marvell/prestera/Makefile
index d395f4131648..df14cee80153 100644
--- a/drivers/net/ethernet/marvell/prestera/Makefile
+++ b/drivers/net/ethernet/marvell/prestera/Makefile
@@ -4,6 +4,6 @@ prestera-objs		:= prestera_main.o prestera_hw.o prestera_dsa.o \
 			   prestera_rxtx.o prestera_devlink.o prestera_ethtool.o \
 			   prestera_switchdev.o prestera_acl.o prestera_flow.o \
 			   prestera_flower.o prestera_span.o prestera_counter.o \
-			   prestera_router.o prestera_router_hw.o
+			   prestera_router.o prestera_router_hw.o prestera_matchall.o
 
 obj-$(CONFIG_PRESTERA_PCI)	+= prestera_pci.o
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_flow.c b/drivers/net/ethernet/marvell/prestera/prestera_flow.c
index 2262693bd5cf..3f81eef167fa 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_flow.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_flow.c
@@ -7,8 +7,9 @@
 #include "prestera.h"
 #include "prestera_acl.h"
 #include "prestera_flow.h"
-#include "prestera_span.h"
 #include "prestera_flower.h"
+#include "prestera_matchall.h"
+#include "prestera_span.h"
 
 static LIST_HEAD(prestera_block_cb_list);
 
@@ -17,9 +18,9 @@ static int prestera_flow_block_mall_cb(struct prestera_flow_block *block,
 {
 	switch (f->command) {
 	case TC_CLSMATCHALL_REPLACE:
-		return prestera_span_replace(block, f);
+		return prestera_mall_replace(block, f);
 	case TC_CLSMATCHALL_DESTROY:
-		prestera_span_destroy(block);
+		prestera_mall_destroy(block);
 		return 0;
 	default:
 		return -EOPNOTSUPP;
@@ -263,7 +264,7 @@ static void prestera_setup_flow_block_unbind(struct prestera_port *port,
 
 	block = flow_block_cb_priv(block_cb);
 
-	prestera_span_destroy(block);
+	prestera_mall_destroy(block);
 
 	err = prestera_flow_block_unbind(block, port);
 	if (err)
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_matchall.c b/drivers/net/ethernet/marvell/prestera/prestera_matchall.c
new file mode 100644
index 000000000000..54573c6a6fe2
--- /dev/null
+++ b/drivers/net/ethernet/marvell/prestera/prestera_matchall.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
+/* Copyright (c) 2019-2022 Marvell International Ltd. All rights reserved */
+
+#include <linux/kernel.h>
+#include <linux/list.h>
+
+#include "prestera.h"
+#include "prestera_hw.h"
+#include "prestera_flow.h"
+#include "prestera_flower.h"
+#include "prestera_matchall.h"
+#include "prestera_span.h"
+
+int prestera_mall_replace(struct prestera_flow_block *block,
+			  struct tc_cls_matchall_offload *f)
+{
+	struct prestera_flow_block_binding *binding;
+	__be16 protocol = f->common.protocol;
+	struct flow_action_entry *act;
+	struct prestera_port *port;
+	int err;
+
+	if (!flow_offload_has_one_action(&f->rule->action)) {
+		NL_SET_ERR_MSG(f->common.extack,
+			       "Only singular actions are supported");
+		return -EOPNOTSUPP;
+	}
+
+	act = &f->rule->action.entries[0];
+
+	if (!prestera_netdev_check(act->dev)) {
+		NL_SET_ERR_MSG(f->common.extack,
+			       "Only Marvell Prestera port is supported");
+		return -EINVAL;
+	}
+	if (!tc_cls_can_offload_and_chain0(act->dev, &f->common))
+		return -EOPNOTSUPP;
+	if (act->id != FLOW_ACTION_MIRRED)
+		return -EOPNOTSUPP;
+	if (protocol != htons(ETH_P_ALL))
+		return -EOPNOTSUPP;
+
+	port = netdev_priv(act->dev);
+
+	list_for_each_entry(binding, &block->binding_list, list) {
+		err = prestera_span_rule_add(binding, port);
+		if (err)
+			goto rollback;
+	}
+
+	return 0;
+
+rollback:
+	list_for_each_entry_continue_reverse(binding,
+					     &block->binding_list, list)
+		prestera_span_rule_del(binding);
+	return err;
+}
+
+void prestera_mall_destroy(struct prestera_flow_block *block)
+{
+	struct prestera_flow_block_binding *binding;
+
+	list_for_each_entry(binding, &block->binding_list, list)
+		prestera_span_rule_del(binding);
+}
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_matchall.h b/drivers/net/ethernet/marvell/prestera/prestera_matchall.h
new file mode 100644
index 000000000000..31ad4d02ecbb
--- /dev/null
+++ b/drivers/net/ethernet/marvell/prestera/prestera_matchall.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
+/* Copyright (c) 2022 Marvell International Ltd. All rights reserved. */
+
+#ifndef _PRESTERA_MATCHALL_H_
+#define _PRESTERA_MATCHALL_H_
+
+#include <net/pkt_cls.h>
+
+struct prestera_flow_block;
+
+int prestera_mall_replace(struct prestera_flow_block *block,
+			  struct tc_cls_matchall_offload *f);
+void prestera_mall_destroy(struct prestera_flow_block *block);
+
+#endif /* _PRESTERA_MATCHALL_H_ */
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_span.c b/drivers/net/ethernet/marvell/prestera/prestera_span.c
index 845e9d8c8cc7..766413b9ba1b 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_span.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_span.c
@@ -120,8 +120,8 @@ static int prestera_span_put(struct prestera_switch *sw, u8 span_id)
 	return 0;
 }
 
-static int prestera_span_rule_add(struct prestera_flow_block_binding *binding,
-				  struct prestera_port *to_port)
+int prestera_span_rule_add(struct prestera_flow_block_binding *binding,
+			   struct prestera_port *to_port)
 {
 	struct prestera_switch *sw = binding->port->sw;
 	u8 span_id;
@@ -145,7 +145,7 @@ static int prestera_span_rule_add(struct prestera_flow_block_binding *binding,
 	return 0;
 }
 
-static int prestera_span_rule_del(struct prestera_flow_block_binding *binding)
+int prestera_span_rule_del(struct prestera_flow_block_binding *binding)
 {
 	int err;
 
@@ -161,60 +161,6 @@ static int prestera_span_rule_del(struct prestera_flow_block_binding *binding)
 	return 0;
 }
 
-int prestera_span_replace(struct prestera_flow_block *block,
-			  struct tc_cls_matchall_offload *f)
-{
-	struct prestera_flow_block_binding *binding;
-	__be16 protocol = f->common.protocol;
-	struct flow_action_entry *act;
-	struct prestera_port *port;
-	int err;
-
-	if (!flow_offload_has_one_action(&f->rule->action)) {
-		NL_SET_ERR_MSG(f->common.extack,
-			       "Only singular actions are supported");
-		return -EOPNOTSUPP;
-	}
-
-	act = &f->rule->action.entries[0];
-
-	if (!prestera_netdev_check(act->dev)) {
-		NL_SET_ERR_MSG(f->common.extack,
-			       "Only Marvell Prestera port is supported");
-		return -EINVAL;
-	}
-	if (!tc_cls_can_offload_and_chain0(act->dev, &f->common))
-		return -EOPNOTSUPP;
-	if (act->id != FLOW_ACTION_MIRRED)
-		return -EOPNOTSUPP;
-	if (protocol != htons(ETH_P_ALL))
-		return -EOPNOTSUPP;
-
-	port = netdev_priv(act->dev);
-
-	list_for_each_entry(binding, &block->binding_list, list) {
-		err = prestera_span_rule_add(binding, port);
-		if (err)
-			goto rollback;
-	}
-
-	return 0;
-
-rollback:
-	list_for_each_entry_continue_reverse(binding,
-					     &block->binding_list, list)
-		prestera_span_rule_del(binding);
-	return err;
-}
-
-void prestera_span_destroy(struct prestera_flow_block *block)
-{
-	struct prestera_flow_block_binding *binding;
-
-	list_for_each_entry(binding, &block->binding_list, list)
-		prestera_span_rule_del(binding);
-}
-
 int prestera_span_init(struct prestera_switch *sw)
 {
 	struct prestera_span *span;
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_span.h b/drivers/net/ethernet/marvell/prestera/prestera_span.h
index f0644521f78a..4958ce820b52 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_span.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera_span.h
@@ -8,13 +8,15 @@
 
 #define PRESTERA_SPAN_INVALID_ID -1
 
+struct prestera_port;
 struct prestera_switch;
-struct prestera_flow_block;
+struct prestera_flow_block_binding;
 
 int prestera_span_init(struct prestera_switch *sw);
 void prestera_span_fini(struct prestera_switch *sw);
-int prestera_span_replace(struct prestera_flow_block *block,
-			  struct tc_cls_matchall_offload *f);
-void prestera_span_destroy(struct prestera_flow_block *block);
+
+int prestera_span_rule_add(struct prestera_flow_block_binding *binding,
+			   struct prestera_port *to_port);
+int prestera_span_rule_del(struct prestera_flow_block_binding *binding);
 
 #endif /* _PRESTERA_SPAN_H_ */
-- 
2.25.1

Hello,

On Tue, 2022-08-23 at 14:39 +0300, Maksym Glubokiy wrote:
> From: Serhiy Boiko <serhiy.boiko@plvision.eu>
> 
> This commit adds more clarity to handling of TC_CLSMATCHALL_REPLACE and
> TC_CLSMATCHALL_DESTROY events by calling newly added *_mall_*() handlers
> instead of directly calling SPAN API.
> 
> This also extracts matchall rules management out of SPAN API since SPAN
> is a hardware module which is used to implement 'matchall egress mirred'
> action only.
> 
> Signed-off-by: Taras Chornyi <tchornyi@marvell.com>
> Signed-off-by: Serhiy Boiko <serhiy.boiko@plvision.eu>
> Signed-off-by: Maksym Glubokiy <maksym.glubokiy@plvision.eu>

This SoB chain is not clear to me. Did Taras co-developed the patch? In
that case a Co-developed-by: tag is missing, as the first tag in the
list. Otherwise why is Taras' SoB there?

Thanks!

Paolo

On Thu, Aug 25, 2022 at 01:11:35PM +0200, Paolo Abeni wrote:
> Hello,
> 
> On Tue, 2022-08-23 at 14:39 +0300, Maksym Glubokiy wrote:
> > From: Serhiy Boiko <serhiy.boiko@plvision.eu>
> > 
> > This commit adds more clarity to handling of TC_CLSMATCHALL_REPLACE and
> > TC_CLSMATCHALL_DESTROY events by calling newly added *_mall_*() handlers
> > instead of directly calling SPAN API.
> > 
> > This also extracts matchall rules management out of SPAN API since SPAN
> > is a hardware module which is used to implement 'matchall egress mirred'
> > action only.
> > 
> > Signed-off-by: Taras Chornyi <tchornyi@marvell.com>
> > Signed-off-by: Serhiy Boiko <serhiy.boiko@plvision.eu>
> > Signed-off-by: Maksym Glubokiy <maksym.glubokiy@plvision.eu>
> 
> This SoB chain is not clear to me. Did Taras co-developed the patch? In
> that case a Co-developed-by: tag is missing, as the first tag in the
> list. Otherwise why is Taras' SoB there?
There are new files being added with Marvel's copyright and my understanding is
that there must be someone from Marvel on SoB list in such a case.