diff mbox series

[net-next,v2,6/8] net: microchip: vcap api: Check chains when adding a tc flower filter

Message ID 20230106085317.1720282-7-steen.hegelund@microchip.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Add support for two classes of VCAP rules | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 2 this patch: 2
netdev/cc_maintainers success CCed 11 of 11 maintainers
netdev/build_clang success Errors and warnings before: 1 this patch: 1
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 2 this patch: 2
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 219 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Steen Hegelund Jan. 6, 2023, 8:53 a.m. UTC
This changes the way the chain information verified when adding a new tc
flower filter.

When adding a flower filter it is now checked that the filter contains a
goto action to one of the IS2 VCAP lookups, except for the last lookup
which may omit this goto action.

It is also checked if you attempt to add multiple matchall filters to
enable the same VCAP lookup.  This will be rejected.

Fixes: 4426b78c626d ("net: lan966x: Add port keyset config and callback interface")
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
---
 .../microchip/lan966x/lan966x_tc_flower.c     | 30 +++++-----
 .../microchip/sparx5/sparx5_tc_flower.c       | 28 +++++----
 .../net/ethernet/microchip/vcap/vcap_api.c    | 59 +++++++++++--------
 .../ethernet/microchip/vcap/vcap_api_client.h |  2 +
 .../ethernet/microchip/vcap/vcap_api_kunit.c  |  8 +--
 5 files changed, 72 insertions(+), 55 deletions(-)
diff mbox series

