net/bnxt: modify mark manager validity checks

The ULP mark manager originally assumed that zero was an invalid
mark and used it for invalidation and deletion.  The mark manager
now supports adding zero as a mark, flags for validity and type,
and adds explicit bounds checking instead of relying on mask.

Signed-off-by: Kishore Padmanabha <kishore.padmanabha@broadcom.com>
Reviewed-by: Mike Baucom <michael.baucom@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

diff --git a/drivers/net/bnxt/tf_ulp/ulp_mapper.c b/drivers/net/bnxt/tf_ulp/ulp_mapper.c
index 9ea6fdb..938b88e 100644 (file)
--- a/drivers/net/bnxt/tf_ulp/ulp_mapper.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_mapper.c
@@ -459,18 +459,9 @@ static inline int32_t
 ulp_mapper_mark_free(struct bnxt_ulp_context *ulp,
                     struct ulp_flow_db_res_params *res)
 {
-       uint32_t flag;
-       uint32_t fid;
-       uint32_t gfid;
-
-       fid       = (uint32_t)res->resource_hndl;
-       TF_GET_FLAG_FROM_FLOW_ID(fid, flag);
-       TF_GET_GFID_FROM_FLOW_ID(fid, gfid);
-
        return ulp_mark_db_mark_del(ulp,
-                                   (flag == TF_GFID_TABLE_EXTERNAL),
-                                   gfid,
-                                   0);
+                                   res->resource_type,
+                                   res->resource_hndl);
 }
 
 /*
@@ -1318,10 +1309,9 @@ ulp_mapper_em_tbl_process(struct bnxt_ulp_mapper_parms *parms,
                mark = tfp_be_to_cpu_32(val);
 
                TF_GET_GFID_FROM_FLOW_ID(iparms.flow_id, gfid);
-               TF_GET_FLAG_FROM_FLOW_ID(iparms.flow_id, flag);
-
+               flag = BNXT_ULP_MARK_GLOBAL_HW_FID;
                rc = ulp_mark_db_mark_add(parms->ulp_ctx,
-                                         (flag == TF_GFID_TABLE_EXTERNAL),
+                                         flag,
                                          gfid,
                                          mark);
                if (rc) {
@@ -1336,8 +1326,8 @@ ulp_mapper_em_tbl_process(struct bnxt_ulp_mapper_parms *parms,
                memset(&fid_parms, 0, sizeof(fid_parms));
                fid_parms.direction     = tbl->direction;
                fid_parms.resource_func = BNXT_ULP_RESOURCE_FUNC_HW_FID;
-               fid_parms.resource_type = tbl->table_type;
-               fid_parms.resource_hndl = iparms.flow_id;
+               fid_parms.resource_type = flag;
+               fid_parms.resource_hndl = gfid;
                fid_parms.critical_resource = 0;
 
                rc = ulp_flow_db_resource_add(parms->ulp_ctx,

diff --git a/drivers/net/bnxt/tf_ulp/ulp_mark_mgr.c b/drivers/net/bnxt/tf_ulp/ulp_mark_mgr.c
index ad83531..9e8b81e 100644 (file)
--- a/drivers/net/bnxt/tf_ulp/ulp_mark_mgr.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_mark_mgr.c
@@ -14,6 +14,13 @@
 #include "ulp_template_db.h"
 #include "ulp_template_struct.h"
 
+#define ULP_MARK_DB_ENTRY_SET_VALID(mark_info) ((mark_info)->flags |=\
+                                               BNXT_ULP_MARK_VALID)
+#define ULP_MARK_DB_ENTRY_IS_INVALID(mark_info) (!((mark_info)->flags &\
+                                                  BNXT_ULP_MARK_VALID))
+#define ULP_MARK_DB_ENTRY_IS_GLOBAL_HW_FID(mark_info) ((mark_info)->flags &\
+                                               BNXT_ULP_MARK_GLOBAL_HW_FID)
+
 static inline uint32_t
 ulp_mark_db_idx_get(bool is_gfid, uint32_t fid, struct bnxt_ulp_mark_tbl *mtbl)
 {
@@ -25,52 +32,14 @@ ulp_mark_db_idx_get(bool is_gfid, uint32_t fid, struct bnxt_ulp_mark_tbl *mtbl)
 
                /* Need to truncate anything beyond supported flows */
                idx &= mtbl->gfid_mask;
-
                if (hashtype)
                        idx |= mtbl->gfid_type_bit;
        } else {
                idx = fid;
        }
-
        return idx;
 }
 
