From: "Singh, Jasvinder" <jasvinder.singh@intel.com>
To: "Lu, Wenzhuo" <wenzhuo.lu@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>,
"Yigit, Ferruh" <ferruh.yigit@intel.com>,
"thomas@monjalon.net" <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH v4 4/4] net/softnic: add TM hierarchy related ops
Date: Thu, 28 Sep 2017 08:39:34 +0000 [thread overview]
Message-ID: <54CBAA185211B4429112C315DA58FF6D3325C667@IRSMSX103.ger.corp.intel.com> (raw)
In-Reply-To: <6A0DE07E22DDAD4C9103DF62FEBC09093B6BD299@shsmsx102.ccr.corp.intel.com>
> Hi Jasvinder,
>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jasvinder Singh
> > Sent: Monday, September 18, 2017 5:10 PM
> > To: dev@dpdk.org
> > Cc: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Yigit,
> > Ferruh <ferruh.yigit@intel.com>; thomas@monjalon.net
> > Subject: [dpdk-dev] [PATCH v4 4/4] net/softnic: add TM hierarchy
> > related ops
> >
> > Implement ethdev TM hierarchy related APIs in SoftNIC PMD.
> >
> > Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > ---
> > drivers/net/softnic/rte_eth_softnic_internals.h | 41 +
> > drivers/net/softnic/rte_eth_softnic_tm.c | 2776
> > ++++++++++++++++++++++-
> > 2 files changed, 2813 insertions(+), 4 deletions(-)
>
>
> > +
> > +static uint32_t
> > +tm_node_subport_id(struct rte_eth_dev *dev, struct tm_node
> > *subport_node)
> > +{
> > + struct pmd_internals *p = dev->data->dev_private;
> > + struct tm_node_list *nl = &p->soft.tm.h.nodes;
> > + struct tm_node *ns;
> > + uint32_t subport_id;
> > +
> > + subport_id = 0;
> > + TAILQ_FOREACH(ns, nl, node) {
> > + if (ns->level != TM_NODE_LEVEL_SUBPORT)
> > + continue;
> > +
> > + if (ns->node_id == subport_node->node_id)
> > + return subport_id;
> > +
> > + subport_id++;
> > + }
> > +
> > + return UINT32_MAX;
> UINT32_MAX means invalid number, right? Better define a specific MACRO
> for the invalid number in case you may not want to use 0xff.. or uint32.
> The same suggestion for the below functions.
Ok.
> > +static int
> > +shaper_profile_check(struct rte_eth_dev *dev,
> > + uint32_t shaper_profile_id,
> > + struct rte_tm_shaper_params *profile,
> > + struct rte_tm_error *error)
> > +{
> > + struct tm_shaper_profile *sp;
> > +
> > + /* Shaper profile ID must not be NONE. */
> > + if (shaper_profile_id == RTE_TM_SHAPER_PROFILE_ID_NONE)
> > + return -rte_tm_error_set(error,
> > + EINVAL,
> > + RTE_TM_ERROR_TYPE_SHAPER_PROFILE_ID,
> > + NULL,
> > + rte_strerror(EINVAL));
> > +
> > + /* Shaper profile must not exist. */
> > + sp = tm_shaper_profile_search(dev, shaper_profile_id);
> > + if (sp)
> > + return -rte_tm_error_set(error,
> > + EEXIST,
> > + RTE_TM_ERROR_TYPE_SHAPER_PROFILE_ID,
> > + NULL,
> > + rte_strerror(EEXIST));
> > +
> > + /* Profile must not be NULL. */
> > + if (profile == NULL)
> > + return -rte_tm_error_set(error,
> > + EINVAL,
> > + RTE_TM_ERROR_TYPE_SHAPER_PROFILE,
> > + NULL,
> > + rte_strerror(EINVAL));
> A slight suggestion. We can do the easiest check at first.
We preferred to perform checks as per the arguments order specified in the function definition so that all the parameter could
be scanned in a systematic manner instead of picking them randomly.
> > +
> > +/* Traffic manager shaper profile add */ static int
> > +pmd_tm_shaper_profile_add(struct rte_eth_dev *dev,
> > + uint32_t shaper_profile_id,
> > + struct rte_tm_shaper_params *profile,
> > + struct rte_tm_error *error)
> > +{
> > + struct pmd_internals *p = dev->data->dev_private;
> > + struct tm_shaper_profile_list *spl = &p->soft.tm.h.shaper_profiles;
> > + struct tm_shaper_profile *sp;
> > + int status;
> > +
> > + /* Check input params */
> > + status = shaper_profile_check(dev, shaper_profile_id, profile, error);
> > + if (status)
> > + return status;
> > +
> > + /* Memory allocation */
> > + sp = calloc(1, sizeof(struct tm_shaper_profile));
> Just curious, why not use rte_zmalloc?
This relates to high level hierarchy specification objects which doesn't need to be allocated on specific
numa node as it is not used once the hierarchy is committed. All these objects gets eventually translated into
TM implementation (librte_sched) specific objects. These objects are allocated using rte_zmalloc and needs
lots of memory (approx. 2M mbufs for single instance of TM hierarchy ports) on specific numa node.
> > + if (sp == NULL)
> > + return -rte_tm_error_set(error,
> > + ENOMEM,
> > + RTE_TM_ERROR_TYPE_UNSPECIFIED,
> > + NULL,
> > + rte_strerror(ENOMEM));
> > +
> > + /* Fill in */
> > + sp->shaper_profile_id = shaper_profile_id;
> > + memcpy(&sp->params, profile, sizeof(sp->params));
> > +
> > + /* Add to list */
> > + TAILQ_INSERT_TAIL(spl, sp, node);
> > + p->soft.tm.h.n_shaper_profiles++;
> > +
> > + return 0;
> > +}
> > +
>
> > +
> > +static struct tm_node *
> > +tm_shared_shaper_get_tc(struct rte_eth_dev *dev,
> > + struct tm_shared_shaper *ss)
> > +{
> > + struct pmd_internals *p = dev->data->dev_private;
> > + struct tm_node_list *nl = &p->soft.tm.h.nodes;
> > + struct tm_node *n;
> > +
> > + TAILQ_FOREACH(n, nl, node) {
> > + if ((n->level != TM_NODE_LEVEL_TC) ||
> > + (n->params.n_shared_shapers == 0) ||
> > + (n->params.shared_shaper_id[0] != ss-
> > >shared_shaper_id))
> According to node_add_check_tc, only one shared shaper supported, right?
> Better adding some comments here?
The subport has 4 shared shapers for each of the pipe traffic classes. Will add comment.
> > + continue;
> > +
> > + return n;
> > + }
> > +
> > + return NULL;
> > +}
>
>
>
> > +
> > +static void
> > +pipe_profile_build(struct rte_eth_dev *dev,
> > + struct tm_node *np,
> > + struct rte_sched_pipe_params *pp)
> > +{
> > + struct pmd_internals *p = dev->data->dev_private;
> > + struct tm_hierarchy *h = &p->soft.tm.h;
> > + struct tm_node_list *nl = &h->nodes;
> > + struct tm_node *nt, *nq;
> > +
> > + memset(pp, 0, sizeof(*pp));
> > +
> > + /* Pipe */
> > + pp->tb_rate = np->shaper_profile->params.peak.rate;
> > + pp->tb_size = np->shaper_profile->params.peak.size;
> > +
> > + /* Traffic Class (TC) */
> > + pp->tc_period = 40;
> 40 means? A MACRO is better?
will add macro. Thanks.
> > +
> > +static int
> > +pipe_profile_free_exists(struct rte_eth_dev *dev,
> > + uint32_t *pipe_profile_id)
> > +{
> > + struct pmd_internals *p = dev->data->dev_private;
> > + struct tm_params *t = &p->soft.tm.params;
> > +
> > + if (t->n_pipe_profiles < RTE_SCHED_PIPE_PROFILES_PER_PORT) {
> > + *pipe_profile_id = t->n_pipe_profiles;
> > + return 1;
> Returning true or false is easier to understand?
Ok.
> Also the same concern of the naming as patch 3.
Ok. Will bring clarity in the names.
Thanks for the comments and time.
next prev parent reply other threads:[~2017-09-28 8:39 UTC|newest]
Thread overview: 79+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-26 18:11 [dpdk-dev] [PATCH 0/2] net/softnic: sw fall-back for traffic management Jasvinder Singh
2017-05-26 18:11 ` [dpdk-dev] [PATCH 1/2] net/softnic: add softnic PMD " Jasvinder Singh
2017-06-26 16:43 ` [dpdk-dev] [PATCH v2 0/2] net/softnic: sw fall-back " Jasvinder Singh
2017-06-26 16:43 ` [dpdk-dev] [PATCH v2 1/2] net/softnic: add softnic PMD " Jasvinder Singh
2017-08-11 12:49 ` [dpdk-dev] [PATCH v3 0/4] net/softnic: sw fall-back pmd for traffic mgmt and others Jasvinder Singh
2017-08-11 12:49 ` [dpdk-dev] [PATCH v3 1/4] net/softnic: add softnic PMD Jasvinder Singh
2017-09-05 14:53 ` Ferruh Yigit
2017-09-08 9:30 ` Singh, Jasvinder
2017-09-08 9:48 ` Ferruh Yigit
2017-09-08 10:42 ` Singh, Jasvinder
2017-09-18 9:10 ` [dpdk-dev] [PATCH v4 0/4] net/softnic: sw fall-back pmd for traffic mgmt and others Jasvinder Singh
2017-09-18 9:10 ` [dpdk-dev] [PATCH v4 1/4] net/softnic: add softnic PMD Jasvinder Singh
2017-09-18 16:58 ` Singh, Jasvinder
2017-09-18 19:09 ` Thomas Monjalon
2017-09-18 9:10 ` [dpdk-dev] [PATCH v4 2/4] net/softnic: add traffic management support Jasvinder Singh
2017-09-25 1:58 ` Lu, Wenzhuo
2017-09-28 8:14 ` Singh, Jasvinder
2017-09-29 14:04 ` [dpdk-dev] [PATCH v5 0/5] net/softnic: sw fall-back pmd for traffic mgmt and others Jasvinder Singh
2017-09-29 14:04 ` [dpdk-dev] [PATCH v5 1/5] net/softnic: add softnic PMD Jasvinder Singh
2017-09-29 14:04 ` [dpdk-dev] [PATCH v5 2/5] net/softnic: add traffic management support Jasvinder Singh
2017-10-06 16:59 ` [dpdk-dev] [PATCH v6 0/5] net/softnic: sw fall-back pmd for traffic mgmt and others Jasvinder Singh
2017-10-06 16:59 ` [dpdk-dev] [PATCH v6 1/5] net/softnic: add softnic PMD Jasvinder Singh
2017-10-09 12:58 ` [dpdk-dev] [PATCH v7 0/5] net/softnic: sw fall-back pmd for traffic mgmt and others Jasvinder Singh
2017-10-09 12:58 ` [dpdk-dev] [PATCH v7 1/5] net/softnic: add softnic PMD Jasvinder Singh
2017-10-09 20:18 ` Ferruh Yigit
2017-10-10 10:08 ` Singh, Jasvinder
2017-10-10 10:18 ` [dpdk-dev] [PATCH v8 0/5] net/softnic: sw fall-back pmd for traffic mgmt and others Jasvinder Singh
2017-10-10 10:18 ` [dpdk-dev] [PATCH v8 1/5] net/softnic: add softnic PMD Jasvinder Singh
2017-10-11 23:18 ` Thomas Monjalon
2017-10-12 8:22 ` Singh, Jasvinder
2017-10-10 10:18 ` [dpdk-dev] [PATCH v8 2/5] net/softnic: add traffic management support Jasvinder Singh
2017-10-10 10:18 ` [dpdk-dev] [PATCH v8 3/5] net/softnic: add TM capabilities ops Jasvinder Singh
2017-10-10 10:18 ` [dpdk-dev] [PATCH v8 4/5] net/softnic: add TM hierarchy related ops Jasvinder Singh
2017-10-10 10:18 ` [dpdk-dev] [PATCH v8 5/5] app/testpmd: add traffic management forwarding mode Jasvinder Singh
2017-10-10 18:24 ` Ferruh Yigit
2017-10-10 18:31 ` [dpdk-dev] [PATCH v8 0/5] net/softnic: sw fall-back pmd for traffic mgmt and others Ferruh Yigit
2017-10-10 19:09 ` Singh, Jasvinder
2017-10-09 12:58 ` [dpdk-dev] [PATCH v7 2/5] net/softnic: add traffic management support Jasvinder Singh
2017-10-09 12:58 ` [dpdk-dev] [PATCH v7 3/5] net/softnic: add TM capabilities ops Jasvinder Singh
2017-10-09 12:58 ` [dpdk-dev] [PATCH v7 4/5] net/softnic: add TM hierarchy related ops Jasvinder Singh
2017-10-09 12:58 ` [dpdk-dev] [PATCH v7 5/5] app/testpmd: add traffic management forwarding mode Jasvinder Singh
2017-10-09 20:17 ` Ferruh Yigit
2017-10-10 10:07 ` Singh, Jasvinder
2017-10-06 17:00 ` [dpdk-dev] [PATCH v6 2/5] net/softnic: add traffic management support Jasvinder Singh
2017-10-06 17:00 ` [dpdk-dev] [PATCH v6 3/5] net/softnic: add TM capabilities ops Jasvinder Singh
2017-10-06 17:00 ` [dpdk-dev] [PATCH v6 4/5] net/softnic: add TM hierarchy related ops Jasvinder Singh
2017-10-06 17:00 ` [dpdk-dev] [PATCH v6 5/5] app/testpmd: add traffic management forwarding mode Jasvinder Singh
2017-10-06 18:57 ` [dpdk-dev] [PATCH v6 0/5] net/softnic: sw fall-back pmd for traffic mgmt and others Ferruh Yigit
2017-10-09 11:32 ` Singh, Jasvinder
2017-09-29 14:04 ` [dpdk-dev] [PATCH v5 3/5] net/softnic: add TM capabilities ops Jasvinder Singh
2017-09-29 14:04 ` [dpdk-dev] [PATCH v5 4/5] net/softnic: add TM hierarchy related ops Jasvinder Singh
2017-09-29 14:04 ` [dpdk-dev] [PATCH v5 5/5] app/testpmd: add traffic management forwarding mode Jasvinder Singh
2017-09-18 9:10 ` [dpdk-dev] [PATCH v4 3/4] net/softnic: add TM capabilities ops Jasvinder Singh
2017-09-25 2:33 ` Lu, Wenzhuo
2017-09-28 8:16 ` Singh, Jasvinder
2017-09-18 9:10 ` [dpdk-dev] [PATCH v4 4/4] net/softnic: add TM hierarchy related ops Jasvinder Singh
2017-09-25 7:14 ` Lu, Wenzhuo
2017-09-28 8:39 ` Singh, Jasvinder [this message]
2017-09-20 15:35 ` [dpdk-dev] [PATCH v4 0/4] net/softnic: sw fall-back pmd for traffic mgmt and others Thomas Monjalon
2017-09-22 22:07 ` Singh, Jasvinder
2017-10-06 10:40 ` Dumitrescu, Cristian
2017-10-06 12:13 ` Thomas Monjalon
2017-08-11 12:49 ` [dpdk-dev] [PATCH v3 2/4] net/softnic: add traffic management support Jasvinder Singh
2017-08-11 12:49 ` [dpdk-dev] [PATCH v3 3/4] net/softnic: add TM capabilities ops Jasvinder Singh
2017-08-11 12:49 ` [dpdk-dev] [PATCH v3 4/4] net/softnic: add TM hierarchy related ops Jasvinder Singh
2017-09-08 17:08 ` [dpdk-dev] [PATCH v3 0/4] net/softnic: sw fall-back pmd for traffic mgmt and others Dumitrescu, Cristian
2017-06-26 16:43 ` [dpdk-dev] [PATCH v2 2/2] net/softnic: add traffic management ops Jasvinder Singh
2017-05-26 18:11 ` [dpdk-dev] [PATCH " Jasvinder Singh
2017-06-07 14:32 ` [dpdk-dev] [PATCH 0/2] net/softnic: sw fall-back for traffic management Thomas Monjalon
2017-06-08 13:27 ` Dumitrescu, Cristian
2017-06-08 13:59 ` Thomas Monjalon
2017-06-08 15:27 ` Dumitrescu, Cristian
2017-06-08 16:16 ` Thomas Monjalon
2017-06-08 16:43 ` Dumitrescu, Cristian
2017-07-04 23:48 ` Thomas Monjalon
2017-07-05 9:32 ` Dumitrescu, Cristian
2017-07-05 10:17 ` Thomas Monjalon
2017-08-11 15:28 ` Stephen Hemminger
2017-08-11 16:22 ` Dumitrescu, Cristian
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54CBAA185211B4429112C315DA58FF6D3325C667@IRSMSX103.ger.corp.intel.com \
--to=jasvinder.singh@intel.com \
--cc=cristian.dumitrescu@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=thomas@monjalon.net \
--cc=wenzhuo.lu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).