Patch

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c b/drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c
index ba3fa917d6b7..b66a8725a071 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c
@@ -82,8 +82,8 @@  static int lan966x_tc_flower_use_dissectors(struct flow_cls_offload *f,
 }
 
 static int lan966x_tc_flower_action_check(struct vcap_control *vctrl,
-					  struct flow_cls_offload *fco,
-					  struct vcap_admin *admin)
+					  struct net_device *dev,
+					  struct flow_cls_offload *fco)
 {
 	struct flow_rule *rule = flow_cls_offload_flow_rule(fco);
 	struct flow_action_entry *actent, *last_actent = NULL;
@@ -109,21 +109,23 @@  static int lan966x_tc_flower_action_check(struct vcap_control *vctrl,
 		last_actent = actent; /* Save last action for later check */
 	}
 
-	/* Check that last action is a goto */
-	if (last_actent->id != FLOW_ACTION_GOTO) {
+	/* Check that last action is a goto
+	 * The last chain/lookup does not need to have goto action
+	 */
+	if (last_actent->id == FLOW_ACTION_GOTO) {
+		/* Check if the destination chain is in one of the VCAPs */
+		if (!vcap_is_next_lookup(vctrl, fco->common.chain_index,
+					 last_actent->chain_index)) {
+			NL_SET_ERR_MSG_MOD(fco->common.extack,
+					   "Invalid goto chain");
+			return -EINVAL;
+		}
+	} else if (!vcap_is_last_chain(vctrl, fco->common.chain_index)) {
 		NL_SET_ERR_MSG_MOD(fco->common.extack,
 				   "Last action must be 'goto'");
 		return -EINVAL;
 	}
 
-	/* Check if the goto chain is in the next lookup */
-	if (!vcap_is_next_lookup(vctrl, fco->common.chain_index,
-				 last_actent->chain_index)) {
-		NL_SET_ERR_MSG_MOD(fco->common.extack,
-				   "Invalid goto chain");
-		return -EINVAL;
-	}
-
 	/* Catch unsupported combinations of actions */
 	if (action_mask & BIT(FLOW_ACTION_TRAP) &&
 	    action_mask & BIT(FLOW_ACTION_ACCEPT)) {
@@ -145,8 +147,8 @@  static int lan966x_tc_flower_add(struct lan966x_port *port,
 	struct vcap_rule *vrule;
 	int err, idx;
 
-	err = lan966x_tc_flower_action_check(port->lan966x->vcap_ctrl, f,
-					     admin);
+	err = lan966x_tc_flower_action_check(port->lan966x->vcap_ctrl,
+					     port->dev, f);
 	if (err)
 		return err;
 
diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c b/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
index 1ed304a816cc..986e41d3bb28 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
@@ -573,8 +573,8 @@  static int sparx5_tc_use_dissectors(struct flow_cls_offload *fco,
 }
 
 static int sparx5_tc_flower_action_check(struct vcap_control *vctrl,
-					 struct flow_cls_offload *fco,
-					 struct vcap_admin *admin)
+					 struct net_device *ndev,
+					 struct flow_cls_offload *fco)
 {
 	struct flow_rule *rule = flow_cls_offload_flow_rule(fco);
 	struct flow_action_entry *actent, *last_actent = NULL;
@@ -600,21 +600,23 @@  static int sparx5_tc_flower_action_check(struct vcap_control *vctrl,
 		last_actent = actent; /* Save last action for later check */
 	}
 
-	/* Check that last action is a goto */
-	if (last_actent->id != FLOW_ACTION_GOTO) {
+	/* Check if last action is a goto
+	 * The last chain/lookup does not need to have a goto action
+	 */
+	if (last_actent->id == FLOW_ACTION_GOTO) {
+		/* Check if the destination chain is in one of the VCAPs */
+		if (!vcap_is_next_lookup(vctrl, fco->common.chain_index,
+					 last_actent->chain_index)) {
+			NL_SET_ERR_MSG_MOD(fco->common.extack,
+					   "Invalid goto chain");
+			return -EINVAL;
+		}
+	} else if (!vcap_is_last_chain(vctrl, fco->common.chain_index)) {
 		NL_SET_ERR_MSG_MOD(fco->common.extack,
 				   "Last action must be 'goto'");
 		return -EINVAL;
 	}
 
-	/* Check if the goto chain is in the next lookup */
-	if (!vcap_is_next_lookup(vctrl, fco->common.chain_index,
-				 last_actent->chain_index)) {
-		NL_SET_ERR_MSG_MOD(fco->common.extack,
-				   "Invalid goto chain");
-		return -EINVAL;
-	}
-
 	/* Catch unsupported combinations of actions */
 	if (action_mask & BIT(FLOW_ACTION_TRAP) &&
 	    action_mask & BIT(FLOW_ACTION_ACCEPT)) {
@@ -833,7 +835,7 @@  static int sparx5_tc_flower_replace(struct net_device *ndev,
 
 	vctrl = port->sparx5->vcap_ctrl;
 
-	err = sparx5_tc_flower_action_check(vctrl, fco, admin);
+	err = sparx5_tc_flower_action_check(vctrl, ndev, fco);
 	if (err)
 		return err;
 
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api.c b/drivers/net/ethernet/microchip/vcap/vcap_api.c
index a5572bcab8e6..2cc6e94077a4 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api.c
@@ -1553,39 +1553,31 @@  struct vcap_admin *vcap_find_admin(struct vcap_control *vctrl, int cid)
 }
 EXPORT_SYMBOL_GPL(vcap_find_admin);
 
-/* Is the next chain id in the following lookup, possible in another VCAP */
-bool vcap_is_next_lookup(struct vcap_control *vctrl, int cur_cid, int next_cid)
+/* Is the next chain id in one of the following lookups
+ * For now this does not support filters linked to other filters using
+ * keys and actions. That will be added later.
+ */
+bool vcap_is_next_lookup(struct vcap_control *vctrl, int src_cid, int dst_cid)
 {
-	struct vcap_admin *admin, *next_admin;
-	int lookup, next_lookup;
+	struct vcap_admin *admin;
+	int next_cid;
 
-	/* The offset must be at least one lookup */
-	if (next_cid < cur_cid + VCAP_CID_LOOKUP_SIZE)
+	if (vcap_api_check(vctrl))
 		return false;
 
-	if (vcap_api_check(vctrl))
+	/* The offset must be at least one lookup, round up */
+	next_cid = src_cid + VCAP_CID_LOOKUP_SIZE;
+	next_cid /= VCAP_CID_LOOKUP_SIZE;
+	next_cid *= VCAP_CID_LOOKUP_SIZE;
+
+	if (dst_cid < next_cid)
 		return false;
 
-	admin = vcap_find_admin(vctrl, cur_cid);
+	admin = vcap_find_admin(vctrl, dst_cid);
 	if (!admin)
 		return false;
 
-	/* If no VCAP contains the next chain, the next chain must be beyond
-	 * the last chain in the current VCAP
-	 */
-	next_admin = vcap_find_admin(vctrl, next_cid);
-	if (!next_admin)
-		return next_cid > admin->last_cid;
-
-	lookup = vcap_chain_id_to_lookup(admin, cur_cid);
-	next_lookup = vcap_chain_id_to_lookup(next_admin, next_cid);
-
-	/* Next lookup must be the following lookup */
-	if (admin == next_admin || admin->vtype == next_admin->vtype)
-		return next_lookup == lookup + 1;
-
-	/* Must be the first lookup in the next VCAP instance */
-	return next_lookup == 0;
+	return true;
 }
 EXPORT_SYMBOL_GPL(vcap_is_next_lookup);
 
@@ -2718,6 +2710,25 @@  int vcap_enable_lookups(struct vcap_control *vctrl, struct net_device *ndev,
 }
 EXPORT_SYMBOL_GPL(vcap_enable_lookups);
 
+/* Is this chain id the last lookup of all VCAPs */
+bool vcap_is_last_chain(struct vcap_control *vctrl, int cid)
+{
+	struct vcap_admin *admin;
+	int lookup;
+
+	if (vcap_api_check(vctrl))
+		return false;
+
+	admin = vcap_find_admin(vctrl, cid);
+	if (!admin)
+		return false;
+
+	/* This must be the last lookup in this VCAP type */
+	lookup = vcap_chain_id_to_lookup(admin, cid);
+	return lookup == admin->lookups - 1;
+}
+EXPORT_SYMBOL_GPL(vcap_is_last_chain);
+
 /* Set a rule counter id (for certain vcaps only) */
 void vcap_rule_set_counter_id(struct vcap_rule *rule, u32 counter_id)
 {
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_client.h b/drivers/net/ethernet/microchip/vcap/vcap_api_client.h
index e07dc8d3c639..f44228436051 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_client.h
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_client.h
@@ -217,6 +217,8 @@  const struct vcap_field *vcap_lookup_keyfield(struct vcap_rule *rule,
 int vcap_lookup_rule_by_cookie(struct vcap_control *vctrl, u64 cookie);
 /* Is the next chain id in the following lookup, possible in another VCAP */
 bool vcap_is_next_lookup(struct vcap_control *vctrl, int cur_cid, int next_cid);
+/* Is this chain id the last lookup of all VCAPs */
+bool vcap_is_last_chain(struct vcap_control *vctrl, int cid);
 /* Provide all rules via a callback interface */
 int vcap_rule_iter(struct vcap_control *vctrl,
 		   int (*callback)(void *, struct vcap_rule *), void *arg);
diff --git a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
index cc6a62338162..fdef9102a9b3 100644
--- a/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
+++ b/drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c
@@ -1865,7 +1865,7 @@  static void vcap_api_next_lookup_basic_test(struct kunit *test)
 	ret = vcap_is_next_lookup(&test_vctrl, 8300000, 8301000);
 	KUNIT_EXPECT_EQ(test, false, ret);
 	ret = vcap_is_next_lookup(&test_vctrl, 8300000, 8401000);
-	KUNIT_EXPECT_EQ(test, true, ret);
+	KUNIT_EXPECT_EQ(test, false, ret);
 }
 
 static void vcap_api_next_lookup_advanced_test(struct kunit *test)
@@ -1926,9 +1926,9 @@  static void vcap_api_next_lookup_advanced_test(struct kunit *test)
 	ret = vcap_is_next_lookup(&test_vctrl, 1100000, 1201000);
 	KUNIT_EXPECT_EQ(test, true, ret);
 	ret = vcap_is_next_lookup(&test_vctrl, 1100000, 1301000);
-	KUNIT_EXPECT_EQ(test, false, ret);
+	KUNIT_EXPECT_EQ(test, true, ret);
 	ret = vcap_is_next_lookup(&test_vctrl, 1100000, 8101000);
-	KUNIT_EXPECT_EQ(test, false, ret);
+	KUNIT_EXPECT_EQ(test, true, ret);
 	ret = vcap_is_next_lookup(&test_vctrl, 1300000, 1401000);
 	KUNIT_EXPECT_EQ(test, true, ret);
 	ret = vcap_is_next_lookup(&test_vctrl, 1400000, 1501000);
@@ -1944,7 +1944,7 @@  static void vcap_api_next_lookup_advanced_test(struct kunit *test)
 	ret = vcap_is_next_lookup(&test_vctrl, 8300000, 8301000);
 	KUNIT_EXPECT_EQ(test, false, ret);
 	ret = vcap_is_next_lookup(&test_vctrl, 8300000, 8401000);
-	KUNIT_EXPECT_EQ(test, true, ret);
+	KUNIT_EXPECT_EQ(test, false, ret);
 }
 
 static void vcap_api_filter_unsupported_keys_test(struct kunit *test)