-static int32_t
-ulp_mark_db_mark_set(struct bnxt_ulp_context *ctxt,
-                    bool is_gfid,
-                    uint32_t fid,
-                    uint32_t mark)
-{
-       struct          bnxt_ulp_mark_tbl *mtbl;
-       uint32_t        idx = 0;
-
-       if (!ctxt) {
-               BNXT_TF_DBG(ERR, "Invalid ulp context\n");
-               return -EINVAL;
-       }
-
-       mtbl = bnxt_ulp_cntxt_ptr2_mark_db_get(ctxt);
-       if (!mtbl) {
-               BNXT_TF_DBG(ERR, "Unable to get Mark DB\n");
-               return -EINVAL;
-       }
-
-       idx = ulp_mark_db_idx_get(is_gfid, fid, mtbl);
-
-       if (is_gfid) {
-               BNXT_TF_DBG(DEBUG, "Set GFID[0x%0x] = 0x%0x\n", idx, mark);
-
-               mtbl->gfid_tbl[idx].mark_id = mark;
-               mtbl->gfid_tbl[idx].valid = true;
-       } else {
-               /* For the LFID, the FID is used as the index */
-               mtbl->lfid_tbl[fid].mark_id = mark;
-               mtbl->lfid_tbl[fid].valid = true;
-       }
-
-       return 0;
-}
-
 /*
  * Allocate and Initialize all Mark Manager resources for this ulp context.
  *
@@ -105,48 +74,48 @@ ulp_mark_db_init(struct bnxt_ulp_context *ctxt)
        if (!mark_tbl)
                goto mem_error;
 
-       /* Need to allocate 2 * Num flows to account for hash type bit. */
+       /* Need to allocate 2 * Num flows to account for hash type bit.*/
+       mark_tbl->lfid_num_entries = dparms->lfid_entries;
        mark_tbl->lfid_tbl = rte_zmalloc("ulp_rx_em_flow_mark_table",
-                                        dparms->lfid_entries *
-                                           sizeof(struct bnxt_lfid_mark_info),
+                                        mark_tbl->lfid_num_entries *
+                                        sizeof(struct bnxt_lfid_mark_info),
                                         0);
-
        if (!mark_tbl->lfid_tbl)
                goto mem_error;
 
-       /* Need to allocate 2 * Num flows to account for hash type bit. */
+       /* Need to allocate 2 * Num flows to account for hash type bit */
+       mark_tbl->gfid_num_entries = dparms->gfid_entries;
        mark_tbl->gfid_tbl = rte_zmalloc("ulp_rx_eem_flow_mark_table",
-                                        2 * dparms->num_flows *
-                                           sizeof(struct bnxt_gfid_mark_info),
+                                        mark_tbl->gfid_num_entries *
+                                        sizeof(struct bnxt_gfid_mark_info),
                                         0);
        if (!mark_tbl->gfid_tbl)
                goto mem_error;
 
        /*
-        * TBD: This needs to be generalized for better mark handling
         * These values are used to compress the FID to the allowable index
-        * space.  The FID from hw may be the full hash.
+        * space.  The FID from hw may be the full hash which may be a big
+        * value to allocate and so allocate only needed hash values.
+        * gfid mask is the number of flow entries for the each left/right
+        * hash  The gfid type bit is used to get to the higher or lower hash
+        * entries.
         */
-       mark_tbl->gfid_max      = dparms->gfid_entries - 1;
-       mark_tbl->gfid_mask     = (dparms->gfid_entries / 2) - 1;
-       mark_tbl->gfid_type_bit = (dparms->gfid_entries / 2);
+       mark_tbl->gfid_mask     = (mark_tbl->gfid_num_entries / 2) - 1;
+       mark_tbl->gfid_type_bit = (mark_tbl->gfid_num_entries / 2);
 
        BNXT_TF_DBG(DEBUG, "GFID Max = 0x%08x\nGFID MASK = 0x%08x\n",
-                   mark_tbl->gfid_max,
+                   mark_tbl->gfid_num_entries - 1,
                    mark_tbl->gfid_mask);
 
        /* Add the mark tbl to the ulp context. */
        bnxt_ulp_cntxt_ptr2_mark_db_set(ctxt, mark_tbl);
-
        return 0;
 
 mem_error:
        rte_free(mark_tbl->gfid_tbl);
        rte_free(mark_tbl->lfid_tbl);
        rte_free(mark_tbl);
-       BNXT_TF_DBG(DEBUG,
-                   "Failed to allocate memory for mark mgr\n");
-
+       BNXT_TF_DBG(DEBUG, "Failed to allocate memory for mark mgr\n");
        return -ENOMEM;
 }
 
