[PATCH net-next] net: prestera: flower template support

[PATCH net-next] net: prestera: flower template support

[Volodymyr Mytnyk]

From: Volodymyr Mytnyk <vmytnyk@marvell.com>

Add user template explicit support. At this moment, max
TCAM rule size is utilized for all rules, doesn't matter
which and how much flower matches are provided by user. It
means that some of TCAM space is wasted, which impacts
the number of filters that can be offloaded.

Introducing the template, allows to have more HW offloaded
filters.

Example:
  tc qd add dev PORT clsact
  tc chain add dev PORT ingress protocol ip \
    flower dst_ip 0.0.0.0/16
  tc filter add dev PORT ingress protocol ip \
    flower skip_sw dst_ip 1.2.3.4/16 action drop

Signed-off-by: Volodymyr Mytnyk <vmytnyk@marvell.com>
---
 .../net/ethernet/marvell/prestera/prestera_acl.c   | 60 ++++++++++++++++++-
 .../net/ethernet/marvell/prestera/prestera_acl.h   |  2 +
 .../net/ethernet/marvell/prestera/prestera_flow.c  |  7 +++
 .../net/ethernet/marvell/prestera/prestera_flow.h  |  4 +-
 .../ethernet/marvell/prestera/prestera_flower.c    | 70 ++++++++++++++++++++++
 .../ethernet/marvell/prestera/prestera_flower.h    |  5 ++
 6 files changed, 145 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_acl.c b/drivers/net/ethernet/marvell/prestera/prestera_acl.c
