Hi Savinay, Trying to summarize the main points of what we discussed and agreed last week for the benefit of all ...
> -----Original Message----- > From: Dharmappa, Savinay <savinay.dharma...@intel.com> > Sent: Thursday, September 17, 2020 9:43 AM > To: Singh, Jasvinder <jasvinder.si...@intel.com>; Dumitrescu, Cristian > <cristian.dumitre...@intel.com>; dev@dpdk.org > Cc: Dharmappa, Savinay <savinay.dharma...@intel.com> > Subject: [PATCH v4 3/9] sched: add subport profile add and config api > > Add apis to add new subport profile and configure it. > > Signed-off-by: Savinay Dharmappa <savinay.dharma...@intel.com> > Signed-off-by: Jasvinder Singh <jasvinder.si...@intel.com> > --- > lib/librte_sched/rte_sched.c | 118 > +++++++++++++++++++++++++++++++++ > lib/librte_sched/rte_sched.h | 45 +++++++++++++ > lib/librte_sched/rte_sched_version.map | 2 + > 3 files changed, 165 insertions(+) > > diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c > index 5fa7865..23aaec4 100644 > --- a/lib/librte_sched/rte_sched.c > +++ b/lib/librte_sched/rte_sched.c > @@ -174,6 +174,8 @@ struct rte_sched_subport { > /* Statistics */ > struct rte_sched_subport_stats stats __rte_cache_aligned; > > + /* subport profile */ > + uint32_t profile; > /* Subport pipes */ > uint32_t n_pipes_per_subport_enabled; > uint32_t n_pipe_profiles; > @@ -1343,6 +1345,56 @@ rte_sched_subport_config(struct rte_sched_port > *port, > } > > int > +rte_sched_subport_profile_config(struct rte_sched_port *port, > + uint32_t subport_id, > + uint32_t profile_id) > +{ > + int i; > + struct rte_sched_subport_profile *params; > + uint32_t n_subports = subport_id + 1; > + struct rte_sched_subport *s; > + > + if (port == NULL) { > + RTE_LOG(ERR, SCHED, > + "%s: Incorrect value for parameter port\n", > __func__); > + return -EINVAL; > + } > + > + if (subport_id >= port->n_subports_per_port) { > + RTE_LOG(ERR, SCHED, "%s: " > + "Incorrect value for parameter subport id\n", __func__); > + > + rte_sched_free_memory(port, n_subports); > + return -EINVAL; > + } > + > + params = port->subport_profiles + profile_id; > + > + s = port->subports[subport_id]; > + > + s->tb_credits = params->tb_size / 2; > + > + s->tc_time = port->time + params->tc_period; > + > + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > + if (s->qsize[i]) > + s->tc_credits[i] = > + params->tc_credits_per_period[i]; > + else > + params->tc_credits_per_period[i] = 0; > + > +#ifdef RTE_SCHED_SUBPORT_TC_OV > + s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(params- > >tc_period, > + s->pipe_tc_be_rate_max); > +#endif > + s->profile = profile_id; > + > + rte_sched_port_log_subport_profile(port, profile_id); > + > + return 0; > +} > + > +int > rte_sched_pipe_config(struct rte_sched_port *port, > uint32_t subport_id, > uint32_t pipe_id, > @@ -1526,6 +1578,72 @@ rte_sched_subport_pipe_profile_add(struct > rte_sched_port *port, > return 0; > } > > +int > +rte_sched_port_subport_profile_add(struct rte_sched_port *port, > + struct rte_sched_subport_profile_params *params, > + uint32_t *subport_profile_id) > +{ > + int status; > + uint32_t i; > + struct rte_sched_subport_profile *dst; > + > + /* Port */ > + if (port == NULL) { > + RTE_LOG(ERR, SCHED, "%s: " > + "Incorrect value for parameter port\n", __func__); > + return -EINVAL; > + } > + > + if (params == NULL) { > + RTE_LOG(ERR, SCHED, "%s: " > + "Incorrect value for parameter profile\n", __func__); > + return -EINVAL; > + } > + > + if (subport_profile_id == NULL) { > + RTE_LOG(ERR, SCHED, "%s: " > + "Incorrect value for parameter subport_profile_id\n", > + __func__); > + return -EINVAL; > + } > + > + dst = port->subport_profiles + port->n_subport_profiles; > + > + /* Subport profiles exceeds the max limit */ > + if (port->n_subport_profiles >= port->n_max_subport_profiles) { > + RTE_LOG(ERR, SCHED, "%s: " > + "Number of subport profiles exceeds the max limit\n", > + __func__); > + return -EINVAL; > + } > + > + status = subport_profile_check(params, port->rate); > + if (status != 0) { > + RTE_LOG(ERR, SCHED, > + "%s: subport profile check failed(%d)\n", __func__, status); > + return -EINVAL; > + } > + > + rte_sched_subport_profile_convert(params, dst, port->rate); > + > + /* Subport profile should not exists */ > + for (i = 0; i < port->n_subport_profiles; i++) > + if (memcmp(port->subport_profiles + i, > + dst, sizeof(*dst)) == 0) { > + RTE_LOG(ERR, SCHED, > + "%s: subport profile exists\n", __func__); > + return -EINVAL; > + } > + > + /* Subport profile commit */ > + *subport_profile_id = port->n_subport_profiles; > + port->n_subport_profiles++; > + > + rte_sched_port_log_subport_profile(port, *subport_profile_id); > + > + return 0; > +} > + > static inline uint32_t > rte_sched_port_qindex(struct rte_sched_port *port, > uint32_t subport, > diff --git a/lib/librte_sched/rte_sched.h b/lib/librte_sched/rte_sched.h > index 39339b7..a7c2638 100644 > --- a/lib/librte_sched/rte_sched.h > +++ b/lib/librte_sched/rte_sched.h > @@ -337,6 +337,29 @@ rte_sched_subport_pipe_profile_add(struct > rte_sched_port *port, > uint32_t *pipe_profile_id); > > /** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice. > + * > + * Hierarchical scheduler subport bandwidth profile add > + * Note that this function is safe to use in runtime for adding new > + * subport bandwidth profile as it doesn't have any impact on hiearchical > + * structure of the scheduler. > + * @param port > + * Handle to port scheduler instance > + * @param struct rte_sched_subport_profile > + * Subport bandwidth profile > + * @param subport_profile_d > + * Subport profile id > + * @return > + * 0 upon success, error code otherwise > + */ > +__rte_experimental > +int > +rte_sched_port_subport_profile_add(struct rte_sched_port *port, > + struct rte_sched_subport_profile_params *profile, > + uint32_t *subport_profile_id); > + Please make sure the internal subport profile table is freed correctly, which is not the case currently. > +/** > * Hierarchical scheduler subport configuration > * > * @param port > @@ -354,6 +377,28 @@ rte_sched_subport_config(struct rte_sched_port > *port, > struct rte_sched_subport_params *params); > > /** > + * @warning > + * @b EXPERIMENTAL: this API may change without prior notice. > + * > + * Hierarchical scheduler subport profile configuration > + * Note that this function is safe to use in runtime for applying any > specific > + * subport bandwidth profile as it doesn't have any impact on hiearchical > + * structure of the scheduler. > + * @param port > + * Handle to port scheduler instance > + * @param subport_id > + * Subport ID > + * @param profile_d > + * Subport profile id > + * @return > + * 0 upon success, error code otherwise > + */ > +__rte_experimental > +int > +rte_sched_subport_profile_config(struct rte_sched_port *port, > + uint32_t subport_id, > + uint32_t profile_id); This function should be melted into rte_sched_subport_config(), which should get an extra profile_id argument. This preserves the symmetry with rte_sched_pipe_config(), which is both an init time and a run-time config update function, and also avoid user confusion and error prone situation when only rte_sched_subport_config() is called while rte_sched_subport_profile_config() is not. First time rte_sched_subport_config() is called (init time), its params argument should be non-NULL; this argument should be ignored (with NULL as the recommended value) for any subsequent calls. A subport-internal init_done flag could be maintained to distinguish between first and any subsequent calls. This should also be documented in the argument description. > +/** > * Hierarchical scheduler pipe configuration > * > * @param port > diff --git a/lib/librte_sched/rte_sched_version.map > b/lib/librte_sched/rte_sched_version.map > index 3faef6f..e64335f 100644 > --- a/lib/librte_sched/rte_sched_version.map > +++ b/lib/librte_sched/rte_sched_version.map > @@ -28,4 +28,6 @@ EXPERIMENTAL { > global: > > rte_sched_subport_pipe_profile_add; > + rte_sched_port_subport_profile_add; > + rte_sched_subport_profile_config; > }; > -- Need to increment the ABI version here, as new data structures and API function changes took place, inline with the deprecation note. > 2.7.4