@@ -208,7 +177,8 @@ ulp_mark_db_mark_get(struct bnxt_ulp_context *ctxt,
        idx = ulp_mark_db_idx_get(is_gfid, fid, mtbl);
 
        if (is_gfid) {
-               if (!mtbl->gfid_tbl[idx].valid)
+               if (idx >= mtbl->gfid_num_entries ||
+                   ULP_MARK_DB_ENTRY_IS_INVALID(&mtbl->gfid_tbl[idx]))
                        return -EINVAL;
 
                BNXT_TF_DBG(DEBUG, "Get GFID[0x%0x] = 0x%0x\n",
@@ -216,7 +186,8 @@ ulp_mark_db_mark_get(struct bnxt_ulp_context *ctxt,
 
                *mark = mtbl->gfid_tbl[idx].mark_id;
        } else {
-               if (!mtbl->gfid_tbl[idx].valid)
+               if (idx >= mtbl->lfid_num_entries ||
+                   ULP_MARK_DB_ENTRY_IS_INVALID(&mtbl->lfid_tbl[idx]))
                        return -EINVAL;
 
                BNXT_TF_DBG(DEBUG, "Get LFID[0x%0x] = 0x%0x\n",
@@ -233,7 +204,7 @@ ulp_mark_db_mark_get(struct bnxt_ulp_context *ctxt,
  *
  * ctxt [in] The ulp context for the mark manager
  *
- * is_gfid [in] The type of fid (GFID or LFID)
+ * mark_flag [in] mark flags.
  *
  * fid [in] The flow id that is returned by HW in BD
  *
@@ -242,11 +213,47 @@ ulp_mark_db_mark_get(struct bnxt_ulp_context *ctxt,
  */
 int32_t
 ulp_mark_db_mark_add(struct bnxt_ulp_context *ctxt,
-                    bool is_gfid,
-                    uint32_t gfid,
+                    uint32_t mark_flag,
+                    uint32_t fid,
                     uint32_t mark)
 {
-       return ulp_mark_db_mark_set(ctxt, is_gfid, gfid, mark);
+       struct bnxt_ulp_mark_tbl *mtbl;
+       uint32_t idx = 0;
+       bool is_gfid;
+
+       if (!ctxt) {
+               BNXT_TF_DBG(ERR, "Invalid ulp context\n");
+               return -EINVAL;
+       }
+
+       mtbl = bnxt_ulp_cntxt_ptr2_mark_db_get(ctxt);
+       if (!mtbl) {
+               BNXT_TF_DBG(ERR, "Unable to get Mark DB\n");
+               return -EINVAL;
+       }
+
+       is_gfid = (mark_flag & BNXT_ULP_MARK_GLOBAL_HW_FID);
+       if (is_gfid) {
+               idx = ulp_mark_db_idx_get(is_gfid, fid, mtbl);
+               if (idx >= mtbl->gfid_num_entries) {
+                       BNXT_TF_DBG(ERR, "Mark index greater than allocated\n");
+                       return -EINVAL;
+               }
+               BNXT_TF_DBG(DEBUG, "Set GFID[0x%0x] = 0x%0x\n", idx, mark);
+               mtbl->gfid_tbl[idx].mark_id = mark;
+               ULP_MARK_DB_ENTRY_SET_VALID(&mtbl->gfid_tbl[idx]);
+
+       } else {
+               /* For the LFID, the FID is used as the index */
+               if (fid >= mtbl->lfid_num_entries) {
+                       BNXT_TF_DBG(ERR, "Mark index greater than allocated\n");
+                       return -EINVAL;
+               }
+               mtbl->lfid_tbl[fid].mark_id = mark;
+               ULP_MARK_DB_ENTRY_SET_VALID(&mtbl->lfid_tbl[fid]);
+       }
+
+       return 0;
 }
 
 /*
@@ -254,18 +261,51 @@ ulp_mark_db_mark_add(struct bnxt_ulp_context *ctxt,
  *
  * ctxt [in] The ulp context for the mark manager
  *
- * is_gfid [in] The type of fid (GFID or LFID)
+ * mark_flag [in] mark flags.
  *
  * fid [in] The flow id that is returned by HW in BD
  *
- * mark [in] The mark to be associated with the FID
- *
  */
 int32_t
 ulp_mark_db_mark_del(struct bnxt_ulp_context *ctxt,
-                    bool is_gfid,
-                    uint32_t gfid,
-                    uint32_t mark  __rte_unused)
+                    uint32_t mark_flag,
+                    uint32_t fid)
 {
-       return ulp_mark_db_mark_set(ctxt, is_gfid, gfid, ULP_MARK_INVALID);
+       struct bnxt_ulp_mark_tbl *mtbl;
+       uint32_t idx = 0;
+       bool is_gfid;
+
+       if (!ctxt) {
+               BNXT_TF_DBG(ERR, "Invalid ulp context\n");
+               return -EINVAL;
+       }
+
+       mtbl = bnxt_ulp_cntxt_ptr2_mark_db_get(ctxt);
+       if (!mtbl) {
+               BNXT_TF_DBG(ERR, "Unable to get Mark DB\n");
+               return -EINVAL;
+       }
+
+       is_gfid = (mark_flag & BNXT_ULP_MARK_GLOBAL_HW_FID);
+       if (is_gfid) {
+               idx = ulp_mark_db_idx_get(is_gfid, fid, mtbl);
+               if (idx >= mtbl->gfid_num_entries) {
+                       BNXT_TF_DBG(ERR, "Mark index greater than allocated\n");
+                       return -EINVAL;
+               }
+               BNXT_TF_DBG(DEBUG, "Reset GFID[0x%0x]\n", idx);
+               memset(&mtbl->gfid_tbl[idx], 0,
+                      sizeof(struct bnxt_gfid_mark_info));
+
+       } else {
+               /* For the LFID, the FID is used as the index */
+               if (fid >= mtbl->lfid_num_entries) {
+                       BNXT_TF_DBG(ERR, "Mark index greater than allocated\n");
+                       return -EINVAL;
+               }
+               memset(&mtbl->lfid_tbl[fid], 0,
+                      sizeof(struct bnxt_lfid_mark_info));
+       }
+
+       return 0;
 }

diff --git a/drivers/net/bnxt/tf_ulp/ulp_mark_mgr.h b/drivers/net/bnxt/tf_ulp/ulp_mark_mgr.h
index 0f8a5e5..fd0d840 100644 (file)
--- a/drivers/net/bnxt/tf_ulp/ulp_mark_mgr.h
+++ b/drivers/net/bnxt/tf_ulp/ulp_mark_mgr.h
@@ -8,23 +8,27 @@
 
 #include "bnxt_ulp.h"
 
-#define ULP_MARK_INVALID (0)
+#define BNXT_ULP_MARK_VALID   0x1
+#define BNXT_ULP_MARK_GLOBAL_HW_FID 0x4
+#define BNXT_ULP_MARK_LOCAL_HW_FID 0x8
+
 struct bnxt_lfid_mark_info {
        uint16_t        mark_id;
-       bool            valid;
+       uint16_t        flags;
 };
 
 struct bnxt_gfid_mark_info {
        uint32_t        mark_id;
-       bool            valid;
+       uint16_t        flags;
 };
 
 struct bnxt_ulp_mark_tbl {
        struct bnxt_lfid_mark_info      *lfid_tbl;
        struct bnxt_gfid_mark_info      *gfid_tbl;
+       uint32_t                        lfid_num_entries;
+       uint32_t                        gfid_num_entries;
        uint32_t                        gfid_mask;
        uint32_t                        gfid_type_bit;
-       uint32_t                        gfid_max;
 };
 
 /*
@@ -77,7 +81,7 @@ ulp_mark_db_mark_get(struct bnxt_ulp_context *ctxt,
  *
  * ctxt [in] The ulp context for the mark manager
  *
- * is_gfid [in] The type of fid (GFID or LFID)
+ * mark_flag [in] mark flags.
  *
  * fid [in] The flow id that is returned by HW in BD
  *
@@ -86,7 +90,7 @@ ulp_mark_db_mark_get(struct bnxt_ulp_context *ctxt,
  */
 int32_t
 ulp_mark_db_mark_add(struct bnxt_ulp_context *ctxt,
-                    bool is_gfid,
+                    uint32_t mark_flag,
                     uint32_t gfid,
                     uint32_t mark);
 
@@ -95,17 +99,14 @@ ulp_mark_db_mark_add(struct bnxt_ulp_context *ctxt,
  *
  * ctxt [in] The ulp context for the mark manager
  *
- * is_gfid [in] The type of fid (GFID or LFID)
+ * mark_flag [in] mark flags
  *
  * fid [in] The flow id that is returned by HW in BD
  *
- * mark [in] The mark to be associated with the FID
- *
  */
 int32_t
 ulp_mark_db_mark_del(struct bnxt_ulp_context *ctxt,
-                    bool is_gfid,
-                    uint32_t gfid,
-                    uint32_t mark);
+                    uint32_t mark_flag,
+                    uint32_t gfid);
 
 #endif /* _ULP_MARK_MGR_H_ */