net: microchip: vcap api: Check chains when adding a tc flower filter
authorSteen Hegelund <steen.hegelund@microchip.com>
Sat, 14 Jan 2023 13:42:40 +0000 (14:42 +0100)
committerDavid S. Miller <davem@davemloft.net>
Mon, 16 Jan 2023 13:45:17 +0000 (13:45 +0000)
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.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/ethernet/microchip/lan966x/lan966x_tc_flower.c
drivers/net/ethernet/microchip/sparx5/sparx5_tc_flower.c
drivers/net/ethernet/microchip/vcap/vcap_api.c
drivers/net/ethernet/microchip/vcap/vcap_api_client.h
drivers/net/ethernet/microchip/vcap/vcap_api_kunit.c

index ba3fa917d6b789750e4ba3a430ea0eac30face54..b66a8725a071273126a687c442bcbb524a6d94d2 100644 (file)
@@ -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;
 
index 1ed304a816ccde2d30f98e502f1a3a3f7ca2d636..986e41d3bb28bf38bb908389c922f36e5f44c1f1 100644 (file)
@@ -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;
 
index a5572bcab8e68177a87b5ad4ef047d20ad4180fe..2cc6e94077a4d69223bb272bdb8c4c07bc8b35b4 100644 (file)
@@ -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)
 {
index e07dc8d3c639065a6ee8276aa3ea7f935bd554f7..f442284360515e471133537978b7c274af3a5d05 100644 (file)
@@ -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);
index cc6a623381629d186fdbe842cd4924c83e57d17e..fdef9102a9b32441bc3d32e96cf4c6ad0c3b4c2d 100644 (file)
@@ -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)