index da0b6525ef9a..1371c1ae59a3 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_acl.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_acl.c
@@ -88,8 +88,8 @@ prestera_acl_ruleset_create(struct prestera_acl *acl,
 			    struct prestera_flow_block *block)
 {
 	struct prestera_acl_ruleset *ruleset;
+	u32 uid = 0;
 	int err;
-	u32 uid;
 
 	ruleset = kzalloc(sizeof(*ruleset), GFP_KERNEL);
 	if (!ruleset)
@@ -125,6 +125,12 @@ prestera_acl_ruleset_create(struct prestera_acl *acl,
 	return ERR_PTR(err);
 }
 
+void prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
+				      void *keymask)
+{
+	ruleset->keymask = kmemdup(keymask, ACL_KEYMASK_SIZE, GFP_KERNEL);
+}
+
 int prestera_acl_ruleset_offload(struct prestera_acl_ruleset *ruleset)
 {
 	u32 vtcam_id;
@@ -559,6 +565,49 @@ prestera_acl_rule_entry_create(struct prestera_acl *acl,
 	return NULL;
 }
 
+static int __prestera_acl_vtcam_id_try_fit(struct prestera_acl *acl, u8 lookup,
+					   void *keymask, u32 *vtcam_id)
+{
+	struct prestera_acl_vtcam *vtcam;
+	int i;
+
+	list_for_each_entry(vtcam, &acl->vtcam_list, list) {
+		if (lookup != vtcam->lookup)
+			continue;
+
+		if (!keymask && !vtcam->is_keymask_set)
+			goto vtcam_found;
+
+		if (!(keymask && vtcam->is_keymask_set))
+			continue;
+
+		/* try to fit with vtcam keymask */
+		for (i = 0; i < __PRESTERA_ACL_RULE_MATCH_TYPE_MAX; i++) {
+			__be32 __keymask = ((__be32 *)keymask)[i];
+
+			if (!__keymask)
+				/* vtcam keymask in not interested */
+				continue;
+
+			if (__keymask & ~vtcam->keymask[i])
+				/* keymask does not fit the vtcam keymask */
+				break;
+		}
+
+		if (i == __PRESTERA_ACL_RULE_MATCH_TYPE_MAX)
+			/* keymask fits vtcam keymask, return it */
+			goto vtcam_found;
+	}
+
+	/* nothing is found */
+	return -ENOENT;
+
+vtcam_found:
+	refcount_inc(&vtcam->refcount);
+	*vtcam_id = vtcam->id;
+	return 0;
+}
+
 int prestera_acl_vtcam_id_get(struct prestera_acl *acl, u8 lookup,
 			      void *keymask, u32 *vtcam_id)
 {
@@ -595,7 +644,14 @@ int prestera_acl_vtcam_id_get(struct prestera_acl *acl, u8 lookup,
 				       PRESTERA_HW_VTCAM_DIR_INGRESS);
 	if (err) {
 		kfree(vtcam);
-		return err;
+
+		/* cannot create new, try to fit into existing vtcam */
+		if (__prestera_acl_vtcam_id_try_fit(acl, lookup,
+						    keymask, &new_vtcam_id))
+			return err;
+
+		*vtcam_id = new_vtcam_id;
+		return 0;
 	}
 
 	vtcam->id = new_vtcam_id;
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_acl.h b/drivers/net/ethernet/marvell/prestera/prestera_acl.h
index 4e6006b4531f..40f6c1d961fa 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_acl.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera_acl.h
@@ -151,6 +151,8 @@ prestera_acl_ruleset_get(struct prestera_acl *acl,
 struct prestera_acl_ruleset *
 prestera_acl_ruleset_lookup(struct prestera_acl *acl,
 			    struct prestera_flow_block *block);
+void prestera_acl_ruleset_keymask_set(struct prestera_acl_ruleset *ruleset,
+				      void *keymask);
 bool prestera_acl_ruleset_is_offload(struct prestera_acl_ruleset *ruleset);
 int prestera_acl_ruleset_offload(struct prestera_acl_ruleset *ruleset);
 void prestera_acl_ruleset_put(struct prestera_acl_ruleset *ruleset);
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_flow.c b/drivers/net/ethernet/marvell/prestera/prestera_flow.c
index 94a1feb3d9e1..d849f046ece7 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_flow.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_flow.c
@@ -40,6 +40,11 @@ static int prestera_flow_block_flower_cb(struct prestera_flow_block *block,
 		return 0;
 	case FLOW_CLS_STATS:
 		return prestera_flower_stats(block, f);
+	case FLOW_CLS_TMPLT_CREATE:
+		return prestera_flower_tmplt_create(block, f);
+	case FLOW_CLS_TMPLT_DESTROY:
+		prestera_flower_tmplt_destroy(block, f);
+		return 0;
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -64,6 +69,8 @@ static void prestera_flow_block_destroy(void *cb_priv)
 {
 	struct prestera_flow_block *block = cb_priv;
 
+	prestera_flower_template_cleanup(block);
+
 	WARN_ON(!list_empty(&block->binding_list));
 
 	kfree(block);
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_flow.h b/drivers/net/ethernet/marvell/prestera/prestera_flow.h
index 5863acf06005..1ea5b745bf72 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_flow.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera_flow.h
@@ -8,6 +8,7 @@
 
 struct prestera_port;
 struct prestera_switch;
+struct prestera_flower_template;
 
 struct prestera_flow_block_binding {
 	struct list_head list;
@@ -18,10 +19,11 @@ struct prestera_flow_block_binding {
 struct prestera_flow_block {
 	struct list_head binding_list;
 	struct prestera_switch *sw;
-	unsigned int rule_count;
 	struct net *net;
 	struct prestera_acl_ruleset *ruleset_zero;
 	struct flow_block_cb *block_cb;
+	struct prestera_flower_template *tmplt;
+	unsigned int rule_count;
 };
 
 int prestera_flow_block_setup(struct prestera_port *port,
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_flower.c b/drivers/net/ethernet/marvell/prestera/prestera_flower.c
index c1dc4e49b07f..19c1417fd05f 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_flower.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_flower.c
@@ -6,6 +6,21 @@
 #include "prestera_flow.h"
 #include "prestera_flower.h"
 
+struct prestera_flower_template {
+	struct prestera_acl_ruleset *ruleset;
+};
+
+void prestera_flower_template_cleanup(struct prestera_flow_block *block)
+{
+	if (block->tmplt) {
+		/* put the reference to the ruleset kept in create */
+		prestera_acl_ruleset_put(block->tmplt->ruleset);
+		kfree(block->tmplt);
+		block->tmplt = NULL;
+		return;
+	}
+}
+
 static int prestera_flower_parse_actions(struct prestera_flow_block *block,
 					 struct prestera_acl_rule *rule,
 					 struct flow_action *flow_action,
@@ -310,6 +325,61 @@ void prestera_flower_destroy(struct prestera_flow_block *block,
 
 }
 
+int prestera_flower_tmplt_create(struct prestera_flow_block *block,
+				 struct flow_cls_offload *f)
+{
+	struct prestera_flower_template *template;
+	struct prestera_acl_ruleset *ruleset;
+	struct prestera_acl_rule rule;
+	int err;
+
+	memset(&rule, 0, sizeof(rule));
+	err = prestera_flower_parse(block, &rule, f);
+	if (err)
+		return err;
+
+	template = kmalloc(sizeof(*template), GFP_KERNEL);
+	if (!template) {
+		err = -ENOMEM;
+		goto err_malloc;
+	}
+
+	prestera_acl_rule_keymask_pcl_id_set(&rule, 0);
+	ruleset = prestera_acl_ruleset_get(block->sw->acl, block);
+	if (IS_ERR_OR_NULL(ruleset)) {
+		err = -EINVAL;
+		goto err_ruleset_get;
+	}
+
+	/* preserve keymask/template to this ruleset */
+	prestera_acl_ruleset_keymask_set(ruleset, rule.re_key.match.mask);
+
+	/* skip error, as it is not possible to reject template operation,
+	 * so, keep the reference to the ruleset for rules to be added
+	 * to that ruleset later. In case of offload fail, the ruleset
+	 * will be offloaded again during adding a new rule. Also,
+	 * unlikly possble that ruleset is already offloaded at this staage.
+	 */
+	prestera_acl_ruleset_offload(ruleset);
+
+	/* keep the reference to the ruleset */
+	template->ruleset = ruleset;
+	block->tmplt = template;
+	return 0;
+
+err_ruleset_get:
+	kfree(template);
+err_malloc:
+	NL_SET_ERR_MSG_MOD(f->common.extack, "Create chain template failed");
+	return err;
+}
+
+void prestera_flower_tmplt_destroy(struct prestera_flow_block *block,
+				   struct flow_cls_offload *f)
+{
+	prestera_flower_template_cleanup(block);
+}
+
 int prestera_flower_stats(struct prestera_flow_block *block,
 			  struct flow_cls_offload *f)
 {
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_flower.h b/drivers/net/ethernet/marvell/prestera/prestera_flower.h
index c6182473efa5..dc3aa4280e9f 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_flower.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera_flower.h
@@ -15,5 +15,10 @@ void prestera_flower_destroy(struct prestera_flow_block *block,
 			     struct flow_cls_offload *f);
 int prestera_flower_stats(struct prestera_flow_block *block,
 			  struct flow_cls_offload *f);
+int prestera_flower_tmplt_create(struct prestera_flow_block *block,
+				 struct flow_cls_offload *f);
+void prestera_flower_tmplt_destroy(struct prestera_flow_block *block,
+				   struct flow_cls_offload *f);
+void prestera_flower_template_cleanup(struct prestera_flow_block *block);
 
 #endif /* _PRESTERA_FLOWER_H_ */
-- 
2.7.4

Re: [PATCH net-next] net: prestera: flower template support

[Jamal Hadi Salim]

On 2021-12-02 10:50, Volodymyr Mytnyk wrote:
> From: Volodymyr Mytnyk<vmytnyk@marvell.com>
> 
> Add user template explicit support. At this moment, max
> TCAM rule size is utilized for all rules, doesn't matter
> which and how much flower matches are provided by user. It
> means that some of TCAM space is wasted, which impacts
> the number of filters that can be offloaded.
> 
> Introducing the template, allows to have more HW offloaded
> filters.
> 
> Example:
>    tc qd add dev PORT clsact
>    tc chain add dev PORT ingress protocol ip \
>      flower dst_ip 0.0.0.0/16

"chain" or "filter"?

>    tc filter add dev PORT ingress protocol ip \
>      flower skip_sw dst_ip 1.2.3.4/16 action drop

You are not using tc priority? Above will result in two priorities
(the 0.0.0.0 entry will be more important) and in classical flower
approach two  different tables.
I am wondering how you map the table to the TCAM.
Is the priority sorting entirely based on masks in hardware?

cheers,
jamal

RE: [PATCH net-next] net: prestera: flower template support

[Volodymyr Mytnyk]

Hi Jamal,

>
> > From: Volodymyr Mytnyk<vmytnyk@marvell.com>
> > 
> > Add user template explicit support. At this moment, max TCAM rule size 
> > is utilized for all rules, doesn't matter which and how much flower 
> > matches are provided by user. It means that some of TCAM space is 
> > wasted, which impacts the number of filters that can be offloaded.
> > 
> > Introducing the template, allows to have more HW offloaded filters.
> > 
> > Example:
> >    tc qd add dev PORT clsact
> >    tc chain add dev PORT ingress protocol ip \
> >      flower dst_ip 0.0.0.0/16
> 
> "chain" or "filter"?

tc chain add ... flower [tempalte] is the command to add explicitly chain with a given template

tc filter ... is the command to add a filter itself in that chain

> 
> >    tc filter add dev PORT ingress protocol ip \
> >      flower skip_sw dst_ip 1.2.3.4/16 action drop
> 
> You are not using tc priority? Above will result in two priorities (the 0.0.0.0 entry will be more important) and in classical flower approach two  different tables.
> I am wondering how you map the table to the TCAM.
> Is the priority sorting entirely based on masks in hardware?

Kernel tc filter priority is used as a priority for HW rule (see flower implementation).

> 
> cheers,
> jamal

Regards,
  Volodymyr

Re: [PATCH net-next] net: prestera: flower template support

[Jamal Hadi Salim]

On 2021-12-02 12:39, Volodymyr Mytnyk wrote:
> Hi Jamal,
> 
>>
>>> From: Volodymyr Mytnyk<vmytnyk@marvell.com>
>>>
>>> Add user template explicit support. At this moment, max TCAM rule size
>>> is utilized for all rules, doesn't matter which and how much flower
>>> matches are provided by user. It means that some of TCAM space is
>>> wasted, which impacts the number of filters that can be offloaded.
>>>
>>> Introducing the template, allows to have more HW offloaded filters.
>>>
>>> Example:
>>>     tc qd add dev PORT clsact
>>>     tc chain add dev PORT ingress protocol ip \
>>>       flower dst_ip 0.0.0.0/16
>>
>> "chain" or "filter"?
> 
> tc chain add ... flower [tempalte] is the command to add explicitly chain with a given template
> 

I guess you are enforcing the template on chain 0. My brain
was  expecting chain id to be called out.


> tc filter ... is the command to add a filter itself in that chain
> 

Got it.


>> You are not using tc priority? Above will result in two priorities (the 0.0.0.0 entry will be more important) and in classical flower approach two  different tables.
>> I am wondering how you map the table to the TCAM.
>> Is the priority sorting entirely based on masks in hardware?
> 
> Kernel tc filter priority is used as a priority for HW rule (see flower implementation).

The TCAM however should be able to accept many masks - is the idea
here to enforce some mask per chain and then have priority being the
priorities handle conflict? What happens when you explicitly specify
priority. If you dont specify it the kernel provides it and essentially
resolution is based on the order in which the rules are entered..

cheers,
jamal

Re: [PATCH net-next] net: prestera: flower template support

[Volodymyr Mytnyk]

Hi Jamal,

>
> > Hi Jamal,
> > 
> >>
> >>> From: Volodymyr Mytnyk<vmytnyk@marvell.com>
> >>>
> >>> Add user template explicit support. At this moment, max TCAM rule size
> >>> is utilized for all rules, doesn't matter which and how much flower
> >>> matches are provided by user. It means that some of TCAM space is
> >>> wasted, which impacts the number of filters that can be offloaded.
> >>>
> >>> Introducing the template, allows to have more HW offloaded filters.
> >>>
> >>> Example:
> >>>     tc qd add dev PORT clsact
> >>>     tc chain add dev PORT ingress protocol ip \
> >>>       flower dst_ip 0.0.0.0/16
> >>
> >> "chain" or "filter"?
> > 
> > tc chain add ... flower [tempalte] is the command to add explicitly chain with a given template
> > 
> 
> I guess you are enforcing the template on chain 0. My brain
> was  expecting chain id to be called out.
> 

chain 0 is the default chain id for "tc chain" & "tc filter" command,
so, that's why I did not mention it in the command line. Please note,
this patch adds only template support. Chains are not supported yet,
and will be added later.

> 
> > tc filter ... is the command to add a filter itself in that chain
> > 
> 
> Got it.
> 
> 
> >> You are not using tc priority? Above will result in two priorities (the 0.0.0.0 entry will be more important) and in classical flower approach two  different tables.
> >> I am wondering how you map the table to the TCAM.
> >> Is the priority sorting entirely based on masks in hardware?
> > 
> > Kernel tc filter priority is used as a priority for HW rule (see flower implementation).
> 
> The TCAM however should be able to accept many masks - is the idea
> here to enforce some mask per chain and then have priority being the
> priorities handle conflict? What happens when you explicitly specify
> priority. If you dont specify it the kernel provides it and essentially
> resolution is based on the order in which the rules are entered..

The HW rule insert/delete into TCAM is done by the FW itself. It means,
that the FW will take care about prio and (re)order the rule based on the
priority provided by user/kernel. So, kernel driver just need to provide
prio to the FW when adding the rule into the HW.

> 
> cheers,
> jamal

Thanks and Regards,
  Volodymyr