| Message ID | 20240530101515.201635-1-saikrishnag@marvell.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v3] octeontx2-pf: Add ucast filter count configurability via devlink. | expand |
On 5/30/2024 3:15 AM, Sai Krishna wrote: > Added a devlink param to set/modify unicast filter count. Currently > it's hardcoded with a macro. > > commands: > > To get the current unicast filter count > # devlink dev param show pci/0002:02:00.0 name unicast_filter_count > > To change/set the unicast filter count > # devlink dev param set pci/0002:02:00.0 name unicast_filter_count > value 5 cmode runtime > A bit of explanation about why this needs to be configurable would be useful. What is the impact of lowering or raising this value? Presumably you need to change the MCAM table size? Lowering this on one port might enable raising it on another? It seems reasonable to me, but it is helpful to provide such motivations in the commit message. > Signed-off-by: Sai Krishna <saikrishnag@marvell.com> > --- > v3: > - Addressed review comments given by Jakub Kicinski > 1. Documented unicast_filter_count devlink param > 2. Minor change to match upstream code base > v2: > - Addressed review comments given by Simon Horman > 1. Updated the commit message with example commads > 2. Modified/optimized conditions > > .../networking/devlink/octeontx2.rst | 16 +++++ > .../marvell/octeontx2/nic/otx2_common.h | 7 +- > .../marvell/octeontx2/nic/otx2_devlink.c | 64 +++++++++++++++++++ > .../marvell/octeontx2/nic/otx2_flows.c | 20 +++--- > .../ethernet/marvell/octeontx2/nic/otx2_pf.c | 2 +- > 5 files changed, 95 insertions(+), 14 deletions(-) > > diff --git a/Documentation/networking/devlink/octeontx2.rst b/Documentation/networking/devlink/octeontx2.rst > index 610de99b728a..5910289b4d4e 100644 > --- a/Documentation/networking/devlink/octeontx2.rst > +++ b/Documentation/networking/devlink/octeontx2.rst > @@ -40,3 +40,19 @@ The ``octeontx2 AF`` driver implements the following driver-specific parameters. > - runtime > - Use to set the quantum which hardware uses for scheduling among transmit queues. > Hardware uses weighted DWRR algorithm to schedule among all transmit queues. > + > +The ``octeontx2 PF`` driver implements the following driver-specific parameters. > + > +.. list-table:: Driver-specific parameters implemented > + :widths: 5 5 5 85 > + > + * - Name > + - Type > + - Mode > + - Description > + * - ``unicast_filter_count`` > + - u8 > + - runtime > + - Used to Set/modify unicast filter count, which helps in better utilization of > + resources against possible wastage(un-used) with current scheme of hardcoded > + unicast count. The text here could be worded a little better. Once the patch is applied then hard coding is no longer the "current scheme". I might have worded this like: "Set the maximum number of unicast filters that can be programmed for the device. This can be used to achieve better device resource utilization, avoiding over consumption of unused MCAM table entries." Or something similar.
> -----Original Message----- > From: Jacob Keller <jacob.e.keller@intel.com> > Sent: Friday, May 31, 2024 12:08 AM > To: Sai Krishna Gajula <saikrishnag@marvell.com>; davem@davemloft.net; > edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; > netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Sunil Kovvuri > Goutham <sgoutham@marvell.com>; Geethasowjanya Akula > <gakula@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>; > Subbaraya Sundeep Bhatta <sbhatta@marvell.com> > Subject: Re: [net-next PATCH v3] octeontx2-pf: Add ucast filter > count configurability via devlink. > > > On 5/30/2024 3:15 AM, Sai Krishna wrote: > > Added a devlink param to set/modify unicast filter count. Currently > > it's hardcoded with a macro. > > > > commands: > > > > To get the current unicast filter count # devlink dev param show > > pci/0002:02:00.0 name unicast_filter_count > > > > To change/set the unicast filter count # devlink dev param set > > pci/0002:02:00.0 name unicast_filter_count value 5 cmode runtime > > > > A bit of explanation about why this needs to be configurable would be > useful. What is the impact of lowering or raising this value? Presumably > you need to change the MCAM table size? Lowering this on one port might > enable raising it on another? > > It seems reasonable to me, but it is helpful to provide such motivations > in the commit message. Ack, will add more info to commit message in patch V4. > > > Signed-off-by: Sai Krishna <saikrishnag@marvell.com> > > --- > > v3: > > - Addressed review comments given by Jakub Kicinski > > 1. Documented unicast_filter_count devlink param > > 2. Minor change to match upstream code base > > v2: > > - Addressed review comments given by Simon Horman > > 1. Updated the commit message with example commads > > 2. Modified/optimized conditions > > > > .../networking/devlink/octeontx2.rst | 16 +++++ > > .../marvell/octeontx2/nic/otx2_common.h | 7 +- > > .../marvell/octeontx2/nic/otx2_devlink.c | 64 +++++++++++++++++++ > > .../marvell/octeontx2/nic/otx2_flows.c | 20 +++--- > > .../ethernet/marvell/octeontx2/nic/otx2_pf.c | 2 +- > > 5 files changed, 95 insertions(+), 14 deletions(-) > > > > diff --git a/Documentation/networking/devlink/octeontx2.rst > b/Documentation/networking/devlink/octeontx2.rst > > index 610de99b728a..5910289b4d4e 100644 > > --- a/Documentation/networking/devlink/octeontx2.rst > > +++ b/Documentation/networking/devlink/octeontx2.rst > > @@ -40,3 +40,19 @@ The ``octeontx2 AF`` driver implements the following > driver-specific parameters. > > - runtime > > - Use to set the quantum which hardware uses for scheduling among > transmit queues. > > Hardware uses weighted DWRR algorithm to schedule among all > transmit queues. > > + > > +The ``octeontx2 PF`` driver implements the following driver-specific > parameters. > > + > > +.. list-table:: Driver-specific parameters implemented > > + :widths: 5 5 5 85 > > + > > + * - Name > > + - Type > > + - Mode > > + - Description > > + * - ``unicast_filter_count`` > > + - u8 > > + - runtime > > + - Used to Set/modify unicast filter count, which helps in better utilization > of > > + resources against possible wastage(un-used) with current scheme of > hardcoded > > + unicast count. > > The text here could be worded a little better. Once the patch is applied > then hard coding is no longer the "current scheme". > > I might have worded this like: > > "Set the maximum number of unicast filters that can be programmed for > the device. This can be used to achieve better device resource > utilization, avoiding over consumption of unused MCAM table entries." > > Or something similar. Ack, will re-word the text.. in patch V4. Thanks, Sai
diff --git a/Documentation/networking/devlink/octeontx2.rst b/Documentation/networking/devlink/octeontx2.rst index 610de99b728a..5910289b4d4e 100644 --- a/Documentation/networking/devlink/octeontx2.rst +++ b/Documentation/networking/devlink/octeontx2.rst @@ -40,3 +40,19 @@ The ``octeontx2 AF`` driver implements the following driver-specific parameters. - runtime - Use to set the quantum which hardware uses for scheduling among transmit queues. Hardware uses weighted DWRR algorithm to schedule among all transmit queues. + +The ``octeontx2 PF`` driver implements the following driver-specific parameters. + +.. list-table:: Driver-specific parameters implemented + :widths: 5 5 5 85 + + * - Name + - Type + - Mode + - Description + * - ``unicast_filter_count`` + - u8 + - runtime + - Used to Set/modify unicast filter count, which helps in better utilization of + resources against possible wastage(un-used) with current scheme of hardcoded + unicast count. diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h index 24fbbef265a6..f27a3456ae64 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h @@ -346,12 +346,9 @@ struct otx2_flow_config { u16 *def_ent; u16 nr_flows; #define OTX2_DEFAULT_FLOWCOUNT 16 -#define OTX2_MAX_UNICAST_FLOWS 8 +#define OTX2_DEFAULT_UNICAST_FLOWS 4 #define OTX2_MAX_VLAN_FLOWS 1 #define OTX2_MAX_TC_FLOWS OTX2_DEFAULT_FLOWCOUNT -#define OTX2_MCAM_COUNT (OTX2_DEFAULT_FLOWCOUNT + \ - OTX2_MAX_UNICAST_FLOWS + \ - OTX2_MAX_VLAN_FLOWS) u16 unicast_offset; u16 rx_vlan_offset; u16 vf_vlan_offset; @@ -365,6 +362,7 @@ struct otx2_flow_config { u16 max_flows; refcount_t mark_flows; struct list_head flow_list_tc; + u8 ucast_flt_cnt; bool ntuple; }; @@ -1067,6 +1065,7 @@ int otx2_handle_ntuple_tc_features(struct net_device *netdev, int otx2_smq_flush(struct otx2_nic *pfvf, int smq); void otx2_free_bufs(struct otx2_nic *pfvf, struct otx2_pool *pool, u64 iova, int size); +int otx2_mcam_entry_init(struct otx2_nic *pfvf); /* tc support */ int otx2_init_tc(struct otx2_nic *nic); diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c index 99ddf31269d9..79cd8f1777d4 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c @@ -64,9 +64,68 @@ static int otx2_dl_mcam_count_get(struct devlink *devlink, u32 id, return 0; } +static int otx2_dl_ucast_flt_cnt_set(struct devlink *devlink, u32 id, + struct devlink_param_gset_ctx *ctx, + struct netlink_ext_ack *extack) +{ + struct otx2_devlink *otx2_dl = devlink_priv(devlink); + struct otx2_nic *pfvf = otx2_dl->pfvf; + int err; + + pfvf->flow_cfg->ucast_flt_cnt = ctx->val.vu8; + + otx2_mcam_flow_del(pfvf); + err = otx2_mcam_entry_init(pfvf); + if (err) + return err; + + return 0; +} + +static int otx2_dl_ucast_flt_cnt_get(struct devlink *devlink, u32 id, + struct devlink_param_gset_ctx *ctx) +{ + struct otx2_devlink *otx2_dl = devlink_priv(devlink); + struct otx2_nic *pfvf = otx2_dl->pfvf; + + ctx->val.vu8 = pfvf->flow_cfg ? pfvf->flow_cfg->ucast_flt_cnt : 0; + + return 0; +} + +static int otx2_dl_ucast_flt_cnt_validate(struct devlink *devlink, u32 id, + union devlink_param_value val, + struct netlink_ext_ack *extack) +{ + struct otx2_devlink *otx2_dl = devlink_priv(devlink); + struct otx2_nic *pfvf = otx2_dl->pfvf; + + /* Check for UNICAST filter support*/ + if (!(pfvf->flags & OTX2_FLAG_UCAST_FLTR_SUPPORT)) { + NL_SET_ERR_MSG_MOD(extack, + "Unicast filter not enabled"); + return -EINVAL; + } + + if (!pfvf->flow_cfg) { + NL_SET_ERR_MSG_MOD(extack, + "pfvf->flow_cfg not initialized"); + return -EINVAL; + } + + if (pfvf->flow_cfg->nr_flows) { + NL_SET_ERR_MSG_MOD(extack, + "Cannot modify count when there are active rules"); + return -EINVAL; + } + + return 0; +} + enum otx2_dl_param_id { OTX2_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX, OTX2_DEVLINK_PARAM_ID_MCAM_COUNT, + OTX2_DEVLINK_PARAM_ID_UCAST_FLT_CNT, }; static const struct devlink_param otx2_dl_params[] = { @@ -75,6 +134,11 @@ static const struct devlink_param otx2_dl_params[] = { BIT(DEVLINK_PARAM_CMODE_RUNTIME), otx2_dl_mcam_count_get, otx2_dl_mcam_count_set, otx2_dl_mcam_count_validate), + DEVLINK_PARAM_DRIVER(OTX2_DEVLINK_PARAM_ID_UCAST_FLT_CNT, + "unicast_filter_count", DEVLINK_PARAM_TYPE_U8, + BIT(DEVLINK_PARAM_CMODE_RUNTIME), + otx2_dl_ucast_flt_cnt_get, otx2_dl_ucast_flt_cnt_set, + otx2_dl_ucast_flt_cnt_validate), }; static const struct devlink_ops otx2_devlink_ops = { diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c index bc5819237ed7..98c31a16c70b 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c @@ -12,8 +12,6 @@ #define OTX2_DEFAULT_ACTION 0x1 -static int otx2_mcam_entry_init(struct otx2_nic *pfvf); - struct otx2_flow { struct ethtool_rx_flow_spec flow_spec; struct list_head list; @@ -161,7 +159,7 @@ int otx2_alloc_mcam_entries(struct otx2_nic *pfvf, u16 count) } EXPORT_SYMBOL(otx2_alloc_mcam_entries); -static int otx2_mcam_entry_init(struct otx2_nic *pfvf) +int otx2_mcam_entry_init(struct otx2_nic *pfvf) { struct otx2_flow_config *flow_cfg = pfvf->flow_cfg; struct npc_get_field_status_req *freq; @@ -172,7 +170,7 @@ static int otx2_mcam_entry_init(struct otx2_nic *pfvf) int ent, count; vf_vlan_max_flows = pfvf->total_vfs * OTX2_PER_VF_VLAN_FLOWS; - count = OTX2_MAX_UNICAST_FLOWS + + count = flow_cfg->ucast_flt_cnt + OTX2_MAX_VLAN_FLOWS + vf_vlan_max_flows; flow_cfg->def_ent = devm_kmalloc_array(pfvf->dev, count, @@ -214,7 +212,7 @@ static int otx2_mcam_entry_init(struct otx2_nic *pfvf) flow_cfg->vf_vlan_offset = 0; flow_cfg->unicast_offset = vf_vlan_max_flows; flow_cfg->rx_vlan_offset = flow_cfg->unicast_offset + - OTX2_MAX_UNICAST_FLOWS; + flow_cfg->ucast_flt_cnt; pfvf->flags |= OTX2_FLAG_UCAST_FLTR_SUPPORT; /* Check if NPC_DMAC field is supported @@ -255,6 +253,7 @@ static int otx2_mcam_entry_init(struct otx2_nic *pfvf) refcount_set(&flow_cfg->mark_flows, 1); return 0; } +EXPORT_SYMBOL(otx2_mcam_entry_init); /* TODO : revisit on size */ #define OTX2_DMAC_FLTR_BITMAP_SZ (4 * 2048 + 32) @@ -302,6 +301,8 @@ int otx2_mcam_flow_init(struct otx2_nic *pf) INIT_LIST_HEAD(&pf->flow_cfg->flow_list); INIT_LIST_HEAD(&pf->flow_cfg->flow_list_tc); + pf->flow_cfg->ucast_flt_cnt = OTX2_DEFAULT_UNICAST_FLOWS; + /* Allocate bare minimum number of MCAM entries needed for * unicast and ntuple filters. */ @@ -314,7 +315,7 @@ int otx2_mcam_flow_init(struct otx2_nic *pf) return 0; pf->mac_table = devm_kzalloc(pf->dev, sizeof(struct otx2_mac_table) - * OTX2_MAX_UNICAST_FLOWS, GFP_KERNEL); + * pf->flow_cfg->ucast_flt_cnt, GFP_KERNEL); if (!pf->mac_table) return -ENOMEM; @@ -356,7 +357,7 @@ static int otx2_do_add_macfilter(struct otx2_nic *pf, const u8 *mac) return -ENOMEM; /* dont have free mcam entries or uc list is greater than alloted */ - if (netdev_uc_count(pf->netdev) > OTX2_MAX_UNICAST_FLOWS) + if (netdev_uc_count(pf->netdev) > pf->flow_cfg->ucast_flt_cnt) return -ENOMEM; mutex_lock(&pf->mbox.lock); @@ -367,7 +368,7 @@ static int otx2_do_add_macfilter(struct otx2_nic *pf, const u8 *mac) } /* unicast offset starts with 32 0..31 for ntuple */ - for (i = 0; i < OTX2_MAX_UNICAST_FLOWS; i++) { + for (i = 0; i < pf->flow_cfg->ucast_flt_cnt; i++) { if (pf->mac_table[i].inuse) continue; ether_addr_copy(pf->mac_table[i].addr, mac); @@ -410,7 +411,7 @@ static bool otx2_get_mcamentry_for_mac(struct otx2_nic *pf, const u8 *mac, { int i; - for (i = 0; i < OTX2_MAX_UNICAST_FLOWS; i++) { + for (i = 0; i < pf->flow_cfg->ucast_flt_cnt; i++) { if (!pf->mac_table[i].inuse) continue; @@ -1394,6 +1395,7 @@ int otx2_destroy_mcam_flows(struct otx2_nic *pfvf) } pfvf->flags &= ~OTX2_FLAG_MCAM_ENTRIES_ALLOC; + flow_cfg->max_flows = 0; mutex_unlock(&pfvf->mbox.lock); return 0; diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c index f5bce3e326cc..ff05ea20409a 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c @@ -1714,7 +1714,7 @@ static void otx2_do_set_rx_mode(struct otx2_nic *pf) return; if ((netdev->flags & IFF_PROMISC) || - (netdev_uc_count(netdev) > OTX2_MAX_UNICAST_FLOWS)) { + (netdev_uc_count(netdev) > pf->flow_cfg->ucast_flt_cnt)) { promisc = true; }
Added a devlink param to set/modify unicast filter count. Currently it's hardcoded with a macro. commands: To get the current unicast filter count # devlink dev param show pci/0002:02:00.0 name unicast_filter_count To change/set the unicast filter count # devlink dev param set pci/0002:02:00.0 name unicast_filter_count value 5 cmode runtime Signed-off-by: Sai Krishna <saikrishnag@marvell.com> --- v3: - Addressed review comments given by Jakub Kicinski 1. Documented unicast_filter_count devlink param 2. Minor change to match upstream code base v2: - Addressed review comments given by Simon Horman 1. Updated the commit message with example commads 2. Modified/optimized conditions .../networking/devlink/octeontx2.rst | 16 +++++ .../marvell/octeontx2/nic/otx2_common.h | 7 +- .../marvell/octeontx2/nic/otx2_devlink.c | 64 +++++++++++++++++++ .../marvell/octeontx2/nic/otx2_flows.c | 20 +++--- .../ethernet/marvell/octeontx2/nic/otx2_pf.c | 2 +- 5 files changed, 95 insertions(+), 14 deletions(-)