| Message ID | 20241029035739.1981839-3-lcherian@marvell.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Knobs for NPC default rule counters | expand |
On Tue, Oct 29, 2024 at 09:27:38AM +0530, Linu Cherian wrote: > Add devlink knobs to enable/disable counters on NPC > default rule entries. > > Sample command to enable default rule counters: > devlink dev param set <dev> name npc_def_rule_cntr value true cmode runtime > > Sample command to read the counter: > cat /sys/kernel/debug/cn10k/npc/mcam_rules > > Signed-off-by: Linu Cherian <lcherian@marvell.com> Reviewed-by: Simon Horman <horms@kernel.org>
On Tue, 29 Oct 2024 09:27:38 +0530 Linu Cherian wrote: > + struct npc_install_flow_rsp rsp = { 0 }; @rsp is reused in the loop, either it doesn't have to be inited at all, or it has to be inited before every use > + struct npc_mcam *mcam = &rvu->hw->mcam; > + struct rvu_npc_mcam_rule *rule; > + int blkaddr; > + > + blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPC, 0); > + if (blkaddr < 0) > + return -EINVAL; > + > + mutex_lock(&mcam->lock); > + list_for_each_entry(rule, &mcam->mcam_rules, list) { > + if (!is_mcam_entry_enabled(rvu, mcam, blkaddr, rule->entry)) > + continue; > + if (!rule->default_rule) > + continue; > + if (enable && !rule->has_cntr) { /* Alloc and map new counter */ > + __rvu_mcam_add_counter_to_rule(rvu, rule->owner, > + rule, &rsp); > + if (rsp.counter < 0) { > + dev_err(rvu->dev, "%s: Err to allocate cntr for default rule (err=%d)\n", > + __func__, rsp.counter); > + break; shouldn't you "unwind" in this case? We'll leave the counter enabled for some rules and disabled for others > + } > + npc_map_mcam_entry_and_cntr(rvu, mcam, blkaddr, > + rule->entry, rsp.counter); > + } > + > + if (enable && rule->has_cntr) /* Reset counter before use */ { > + rvu_write64(rvu, blkaddr, > + NPC_AF_MATCH_STATX(rule->cntr), 0x0); > + continue; so setting to enabled while already enabled resets the value? If so that's neither documented, nor.. usual. > + } > + > + if (!enable && rule->has_cntr) /* Free and unmap counter */ { > + __rvu_mcam_remove_counter_from_rule(rvu, rule->owner, > + rule); > + } unnecesary parenthesis > + }
Hi Jakub, On 2024-11-04 at 01:23:10, Jakub Kicinski (kuba@kernel.org) wrote: > On Tue, 29 Oct 2024 09:27:38 +0530 Linu Cherian wrote: > > + struct npc_install_flow_rsp rsp = { 0 }; > > @rsp is reused in the loop, either it doesn't have to be inited at all, > or it has to be inited before every use Ack. It doesnt have to be inited. Will remove. > > > + struct npc_mcam *mcam = &rvu->hw->mcam; > > + struct rvu_npc_mcam_rule *rule; > > + int blkaddr; > > + > > + blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPC, 0); > > + if (blkaddr < 0) > > + return -EINVAL; > > + > > + mutex_lock(&mcam->lock); > > + list_for_each_entry(rule, &mcam->mcam_rules, list) { > > + if (!is_mcam_entry_enabled(rvu, mcam, blkaddr, rule->entry)) > > + continue; > > + if (!rule->default_rule) > > + continue; > > + if (enable && !rule->has_cntr) { /* Alloc and map new counter */ > > + __rvu_mcam_add_counter_to_rule(rvu, rule->owner, > > + rule, &rsp); > > + if (rsp.counter < 0) { > > + dev_err(rvu->dev, "%s: Err to allocate cntr for default rule (err=%d)\n", > > + __func__, rsp.counter); > > + break; > > shouldn't you "unwind" in this case? We'll leave the counter enabled > for some rules and disabled for others Wanted to keep a best effort approach here, will make it clear in the documentation. > > > + } > > + npc_map_mcam_entry_and_cntr(rvu, mcam, blkaddr, > > + rule->entry, rsp.counter); > > + } > > + > > + if (enable && rule->has_cntr) /* Reset counter before use */ { > > + rvu_write64(rvu, blkaddr, > > + NPC_AF_MATCH_STATX(rule->cntr), 0x0); > > + continue; > > so setting to enabled while already enabled resets the value? > If so that's neither documented, nor.. usual. Will move this code under, if (enable && !rule->has_cntr) case. Infact, all enablement happens only from devlink control, hence the above if check is really not required and can be confusing. > > > + } > > + > > + if (!enable && rule->has_cntr) /* Free and unmap counter */ { > > + __rvu_mcam_remove_counter_from_rule(rvu, rule->owner, > > + rule); > > + } > > unnecesary parenthesis Ack. Thanks, Linu Cherian.
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h index d92a5f47a476..e8c6a6fe9bd5 100644 --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h @@ -525,6 +525,7 @@ struct rvu { struct mutex alias_lock; /* Serialize bar2 alias access */ int vfs; /* Number of VFs attached to RVU */ u16 vf_devid; /* VF devices id */ + bool def_rule_cntr_en; int nix_blkaddr[MAX_NIX_BLKS]; /* Mbox */ @@ -989,6 +990,7 @@ void npc_set_mcam_action(struct rvu *rvu, struct npc_mcam *mcam, void npc_read_mcam_entry(struct rvu *rvu, struct npc_mcam *mcam, int blkaddr, u16 src, struct mcam_entry *entry, u8 *intf, u8 *ena); +int npc_config_cntr_default_entries(struct rvu *rvu, bool enable); bool is_cgx_config_permitted(struct rvu *rvu, u16 pcifunc); bool is_mac_feature_supported(struct rvu *rvu, int pf, int feature); u32 rvu_cgx_get_fifolen(struct rvu *rvu); diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c index 7498ab429963..9c26e19a860b 100644 --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c @@ -1238,6 +1238,7 @@ enum rvu_af_dl_param_id { RVU_AF_DEVLINK_PARAM_ID_DWRR_MTU, RVU_AF_DEVLINK_PARAM_ID_NPC_MCAM_ZONE_PERCENT, RVU_AF_DEVLINK_PARAM_ID_NPC_EXACT_FEATURE_DISABLE, + RVU_AF_DEVLINK_PARAM_ID_NPC_DEF_RULE_CNTR_ENABLE, RVU_AF_DEVLINK_PARAM_ID_NIX_MAXLF, }; @@ -1358,6 +1359,32 @@ static int rvu_af_dl_npc_mcam_high_zone_percent_validate(struct devlink *devlink return 0; } +static int rvu_af_dl_npc_def_rule_cntr_get(struct devlink *devlink, u32 id, + struct devlink_param_gset_ctx *ctx) +{ + struct rvu_devlink *rvu_dl = devlink_priv(devlink); + struct rvu *rvu = rvu_dl->rvu; + + ctx->val.vbool = rvu->def_rule_cntr_en; + + return 0; +} + +static int rvu_af_dl_npc_def_rule_cntr_set(struct devlink *devlink, u32 id, + struct devlink_param_gset_ctx *ctx, + struct netlink_ext_ack *extack) +{ + struct rvu_devlink *rvu_dl = devlink_priv(devlink); + struct rvu *rvu = rvu_dl->rvu; + int err; + + err = npc_config_cntr_default_entries(rvu, ctx->val.vbool); + if (!err) + rvu->def_rule_cntr_en = ctx->val.vbool; + + return err; +} + static int rvu_af_dl_nix_maxlf_get(struct devlink *devlink, u32 id, struct devlink_param_gset_ctx *ctx) { @@ -1444,6 +1471,11 @@ static const struct devlink_param rvu_af_dl_params[] = { rvu_af_dl_npc_mcam_high_zone_percent_get, rvu_af_dl_npc_mcam_high_zone_percent_set, rvu_af_dl_npc_mcam_high_zone_percent_validate), + DEVLINK_PARAM_DRIVER(RVU_AF_DEVLINK_PARAM_ID_NPC_DEF_RULE_CNTR_ENABLE, + "npc_def_rule_cntr", DEVLINK_PARAM_TYPE_BOOL, + BIT(DEVLINK_PARAM_CMODE_RUNTIME), + rvu_af_dl_npc_def_rule_cntr_get, + rvu_af_dl_npc_def_rule_cntr_set, NULL), DEVLINK_PARAM_DRIVER(RVU_AF_DEVLINK_PARAM_ID_NIX_MAXLF, "nix_maxlf", DEVLINK_PARAM_TYPE_U16, BIT(DEVLINK_PARAM_CMODE_RUNTIME), diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c index c4ef1e83cc46..9e39c3149a4f 100644 --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c @@ -2691,6 +2691,51 @@ void npc_mcam_rsrcs_reserve(struct rvu *rvu, int blkaddr, int entry_idx) npc_mcam_set_bit(mcam, entry_idx); } +int npc_config_cntr_default_entries(struct rvu *rvu, bool enable) +{ + struct npc_install_flow_rsp rsp = { 0 }; + struct npc_mcam *mcam = &rvu->hw->mcam; + struct rvu_npc_mcam_rule *rule; + int blkaddr; + + blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPC, 0); + if (blkaddr < 0) + return -EINVAL; + + mutex_lock(&mcam->lock); + list_for_each_entry(rule, &mcam->mcam_rules, list) { + if (!is_mcam_entry_enabled(rvu, mcam, blkaddr, rule->entry)) + continue; + if (!rule->default_rule) + continue; + if (enable && !rule->has_cntr) { /* Alloc and map new counter */ + __rvu_mcam_add_counter_to_rule(rvu, rule->owner, + rule, &rsp); + if (rsp.counter < 0) { + dev_err(rvu->dev, "%s: Err to allocate cntr for default rule (err=%d)\n", + __func__, rsp.counter); + break; + } + npc_map_mcam_entry_and_cntr(rvu, mcam, blkaddr, + rule->entry, rsp.counter); + } + + if (enable && rule->has_cntr) /* Reset counter before use */ { + rvu_write64(rvu, blkaddr, + NPC_AF_MATCH_STATX(rule->cntr), 0x0); + continue; + } + + if (!enable && rule->has_cntr) /* Free and unmap counter */ { + __rvu_mcam_remove_counter_from_rule(rvu, rule->owner, + rule); + } + } + mutex_unlock(&mcam->lock); + + return 0; +} + int rvu_mbox_handler_npc_mcam_alloc_entry(struct rvu *rvu, struct npc_mcam_alloc_entry_req *req, struct npc_mcam_alloc_entry_rsp *rsp)
Add devlink knobs to enable/disable counters on NPC default rule entries. Sample command to enable default rule counters: devlink dev param set <dev> name npc_def_rule_cntr value true cmode runtime Sample command to read the counter: cat /sys/kernel/debug/cn10k/npc/mcam_rules Signed-off-by: Linu Cherian <lcherian@marvell.com> --- Changelog from v3: No changes. .../net/ethernet/marvell/octeontx2/af/rvu.h | 2 + .../marvell/octeontx2/af/rvu_devlink.c | 32 +++++++++++++ .../ethernet/marvell/octeontx2/af/rvu_npc.c | 45 +++++++++++++++++++ 3 files changed, 79 insertions(+)