* [PATCH v3] sched: enable/disable TC OV at runtime @ 2022-04-27 8:58 Marcin Danilewicz 2022-04-27 9:36 ` Dumitrescu, Cristian 2022-04-27 15:53 ` Stephen Hemminger 0 siblings, 2 replies; 12+ messages in thread From: Marcin Danilewicz @ 2022-04-27 8:58 UTC (permalink / raw) To: dev Added new API to enable or disable TC over subscription for best effort traffic class at subport level. Added changes after review and increased throughput. By default TC OV is disabled. Signed-off-by: Marcin Danilewicz <marcinx.danilewicz@intel.com> --- lib/sched/rte_sched.c | 189 +++++++++++++++++++++++++++++++++++------- lib/sched/rte_sched.h | 18 ++++ lib/sched/version.map | 3 + 3 files changed, 178 insertions(+), 32 deletions(-) diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c index ec74bee939..6e7d81df46 100644 --- a/lib/sched/rte_sched.c +++ b/lib/sched/rte_sched.c @@ -213,6 +213,9 @@ struct rte_sched_subport { uint8_t *bmp_array; struct rte_mbuf **queue_array; uint8_t memory[0] __rte_cache_aligned; + + /* TC oversubscription activation */ + int is_tc_ov_enabled; } __rte_cache_aligned; struct rte_sched_port { @@ -1165,6 +1168,45 @@ rte_sched_cman_config(struct rte_sched_port *port, } #endif +int +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, + uint32_t subport_id, + bool tc_ov_enable) +{ + struct rte_sched_subport *s; + struct rte_sched_subport_profile *profile; + + 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__); + return -EINVAL; + } + + s = port->subports[subport_id]; + s->is_tc_ov_enabled = tc_ov_enable ? 1 : 0; + + if (s->is_tc_ov_enabled) { + /* TC oversubscription */ + s->tc_ov_wm_min = port->mtu; + s->tc_ov_period_id = 0; + s->tc_ov = 0; + s->tc_ov_n = 0; + s->tc_ov_rate = 0; + + profile = port->subport_profiles + s->profile; + s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile->tc_period, + s->pipe_tc_be_rate_max); + s->tc_ov_wm = s->tc_ov_wm_max; + } + return 0; +} + int rte_sched_subport_config(struct rte_sched_port *port, uint32_t subport_id, @@ -1254,6 +1296,9 @@ rte_sched_subport_config(struct rte_sched_port *port, s->n_pipe_profiles = params->n_pipe_profiles; s->n_max_pipe_profiles = params->n_max_pipe_profiles; + /* TC over-subscription is disabled by default */ + s->is_tc_ov_enabled = 0; + #ifdef RTE_SCHED_CMAN if (params->cman_params != NULL) { s->cman_enabled = true; @@ -1316,13 +1361,6 @@ rte_sched_subport_config(struct rte_sched_port *port, for (i = 0; i < RTE_SCHED_PORT_N_GRINDERS; i++) s->grinder_base_bmp_pos[i] = RTE_SCHED_PIPE_INVALID; - - /* TC oversubscription */ - s->tc_ov_wm_min = port->mtu; - s->tc_ov_period_id = 0; - s->tc_ov = 0; - s->tc_ov_n = 0; - s->tc_ov_rate = 0; } { @@ -1342,9 +1380,6 @@ rte_sched_subport_config(struct rte_sched_port *port, else profile->tc_credits_per_period[i] = 0; - s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile->tc_period, - s->pipe_tc_be_rate_max); - s->tc_ov_wm = s->tc_ov_wm_max; s->profile = subport_profile_id; } @@ -1417,17 +1452,20 @@ rte_sched_pipe_config(struct rte_sched_port *port, double pipe_tc_be_rate = (double) params->tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] / (double) params->tc_period; - uint32_t tc_be_ov = s->tc_ov; - /* Unplug pipe from its subport */ - s->tc_ov_n -= params->tc_ov_weight; - s->tc_ov_rate -= pipe_tc_be_rate; - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; + if (s->is_tc_ov_enabled) { + uint32_t tc_be_ov = s->tc_ov; - if (s->tc_ov != tc_be_ov) { - RTE_LOG(DEBUG, SCHED, - "Subport %u Best-effort TC oversubscription is OFF (%.4lf >= %.4lf)\n", - subport_id, subport_tc_be_rate, s->tc_ov_rate); + /* Unplug pipe from its subport */ + s->tc_ov_n -= params->tc_ov_weight; + s->tc_ov_rate -= pipe_tc_be_rate; + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; + + if (s->tc_ov != tc_be_ov) { + RTE_LOG(DEBUG, SCHED, + "Subport %u Best-effort TC oversubscription is OFF (%.4lf >= %.4lf)\n", + subport_id, subport_tc_be_rate, s->tc_ov_rate); + } } /* Reset the pipe */ @@ -1460,19 +1498,22 @@ rte_sched_pipe_config(struct rte_sched_port *port, double pipe_tc_be_rate = (double) params->tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] / (double) params->tc_period; - uint32_t tc_be_ov = s->tc_ov; - s->tc_ov_n += params->tc_ov_weight; - s->tc_ov_rate += pipe_tc_be_rate; - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; + if (s->is_tc_ov_enabled) { + uint32_t tc_be_ov = s->tc_ov; + + s->tc_ov_n += params->tc_ov_weight; + s->tc_ov_rate += pipe_tc_be_rate; + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; - if (s->tc_ov != tc_be_ov) { - RTE_LOG(DEBUG, SCHED, - "Subport %u Best effort TC oversubscription is ON (%.4lf < %.4lf)\n", - subport_id, subport_tc_be_rate, s->tc_ov_rate); + if (s->tc_ov != tc_be_ov) { + RTE_LOG(DEBUG, SCHED, + "Subport %u Best effort TC oversubscription is ON (%.4lf < %.4lf)\n", + subport_id, subport_tc_be_rate, s->tc_ov_rate); + } + p->tc_ov_period_id = s->tc_ov_period_id; + p->tc_ov_credits = s->tc_ov_wm; } - p->tc_ov_period_id = s->tc_ov_period_id; - p->tc_ov_credits = s->tc_ov_wm; } return 0; @@ -2318,6 +2359,45 @@ grinder_credits_update(struct rte_sched_port *port, pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); pipe->tb_time += n_periods * params->tb_period; + /* Subport TCs */ + if (unlikely(port->time >= subport->tc_time)) { + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) + subport->tc_credits[i] = sp->tc_credits_per_period[i]; + + subport->tc_time = port->time + sp->tc_period; + } + + /* Pipe TCs */ + if (unlikely(port->time >= pipe->tc_time)) { + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) + pipe->tc_credits[i] = params->tc_credits_per_period[i]; + pipe->tc_time = port->time + params->tc_period; + } +} + +static inline void +grinder_credits_update_with_tc_ov(struct rte_sched_port *port, + struct rte_sched_subport *subport, uint32_t pos) +{ + struct rte_sched_grinder *grinder = subport->grinder + pos; + struct rte_sched_pipe *pipe = grinder->pipe; + struct rte_sched_pipe_profile *params = grinder->pipe_params; + struct rte_sched_subport_profile *sp = grinder->subport_params; + uint64_t n_periods; + uint32_t i; + + /* Subport TB */ + n_periods = (port->time - subport->tb_time) / sp->tb_period; + subport->tb_credits += n_periods * sp->tb_credits_per_period; + subport->tb_credits = RTE_MIN(subport->tb_credits, sp->tb_size); + subport->tb_time += n_periods * sp->tb_period; + + /* Pipe TB */ + n_periods = (port->time - pipe->tb_time) / params->tb_period; + pipe->tb_credits += n_periods * params->tb_credits_per_period; + pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); + pipe->tb_time += n_periods * params->tb_period; + /* Subport TCs */ if (unlikely(port->time >= subport->tc_time)) { subport->tc_ov_wm = @@ -2348,6 +2428,39 @@ grinder_credits_update(struct rte_sched_port *port, static inline int grinder_credits_check(struct rte_sched_port *port, struct rte_sched_subport *subport, uint32_t pos) +{ + struct rte_sched_grinder *grinder = subport->grinder + pos; + struct rte_sched_pipe *pipe = grinder->pipe; + struct rte_mbuf *pkt = grinder->pkt; + uint32_t tc_index = grinder->tc_index; + uint64_t pkt_len = pkt->pkt_len + port->frame_overhead; + uint64_t subport_tb_credits = subport->tb_credits; + uint64_t subport_tc_credits = subport->tc_credits[tc_index]; + uint64_t pipe_tb_credits = pipe->tb_credits; + uint64_t pipe_tc_credits = pipe->tc_credits[tc_index]; + int enough_credits; + + /* Check pipe and subport credits */ + enough_credits = (pkt_len <= subport_tb_credits) && + (pkt_len <= subport_tc_credits) && + (pkt_len <= pipe_tb_credits) && + (pkt_len <= pipe_tc_credits); + + if (!enough_credits) + return 0; + + /* Update pipe and subport credits */ + subport->tb_credits -= pkt_len; + subport->tc_credits[tc_index] -= pkt_len; + pipe->tb_credits -= pkt_len; + pipe->tc_credits[tc_index] -= pkt_len; + + return 1; +} + +static inline int +grinder_credits_check_with_tc_ov(struct rte_sched_port *port, + struct rte_sched_subport *subport, uint32_t pos) { struct rte_sched_grinder *grinder = subport->grinder + pos; struct rte_sched_pipe *pipe = grinder->pipe; @@ -2403,8 +2516,16 @@ grinder_schedule(struct rte_sched_port *port, uint32_t pkt_len = pkt->pkt_len + port->frame_overhead; uint32_t be_tc_active; - if (!grinder_credits_check(port, subport, pos)) - return 0; + switch (subport->is_tc_ov_enabled) { + case 1: + if (!grinder_credits_check_with_tc_ov(port, subport, pos)) + return 0; + break; + case 0: + if (!grinder_credits_check(port, subport, pos)) + return 0; + break; + } /* Advance port time */ port->time += pkt_len; @@ -2770,7 +2891,11 @@ grinder_handle(struct rte_sched_port *port, subport->profile; grinder_prefetch_tc_queue_arrays(subport, pos); - grinder_credits_update(port, subport, pos); + + if (unlikely(subport->is_tc_ov_enabled)) + grinder_credits_update_with_tc_ov(port, subport, pos); + else + grinder_credits_update(port, subport, pos); grinder->state = e_GRINDER_PREFETCH_MBUF; return 0; diff --git a/lib/sched/rte_sched.h b/lib/sched/rte_sched.h index 5ece64e527..94febe1d94 100644 --- a/lib/sched/rte_sched.h +++ b/lib/sched/rte_sched.h @@ -579,6 +579,24 @@ rte_sched_port_enqueue(struct rte_sched_port *port, struct rte_mbuf **pkts, uint int rte_sched_port_dequeue(struct rte_sched_port *port, struct rte_mbuf **pkts, uint32_t n_pkts); +/** + * Hierarchical scheduler subport TC OV enable/disable config. + * Note that this function is safe to use at runtime + * to enable/disable TC OV for subport. + * + * @param port + * Handle to port scheduler instance + * @param subport_id + * Subport ID + * @param tc_ov_enable + * Boolean flag to enable/disable TC OV + * @return + * 0 upon success, error code otherwise + */ +__rte_experimental +int +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, uint32_t subport_id, bool tc_ov_enable); + #ifdef __cplusplus } #endif diff --git a/lib/sched/version.map b/lib/sched/version.map index d22c07fc9f..c6e994d8df 100644 --- a/lib/sched/version.map +++ b/lib/sched/version.map @@ -34,4 +34,7 @@ EXPERIMENTAL { # added in 21.11 rte_pie_rt_data_init; rte_pie_config_init; + + # added in 22.03 + rte_sched_subport_tc_ov_config; }; -- 2.25.1 -------------------------------------------------------------- Intel Research and Development Ireland Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v3] sched: enable/disable TC OV at runtime 2022-04-27 8:58 [PATCH v3] sched: enable/disable TC OV at runtime Marcin Danilewicz @ 2022-04-27 9:36 ` Dumitrescu, Cristian 2022-04-27 9:37 ` Dumitrescu, Cristian 2022-04-27 15:53 ` Stephen Hemminger 1 sibling, 1 reply; 12+ messages in thread From: Dumitrescu, Cristian @ 2022-04-27 9:36 UTC (permalink / raw) To: Danilewicz, MarcinX, dev Marcin, Every time you send a new version, you need to copy the maintainers and the other relevant people, otherwise there is a high chance we are not going to see your patch, thanks! I only saw this one due to pure chance ;) Regards, Cristian > -----Original Message----- > From: Marcin Danilewicz <marcinx.danilewicz@intel.com> > Sent: Wednesday, April 27, 2022 9:59 AM > To: dev@dpdk.org > Subject: [PATCH v3] sched: enable/disable TC OV at runtime > > Added new API to enable or disable TC over subscription for best > effort traffic class at subport level. > Added changes after review and increased throughput. > > By default TC OV is disabled. > > Signed-off-by: Marcin Danilewicz <marcinx.danilewicz@intel.com> > --- > lib/sched/rte_sched.c | 189 +++++++++++++++++++++++++++++++++++------- > lib/sched/rte_sched.h | 18 ++++ > lib/sched/version.map | 3 + > 3 files changed, 178 insertions(+), 32 deletions(-) > > diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c > index ec74bee939..6e7d81df46 100644 > --- a/lib/sched/rte_sched.c > +++ b/lib/sched/rte_sched.c > @@ -213,6 +213,9 @@ struct rte_sched_subport { > uint8_t *bmp_array; > struct rte_mbuf **queue_array; > uint8_t memory[0] __rte_cache_aligned; > + > + /* TC oversubscription activation */ > + int is_tc_ov_enabled; > } __rte_cache_aligned; > > struct rte_sched_port { > @@ -1165,6 +1168,45 @@ rte_sched_cman_config(struct rte_sched_port > *port, > } > #endif > > +int > +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, > + uint32_t subport_id, > + bool tc_ov_enable) > +{ > + struct rte_sched_subport *s; > + struct rte_sched_subport_profile *profile; > + > + 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__); > + return -EINVAL; > + } > + > + s = port->subports[subport_id]; > + s->is_tc_ov_enabled = tc_ov_enable ? 1 : 0; > + > + if (s->is_tc_ov_enabled) { > + /* TC oversubscription */ > + s->tc_ov_wm_min = port->mtu; > + s->tc_ov_period_id = 0; > + s->tc_ov = 0; > + s->tc_ov_n = 0; > + s->tc_ov_rate = 0; > + > + profile = port->subport_profiles + s->profile; > + s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile- > >tc_period, > + s->pipe_tc_be_rate_max); > + s->tc_ov_wm = s->tc_ov_wm_max; > + } > + return 0; > +} > + > int > rte_sched_subport_config(struct rte_sched_port *port, > uint32_t subport_id, > @@ -1254,6 +1296,9 @@ rte_sched_subport_config(struct rte_sched_port > *port, > s->n_pipe_profiles = params->n_pipe_profiles; > s->n_max_pipe_profiles = params->n_max_pipe_profiles; > > + /* TC over-subscription is disabled by default */ > + s->is_tc_ov_enabled = 0; > + > #ifdef RTE_SCHED_CMAN > if (params->cman_params != NULL) { > s->cman_enabled = true; > @@ -1316,13 +1361,6 @@ rte_sched_subport_config(struct rte_sched_port > *port, > > for (i = 0; i < RTE_SCHED_PORT_N_GRINDERS; i++) > s->grinder_base_bmp_pos[i] = > RTE_SCHED_PIPE_INVALID; > - > - /* TC oversubscription */ > - s->tc_ov_wm_min = port->mtu; > - s->tc_ov_period_id = 0; > - s->tc_ov = 0; > - s->tc_ov_n = 0; > - s->tc_ov_rate = 0; > } > > { > @@ -1342,9 +1380,6 @@ rte_sched_subport_config(struct rte_sched_port > *port, > else > profile->tc_credits_per_period[i] = 0; > > - s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile- > >tc_period, > - s- > >pipe_tc_be_rate_max); > - s->tc_ov_wm = s->tc_ov_wm_max; > s->profile = subport_profile_id; > > } > @@ -1417,17 +1452,20 @@ rte_sched_pipe_config(struct rte_sched_port > *port, > double pipe_tc_be_rate = > (double) params- > >tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] > / (double) params->tc_period; > - uint32_t tc_be_ov = s->tc_ov; > > - /* Unplug pipe from its subport */ > - s->tc_ov_n -= params->tc_ov_weight; > - s->tc_ov_rate -= pipe_tc_be_rate; > - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > + if (s->is_tc_ov_enabled) { > + uint32_t tc_be_ov = s->tc_ov; > > - if (s->tc_ov != tc_be_ov) { > - RTE_LOG(DEBUG, SCHED, > - "Subport %u Best-effort TC oversubscription is > OFF (%.4lf >= %.4lf)\n", > - subport_id, subport_tc_be_rate, s- > >tc_ov_rate); > + /* Unplug pipe from its subport */ > + s->tc_ov_n -= params->tc_ov_weight; > + s->tc_ov_rate -= pipe_tc_be_rate; > + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > + > + if (s->tc_ov != tc_be_ov) { > + RTE_LOG(DEBUG, SCHED, > + "Subport %u Best-effort TC > oversubscription is OFF (%.4lf >= %.4lf)\n", > + subport_id, subport_tc_be_rate, s- > >tc_ov_rate); > + } > } > > /* Reset the pipe */ > @@ -1460,19 +1498,22 @@ rte_sched_pipe_config(struct rte_sched_port > *port, > double pipe_tc_be_rate = > (double) params- > >tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] > / (double) params->tc_period; > - uint32_t tc_be_ov = s->tc_ov; > > - s->tc_ov_n += params->tc_ov_weight; > - s->tc_ov_rate += pipe_tc_be_rate; > - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > + if (s->is_tc_ov_enabled) { > + uint32_t tc_be_ov = s->tc_ov; > + > + s->tc_ov_n += params->tc_ov_weight; > + s->tc_ov_rate += pipe_tc_be_rate; > + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > > - if (s->tc_ov != tc_be_ov) { > - RTE_LOG(DEBUG, SCHED, > - "Subport %u Best effort TC oversubscription is > ON (%.4lf < %.4lf)\n", > - subport_id, subport_tc_be_rate, s- > >tc_ov_rate); > + if (s->tc_ov != tc_be_ov) { > + RTE_LOG(DEBUG, SCHED, > + "Subport %u Best effort TC > oversubscription is ON (%.4lf < %.4lf)\n", > + subport_id, subport_tc_be_rate, s- > >tc_ov_rate); > + } > + p->tc_ov_period_id = s->tc_ov_period_id; > + p->tc_ov_credits = s->tc_ov_wm; > } > - p->tc_ov_period_id = s->tc_ov_period_id; > - p->tc_ov_credits = s->tc_ov_wm; > } > > return 0; > @@ -2318,6 +2359,45 @@ grinder_credits_update(struct rte_sched_port > *port, > pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); > pipe->tb_time += n_periods * params->tb_period; > > + /* Subport TCs */ > + if (unlikely(port->time >= subport->tc_time)) { > + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > + subport->tc_credits[i] = sp->tc_credits_per_period[i]; > + > + subport->tc_time = port->time + sp->tc_period; > + } > + > + /* Pipe TCs */ > + if (unlikely(port->time >= pipe->tc_time)) { > + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > + pipe->tc_credits[i] = params->tc_credits_per_period[i]; > + pipe->tc_time = port->time + params->tc_period; > + } > +} > + > +static inline void > +grinder_credits_update_with_tc_ov(struct rte_sched_port *port, > + struct rte_sched_subport *subport, uint32_t pos) > +{ > + struct rte_sched_grinder *grinder = subport->grinder + pos; > + struct rte_sched_pipe *pipe = grinder->pipe; > + struct rte_sched_pipe_profile *params = grinder->pipe_params; > + struct rte_sched_subport_profile *sp = grinder->subport_params; > + uint64_t n_periods; > + uint32_t i; > + > + /* Subport TB */ > + n_periods = (port->time - subport->tb_time) / sp->tb_period; > + subport->tb_credits += n_periods * sp->tb_credits_per_period; > + subport->tb_credits = RTE_MIN(subport->tb_credits, sp->tb_size); > + subport->tb_time += n_periods * sp->tb_period; > + > + /* Pipe TB */ > + n_periods = (port->time - pipe->tb_time) / params->tb_period; > + pipe->tb_credits += n_periods * params->tb_credits_per_period; > + pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); > + pipe->tb_time += n_periods * params->tb_period; > + > /* Subport TCs */ > if (unlikely(port->time >= subport->tc_time)) { > subport->tc_ov_wm = > @@ -2348,6 +2428,39 @@ grinder_credits_update(struct rte_sched_port > *port, > static inline int > grinder_credits_check(struct rte_sched_port *port, > struct rte_sched_subport *subport, uint32_t pos) > +{ > + struct rte_sched_grinder *grinder = subport->grinder + pos; > + struct rte_sched_pipe *pipe = grinder->pipe; > + struct rte_mbuf *pkt = grinder->pkt; > + uint32_t tc_index = grinder->tc_index; > + uint64_t pkt_len = pkt->pkt_len + port->frame_overhead; > + uint64_t subport_tb_credits = subport->tb_credits; > + uint64_t subport_tc_credits = subport->tc_credits[tc_index]; > + uint64_t pipe_tb_credits = pipe->tb_credits; > + uint64_t pipe_tc_credits = pipe->tc_credits[tc_index]; > + int enough_credits; > + > + /* Check pipe and subport credits */ > + enough_credits = (pkt_len <= subport_tb_credits) && > + (pkt_len <= subport_tc_credits) && > + (pkt_len <= pipe_tb_credits) && > + (pkt_len <= pipe_tc_credits); > + > + if (!enough_credits) > + return 0; > + > + /* Update pipe and subport credits */ > + subport->tb_credits -= pkt_len; > + subport->tc_credits[tc_index] -= pkt_len; > + pipe->tb_credits -= pkt_len; > + pipe->tc_credits[tc_index] -= pkt_len; > + > + return 1; > +} > + > +static inline int > +grinder_credits_check_with_tc_ov(struct rte_sched_port *port, > + struct rte_sched_subport *subport, uint32_t pos) > { > struct rte_sched_grinder *grinder = subport->grinder + pos; > struct rte_sched_pipe *pipe = grinder->pipe; > @@ -2403,8 +2516,16 @@ grinder_schedule(struct rte_sched_port *port, > uint32_t pkt_len = pkt->pkt_len + port->frame_overhead; > uint32_t be_tc_active; > > - if (!grinder_credits_check(port, subport, pos)) > - return 0; > + switch (subport->is_tc_ov_enabled) { > + case 1: > + if (!grinder_credits_check_with_tc_ov(port, subport, pos)) > + return 0; > + break; > + case 0: > + if (!grinder_credits_check(port, subport, pos)) > + return 0; > + break; > + } > > /* Advance port time */ > port->time += pkt_len; > @@ -2770,7 +2891,11 @@ grinder_handle(struct rte_sched_port *port, > subport->profile; > > grinder_prefetch_tc_queue_arrays(subport, pos); > - grinder_credits_update(port, subport, pos); > + > + if (unlikely(subport->is_tc_ov_enabled)) > + grinder_credits_update_with_tc_ov(port, subport, pos); > + else > + grinder_credits_update(port, subport, pos); > > grinder->state = e_GRINDER_PREFETCH_MBUF; > return 0; > diff --git a/lib/sched/rte_sched.h b/lib/sched/rte_sched.h > index 5ece64e527..94febe1d94 100644 > --- a/lib/sched/rte_sched.h > +++ b/lib/sched/rte_sched.h > @@ -579,6 +579,24 @@ rte_sched_port_enqueue(struct rte_sched_port *port, > struct rte_mbuf **pkts, uint > int > rte_sched_port_dequeue(struct rte_sched_port *port, struct rte_mbuf **pkts, > uint32_t n_pkts); > > +/** > + * Hierarchical scheduler subport TC OV enable/disable config. > + * Note that this function is safe to use at runtime > + * to enable/disable TC OV for subport. > + * > + * @param port > + * Handle to port scheduler instance > + * @param subport_id > + * Subport ID > + * @param tc_ov_enable > + * Boolean flag to enable/disable TC OV > + * @return > + * 0 upon success, error code otherwise > + */ > +__rte_experimental > +int > +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, uint32_t > subport_id, bool tc_ov_enable); > + > #ifdef __cplusplus > } > #endif > diff --git a/lib/sched/version.map b/lib/sched/version.map > index d22c07fc9f..c6e994d8df 100644 > --- a/lib/sched/version.map > +++ b/lib/sched/version.map > @@ -34,4 +34,7 @@ EXPERIMENTAL { > # added in 21.11 > rte_pie_rt_data_init; > rte_pie_config_init; > + > + # added in 22.03 > + rte_sched_subport_tc_ov_config; > }; > -- > 2.25.1 > > -------------------------------------------------------------- > Intel Research and Development Ireland Limited > Registered in Ireland > Registered Office: Collinstown Industrial Park, Leixlip, County Kildare > Registered Number: 308263 > > > This e-mail and any attachments may contain confidential material for the sole > use of the intended recipient(s). Any review or distribution by others is > strictly prohibited. If you are not the intended recipient, please contact the > sender and delete all copies. ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v3] sched: enable/disable TC OV at runtime 2022-04-27 9:36 ` Dumitrescu, Cristian @ 2022-04-27 9:37 ` Dumitrescu, Cristian 0 siblings, 0 replies; 12+ messages in thread From: Dumitrescu, Cristian @ 2022-04-27 9:37 UTC (permalink / raw) To: Danilewicz, MarcinX, dev; +Cc: Singh, Jasvinder Adding Jasvinder > -----Original Message----- > From: Dumitrescu, Cristian > Sent: Wednesday, April 27, 2022 10:37 AM > To: Marcin Danilewicz <marcinx.danilewicz@intel.com>; dev@dpdk.org > Subject: RE: [PATCH v3] sched: enable/disable TC OV at runtime > > Marcin, > > Every time you send a new version, you need to copy the maintainers and the > other relevant people, otherwise there is a high chance we are not going to see > your patch, thanks! I only saw this one due to pure chance ;) > > Regards, > Cristian > > > -----Original Message----- > > From: Marcin Danilewicz <marcinx.danilewicz@intel.com> > > Sent: Wednesday, April 27, 2022 9:59 AM > > To: dev@dpdk.org > > Subject: [PATCH v3] sched: enable/disable TC OV at runtime > > > > Added new API to enable or disable TC over subscription for best > > effort traffic class at subport level. > > Added changes after review and increased throughput. > > > > By default TC OV is disabled. > > > > Signed-off-by: Marcin Danilewicz <marcinx.danilewicz@intel.com> > > --- > > lib/sched/rte_sched.c | 189 +++++++++++++++++++++++++++++++++++------- > > lib/sched/rte_sched.h | 18 ++++ > > lib/sched/version.map | 3 + > > 3 files changed, 178 insertions(+), 32 deletions(-) > > > > diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c > > index ec74bee939..6e7d81df46 100644 > > --- a/lib/sched/rte_sched.c > > +++ b/lib/sched/rte_sched.c > > @@ -213,6 +213,9 @@ struct rte_sched_subport { > > uint8_t *bmp_array; > > struct rte_mbuf **queue_array; > > uint8_t memory[0] __rte_cache_aligned; > > + > > + /* TC oversubscription activation */ > > + int is_tc_ov_enabled; > > } __rte_cache_aligned; > > > > struct rte_sched_port { > > @@ -1165,6 +1168,45 @@ rte_sched_cman_config(struct rte_sched_port > > *port, > > } > > #endif > > > > +int > > +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, > > + uint32_t subport_id, > > + bool tc_ov_enable) > > +{ > > + struct rte_sched_subport *s; > > + struct rte_sched_subport_profile *profile; > > + > > + 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__); > > + return -EINVAL; > > + } > > + > > + s = port->subports[subport_id]; > > + s->is_tc_ov_enabled = tc_ov_enable ? 1 : 0; > > + > > + if (s->is_tc_ov_enabled) { > > + /* TC oversubscription */ > > + s->tc_ov_wm_min = port->mtu; > > + s->tc_ov_period_id = 0; > > + s->tc_ov = 0; > > + s->tc_ov_n = 0; > > + s->tc_ov_rate = 0; > > + > > + profile = port->subport_profiles + s->profile; > > + s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile- > > >tc_period, > > + s->pipe_tc_be_rate_max); > > + s->tc_ov_wm = s->tc_ov_wm_max; > > + } > > + return 0; > > +} > > + > > int > > rte_sched_subport_config(struct rte_sched_port *port, > > uint32_t subport_id, > > @@ -1254,6 +1296,9 @@ rte_sched_subport_config(struct rte_sched_port > > *port, > > s->n_pipe_profiles = params->n_pipe_profiles; > > s->n_max_pipe_profiles = params->n_max_pipe_profiles; > > > > + /* TC over-subscription is disabled by default */ > > + s->is_tc_ov_enabled = 0; > > + > > #ifdef RTE_SCHED_CMAN > > if (params->cman_params != NULL) { > > s->cman_enabled = true; > > @@ -1316,13 +1361,6 @@ rte_sched_subport_config(struct rte_sched_port > > *port, > > > > for (i = 0; i < RTE_SCHED_PORT_N_GRINDERS; i++) > > s->grinder_base_bmp_pos[i] = > > RTE_SCHED_PIPE_INVALID; > > - > > - /* TC oversubscription */ > > - s->tc_ov_wm_min = port->mtu; > > - s->tc_ov_period_id = 0; > > - s->tc_ov = 0; > > - s->tc_ov_n = 0; > > - s->tc_ov_rate = 0; > > } > > > > { > > @@ -1342,9 +1380,6 @@ rte_sched_subport_config(struct rte_sched_port > > *port, > > else > > profile->tc_credits_per_period[i] = 0; > > > > - s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile- > > >tc_period, > > - s- > > >pipe_tc_be_rate_max); > > - s->tc_ov_wm = s->tc_ov_wm_max; > > s->profile = subport_profile_id; > > > > } > > @@ -1417,17 +1452,20 @@ rte_sched_pipe_config(struct rte_sched_port > > *port, > > double pipe_tc_be_rate = > > (double) params- > > >tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] > > / (double) params->tc_period; > > - uint32_t tc_be_ov = s->tc_ov; > > > > - /* Unplug pipe from its subport */ > > - s->tc_ov_n -= params->tc_ov_weight; > > - s->tc_ov_rate -= pipe_tc_be_rate; > > - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > > + if (s->is_tc_ov_enabled) { > > + uint32_t tc_be_ov = s->tc_ov; > > > > - if (s->tc_ov != tc_be_ov) { > > - RTE_LOG(DEBUG, SCHED, > > - "Subport %u Best-effort TC oversubscription is > > OFF (%.4lf >= %.4lf)\n", > > - subport_id, subport_tc_be_rate, s- > > >tc_ov_rate); > > + /* Unplug pipe from its subport */ > > + s->tc_ov_n -= params->tc_ov_weight; > > + s->tc_ov_rate -= pipe_tc_be_rate; > > + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > > + > > + if (s->tc_ov != tc_be_ov) { > > + RTE_LOG(DEBUG, SCHED, > > + "Subport %u Best-effort TC > > oversubscription is OFF (%.4lf >= %.4lf)\n", > > + subport_id, subport_tc_be_rate, s- > > >tc_ov_rate); > > + } > > } > > > > /* Reset the pipe */ > > @@ -1460,19 +1498,22 @@ rte_sched_pipe_config(struct rte_sched_port > > *port, > > double pipe_tc_be_rate = > > (double) params- > > >tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] > > / (double) params->tc_period; > > - uint32_t tc_be_ov = s->tc_ov; > > > > - s->tc_ov_n += params->tc_ov_weight; > > - s->tc_ov_rate += pipe_tc_be_rate; > > - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > > + if (s->is_tc_ov_enabled) { > > + uint32_t tc_be_ov = s->tc_ov; > > + > > + s->tc_ov_n += params->tc_ov_weight; > > + s->tc_ov_rate += pipe_tc_be_rate; > > + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > > > > - if (s->tc_ov != tc_be_ov) { > > - RTE_LOG(DEBUG, SCHED, > > - "Subport %u Best effort TC oversubscription is > > ON (%.4lf < %.4lf)\n", > > - subport_id, subport_tc_be_rate, s- > > >tc_ov_rate); > > + if (s->tc_ov != tc_be_ov) { > > + RTE_LOG(DEBUG, SCHED, > > + "Subport %u Best effort TC > > oversubscription is ON (%.4lf < %.4lf)\n", > > + subport_id, subport_tc_be_rate, s- > > >tc_ov_rate); > > + } > > + p->tc_ov_period_id = s->tc_ov_period_id; > > + p->tc_ov_credits = s->tc_ov_wm; > > } > > - p->tc_ov_period_id = s->tc_ov_period_id; > > - p->tc_ov_credits = s->tc_ov_wm; > > } > > > > return 0; > > @@ -2318,6 +2359,45 @@ grinder_credits_update(struct rte_sched_port > > *port, > > pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); > > pipe->tb_time += n_periods * params->tb_period; > > > > + /* Subport TCs */ > > + if (unlikely(port->time >= subport->tc_time)) { > > + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > > + subport->tc_credits[i] = sp->tc_credits_per_period[i]; > > + > > + subport->tc_time = port->time + sp->tc_period; > > + } > > + > > + /* Pipe TCs */ > > + if (unlikely(port->time >= pipe->tc_time)) { > > + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > > + pipe->tc_credits[i] = params->tc_credits_per_period[i]; > > + pipe->tc_time = port->time + params->tc_period; > > + } > > +} > > + > > +static inline void > > +grinder_credits_update_with_tc_ov(struct rte_sched_port *port, > > + struct rte_sched_subport *subport, uint32_t pos) > > +{ > > + struct rte_sched_grinder *grinder = subport->grinder + pos; > > + struct rte_sched_pipe *pipe = grinder->pipe; > > + struct rte_sched_pipe_profile *params = grinder->pipe_params; > > + struct rte_sched_subport_profile *sp = grinder->subport_params; > > + uint64_t n_periods; > > + uint32_t i; > > + > > + /* Subport TB */ > > + n_periods = (port->time - subport->tb_time) / sp->tb_period; > > + subport->tb_credits += n_periods * sp->tb_credits_per_period; > > + subport->tb_credits = RTE_MIN(subport->tb_credits, sp->tb_size); > > + subport->tb_time += n_periods * sp->tb_period; > > + > > + /* Pipe TB */ > > + n_periods = (port->time - pipe->tb_time) / params->tb_period; > > + pipe->tb_credits += n_periods * params->tb_credits_per_period; > > + pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); > > + pipe->tb_time += n_periods * params->tb_period; > > + > > /* Subport TCs */ > > if (unlikely(port->time >= subport->tc_time)) { > > subport->tc_ov_wm = > > @@ -2348,6 +2428,39 @@ grinder_credits_update(struct rte_sched_port > > *port, > > static inline int > > grinder_credits_check(struct rte_sched_port *port, > > struct rte_sched_subport *subport, uint32_t pos) > > +{ > > + struct rte_sched_grinder *grinder = subport->grinder + pos; > > + struct rte_sched_pipe *pipe = grinder->pipe; > > + struct rte_mbuf *pkt = grinder->pkt; > > + uint32_t tc_index = grinder->tc_index; > > + uint64_t pkt_len = pkt->pkt_len + port->frame_overhead; > > + uint64_t subport_tb_credits = subport->tb_credits; > > + uint64_t subport_tc_credits = subport->tc_credits[tc_index]; > > + uint64_t pipe_tb_credits = pipe->tb_credits; > > + uint64_t pipe_tc_credits = pipe->tc_credits[tc_index]; > > + int enough_credits; > > + > > + /* Check pipe and subport credits */ > > + enough_credits = (pkt_len <= subport_tb_credits) && > > + (pkt_len <= subport_tc_credits) && > > + (pkt_len <= pipe_tb_credits) && > > + (pkt_len <= pipe_tc_credits); > > + > > + if (!enough_credits) > > + return 0; > > + > > + /* Update pipe and subport credits */ > > + subport->tb_credits -= pkt_len; > > + subport->tc_credits[tc_index] -= pkt_len; > > + pipe->tb_credits -= pkt_len; > > + pipe->tc_credits[tc_index] -= pkt_len; > > + > > + return 1; > > +} > > + > > +static inline int > > +grinder_credits_check_with_tc_ov(struct rte_sched_port *port, > > + struct rte_sched_subport *subport, uint32_t pos) > > { > > struct rte_sched_grinder *grinder = subport->grinder + pos; > > struct rte_sched_pipe *pipe = grinder->pipe; > > @@ -2403,8 +2516,16 @@ grinder_schedule(struct rte_sched_port *port, > > uint32_t pkt_len = pkt->pkt_len + port->frame_overhead; > > uint32_t be_tc_active; > > > > - if (!grinder_credits_check(port, subport, pos)) > > - return 0; > > + switch (subport->is_tc_ov_enabled) { > > + case 1: > > + if (!grinder_credits_check_with_tc_ov(port, subport, pos)) > > + return 0; > > + break; > > + case 0: > > + if (!grinder_credits_check(port, subport, pos)) > > + return 0; > > + break; > > + } > > > > /* Advance port time */ > > port->time += pkt_len; > > @@ -2770,7 +2891,11 @@ grinder_handle(struct rte_sched_port *port, > > subport->profile; > > > > grinder_prefetch_tc_queue_arrays(subport, pos); > > - grinder_credits_update(port, subport, pos); > > + > > + if (unlikely(subport->is_tc_ov_enabled)) > > + grinder_credits_update_with_tc_ov(port, subport, pos); > > + else > > + grinder_credits_update(port, subport, pos); > > > > grinder->state = e_GRINDER_PREFETCH_MBUF; > > return 0; > > diff --git a/lib/sched/rte_sched.h b/lib/sched/rte_sched.h > > index 5ece64e527..94febe1d94 100644 > > --- a/lib/sched/rte_sched.h > > +++ b/lib/sched/rte_sched.h > > @@ -579,6 +579,24 @@ rte_sched_port_enqueue(struct rte_sched_port > *port, > > struct rte_mbuf **pkts, uint > > int > > rte_sched_port_dequeue(struct rte_sched_port *port, struct rte_mbuf > **pkts, > > uint32_t n_pkts); > > > > +/** > > + * Hierarchical scheduler subport TC OV enable/disable config. > > + * Note that this function is safe to use at runtime > > + * to enable/disable TC OV for subport. > > + * > > + * @param port > > + * Handle to port scheduler instance > > + * @param subport_id > > + * Subport ID > > + * @param tc_ov_enable > > + * Boolean flag to enable/disable TC OV > > + * @return > > + * 0 upon success, error code otherwise > > + */ > > +__rte_experimental > > +int > > +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, uint32_t > > subport_id, bool tc_ov_enable); > > + > > #ifdef __cplusplus > > } > > #endif > > diff --git a/lib/sched/version.map b/lib/sched/version.map > > index d22c07fc9f..c6e994d8df 100644 > > --- a/lib/sched/version.map > > +++ b/lib/sched/version.map > > @@ -34,4 +34,7 @@ EXPERIMENTAL { > > # added in 21.11 > > rte_pie_rt_data_init; > > rte_pie_config_init; > > + > > + # added in 22.03 > > + rte_sched_subport_tc_ov_config; > > }; > > -- > > 2.25.1 > > > > -------------------------------------------------------------- > > Intel Research and Development Ireland Limited > > Registered in Ireland > > Registered Office: Collinstown Industrial Park, Leixlip, County Kildare > > Registered Number: 308263 > > > > > > This e-mail and any attachments may contain confidential material for the > sole > > use of the intended recipient(s). Any review or distribution by others is > > strictly prohibited. If you are not the intended recipient, please contact the > > sender and delete all copies. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] sched: enable/disable TC OV at runtime 2022-04-27 8:58 [PATCH v3] sched: enable/disable TC OV at runtime Marcin Danilewicz 2022-04-27 9:36 ` Dumitrescu, Cristian @ 2022-04-27 15:53 ` Stephen Hemminger 1 sibling, 0 replies; 12+ messages in thread From: Stephen Hemminger @ 2022-04-27 15:53 UTC (permalink / raw) To: Marcin Danilewicz; +Cc: dev On Wed, 27 Apr 2022 08:58:48 +0000 Marcin Danilewicz <marcinx.danilewicz@intel.com> wrote: > Added new API to enable or disable TC over subscription for best > effort traffic class at subport level. > Added changes after review and increased throughput. > > By default TC OV is disabled. > > Signed-off-by: Marcin Danilewicz <marcinx.danilewicz@intel.com> > --- > lib/sched/rte_sched.c | 189 +++++++++++++++++++++++++++++++++++------- > lib/sched/rte_sched.h | 18 ++++ > lib/sched/version.map | 3 + > 3 files changed, 178 insertions(+), 32 deletions(-) > > diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c > index ec74bee939..6e7d81df46 100644 > --- a/lib/sched/rte_sched.c > +++ b/lib/sched/rte_sched.c > @@ -213,6 +213,9 @@ struct rte_sched_subport { > uint8_t *bmp_array; > struct rte_mbuf **queue_array; > uint8_t memory[0] __rte_cache_aligned; > + > + /* TC oversubscription activation */ > + int is_tc_ov_enabled; > } __rte_cache_aligned; Since this is a flag, either use uint8_t or bool? Also, there are holes in that data structure that should be used. struct rte_sched_port { uint32_t n_subports_per_port; /* 0 4 */ uint32_t n_pipes_per_subport; /* 4 4 */ uint32_t n_pipes_per_subport_log2; /* 8 4 */ uint16_t pipe_queue[13]; /* 12 26 */ uint8_t pipe_tc[16]; /* 38 16 */ uint8_t tc_queue[16]; /* 54 16 */ /* XXX 2 bytes hole, try to pack */ /* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */ uint32_t n_subport_profiles; /* 72 4 */ uint32_t n_max_subport_profiles; /* 76 4 */ uint64_t rate; /* 80 8 */ uint32_t mtu; /* 88 4 */ uint32_t frame_overhead; /* 92 4 */ int socket; /* 96 4 */ /* XXX 4 bytes hole, try to pack */ uint64_t time_cpu_cycles; /* 104 8 */ uint64_t time_cpu_bytes; /* 112 8 */ uint64_t time; /* 120 8 */ /* --- cacheline 2 boundary (128 bytes) --- */ struct rte_reciprocal inv_cycles_per_byte; /* 128 8 */ /* XXX last struct has 2 bytes of padding */ uint64_t cycles_per_byte; /* 136 8 */ struct rte_mbuf * * pkts_out; /* 144 8 */ uint32_t n_pkts_out; /* 152 4 */ uint32_t subport_id; /* 156 4 */ struct rte_sched_subport_profile * subport_profiles; /* 160 8 */ /* XXX 24 bytes hole, try to pack */ /* --- cacheline 3 boundary (192 bytes) --- */ struct rte_sched_subport * subports[] __attribute__((__aligned__(64))); /* 192 0 */ /* size: 192, cachelines: 3, members: 22 */ /* sum members: 162, holes: 3, sum holes: 30 */ /* paddings: 1, sum paddings: 2 */ /* forced alignments: 1, forced holes: 1, sum forced holes: 24 */ } __attribute__((__aligned__(64))); > > struct rte_sched_port { > @@ -1165,6 +1168,45 @@ rte_sched_cman_config(struct rte_sched_port *port, > } > #endif > > +int > +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, > + uint32_t subport_id, > + bool tc_ov_enable) > +{ > + struct rte_sched_subport *s; > + struct rte_sched_subport_profile *profile; > + > + 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__); > + return -EINVAL; > + } > + > + s = port->subports[subport_id]; > + s->is_tc_ov_enabled = tc_ov_enable ? 1 : 0; > + > + if (s->is_tc_ov_enabled) { > + /* TC oversubscription */ > + s->tc_ov_wm_min = port->mtu; > + s->tc_ov_period_id = 0; > + s->tc_ov = 0; > + s->tc_ov_n = 0; > + s->tc_ov_rate = 0; > + > + profile = port->subport_profiles + s->profile; > + s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile->tc_period, > + s->pipe_tc_be_rate_max); > + s->tc_ov_wm = s->tc_ov_wm_max; All of tc_ov_XX could be a sub structure and the init might be cleaner. > + } > + return 0; > +} > + > int > rte_sched_subport_config(struct rte_sched_port *port, > uint32_t subport_id, > @@ -1254,6 +1296,9 @@ rte_sched_subport_config(struct rte_sched_port *port, > s->n_pipe_profiles = params->n_pipe_profiles; > s->n_max_pipe_profiles = params->n_max_pipe_profiles; > > + /* TC over-subscription is disabled by default */ > + s->is_tc_ov_enabled = 0; > + > #ifdef RTE_SCHED_CMAN > if (params->cman_params != NULL) { > s->cman_enabled = true; > @@ -1316,13 +1361,6 @@ rte_sched_subport_config(struct rte_sched_port *port, > > for (i = 0; i < RTE_SCHED_PORT_N_GRINDERS; i++) > s->grinder_base_bmp_pos[i] = RTE_SCHED_PIPE_INVALID; > - > - /* TC oversubscription */ > - s->tc_ov_wm_min = port->mtu; > - s->tc_ov_period_id = 0; > - s->tc_ov = 0; > - s->tc_ov_n = 0; > - s->tc_ov_rate = 0; > } > > { > @@ -1342,9 +1380,6 @@ rte_sched_subport_config(struct rte_sched_port *port, > else > profile->tc_credits_per_period[i] = 0; > > - s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile->tc_period, > - s->pipe_tc_be_rate_max); > - s->tc_ov_wm = s->tc_ov_wm_max; > s->profile = subport_profile_id; > > } > @@ -1417,17 +1452,20 @@ rte_sched_pipe_config(struct rte_sched_port *port, > double pipe_tc_be_rate = > (double) params->tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] > / (double) params->tc_period; > - uint32_t tc_be_ov = s->tc_ov; > > - /* Unplug pipe from its subport */ > - s->tc_ov_n -= params->tc_ov_weight; > - s->tc_ov_rate -= pipe_tc_be_rate; > - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > + if (s->is_tc_ov_enabled) { > + uint32_t tc_be_ov = s->tc_ov; > > - if (s->tc_ov != tc_be_ov) { > - RTE_LOG(DEBUG, SCHED, > - "Subport %u Best-effort TC oversubscription is OFF (%.4lf >= %.4lf)\n", > - subport_id, subport_tc_be_rate, s->tc_ov_rate); > + /* Unplug pipe from its subport */ > + s->tc_ov_n -= params->tc_ov_weight; > + s->tc_ov_rate -= pipe_tc_be_rate; > + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > + > + if (s->tc_ov != tc_be_ov) { > + RTE_LOG(DEBUG, SCHED, > + "Subport %u Best-effort TC oversubscription is OFF (%.4lf >= %.4lf)\n", > + subport_id, subport_tc_be_rate, s->tc_ov_rate); > + } > } > > /* Reset the pipe */ > @@ -1460,19 +1498,22 @@ rte_sched_pipe_config(struct rte_sched_port *port, > double pipe_tc_be_rate = > (double) params->tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] > / (double) params->tc_period; > - uint32_t tc_be_ov = s->tc_ov; > > - s->tc_ov_n += params->tc_ov_weight; > - s->tc_ov_rate += pipe_tc_be_rate; > - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > + if (s->is_tc_ov_enabled) { > + uint32_t tc_be_ov = s->tc_ov; > + > + s->tc_ov_n += params->tc_ov_weight; > + s->tc_ov_rate += pipe_tc_be_rate; > + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > > - if (s->tc_ov != tc_be_ov) { > - RTE_LOG(DEBUG, SCHED, > - "Subport %u Best effort TC oversubscription is ON (%.4lf < %.4lf)\n", > - subport_id, subport_tc_be_rate, s->tc_ov_rate); > + if (s->tc_ov != tc_be_ov) { > + RTE_LOG(DEBUG, SCHED, > + "Subport %u Best effort TC oversubscription is ON (%.4lf < %.4lf)\n", > + subport_id, subport_tc_be_rate, s->tc_ov_rate); > + } > + p->tc_ov_period_id = s->tc_ov_period_id; > + p->tc_ov_credits = s->tc_ov_wm; > } > - p->tc_ov_period_id = s->tc_ov_period_id; > - p->tc_ov_credits = s->tc_ov_wm; > } > > return 0; > @@ -2318,6 +2359,45 @@ grinder_credits_update(struct rte_sched_port *port, > pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); > pipe->tb_time += n_periods * params->tb_period; > > + /* Subport TCs */ > + if (unlikely(port->time >= subport->tc_time)) { > + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > + subport->tc_credits[i] = sp->tc_credits_per_period[i]; > + > + subport->tc_time = port->time + sp->tc_period; > + } > + > + /* Pipe TCs */ > + if (unlikely(port->time >= pipe->tc_time)) { > + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > + pipe->tc_credits[i] = params->tc_credits_per_period[i]; > + pipe->tc_time = port->time + params->tc_period; > + } > +} > + > +static inline void > +grinder_credits_update_with_tc_ov(struct rte_sched_port *port, > + struct rte_sched_subport *subport, uint32_t pos) > +{ > + struct rte_sched_grinder *grinder = subport->grinder + pos; > + struct rte_sched_pipe *pipe = grinder->pipe; > + struct rte_sched_pipe_profile *params = grinder->pipe_params; > + struct rte_sched_subport_profile *sp = grinder->subport_params; > + uint64_t n_periods; > + uint32_t i; > + > + /* Subport TB */ > + n_periods = (port->time - subport->tb_time) / sp->tb_period; > + subport->tb_credits += n_periods * sp->tb_credits_per_period; > + subport->tb_credits = RTE_MIN(subport->tb_credits, sp->tb_size); > + subport->tb_time += n_periods * sp->tb_period; > + > + /* Pipe TB */ > + n_periods = (port->time - pipe->tb_time) / params->tb_period; > + pipe->tb_credits += n_periods * params->tb_credits_per_period; > + pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); > + pipe->tb_time += n_periods * params->tb_period; > + > /* Subport TCs */ > if (unlikely(port->time >= subport->tc_time)) { > subport->tc_ov_wm = > @@ -2348,6 +2428,39 @@ grinder_credits_update(struct rte_sched_port *port, > static inline int > grinder_credits_check(struct rte_sched_port *port, > struct rte_sched_subport *subport, uint32_t pos) Either return negative errno, or make return a boolean. > +{ > + struct rte_sched_grinder *grinder = subport->grinder + pos; > + struct rte_sched_pipe *pipe = grinder->pipe; > + struct rte_mbuf *pkt = grinder->pkt; > + uint32_t tc_index = grinder->tc_index; > + uint64_t pkt_len = pkt->pkt_len + port->frame_overhead; > + uint64_t subport_tb_credits = subport->tb_credits; > + uint64_t subport_tc_credits = subport->tc_credits[tc_index]; > + uint64_t pipe_tb_credits = pipe->tb_credits; > + uint64_t pipe_tc_credits = pipe->tc_credits[tc_index]; > + int enough_credits; > + > + /* Check pipe and subport credits */ > + enough_credits = (pkt_len <= subport_tb_credits) && > + (pkt_len <= subport_tc_credits) && > + (pkt_len <= pipe_tb_credits) && > + (pkt_len <= pipe_tc_credits); > + > + if (!enough_credits) > + return 0; > + > + /* Update pipe and subport credits */ > + subport->tb_credits -= pkt_len; > + subport->tc_credits[tc_index] -= pkt_len; > + pipe->tb_credits -= pkt_len; > + pipe->tc_credits[tc_index] -= pkt_len; > + > + return 1; > +} > + > +static inline int > +grinder_credits_check_with_tc_ov(struct rte_sched_port *port, > + struct rte_sched_subport *subport, uint32_t pos) > { > struct rte_sched_grinder *grinder = subport->grinder + pos; > struct rte_sched_pipe *pipe = grinder->pipe; > @@ -2403,8 +2516,16 @@ grinder_schedule(struct rte_sched_port *port, > uint32_t pkt_len = pkt->pkt_len + port->frame_overhead; > uint32_t be_tc_active; > > - if (!grinder_credits_check(port, subport, pos)) > - return 0; > + switch (subport->is_tc_ov_enabled) { > + case 1: > + if (!grinder_credits_check_with_tc_ov(port, subport, pos)) > + return 0; > + break; > + case 0: > + if (!grinder_credits_check(port, subport, pos)) > + return 0; > + break; > + } > > /* Advance port time */ > port->time += pkt_len; > @@ -2770,7 +2891,11 @@ grinder_handle(struct rte_sched_port *port, > subport->profile; > > grinder_prefetch_tc_queue_arrays(subport, pos); > - grinder_credits_update(port, subport, pos); > + > + if (unlikely(subport->is_tc_ov_enabled)) > + grinder_credits_update_with_tc_ov(port, subport, pos); > + else > + grinder_credits_update(port, subport, pos); > > grinder->state = e_GRINDER_PREFETCH_MBUF; > return 0; > diff --git a/lib/sched/rte_sched.h b/lib/sched/rte_sched.h > index 5ece64e527..94febe1d94 100644 > --- a/lib/sched/rte_sched.h > +++ b/lib/sched/rte_sched.h > @@ -579,6 +579,24 @@ rte_sched_port_enqueue(struct rte_sched_port *port, struct rte_mbuf **pkts, uint > int > rte_sched_port_dequeue(struct rte_sched_port *port, struct rte_mbuf **pkts, uint32_t n_pkts); > > +/** > + * Hierarchical scheduler subport TC OV enable/disable config. > + * Note that this function is safe to use at runtime > + * to enable/disable TC OV for subport. > + * > + * @param port > + * Handle to port scheduler instance > + * @param subport_id > + * Subport ID > + * @param tc_ov_enable > + * Boolean flag to enable/disable TC OV > + * @return > + * 0 upon success, error code otherwise > + */ > +__rte_experimental > +int > +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, uint32_t subport_id, bool tc_ov_enable); > + > #ifdef __cplusplus > } > #endif > diff --git a/lib/sched/version.map b/lib/sched/version.map > index d22c07fc9f..c6e994d8df 100644 > --- a/lib/sched/version.map > +++ b/lib/sched/version.map > @@ -34,4 +34,7 @@ EXPERIMENTAL { > # added in 21.11 > rte_pie_rt_data_init; > rte_pie_config_init; > + > + # added in 22.03 > + rte_sched_subport_tc_ov_config; > }; ^ permalink raw reply [flat|nested] 12+ messages in thread
* [dpdk][PATCH 2/2] sched: fix to manage TC OV at runtime @ 2022-04-07 14:51 Marcin Danilewicz 2022-04-27 9:23 ` [PATCH v3] sched: enable/disable " Marcin Danilewicz 0 siblings, 1 reply; 12+ messages in thread From: Marcin Danilewicz @ 2022-04-07 14:51 UTC (permalink / raw) To: dev Added changes after review and increased throughput. Signed-off-by: Marcin Danilewicz <marcinx.danilewicz@intel.com> diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c index 1d05089d00..6e7d81df46 100644 --- a/lib/sched/rte_sched.c +++ b/lib/sched/rte_sched.c @@ -155,7 +155,6 @@ struct rte_sched_subport { uint64_t tc_credits[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE]; /* TC oversubscription */ - uint8_t is_tc_ov_enabled; uint64_t tc_ov_wm; uint64_t tc_ov_wm_min; uint64_t tc_ov_wm_max; @@ -214,6 +213,9 @@ struct rte_sched_subport { uint8_t *bmp_array; struct rte_mbuf **queue_array; uint8_t memory[0] __rte_cache_aligned; + + /* TC oversubscription activation */ + int is_tc_ov_enabled; } __rte_cache_aligned; struct rte_sched_port { @@ -1187,7 +1189,7 @@ rte_sched_subport_tc_ov_config(struct rte_sched_port *port, } s = port->subports[subport_id]; - s->is_tc_ov_enabled = tc_ov_enable; + s->is_tc_ov_enabled = tc_ov_enable ? 1 : 0; if (s->is_tc_ov_enabled) { /* TC oversubscription */ @@ -1294,6 +1296,9 @@ rte_sched_subport_config(struct rte_sched_port *port, s->n_pipe_profiles = params->n_pipe_profiles; s->n_max_pipe_profiles = params->n_max_pipe_profiles; + /* TC over-subscription is disabled by default */ + s->is_tc_ov_enabled = 0; + #ifdef RTE_SCHED_CMAN if (params->cman_params != NULL) { s->cman_enabled = true; @@ -1356,9 +1361,6 @@ rte_sched_subport_config(struct rte_sched_port *port, for (i = 0; i < RTE_SCHED_PORT_N_GRINDERS; i++) s->grinder_base_bmp_pos[i] = RTE_SCHED_PIPE_INVALID; - - /* TC over-subscription is disabled by default */ - s->is_tc_ov_enabled = 0; } { @@ -2514,12 +2516,15 @@ grinder_schedule(struct rte_sched_port *port, uint32_t pkt_len = pkt->pkt_len + port->frame_overhead; uint32_t be_tc_active; - if (unlikely(subport->is_tc_ov_enabled)) { + switch (subport->is_tc_ov_enabled) { + case 1: if (!grinder_credits_check_with_tc_ov(port, subport, pos)) return 0; - } else { + break; + case 0: if (!grinder_credits_check(port, subport, pos)) return 0; + break; } /* Advance port time */ -- 2.25.1 -------------------------------------------------------------- Intel Research and Development Ireland Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3] sched: enable/disable TC OV at runtime 2022-04-07 14:51 [dpdk][PATCH 2/2] sched: fix to manage " Marcin Danilewicz @ 2022-04-27 9:23 ` Marcin Danilewicz 2022-04-27 15:48 ` Singh, Jasvinder 2022-05-09 20:05 ` Dumitrescu, Cristian 0 siblings, 2 replies; 12+ messages in thread From: Marcin Danilewicz @ 2022-04-27 9:23 UTC (permalink / raw) To: dev, jasvinder.singh, cristian.dumitrescu; +Cc: megha.ajmera Added new API to enable or disable TC over subscription for best effort traffic class at subport level. Added changes after review and increased throughput. By default TC OV is disabled. Signed-off-by: Marcin Danilewicz <marcinx.danilewicz@intel.com> --- lib/sched/rte_sched.c | 189 +++++++++++++++++++++++++++++++++++------- lib/sched/rte_sched.h | 18 ++++ lib/sched/version.map | 3 + 3 files changed, 178 insertions(+), 32 deletions(-) diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c index ec74bee939..6e7d81df46 100644 --- a/lib/sched/rte_sched.c +++ b/lib/sched/rte_sched.c @@ -213,6 +213,9 @@ struct rte_sched_subport { uint8_t *bmp_array; struct rte_mbuf **queue_array; uint8_t memory[0] __rte_cache_aligned; + + /* TC oversubscription activation */ + int is_tc_ov_enabled; } __rte_cache_aligned; struct rte_sched_port { @@ -1165,6 +1168,45 @@ rte_sched_cman_config(struct rte_sched_port *port, } #endif +int +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, + uint32_t subport_id, + bool tc_ov_enable) +{ + struct rte_sched_subport *s; + struct rte_sched_subport_profile *profile; + + 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__); + return -EINVAL; + } + + s = port->subports[subport_id]; + s->is_tc_ov_enabled = tc_ov_enable ? 1 : 0; + + if (s->is_tc_ov_enabled) { + /* TC oversubscription */ + s->tc_ov_wm_min = port->mtu; + s->tc_ov_period_id = 0; + s->tc_ov = 0; + s->tc_ov_n = 0; + s->tc_ov_rate = 0; + + profile = port->subport_profiles + s->profile; + s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile->tc_period, + s->pipe_tc_be_rate_max); + s->tc_ov_wm = s->tc_ov_wm_max; + } + return 0; +} + int rte_sched_subport_config(struct rte_sched_port *port, uint32_t subport_id, @@ -1254,6 +1296,9 @@ rte_sched_subport_config(struct rte_sched_port *port, s->n_pipe_profiles = params->n_pipe_profiles; s->n_max_pipe_profiles = params->n_max_pipe_profiles; + /* TC over-subscription is disabled by default */ + s->is_tc_ov_enabled = 0; + #ifdef RTE_SCHED_CMAN if (params->cman_params != NULL) { s->cman_enabled = true; @@ -1316,13 +1361,6 @@ rte_sched_subport_config(struct rte_sched_port *port, for (i = 0; i < RTE_SCHED_PORT_N_GRINDERS; i++) s->grinder_base_bmp_pos[i] = RTE_SCHED_PIPE_INVALID; - - /* TC oversubscription */ - s->tc_ov_wm_min = port->mtu; - s->tc_ov_period_id = 0; - s->tc_ov = 0; - s->tc_ov_n = 0; - s->tc_ov_rate = 0; } { @@ -1342,9 +1380,6 @@ rte_sched_subport_config(struct rte_sched_port *port, else profile->tc_credits_per_period[i] = 0; - s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile->tc_period, - s->pipe_tc_be_rate_max); - s->tc_ov_wm = s->tc_ov_wm_max; s->profile = subport_profile_id; } @@ -1417,17 +1452,20 @@ rte_sched_pipe_config(struct rte_sched_port *port, double pipe_tc_be_rate = (double) params->tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] / (double) params->tc_period; - uint32_t tc_be_ov = s->tc_ov; - /* Unplug pipe from its subport */ - s->tc_ov_n -= params->tc_ov_weight; - s->tc_ov_rate -= pipe_tc_be_rate; - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; + if (s->is_tc_ov_enabled) { + uint32_t tc_be_ov = s->tc_ov; - if (s->tc_ov != tc_be_ov) { - RTE_LOG(DEBUG, SCHED, - "Subport %u Best-effort TC oversubscription is OFF (%.4lf >= %.4lf)\n", - subport_id, subport_tc_be_rate, s->tc_ov_rate); + /* Unplug pipe from its subport */ + s->tc_ov_n -= params->tc_ov_weight; + s->tc_ov_rate -= pipe_tc_be_rate; + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; + + if (s->tc_ov != tc_be_ov) { + RTE_LOG(DEBUG, SCHED, + "Subport %u Best-effort TC oversubscription is OFF (%.4lf >= %.4lf)\n", + subport_id, subport_tc_be_rate, s->tc_ov_rate); + } } /* Reset the pipe */ @@ -1460,19 +1498,22 @@ rte_sched_pipe_config(struct rte_sched_port *port, double pipe_tc_be_rate = (double) params->tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] / (double) params->tc_period; - uint32_t tc_be_ov = s->tc_ov; - s->tc_ov_n += params->tc_ov_weight; - s->tc_ov_rate += pipe_tc_be_rate; - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; + if (s->is_tc_ov_enabled) { + uint32_t tc_be_ov = s->tc_ov; + + s->tc_ov_n += params->tc_ov_weight; + s->tc_ov_rate += pipe_tc_be_rate; + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; - if (s->tc_ov != tc_be_ov) { - RTE_LOG(DEBUG, SCHED, - "Subport %u Best effort TC oversubscription is ON (%.4lf < %.4lf)\n", - subport_id, subport_tc_be_rate, s->tc_ov_rate); + if (s->tc_ov != tc_be_ov) { + RTE_LOG(DEBUG, SCHED, + "Subport %u Best effort TC oversubscription is ON (%.4lf < %.4lf)\n", + subport_id, subport_tc_be_rate, s->tc_ov_rate); + } + p->tc_ov_period_id = s->tc_ov_period_id; + p->tc_ov_credits = s->tc_ov_wm; } - p->tc_ov_period_id = s->tc_ov_period_id; - p->tc_ov_credits = s->tc_ov_wm; } return 0; @@ -2318,6 +2359,45 @@ grinder_credits_update(struct rte_sched_port *port, pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); pipe->tb_time += n_periods * params->tb_period; + /* Subport TCs */ + if (unlikely(port->time >= subport->tc_time)) { + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) + subport->tc_credits[i] = sp->tc_credits_per_period[i]; + + subport->tc_time = port->time + sp->tc_period; + } + + /* Pipe TCs */ + if (unlikely(port->time >= pipe->tc_time)) { + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) + pipe->tc_credits[i] = params->tc_credits_per_period[i]; + pipe->tc_time = port->time + params->tc_period; + } +} + +static inline void +grinder_credits_update_with_tc_ov(struct rte_sched_port *port, + struct rte_sched_subport *subport, uint32_t pos) +{ + struct rte_sched_grinder *grinder = subport->grinder + pos; + struct rte_sched_pipe *pipe = grinder->pipe; + struct rte_sched_pipe_profile *params = grinder->pipe_params; + struct rte_sched_subport_profile *sp = grinder->subport_params; + uint64_t n_periods; + uint32_t i; + + /* Subport TB */ + n_periods = (port->time - subport->tb_time) / sp->tb_period; + subport->tb_credits += n_periods * sp->tb_credits_per_period; + subport->tb_credits = RTE_MIN(subport->tb_credits, sp->tb_size); + subport->tb_time += n_periods * sp->tb_period; + + /* Pipe TB */ + n_periods = (port->time - pipe->tb_time) / params->tb_period; + pipe->tb_credits += n_periods * params->tb_credits_per_period; + pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); + pipe->tb_time += n_periods * params->tb_period; + /* Subport TCs */ if (unlikely(port->time >= subport->tc_time)) { subport->tc_ov_wm = @@ -2348,6 +2428,39 @@ grinder_credits_update(struct rte_sched_port *port, static inline int grinder_credits_check(struct rte_sched_port *port, struct rte_sched_subport *subport, uint32_t pos) +{ + struct rte_sched_grinder *grinder = subport->grinder + pos; + struct rte_sched_pipe *pipe = grinder->pipe; + struct rte_mbuf *pkt = grinder->pkt; + uint32_t tc_index = grinder->tc_index; + uint64_t pkt_len = pkt->pkt_len + port->frame_overhead; + uint64_t subport_tb_credits = subport->tb_credits; + uint64_t subport_tc_credits = subport->tc_credits[tc_index]; + uint64_t pipe_tb_credits = pipe->tb_credits; + uint64_t pipe_tc_credits = pipe->tc_credits[tc_index]; + int enough_credits; + + /* Check pipe and subport credits */ + enough_credits = (pkt_len <= subport_tb_credits) && + (pkt_len <= subport_tc_credits) && + (pkt_len <= pipe_tb_credits) && + (pkt_len <= pipe_tc_credits); + + if (!enough_credits) + return 0; + + /* Update pipe and subport credits */ + subport->tb_credits -= pkt_len; + subport->tc_credits[tc_index] -= pkt_len; + pipe->tb_credits -= pkt_len; + pipe->tc_credits[tc_index] -= pkt_len; + + return 1; +} + +static inline int +grinder_credits_check_with_tc_ov(struct rte_sched_port *port, + struct rte_sched_subport *subport, uint32_t pos) { struct rte_sched_grinder *grinder = subport->grinder + pos; struct rte_sched_pipe *pipe = grinder->pipe; @@ -2403,8 +2516,16 @@ grinder_schedule(struct rte_sched_port *port, uint32_t pkt_len = pkt->pkt_len + port->frame_overhead; uint32_t be_tc_active; - if (!grinder_credits_check(port, subport, pos)) - return 0; + switch (subport->is_tc_ov_enabled) { + case 1: + if (!grinder_credits_check_with_tc_ov(port, subport, pos)) + return 0; + break; + case 0: + if (!grinder_credits_check(port, subport, pos)) + return 0; + break; + } /* Advance port time */ port->time += pkt_len; @@ -2770,7 +2891,11 @@ grinder_handle(struct rte_sched_port *port, subport->profile; grinder_prefetch_tc_queue_arrays(subport, pos); - grinder_credits_update(port, subport, pos); + + if (unlikely(subport->is_tc_ov_enabled)) + grinder_credits_update_with_tc_ov(port, subport, pos); + else + grinder_credits_update(port, subport, pos); grinder->state = e_GRINDER_PREFETCH_MBUF; return 0; diff --git a/lib/sched/rte_sched.h b/lib/sched/rte_sched.h index 5ece64e527..94febe1d94 100644 --- a/lib/sched/rte_sched.h +++ b/lib/sched/rte_sched.h @@ -579,6 +579,24 @@ rte_sched_port_enqueue(struct rte_sched_port *port, struct rte_mbuf **pkts, uint int rte_sched_port_dequeue(struct rte_sched_port *port, struct rte_mbuf **pkts, uint32_t n_pkts); +/** + * Hierarchical scheduler subport TC OV enable/disable config. + * Note that this function is safe to use at runtime + * to enable/disable TC OV for subport. + * + * @param port + * Handle to port scheduler instance + * @param subport_id + * Subport ID + * @param tc_ov_enable + * Boolean flag to enable/disable TC OV + * @return + * 0 upon success, error code otherwise + */ +__rte_experimental +int +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, uint32_t subport_id, bool tc_ov_enable); + #ifdef __cplusplus } #endif diff --git a/lib/sched/version.map b/lib/sched/version.map index d22c07fc9f..c6e994d8df 100644 --- a/lib/sched/version.map +++ b/lib/sched/version.map @@ -34,4 +34,7 @@ EXPERIMENTAL { # added in 21.11 rte_pie_rt_data_init; rte_pie_config_init; + + # added in 22.03 + rte_sched_subport_tc_ov_config; }; -- 2.25.1 -------------------------------------------------------------- Intel Research and Development Ireland Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v3] sched: enable/disable TC OV at runtime 2022-04-27 9:23 ` [PATCH v3] sched: enable/disable " Marcin Danilewicz @ 2022-04-27 15:48 ` Singh, Jasvinder 2022-05-09 20:05 ` Dumitrescu, Cristian 1 sibling, 0 replies; 12+ messages in thread From: Singh, Jasvinder @ 2022-04-27 15:48 UTC (permalink / raw) To: Danilewicz, MarcinX, dev, Dumitrescu, Cristian; +Cc: Ajmera, Megha > -----Original Message----- > From: Danilewicz, MarcinX <marcinx.danilewicz@intel.com> > Sent: Wednesday, April 27, 2022 10:24 AM > To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>; > Dumitrescu, Cristian <cristian.dumitrescu@intel.com> > Cc: Ajmera, Megha <megha.ajmera@intel.com> > Subject: [PATCH v3] sched: enable/disable TC OV at runtime > > Added new API to enable or disable TC over subscription for best effort > traffic class at subport level. > Added changes after review and increased throughput. > > By default TC OV is disabled. > > Signed-off-by: Marcin Danilewicz <marcinx.danilewicz@intel.com> > --- > lib/sched/rte_sched.c | 189 +++++++++++++++++++++++++++++++++++--- > ---- > lib/sched/rte_sched.h | 18 ++++ > lib/sched/version.map | 3 + > 3 files changed, 178 insertions(+), 32 deletions(-) > > diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c index > ec74bee939..6e7d81df46 100644 > --- a/lib/sched/rte_sched.c > +++ b/lib/sched/rte_sched.c > @@ -213,6 +213,9 @@ struct rte_sched_subport { > uint8_t *bmp_array; > struct rte_mbuf **queue_array; > uint8_t memory[0] __rte_cache_aligned; > + > + /* TC oversubscription activation */ > + int is_tc_ov_enabled; > } __rte_cache_aligned; > > struct rte_sched_port { > @@ -1165,6 +1168,45 @@ rte_sched_cman_config(struct rte_sched_port > *port, } #endif > > +int > +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, > + uint32_t subport_id, > + bool tc_ov_enable) > +{ > + struct rte_sched_subport *s; > + struct rte_sched_subport_profile *profile; > + > + 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__); > + return -EINVAL; > + } > + > + s = port->subports[subport_id]; > + s->is_tc_ov_enabled = tc_ov_enable ? 1 : 0; > + > + if (s->is_tc_ov_enabled) { > + /* TC oversubscription */ > + s->tc_ov_wm_min = port->mtu; > + s->tc_ov_period_id = 0; > + s->tc_ov = 0; > + s->tc_ov_n = 0; > + s->tc_ov_rate = 0; > + > + profile = port->subport_profiles + s->profile; > + s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile- > >tc_period, > + s->pipe_tc_be_rate_max); > + s->tc_ov_wm = s->tc_ov_wm_max; > + } > + return 0; > +} > + > int > rte_sched_subport_config(struct rte_sched_port *port, > uint32_t subport_id, > @@ -1254,6 +1296,9 @@ rte_sched_subport_config(struct rte_sched_port > *port, > s->n_pipe_profiles = params->n_pipe_profiles; > s->n_max_pipe_profiles = params->n_max_pipe_profiles; > > + /* TC over-subscription is disabled by default */ > + s->is_tc_ov_enabled = 0; > + > #ifdef RTE_SCHED_CMAN > if (params->cman_params != NULL) { > s->cman_enabled = true; > @@ -1316,13 +1361,6 @@ rte_sched_subport_config(struct rte_sched_port > *port, > > for (i = 0; i < RTE_SCHED_PORT_N_GRINDERS; i++) > s->grinder_base_bmp_pos[i] = > RTE_SCHED_PIPE_INVALID; > - > - /* TC oversubscription */ > - s->tc_ov_wm_min = port->mtu; > - s->tc_ov_period_id = 0; > - s->tc_ov = 0; > - s->tc_ov_n = 0; > - s->tc_ov_rate = 0; > } > > { > @@ -1342,9 +1380,6 @@ rte_sched_subport_config(struct rte_sched_port > *port, > else > profile->tc_credits_per_period[i] = 0; > > - s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile- > >tc_period, > - s- > >pipe_tc_be_rate_max); > - s->tc_ov_wm = s->tc_ov_wm_max; > s->profile = subport_profile_id; > > } > @@ -1417,17 +1452,20 @@ rte_sched_pipe_config(struct rte_sched_port > *port, > double pipe_tc_be_rate = > (double) params- > >tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] > / (double) params->tc_period; > - uint32_t tc_be_ov = s->tc_ov; > > - /* Unplug pipe from its subport */ > - s->tc_ov_n -= params->tc_ov_weight; > - s->tc_ov_rate -= pipe_tc_be_rate; > - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > + if (s->is_tc_ov_enabled) { > + uint32_t tc_be_ov = s->tc_ov; > > - if (s->tc_ov != tc_be_ov) { > - RTE_LOG(DEBUG, SCHED, > - "Subport %u Best-effort TC oversubscription > is OFF (%.4lf >= %.4lf)\n", > - subport_id, subport_tc_be_rate, s- > >tc_ov_rate); > + /* Unplug pipe from its subport */ > + s->tc_ov_n -= params->tc_ov_weight; > + s->tc_ov_rate -= pipe_tc_be_rate; > + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > + > + if (s->tc_ov != tc_be_ov) { > + RTE_LOG(DEBUG, SCHED, > + "Subport %u Best-effort TC > oversubscription is OFF (%.4lf >= %.4lf)\n", > + subport_id, subport_tc_be_rate, s- > >tc_ov_rate); > + } > } > > /* Reset the pipe */ > @@ -1460,19 +1498,22 @@ rte_sched_pipe_config(struct rte_sched_port > *port, > double pipe_tc_be_rate = > (double) params- > >tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] > / (double) params->tc_period; > - uint32_t tc_be_ov = s->tc_ov; > > - s->tc_ov_n += params->tc_ov_weight; > - s->tc_ov_rate += pipe_tc_be_rate; > - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > + if (s->is_tc_ov_enabled) { > + uint32_t tc_be_ov = s->tc_ov; > + > + s->tc_ov_n += params->tc_ov_weight; > + s->tc_ov_rate += pipe_tc_be_rate; > + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > > - if (s->tc_ov != tc_be_ov) { > - RTE_LOG(DEBUG, SCHED, > - "Subport %u Best effort TC oversubscription > is ON (%.4lf < %.4lf)\n", > - subport_id, subport_tc_be_rate, s- > >tc_ov_rate); > + if (s->tc_ov != tc_be_ov) { > + RTE_LOG(DEBUG, SCHED, > + "Subport %u Best effort TC > oversubscription is ON (%.4lf < %.4lf)\n", > + subport_id, subport_tc_be_rate, s- > >tc_ov_rate); > + } > + p->tc_ov_period_id = s->tc_ov_period_id; > + p->tc_ov_credits = s->tc_ov_wm; > } > - p->tc_ov_period_id = s->tc_ov_period_id; > - p->tc_ov_credits = s->tc_ov_wm; > } > > return 0; > @@ -2318,6 +2359,45 @@ grinder_credits_update(struct rte_sched_port > *port, > pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); > pipe->tb_time += n_periods * params->tb_period; > > + /* Subport TCs */ > + if (unlikely(port->time >= subport->tc_time)) { > + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > + subport->tc_credits[i] = sp- > >tc_credits_per_period[i]; > + > + subport->tc_time = port->time + sp->tc_period; > + } > + > + /* Pipe TCs */ > + if (unlikely(port->time >= pipe->tc_time)) { > + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > + pipe->tc_credits[i] = params- > >tc_credits_per_period[i]; > + pipe->tc_time = port->time + params->tc_period; > + } > +} > + > +static inline void > +grinder_credits_update_with_tc_ov(struct rte_sched_port *port, > + struct rte_sched_subport *subport, uint32_t pos) { > + struct rte_sched_grinder *grinder = subport->grinder + pos; > + struct rte_sched_pipe *pipe = grinder->pipe; > + struct rte_sched_pipe_profile *params = grinder->pipe_params; > + struct rte_sched_subport_profile *sp = grinder->subport_params; > + uint64_t n_periods; > + uint32_t i; > + > + /* Subport TB */ > + n_periods = (port->time - subport->tb_time) / sp->tb_period; > + subport->tb_credits += n_periods * sp->tb_credits_per_period; > + subport->tb_credits = RTE_MIN(subport->tb_credits, sp->tb_size); > + subport->tb_time += n_periods * sp->tb_period; > + > + /* Pipe TB */ > + n_periods = (port->time - pipe->tb_time) / params->tb_period; > + pipe->tb_credits += n_periods * params->tb_credits_per_period; > + pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); > + pipe->tb_time += n_periods * params->tb_period; > + > /* Subport TCs */ > if (unlikely(port->time >= subport->tc_time)) { > subport->tc_ov_wm = > @@ -2348,6 +2428,39 @@ grinder_credits_update(struct rte_sched_port > *port, static inline int grinder_credits_check(struct rte_sched_port *port, > struct rte_sched_subport *subport, uint32_t pos) > +{ > + struct rte_sched_grinder *grinder = subport->grinder + pos; > + struct rte_sched_pipe *pipe = grinder->pipe; > + struct rte_mbuf *pkt = grinder->pkt; > + uint32_t tc_index = grinder->tc_index; > + uint64_t pkt_len = pkt->pkt_len + port->frame_overhead; > + uint64_t subport_tb_credits = subport->tb_credits; > + uint64_t subport_tc_credits = subport->tc_credits[tc_index]; > + uint64_t pipe_tb_credits = pipe->tb_credits; > + uint64_t pipe_tc_credits = pipe->tc_credits[tc_index]; > + int enough_credits; > + > + /* Check pipe and subport credits */ > + enough_credits = (pkt_len <= subport_tb_credits) && > + (pkt_len <= subport_tc_credits) && > + (pkt_len <= pipe_tb_credits) && > + (pkt_len <= pipe_tc_credits); > + > + if (!enough_credits) > + return 0; > + > + /* Update pipe and subport credits */ > + subport->tb_credits -= pkt_len; > + subport->tc_credits[tc_index] -= pkt_len; > + pipe->tb_credits -= pkt_len; > + pipe->tc_credits[tc_index] -= pkt_len; > + > + return 1; > +} > + > +static inline int > +grinder_credits_check_with_tc_ov(struct rte_sched_port *port, > + struct rte_sched_subport *subport, uint32_t pos) > { > struct rte_sched_grinder *grinder = subport->grinder + pos; > struct rte_sched_pipe *pipe = grinder->pipe; @@ -2403,8 +2516,16 > @@ grinder_schedule(struct rte_sched_port *port, > uint32_t pkt_len = pkt->pkt_len + port->frame_overhead; > uint32_t be_tc_active; > > - if (!grinder_credits_check(port, subport, pos)) > - return 0; > + switch (subport->is_tc_ov_enabled) { > + case 1: > + if (!grinder_credits_check_with_tc_ov(port, subport, pos)) > + return 0; > + break; > + case 0: > + if (!grinder_credits_check(port, subport, pos)) > + return 0; > + break; > + } > > /* Advance port time */ > port->time += pkt_len; > @@ -2770,7 +2891,11 @@ grinder_handle(struct rte_sched_port *port, > subport->profile; > > grinder_prefetch_tc_queue_arrays(subport, pos); > - grinder_credits_update(port, subport, pos); > + > + if (unlikely(subport->is_tc_ov_enabled)) > + grinder_credits_update_with_tc_ov(port, subport, > pos); > + else > + grinder_credits_update(port, subport, pos); > > grinder->state = e_GRINDER_PREFETCH_MBUF; > return 0; > diff --git a/lib/sched/rte_sched.h b/lib/sched/rte_sched.h index > 5ece64e527..94febe1d94 100644 > --- a/lib/sched/rte_sched.h > +++ b/lib/sched/rte_sched.h > @@ -579,6 +579,24 @@ rte_sched_port_enqueue(struct rte_sched_port > *port, struct rte_mbuf **pkts, uint int rte_sched_port_dequeue(struct > rte_sched_port *port, struct rte_mbuf **pkts, uint32_t n_pkts); > > +/** > + * Hierarchical scheduler subport TC OV enable/disable config. > + * Note that this function is safe to use at runtime > + * to enable/disable TC OV for subport. > + * > + * @param port > + * Handle to port scheduler instance > + * @param subport_id > + * Subport ID > + * @param tc_ov_enable > + * Boolean flag to enable/disable TC OV > + * @return > + * 0 upon success, error code otherwise > + */ > +__rte_experimental > +int > +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, uint32_t > +subport_id, bool tc_ov_enable); > + > #ifdef __cplusplus > } > #endif > diff --git a/lib/sched/version.map b/lib/sched/version.map index > d22c07fc9f..c6e994d8df 100644 > --- a/lib/sched/version.map > +++ b/lib/sched/version.map > @@ -34,4 +34,7 @@ EXPERIMENTAL { > # added in 21.11 > rte_pie_rt_data_init; > rte_pie_config_init; > + > + # added in 22.03 > + rte_sched_subport_tc_ov_config; > }; > -- > 2.25.1 Hi Marcin, I don't see any note on the changes made in this version with respect to previous versions. Can you include them in future version? Also, I had some comments on the first version of this patch, I don't see any response. ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v3] sched: enable/disable TC OV at runtime 2022-04-27 9:23 ` [PATCH v3] sched: enable/disable " Marcin Danilewicz 2022-04-27 15:48 ` Singh, Jasvinder @ 2022-05-09 20:05 ` Dumitrescu, Cristian 2022-05-10 6:40 ` Ajmera, Megha 1 sibling, 1 reply; 12+ messages in thread From: Dumitrescu, Cristian @ 2022-05-09 20:05 UTC (permalink / raw) To: Danilewicz, MarcinX, dev, Singh, Jasvinder; +Cc: Ajmera, Megha Hi Marcin, > -----Original Message----- > From: Danilewicz, MarcinX <marcinx.danilewicz@intel.com> > Sent: Wednesday, April 27, 2022 10:24 AM > To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu, > Cristian <cristian.dumitrescu@intel.com> > Cc: Ajmera, Megha <megha.ajmera@intel.com> > Subject: [PATCH v3] sched: enable/disable TC OV at runtime We are not trying to enable/disable the traffic class oversubscription feature at run-time, but at initialization. If cat, we should prohibit changing this post-initialization. Also the name of the feature should not be abbreviated in the patch title. I suggest you rework the title to: [PATCH] sched: enable traffic class oversubscription conditionally > > Added new API to enable or disable TC over subscription for best > effort traffic class at subport level. > Added changes after review and increased throughput. > > By default TC OV is disabled. It should be the other way around, the TC_OV should be enabled by default. The TC oversubscription is a more natural way to use this library, we usually want to disable this feature just for better performance in case this functionality is not needed. Please initialize the tc_ov flag accordingly. > > Signed-off-by: Marcin Danilewicz <marcinx.danilewicz@intel.com> > --- > lib/sched/rte_sched.c | 189 +++++++++++++++++++++++++++++++++++------- > lib/sched/rte_sched.h | 18 ++++ > lib/sched/version.map | 3 + > 3 files changed, 178 insertions(+), 32 deletions(-) > > diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c > index ec74bee939..6e7d81df46 100644 > --- a/lib/sched/rte_sched.c > +++ b/lib/sched/rte_sched.c > @@ -213,6 +213,9 @@ struct rte_sched_subport { > uint8_t *bmp_array; > struct rte_mbuf **queue_array; > uint8_t memory[0] __rte_cache_aligned; > + > + /* TC oversubscription activation */ > + int is_tc_ov_enabled; How about we simplify the name of this variable to: tc_ov_enabled ? > } __rte_cache_aligned; > > struct rte_sched_port { > @@ -1165,6 +1168,45 @@ rte_sched_cman_config(struct rte_sched_port > *port, > } > #endif > > +int > +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, > + uint32_t subport_id, > + bool tc_ov_enable) > +{ > + struct rte_sched_subport *s; > + struct rte_sched_subport_profile *profile; > + > + 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__); > + return -EINVAL; > + } > + > + s = port->subports[subport_id]; > + s->is_tc_ov_enabled = tc_ov_enable ? 1 : 0; > + > + if (s->is_tc_ov_enabled) { > + /* TC oversubscription */ > + s->tc_ov_wm_min = port->mtu; > + s->tc_ov_period_id = 0; > + s->tc_ov = 0; > + s->tc_ov_n = 0; > + s->tc_ov_rate = 0; > + > + profile = port->subport_profiles + s->profile; > + s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile- > >tc_period, > + s->pipe_tc_be_rate_max); > + s->tc_ov_wm = s->tc_ov_wm_max; > + } > + return 0; > +} This function should not exist, please remove it and keep the initial code that computes the tc_ov related variable regardless of whether tc_ov is enabled or not. All the tc_ov related variables have the tc_ov particle in their name, so there is no clash. This is initialization code, so no performance overhead. Let's keep the code unmodified and compute both the tc_ov and the non-tc_ov varables at initialization, regardless of whether the feature is enabled or not. This comment is applicable to all the initialization code, please adjust all the init code accordingly. There should be no diff showing in the patch for any of the init code! For this file "rte_sched.c", your patch should contain just two additional run-time functions, i.e. the non-tc-ov version of functions grinder_credits_update() and grindler_credits_check(), and the small code required to test when to use the tc-ov vs. the non-tc_ov version, makes sense? > + > int > is_tc_ov_enabled (struct rte_sched_port *port, > uint32_t subport_id, > @@ -1254,6 +1296,9 @@ rte_sched_subport_config(struct rte_sched_port > *port, > s->n_pipe_profiles = params->n_pipe_profiles; > s->n_max_pipe_profiles = params->n_max_pipe_profiles; > > + /* TC over-subscription is disabled by default */ > + s->is_tc_ov_enabled = 0; > + By default, this feature should be enabled: s->is_tc_ov_enabled = 1; > #ifdef RTE_SCHED_CMAN > if (params->cman_params != NULL) { > s->cman_enabled = true; > @@ -1316,13 +1361,6 @@ rte_sched_subport_config(struct rte_sched_port > *port, > > for (i = 0; i < RTE_SCHED_PORT_N_GRINDERS; i++) > s->grinder_base_bmp_pos[i] = > RTE_SCHED_PIPE_INVALID; > - > - /* TC oversubscription */ > - s->tc_ov_wm_min = port->mtu; > - s->tc_ov_period_id = 0; > - s->tc_ov = 0; > - s->tc_ov_n = 0; > - s->tc_ov_rate = 0; > } > > { > @@ -1342,9 +1380,6 @@ rte_sched_subport_config(struct rte_sched_port > *port, > else > profile->tc_credits_per_period[i] = 0; > > - s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile- > >tc_period, > - s- > >pipe_tc_be_rate_max); > - s->tc_ov_wm = s->tc_ov_wm_max; > s->profile = subport_profile_id; > > } > @@ -1417,17 +1452,20 @@ rte_sched_pipe_config(struct rte_sched_port > *port, > double pipe_tc_be_rate = > (double) params- > >tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] > / (double) params->tc_period; > - uint32_t tc_be_ov = s->tc_ov; > > - /* Unplug pipe from its subport */ > - s->tc_ov_n -= params->tc_ov_weight; > - s->tc_ov_rate -= pipe_tc_be_rate; > - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > + if (s->is_tc_ov_enabled) { > + uint32_t tc_be_ov = s->tc_ov; > > - if (s->tc_ov != tc_be_ov) { > - RTE_LOG(DEBUG, SCHED, > - "Subport %u Best-effort TC oversubscription is > OFF (%.4lf >= %.4lf)\n", > - subport_id, subport_tc_be_rate, s- > >tc_ov_rate); > + /* Unplug pipe from its subport */ > + s->tc_ov_n -= params->tc_ov_weight; > + s->tc_ov_rate -= pipe_tc_be_rate; > + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > + > + if (s->tc_ov != tc_be_ov) { > + RTE_LOG(DEBUG, SCHED, > + "Subport %u Best-effort TC > oversubscription is OFF (%.4lf >= %.4lf)\n", > + subport_id, subport_tc_be_rate, s- > >tc_ov_rate); > + } > } > > /* Reset the pipe */ > @@ -1460,19 +1498,22 @@ rte_sched_pipe_config(struct rte_sched_port > *port, > double pipe_tc_be_rate = > (double) params- > >tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] > / (double) params->tc_period; > - uint32_t tc_be_ov = s->tc_ov; > > - s->tc_ov_n += params->tc_ov_weight; > - s->tc_ov_rate += pipe_tc_be_rate; > - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > + if (s->is_tc_ov_enabled) { > + uint32_t tc_be_ov = s->tc_ov; > + > + s->tc_ov_n += params->tc_ov_weight; > + s->tc_ov_rate += pipe_tc_be_rate; > + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > > - if (s->tc_ov != tc_be_ov) { > - RTE_LOG(DEBUG, SCHED, > - "Subport %u Best effort TC oversubscription is > ON (%.4lf < %.4lf)\n", > - subport_id, subport_tc_be_rate, s- > >tc_ov_rate); > + if (s->tc_ov != tc_be_ov) { > + RTE_LOG(DEBUG, SCHED, > + "Subport %u Best effort TC > oversubscription is ON (%.4lf < %.4lf)\n", > + subport_id, subport_tc_be_rate, s- > >tc_ov_rate); > + } > + p->tc_ov_period_id = s->tc_ov_period_id; > + p->tc_ov_credits = s->tc_ov_wm; > } > - p->tc_ov_period_id = s->tc_ov_period_id; > - p->tc_ov_credits = s->tc_ov_wm; > } > > return 0; > @@ -2318,6 +2359,45 @@ grinder_credits_update(struct rte_sched_port > *port, > pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); > pipe->tb_time += n_periods * params->tb_period; > > + /* Subport TCs */ > + if (unlikely(port->time >= subport->tc_time)) { > + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > + subport->tc_credits[i] = sp->tc_credits_per_period[i]; > + > + subport->tc_time = port->time + sp->tc_period; > + } > + > + /* Pipe TCs */ > + if (unlikely(port->time >= pipe->tc_time)) { > + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > + pipe->tc_credits[i] = params->tc_credits_per_period[i]; > + pipe->tc_time = port->time + params->tc_period; > + } > +} > + > +static inline void > +grinder_credits_update_with_tc_ov(struct rte_sched_port *port, > + struct rte_sched_subport *subport, uint32_t pos) > +{ > + struct rte_sched_grinder *grinder = subport->grinder + pos; > + struct rte_sched_pipe *pipe = grinder->pipe; > + struct rte_sched_pipe_profile *params = grinder->pipe_params; > + struct rte_sched_subport_profile *sp = grinder->subport_params; > + uint64_t n_periods; > + uint32_t i; > + > + /* Subport TB */ > + n_periods = (port->time - subport->tb_time) / sp->tb_period; > + subport->tb_credits += n_periods * sp->tb_credits_per_period; > + subport->tb_credits = RTE_MIN(subport->tb_credits, sp->tb_size); > + subport->tb_time += n_periods * sp->tb_period; > + > + /* Pipe TB */ > + n_periods = (port->time - pipe->tb_time) / params->tb_period; > + pipe->tb_credits += n_periods * params->tb_credits_per_period; > + pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); > + pipe->tb_time += n_periods * params->tb_period; > + > /* Subport TCs */ > if (unlikely(port->time >= subport->tc_time)) { > subport->tc_ov_wm = > @@ -2348,6 +2428,39 @@ grinder_credits_update(struct rte_sched_port > *port, > static inline int > grinder_credits_check(struct rte_sched_port *port, > struct rte_sched_subport *subport, uint32_t pos) > +{ > + struct rte_sched_grinder *grinder = subport->grinder + pos; > + struct rte_sched_pipe *pipe = grinder->pipe; > + struct rte_mbuf *pkt = grinder->pkt; > + uint32_t tc_index = grinder->tc_index; > + uint64_t pkt_len = pkt->pkt_len + port->frame_overhead; > + uint64_t subport_tb_credits = subport->tb_credits; > + uint64_t subport_tc_credits = subport->tc_credits[tc_index]; > + uint64_t pipe_tb_credits = pipe->tb_credits; > + uint64_t pipe_tc_credits = pipe->tc_credits[tc_index]; > + int enough_credits; > + > + /* Check pipe and subport credits */ > + enough_credits = (pkt_len <= subport_tb_credits) && > + (pkt_len <= subport_tc_credits) && > + (pkt_len <= pipe_tb_credits) && > + (pkt_len <= pipe_tc_credits); > + > + if (!enough_credits) > + return 0; > + > + /* Update pipe and subport credits */ > + subport->tb_credits -= pkt_len; > + subport->tc_credits[tc_index] -= pkt_len; > + pipe->tb_credits -= pkt_len; > + pipe->tc_credits[tc_index] -= pkt_len; > + > + return 1; > +} > + > +static inline int > +grinder_credits_check_with_tc_ov(struct rte_sched_port *port, > + struct rte_sched_subport *subport, uint32_t pos) > { > struct rte_sched_grinder *grinder = subport->grinder + pos; > struct rte_sched_pipe *pipe = grinder->pipe; > @@ -2403,8 +2516,16 @@ grinder_schedule(struct rte_sched_port *port, > uint32_t pkt_len = pkt->pkt_len + port->frame_overhead; > uint32_t be_tc_active; > > - if (!grinder_credits_check(port, subport, pos)) > - return 0; > + switch (subport->is_tc_ov_enabled) { > + case 1: > + if (!grinder_credits_check_with_tc_ov(port, subport, pos)) > + return 0; > + break; > + case 0: > + if (!grinder_credits_check(port, subport, pos)) > + return 0; > + break; > + } There should be no switch statement here, please replace with an if statement. I suggest the following: int status; status = subport->tc_ov_enabled ? grinder_credits_check_with_tc_ov(port, subport, pos) : grinder_credits_check(port, subport, pos); if (!status) return 0; > > /* Advance port time */ > port->time += pkt_len; > @@ -2770,7 +2891,11 @@ grinder_handle(struct rte_sched_port *port, > subport->profile; > > grinder_prefetch_tc_queue_arrays(subport, pos); > - grinder_credits_update(port, subport, pos); > + > + if (unlikely(subport->is_tc_ov_enabled)) Please remove the "unlikely" from here, don't put any likely/unlikely here at all. > + grinder_credits_update_with_tc_ov(port, subport, pos); > + else > + grinder_credits_update(port, subport, pos); > > grinder->state = e_GRINDER_PREFETCH_MBUF; > return 0; > diff --git a/lib/sched/rte_sched.h b/lib/sched/rte_sched.h > index 5ece64e527..94febe1d94 100644 > --- a/lib/sched/rte_sched.h > +++ b/lib/sched/rte_sched.h > @@ -579,6 +579,24 @@ rte_sched_port_enqueue(struct rte_sched_port *port, > struct rte_mbuf **pkts, uint > int > rte_sched_port_dequeue(struct rte_sched_port *port, struct rte_mbuf **pkts, > uint32_t n_pkts); > > +/** > + * Hierarchical scheduler subport TC OV enable/disable config. The name of the feature should be fully stated here: traffic class oversubscription, not the abbreviation, please change. > + * Note that this function is safe to use at runtime > + * to enable/disable TC OV for subport. We should actually forbit this rather than encourage it. Calling this function several times does not make sense, and it can create limitations that can come back and byte us in the future, whenever we might need to extend this code, for no reason. Please actually replace with: "This function should be called at the time of subport initialization." > + * > + * @param port > + * Handle to port scheduler instance > + * @param subport_id > + * Subport ID > + * @param tc_ov_enable > + * Boolean flag to enable/disable TC OV > + * @return > + * 0 upon success, error code otherwise > + */ > +__rte_experimental > +int > +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, uint32_t > subport_id, bool tc_ov_enable); > + > #ifdef __cplusplus > } > #endif > diff --git a/lib/sched/version.map b/lib/sched/version.map > index d22c07fc9f..c6e994d8df 100644 > --- a/lib/sched/version.map > +++ b/lib/sched/version.map > @@ -34,4 +34,7 @@ EXPERIMENTAL { > # added in 21.11 > rte_pie_rt_data_init; > rte_pie_config_init; > + > + # added in 22.03 This is not in 22.03, it will hopefully be in 22.07. > + rte_sched_subport_tc_ov_config; > }; > -- > 2.25.1 Regards, Cristian ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v3] sched: enable/disable TC OV at runtime 2022-05-09 20:05 ` Dumitrescu, Cristian @ 2022-05-10 6:40 ` Ajmera, Megha 2022-05-10 9:09 ` Dumitrescu, Cristian 0 siblings, 1 reply; 12+ messages in thread From: Ajmera, Megha @ 2022-05-10 6:40 UTC (permalink / raw) To: Dumitrescu, Cristian, Danilewicz, MarcinX, dev, Singh, Jasvinder Cc: Thakur, Sham Singh Hi Cristian, Marcin, > > -----Original Message----- > > From: Danilewicz, MarcinX <marcinx.danilewicz@intel.com> > > Sent: Wednesday, April 27, 2022 10:24 AM > > To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>; > > Dumitrescu, Cristian <cristian.dumitrescu@intel.com> > > Cc: Ajmera, Megha <megha.ajmera@intel.com> > > Subject: [PATCH v3] sched: enable/disable TC OV at runtime > > We are not trying to enable/disable the traffic class oversubscription feature at > run-time, but at initialization. If cat, we should prohibit changing this post- > initialization. > If we only need this to be configured at initialization time, then we can as well take this flag in subport config API itself. Then there will be no need for a new API. The purpose of new API was to enable/disable this feature at runtime. > Also the name of the feature should not be abbreviated in the patch title. > > I suggest you rework the title to: > [PATCH] sched: enable traffic class oversubscription conditionally > > > > > Added new API to enable or disable TC over subscription for best > > effort traffic class at subport level. > > Added changes after review and increased throughput. > > > > By default TC OV is disabled. > > It should be the other way around, the TC_OV should be enabled by default. The > TC oversubscription is a more natural way to use this library, we usually want to > disable this feature just for better performance in case this functionality is not > needed. Please initialize the tc_ov flag accordingly. > In original code, this feature has always been disabled as it impacts performance. So, in my opinion we should keep it disabled by default and let user enable it when required. > > > > Signed-off-by: Marcin Danilewicz <marcinx.danilewicz@intel.com> > > --- > > lib/sched/rte_sched.c | 189 > > +++++++++++++++++++++++++++++++++++------- > > lib/sched/rte_sched.h | 18 ++++ > > lib/sched/version.map | 3 + > > 3 files changed, 178 insertions(+), 32 deletions(-) > > > > diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c index > > ec74bee939..6e7d81df46 100644 > > --- a/lib/sched/rte_sched.c > > +++ b/lib/sched/rte_sched.c > > @@ -213,6 +213,9 @@ struct rte_sched_subport { > > uint8_t *bmp_array; > > struct rte_mbuf **queue_array; > > uint8_t memory[0] __rte_cache_aligned; > > + > > + /* TC oversubscription activation */ > > + int is_tc_ov_enabled; > > How about we simplify the name of this variable to: tc_ov_enabled ? > > > } __rte_cache_aligned; > > > > struct rte_sched_port { > > @@ -1165,6 +1168,45 @@ rte_sched_cman_config(struct rte_sched_port > > *port, } #endif > > > > +int > > +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, > > + uint32_t subport_id, > > + bool tc_ov_enable) > > +{ > > + struct rte_sched_subport *s; > > + struct rte_sched_subport_profile *profile; > > + > > + 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__); > > + return -EINVAL; > > + } > > + > > + s = port->subports[subport_id]; > > + s->is_tc_ov_enabled = tc_ov_enable ? 1 : 0; > > + > > + if (s->is_tc_ov_enabled) { > > + /* TC oversubscription */ > > + s->tc_ov_wm_min = port->mtu; > > + s->tc_ov_period_id = 0; > > + s->tc_ov = 0; > > + s->tc_ov_n = 0; > > + s->tc_ov_rate = 0; > > + > > + profile = port->subport_profiles + s->profile; > > + s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile- > > >tc_period, > > + s->pipe_tc_be_rate_max); > > + s->tc_ov_wm = s->tc_ov_wm_max; > > + } > > + return 0; > > +} > > This function should not exist, please remove it and keep the initial code that > computes the tc_ov related variable regardless of whether tc_ov is enabled or > not. > > All the tc_ov related variables have the tc_ov particle in their name, so there is > no clash. This is initialization code, so no performance overhead. Let's keep the > code unmodified and compute both the tc_ov and the non-tc_ov varables at > initialization, regardless of whether the feature is enabled or not. > > This comment is applicable to all the initialization code, please adjust all the init > code accordingly. There should be no diff showing in the patch for any of the init > code! > > For this file "rte_sched.c", your patch should contain just two additional run- > time functions, i.e. the non-tc-ov version of functions grinder_credits_update() > and grindler_credits_check(), and the small code required to test when to use > the tc-ov vs. the non-tc_ov version, makes sense? > > > + > > int > > is_tc_ov_enabled (struct rte_sched_port *port, > > uint32_t subport_id, > > @@ -1254,6 +1296,9 @@ rte_sched_subport_config(struct rte_sched_port > > *port, > > s->n_pipe_profiles = params->n_pipe_profiles; > > s->n_max_pipe_profiles = params->n_max_pipe_profiles; > > > > + /* TC over-subscription is disabled by default */ > > + s->is_tc_ov_enabled = 0; > > + > > By default, this feature should be enabled: > s->is_tc_ov_enabled = 1; > > > #ifdef RTE_SCHED_CMAN > > if (params->cman_params != NULL) { > > s->cman_enabled = true; > > @@ -1316,13 +1361,6 @@ rte_sched_subport_config(struct rte_sched_port > > *port, > > > > for (i = 0; i < RTE_SCHED_PORT_N_GRINDERS; i++) > > s->grinder_base_bmp_pos[i] = > > RTE_SCHED_PIPE_INVALID; > > - > > - /* TC oversubscription */ > > - s->tc_ov_wm_min = port->mtu; > > - s->tc_ov_period_id = 0; > > - s->tc_ov = 0; > > - s->tc_ov_n = 0; > > - s->tc_ov_rate = 0; > > } > > > > { > > @@ -1342,9 +1380,6 @@ rte_sched_subport_config(struct rte_sched_port > > *port, > > else > > profile->tc_credits_per_period[i] = 0; > > > > - s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile- > > >tc_period, > > - s- > > >pipe_tc_be_rate_max); > > - s->tc_ov_wm = s->tc_ov_wm_max; > > s->profile = subport_profile_id; > > > > } > > @@ -1417,17 +1452,20 @@ rte_sched_pipe_config(struct rte_sched_port > > *port, > > double pipe_tc_be_rate = > > (double) params- > > >tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] > > / (double) params->tc_period; > > - uint32_t tc_be_ov = s->tc_ov; > > > > - /* Unplug pipe from its subport */ > > - s->tc_ov_n -= params->tc_ov_weight; > > - s->tc_ov_rate -= pipe_tc_be_rate; > > - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > > + if (s->is_tc_ov_enabled) { > > + uint32_t tc_be_ov = s->tc_ov; > > > > - if (s->tc_ov != tc_be_ov) { > > - RTE_LOG(DEBUG, SCHED, > > - "Subport %u Best-effort TC oversubscription is > > OFF (%.4lf >= %.4lf)\n", > > - subport_id, subport_tc_be_rate, s- > > >tc_ov_rate); > > + /* Unplug pipe from its subport */ > > + s->tc_ov_n -= params->tc_ov_weight; > > + s->tc_ov_rate -= pipe_tc_be_rate; > > + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > > + > > + if (s->tc_ov != tc_be_ov) { > > + RTE_LOG(DEBUG, SCHED, > > + "Subport %u Best-effort TC > > oversubscription is OFF (%.4lf >= %.4lf)\n", > > + subport_id, subport_tc_be_rate, s- > > >tc_ov_rate); > > + } > > } > > > > /* Reset the pipe */ > > @@ -1460,19 +1498,22 @@ rte_sched_pipe_config(struct rte_sched_port > > *port, > > double pipe_tc_be_rate = > > (double) params- > > >tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] > > / (double) params->tc_period; > > - uint32_t tc_be_ov = s->tc_ov; > > > > - s->tc_ov_n += params->tc_ov_weight; > > - s->tc_ov_rate += pipe_tc_be_rate; > > - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > > + if (s->is_tc_ov_enabled) { > > + uint32_t tc_be_ov = s->tc_ov; > > + > > + s->tc_ov_n += params->tc_ov_weight; > > + s->tc_ov_rate += pipe_tc_be_rate; > > + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > > > > - if (s->tc_ov != tc_be_ov) { > > - RTE_LOG(DEBUG, SCHED, > > - "Subport %u Best effort TC oversubscription is > > ON (%.4lf < %.4lf)\n", > > - subport_id, subport_tc_be_rate, s- > > >tc_ov_rate); > > + if (s->tc_ov != tc_be_ov) { > > + RTE_LOG(DEBUG, SCHED, > > + "Subport %u Best effort TC > > oversubscription is ON (%.4lf < %.4lf)\n", > > + subport_id, subport_tc_be_rate, s- > > >tc_ov_rate); > > + } > > + p->tc_ov_period_id = s->tc_ov_period_id; > > + p->tc_ov_credits = s->tc_ov_wm; > > } > > - p->tc_ov_period_id = s->tc_ov_period_id; > > - p->tc_ov_credits = s->tc_ov_wm; > > } > > > > return 0; > > @@ -2318,6 +2359,45 @@ grinder_credits_update(struct rte_sched_port > > *port, > > pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); > > pipe->tb_time += n_periods * params->tb_period; > > > > + /* Subport TCs */ > > + if (unlikely(port->time >= subport->tc_time)) { > > + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > > + subport->tc_credits[i] = sp->tc_credits_per_period[i]; > > + > > + subport->tc_time = port->time + sp->tc_period; > > + } > > + > > + /* Pipe TCs */ > > + if (unlikely(port->time >= pipe->tc_time)) { > > + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > > + pipe->tc_credits[i] = params->tc_credits_per_period[i]; > > + pipe->tc_time = port->time + params->tc_period; > > + } > > +} > > + > > +static inline void > > +grinder_credits_update_with_tc_ov(struct rte_sched_port *port, > > + struct rte_sched_subport *subport, uint32_t pos) { > > + struct rte_sched_grinder *grinder = subport->grinder + pos; > > + struct rte_sched_pipe *pipe = grinder->pipe; > > + struct rte_sched_pipe_profile *params = grinder->pipe_params; > > + struct rte_sched_subport_profile *sp = grinder->subport_params; > > + uint64_t n_periods; > > + uint32_t i; > > + > > + /* Subport TB */ > > + n_periods = (port->time - subport->tb_time) / sp->tb_period; > > + subport->tb_credits += n_periods * sp->tb_credits_per_period; > > + subport->tb_credits = RTE_MIN(subport->tb_credits, sp->tb_size); > > + subport->tb_time += n_periods * sp->tb_period; > > + > > + /* Pipe TB */ > > + n_periods = (port->time - pipe->tb_time) / params->tb_period; > > + pipe->tb_credits += n_periods * params->tb_credits_per_period; > > + pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); > > + pipe->tb_time += n_periods * params->tb_period; > > + > > /* Subport TCs */ > > if (unlikely(port->time >= subport->tc_time)) { > > subport->tc_ov_wm = > > @@ -2348,6 +2428,39 @@ grinder_credits_update(struct rte_sched_port > > *port, static inline int grinder_credits_check(struct rte_sched_port > > *port, > > struct rte_sched_subport *subport, uint32_t pos) > > +{ > > + struct rte_sched_grinder *grinder = subport->grinder + pos; > > + struct rte_sched_pipe *pipe = grinder->pipe; > > + struct rte_mbuf *pkt = grinder->pkt; > > + uint32_t tc_index = grinder->tc_index; > > + uint64_t pkt_len = pkt->pkt_len + port->frame_overhead; > > + uint64_t subport_tb_credits = subport->tb_credits; > > + uint64_t subport_tc_credits = subport->tc_credits[tc_index]; > > + uint64_t pipe_tb_credits = pipe->tb_credits; > > + uint64_t pipe_tc_credits = pipe->tc_credits[tc_index]; > > + int enough_credits; > > + > > + /* Check pipe and subport credits */ > > + enough_credits = (pkt_len <= subport_tb_credits) && > > + (pkt_len <= subport_tc_credits) && > > + (pkt_len <= pipe_tb_credits) && > > + (pkt_len <= pipe_tc_credits); > > + > > + if (!enough_credits) > > + return 0; > > + > > + /* Update pipe and subport credits */ > > + subport->tb_credits -= pkt_len; > > + subport->tc_credits[tc_index] -= pkt_len; > > + pipe->tb_credits -= pkt_len; > > + pipe->tc_credits[tc_index] -= pkt_len; > > + > > + return 1; > > +} > > + > > +static inline int > > +grinder_credits_check_with_tc_ov(struct rte_sched_port *port, > > + struct rte_sched_subport *subport, uint32_t pos) > > { > > struct rte_sched_grinder *grinder = subport->grinder + pos; > > struct rte_sched_pipe *pipe = grinder->pipe; @@ -2403,8 +2516,16 @@ > > grinder_schedule(struct rte_sched_port *port, > > uint32_t pkt_len = pkt->pkt_len + port->frame_overhead; > > uint32_t be_tc_active; > > > > - if (!grinder_credits_check(port, subport, pos)) > > - return 0; > > + switch (subport->is_tc_ov_enabled) { > > + case 1: > > + if (!grinder_credits_check_with_tc_ov(port, subport, pos)) > > + return 0; > > + break; > > + case 0: > > + if (!grinder_credits_check(port, subport, pos)) > > + return 0; > > + break; > > + } > > There should be no switch statement here, please replace with an if statement. I > suggest the following: > > int status; > > status = subport->tc_ov_enabled ? grinder_credits_check_with_tc_ov(port, > subport, pos) : grinder_credits_check(port, subport, pos); if (!status) > return 0; > > > > > /* Advance port time */ > > port->time += pkt_len; > > @@ -2770,7 +2891,11 @@ grinder_handle(struct rte_sched_port *port, > > subport->profile; > > > > grinder_prefetch_tc_queue_arrays(subport, pos); > > - grinder_credits_update(port, subport, pos); > > + > > + if (unlikely(subport->is_tc_ov_enabled)) > > Please remove the "unlikely" from here, don't put any likely/unlikely here at all. > > > + grinder_credits_update_with_tc_ov(port, subport, pos); > > + else > > + grinder_credits_update(port, subport, pos); > > > > grinder->state = e_GRINDER_PREFETCH_MBUF; > > return 0; > > diff --git a/lib/sched/rte_sched.h b/lib/sched/rte_sched.h index > > 5ece64e527..94febe1d94 100644 > > --- a/lib/sched/rte_sched.h > > +++ b/lib/sched/rte_sched.h > > @@ -579,6 +579,24 @@ rte_sched_port_enqueue(struct rte_sched_port > > *port, struct rte_mbuf **pkts, uint int > > rte_sched_port_dequeue(struct rte_sched_port *port, struct rte_mbuf > > **pkts, uint32_t n_pkts); > > > > +/** > > + * Hierarchical scheduler subport TC OV enable/disable config. > > The name of the feature should be fully stated here: traffic class > oversubscription, not the abbreviation, please change. > > > > + * Note that this function is safe to use at runtime > > + * to enable/disable TC OV for subport. > > We should actually forbit this rather than encourage it. Calling this function > several times does not make sense, and it can create limitations that can come > back and byte us in the future, whenever we might need to extend this code, for > no reason. > > Please actually replace with: "This function should be called at the time of > subport initialization." > > > + * > > + * @param port > > + * Handle to port scheduler instance > > + * @param subport_id > > + * Subport ID > > + * @param tc_ov_enable > > + * Boolean flag to enable/disable TC OV > > + * @return > > + * 0 upon success, error code otherwise > > + */ > > +__rte_experimental > > +int > > +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, uint32_t > > subport_id, bool tc_ov_enable); > > + > > #ifdef __cplusplus > > } > > #endif > > diff --git a/lib/sched/version.map b/lib/sched/version.map index > > d22c07fc9f..c6e994d8df 100644 > > --- a/lib/sched/version.map > > +++ b/lib/sched/version.map > > @@ -34,4 +34,7 @@ EXPERIMENTAL { > > # added in 21.11 > > rte_pie_rt_data_init; > > rte_pie_config_init; > > + > > + # added in 22.03 > > This is not in 22.03, it will hopefully be in 22.07. > > > + rte_sched_subport_tc_ov_config; > > }; > > -- > > 2.25.1 > > Regards, > Cristian ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v3] sched: enable/disable TC OV at runtime 2022-05-10 6:40 ` Ajmera, Megha @ 2022-05-10 9:09 ` Dumitrescu, Cristian 2022-05-24 13:33 ` Marcin Danilewicz 0 siblings, 1 reply; 12+ messages in thread From: Dumitrescu, Cristian @ 2022-05-10 9:09 UTC (permalink / raw) To: Ajmera, Megha, Danilewicz, MarcinX, dev, Singh, Jasvinder Cc: Thakur, Sham Singh Hi Megha, > -----Original Message----- > From: Ajmera, Megha <megha.ajmera@intel.com> > Sent: Tuesday, May 10, 2022 7:41 AM > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Danilewicz, MarcinX > <marcinx.danilewicz@intel.com>; dev@dpdk.org; Singh, Jasvinder > <jasvinder.singh@intel.com> > Cc: Thakur, Sham Singh <sham.singh.thakur@intel.com> > Subject: RE: [PATCH v3] sched: enable/disable TC OV at runtime > > Hi Cristian, Marcin, > > > > -----Original Message----- > > > From: Danilewicz, MarcinX <marcinx.danilewicz@intel.com> > > > Sent: Wednesday, April 27, 2022 10:24 AM > > > To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>; > > > Dumitrescu, Cristian <cristian.dumitrescu@intel.com> > > > Cc: Ajmera, Megha <megha.ajmera@intel.com> > > > Subject: [PATCH v3] sched: enable/disable TC OV at runtime > > > > We are not trying to enable/disable the traffic class oversubscription feature > at > > run-time, but at initialization. If cat, we should prohibit changing this post- > > initialization. > > > > If we only need this to be configured at initialization time, then we can as well > take this flag in subport config API itself. Then there will be no need for a new > API. The purpose of new API was to enable/disable this feature at runtime. > Yes, I agree this would be the ideal way to drive this change, but the problem is that modifying the existing subport parameter structure would represent an API change. This would require a deprecation notice, and the patch would be blocked until 22.11 release. Are you willing to wait until 22.11? If not, then adding the configuration function for this flag is the next best thing. > > Also the name of the feature should not be abbreviated in the patch title. > > > > I suggest you rework the title to: > > [PATCH] sched: enable traffic class oversubscription conditionally > > > > > > > > Added new API to enable or disable TC over subscription for best > > > effort traffic class at subport level. > > > Added changes after review and increased throughput. > > > > > > By default TC OV is disabled. > > > > It should be the other way around, the TC_OV should be enabled by default. > The > > TC oversubscription is a more natural way to use this library, we usually want > to > > disable this feature just for better performance in case this functionality is > not > > needed. Please initialize the tc_ov flag accordingly. > > > > In original code, this feature has always been disabled as it impacts > performance. > So, in my opinion we should keep it disabled by default and let user enable it > when required. > In the original code, yes, it had to be explicitly enabled through a build-time flag. This was not the best option, and this is precisely what we are trying to fix with this patch. But on the other hand all the users of these library that I know use it with the TC oversubscription turned on. Functionality is more important for them than performance. Hence my vote now is to enable it by default; those users that prefer performance over functionality can easily turn this feature off with no issues. > > > > > > Signed-off-by: Marcin Danilewicz <marcinx.danilewicz@intel.com> > > > --- > > > lib/sched/rte_sched.c | 189 > > > +++++++++++++++++++++++++++++++++++------- > > > lib/sched/rte_sched.h | 18 ++++ > > > lib/sched/version.map | 3 + > > > 3 files changed, 178 insertions(+), 32 deletions(-) > > > > > > diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c index > > > ec74bee939..6e7d81df46 100644 > > > --- a/lib/sched/rte_sched.c > > > +++ b/lib/sched/rte_sched.c > > > @@ -213,6 +213,9 @@ struct rte_sched_subport { > > > uint8_t *bmp_array; > > > struct rte_mbuf **queue_array; > > > uint8_t memory[0] __rte_cache_aligned; > > > + > > > + /* TC oversubscription activation */ > > > + int is_tc_ov_enabled; > > > > How about we simplify the name of this variable to: tc_ov_enabled ? > > > > > } __rte_cache_aligned; > > > > > > struct rte_sched_port { > > > @@ -1165,6 +1168,45 @@ rte_sched_cman_config(struct rte_sched_port > > > *port, } #endif > > > > > > +int > > > +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, > > > + uint32_t subport_id, > > > + bool tc_ov_enable) > > > +{ > > > + struct rte_sched_subport *s; > > > + struct rte_sched_subport_profile *profile; > > > + > > > + 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__); > > > + return -EINVAL; > > > + } > > > + > > > + s = port->subports[subport_id]; > > > + s->is_tc_ov_enabled = tc_ov_enable ? 1 : 0; > > > + > > > + if (s->is_tc_ov_enabled) { > > > + /* TC oversubscription */ > > > + s->tc_ov_wm_min = port->mtu; > > > + s->tc_ov_period_id = 0; > > > + s->tc_ov = 0; > > > + s->tc_ov_n = 0; > > > + s->tc_ov_rate = 0; > > > + > > > + profile = port->subport_profiles + s->profile; > > > + s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile- > > > >tc_period, > > > + s->pipe_tc_be_rate_max); > > > + s->tc_ov_wm = s->tc_ov_wm_max; > > > + } > > > + return 0; > > > +} > > > > This function should not exist, please remove it and keep the initial code that > > computes the tc_ov related variable regardless of whether tc_ov is enabled > or > > not. > > > > All the tc_ov related variables have the tc_ov particle in their name, so there > is > > no clash. This is initialization code, so no performance overhead. Let's keep > the > > code unmodified and compute both the tc_ov and the non-tc_ov varables at > > initialization, regardless of whether the feature is enabled or not. > > > > This comment is applicable to all the initialization code, please adjust all the > init > > code accordingly. There should be no diff showing in the patch for any of the > init > > code! > > > > For this file "rte_sched.c", your patch should contain just two additional run- > > time functions, i.e. the non-tc-ov version of functions > grinder_credits_update() > > and grindler_credits_check(), and the small code required to test when to use > > the tc-ov vs. the non-tc_ov version, makes sense? > > > > > + > > > int > > > is_tc_ov_enabled (struct rte_sched_port *port, > > > uint32_t subport_id, > > > @@ -1254,6 +1296,9 @@ rte_sched_subport_config(struct rte_sched_port > > > *port, > > > s->n_pipe_profiles = params->n_pipe_profiles; > > > s->n_max_pipe_profiles = params->n_max_pipe_profiles; > > > > > > + /* TC over-subscription is disabled by default */ > > > + s->is_tc_ov_enabled = 0; > > > + > > > > By default, this feature should be enabled: > > s->is_tc_ov_enabled = 1; > > > > > #ifdef RTE_SCHED_CMAN > > > if (params->cman_params != NULL) { > > > s->cman_enabled = true; > > > @@ -1316,13 +1361,6 @@ rte_sched_subport_config(struct rte_sched_port > > > *port, > > > > > > for (i = 0; i < RTE_SCHED_PORT_N_GRINDERS; i++) > > > s->grinder_base_bmp_pos[i] = > > > RTE_SCHED_PIPE_INVALID; > > > - > > > - /* TC oversubscription */ > > > - s->tc_ov_wm_min = port->mtu; > > > - s->tc_ov_period_id = 0; > > > - s->tc_ov = 0; > > > - s->tc_ov_n = 0; > > > - s->tc_ov_rate = 0; > > > } > > > > > > { > > > @@ -1342,9 +1380,6 @@ rte_sched_subport_config(struct rte_sched_port > > > *port, > > > else > > > profile->tc_credits_per_period[i] = 0; > > > > > > - s->tc_ov_wm_max = rte_sched_time_ms_to_bytes(profile- > > > >tc_period, > > > - s- > > > >pipe_tc_be_rate_max); > > > - s->tc_ov_wm = s->tc_ov_wm_max; > > > s->profile = subport_profile_id; > > > > > > } > > > @@ -1417,17 +1452,20 @@ rte_sched_pipe_config(struct rte_sched_port > > > *port, > > > double pipe_tc_be_rate = > > > (double) params- > > > >tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] > > > / (double) params->tc_period; > > > - uint32_t tc_be_ov = s->tc_ov; > > > > > > - /* Unplug pipe from its subport */ > > > - s->tc_ov_n -= params->tc_ov_weight; > > > - s->tc_ov_rate -= pipe_tc_be_rate; > > > - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > > > + if (s->is_tc_ov_enabled) { > > > + uint32_t tc_be_ov = s->tc_ov; > > > > > > - if (s->tc_ov != tc_be_ov) { > > > - RTE_LOG(DEBUG, SCHED, > > > - "Subport %u Best-effort TC oversubscription is > > > OFF (%.4lf >= %.4lf)\n", > > > - subport_id, subport_tc_be_rate, s- > > > >tc_ov_rate); > > > + /* Unplug pipe from its subport */ > > > + s->tc_ov_n -= params->tc_ov_weight; > > > + s->tc_ov_rate -= pipe_tc_be_rate; > > > + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > > > + > > > + if (s->tc_ov != tc_be_ov) { > > > + RTE_LOG(DEBUG, SCHED, > > > + "Subport %u Best-effort TC > > > oversubscription is OFF (%.4lf >= %.4lf)\n", > > > + subport_id, subport_tc_be_rate, s- > > > >tc_ov_rate); > > > + } > > > } > > > > > > /* Reset the pipe */ > > > @@ -1460,19 +1498,22 @@ rte_sched_pipe_config(struct rte_sched_port > > > *port, > > > double pipe_tc_be_rate = > > > (double) params- > > > >tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASS_BE] > > > / (double) params->tc_period; > > > - uint32_t tc_be_ov = s->tc_ov; > > > > > > - s->tc_ov_n += params->tc_ov_weight; > > > - s->tc_ov_rate += pipe_tc_be_rate; > > > - s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > > > + if (s->is_tc_ov_enabled) { > > > + uint32_t tc_be_ov = s->tc_ov; > > > + > > > + s->tc_ov_n += params->tc_ov_weight; > > > + s->tc_ov_rate += pipe_tc_be_rate; > > > + s->tc_ov = s->tc_ov_rate > subport_tc_be_rate; > > > > > > - if (s->tc_ov != tc_be_ov) { > > > - RTE_LOG(DEBUG, SCHED, > > > - "Subport %u Best effort TC oversubscription is > > > ON (%.4lf < %.4lf)\n", > > > - subport_id, subport_tc_be_rate, s- > > > >tc_ov_rate); > > > + if (s->tc_ov != tc_be_ov) { > > > + RTE_LOG(DEBUG, SCHED, > > > + "Subport %u Best effort TC > > > oversubscription is ON (%.4lf < %.4lf)\n", > > > + subport_id, subport_tc_be_rate, s- > > > >tc_ov_rate); > > > + } > > > + p->tc_ov_period_id = s->tc_ov_period_id; > > > + p->tc_ov_credits = s->tc_ov_wm; > > > } > > > - p->tc_ov_period_id = s->tc_ov_period_id; > > > - p->tc_ov_credits = s->tc_ov_wm; > > > } > > > > > > return 0; > > > @@ -2318,6 +2359,45 @@ grinder_credits_update(struct rte_sched_port > > > *port, > > > pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); > > > pipe->tb_time += n_periods * params->tb_period; > > > > > > + /* Subport TCs */ > > > + if (unlikely(port->time >= subport->tc_time)) { > > > + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > > > + subport->tc_credits[i] = sp->tc_credits_per_period[i]; > > > + > > > + subport->tc_time = port->time + sp->tc_period; > > > + } > > > + > > > + /* Pipe TCs */ > > > + if (unlikely(port->time >= pipe->tc_time)) { > > > + for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > > > + pipe->tc_credits[i] = params->tc_credits_per_period[i]; > > > + pipe->tc_time = port->time + params->tc_period; > > > + } > > > +} > > > + > > > +static inline void > > > +grinder_credits_update_with_tc_ov(struct rte_sched_port *port, > > > + struct rte_sched_subport *subport, uint32_t pos) { > > > + struct rte_sched_grinder *grinder = subport->grinder + pos; > > > + struct rte_sched_pipe *pipe = grinder->pipe; > > > + struct rte_sched_pipe_profile *params = grinder->pipe_params; > > > + struct rte_sched_subport_profile *sp = grinder->subport_params; > > > + uint64_t n_periods; > > > + uint32_t i; > > > + > > > + /* Subport TB */ > > > + n_periods = (port->time - subport->tb_time) / sp->tb_period; > > > + subport->tb_credits += n_periods * sp->tb_credits_per_period; > > > + subport->tb_credits = RTE_MIN(subport->tb_credits, sp->tb_size); > > > + subport->tb_time += n_periods * sp->tb_period; > > > + > > > + /* Pipe TB */ > > > + n_periods = (port->time - pipe->tb_time) / params->tb_period; > > > + pipe->tb_credits += n_periods * params->tb_credits_per_period; > > > + pipe->tb_credits = RTE_MIN(pipe->tb_credits, params->tb_size); > > > + pipe->tb_time += n_periods * params->tb_period; > > > + > > > /* Subport TCs */ > > > if (unlikely(port->time >= subport->tc_time)) { > > > subport->tc_ov_wm = > > > @@ -2348,6 +2428,39 @@ grinder_credits_update(struct rte_sched_port > > > *port, static inline int grinder_credits_check(struct rte_sched_port > > > *port, > > > struct rte_sched_subport *subport, uint32_t pos) > > > +{ > > > + struct rte_sched_grinder *grinder = subport->grinder + pos; > > > + struct rte_sched_pipe *pipe = grinder->pipe; > > > + struct rte_mbuf *pkt = grinder->pkt; > > > + uint32_t tc_index = grinder->tc_index; > > > + uint64_t pkt_len = pkt->pkt_len + port->frame_overhead; > > > + uint64_t subport_tb_credits = subport->tb_credits; > > > + uint64_t subport_tc_credits = subport->tc_credits[tc_index]; > > > + uint64_t pipe_tb_credits = pipe->tb_credits; > > > + uint64_t pipe_tc_credits = pipe->tc_credits[tc_index]; > > > + int enough_credits; > > > + > > > + /* Check pipe and subport credits */ > > > + enough_credits = (pkt_len <= subport_tb_credits) && > > > + (pkt_len <= subport_tc_credits) && > > > + (pkt_len <= pipe_tb_credits) && > > > + (pkt_len <= pipe_tc_credits); > > > + > > > + if (!enough_credits) > > > + return 0; > > > + > > > + /* Update pipe and subport credits */ > > > + subport->tb_credits -= pkt_len; > > > + subport->tc_credits[tc_index] -= pkt_len; > > > + pipe->tb_credits -= pkt_len; > > > + pipe->tc_credits[tc_index] -= pkt_len; > > > + > > > + return 1; > > > +} > > > + > > > +static inline int > > > +grinder_credits_check_with_tc_ov(struct rte_sched_port *port, > > > + struct rte_sched_subport *subport, uint32_t pos) > > > { > > > struct rte_sched_grinder *grinder = subport->grinder + pos; > > > struct rte_sched_pipe *pipe = grinder->pipe; @@ -2403,8 +2516,16 > @@ > > > grinder_schedule(struct rte_sched_port *port, > > > uint32_t pkt_len = pkt->pkt_len + port->frame_overhead; > > > uint32_t be_tc_active; > > > > > > - if (!grinder_credits_check(port, subport, pos)) > > > - return 0; > > > + switch (subport->is_tc_ov_enabled) { > > > + case 1: > > > + if (!grinder_credits_check_with_tc_ov(port, subport, pos)) > > > + return 0; > > > + break; > > > + case 0: > > > + if (!grinder_credits_check(port, subport, pos)) > > > + return 0; > > > + break; > > > + } > > > > There should be no switch statement here, please replace with an if > statement. I > > suggest the following: > > > > int status; > > > > status = subport->tc_ov_enabled ? grinder_credits_check_with_tc_ov(port, > > subport, pos) : grinder_credits_check(port, subport, pos); if (!status) > > return 0; > > > > > > > > /* Advance port time */ > > > port->time += pkt_len; > > > @@ -2770,7 +2891,11 @@ grinder_handle(struct rte_sched_port *port, > > > subport->profile; > > > > > > grinder_prefetch_tc_queue_arrays(subport, pos); > > > - grinder_credits_update(port, subport, pos); > > > + > > > + if (unlikely(subport->is_tc_ov_enabled)) > > > > Please remove the "unlikely" from here, don't put any likely/unlikely here at > all. > > > > > + grinder_credits_update_with_tc_ov(port, subport, pos); > > > + else > > > + grinder_credits_update(port, subport, pos); > > > > > > grinder->state = e_GRINDER_PREFETCH_MBUF; > > > return 0; > > > diff --git a/lib/sched/rte_sched.h b/lib/sched/rte_sched.h index > > > 5ece64e527..94febe1d94 100644 > > > --- a/lib/sched/rte_sched.h > > > +++ b/lib/sched/rte_sched.h > > > @@ -579,6 +579,24 @@ rte_sched_port_enqueue(struct rte_sched_port > > > *port, struct rte_mbuf **pkts, uint int > > > rte_sched_port_dequeue(struct rte_sched_port *port, struct rte_mbuf > > > **pkts, uint32_t n_pkts); > > > > > > +/** > > > + * Hierarchical scheduler subport TC OV enable/disable config. > > > > The name of the feature should be fully stated here: traffic class > > oversubscription, not the abbreviation, please change. > > > > > > > + * Note that this function is safe to use at runtime > > > + * to enable/disable TC OV for subport. > > > > We should actually forbit this rather than encourage it. Calling this function > > several times does not make sense, and it can create limitations that can > come > > back and byte us in the future, whenever we might need to extend this code, > for > > no reason. > > > > Please actually replace with: "This function should be called at the time of > > subport initialization." > > > > > + * > > > + * @param port > > > + * Handle to port scheduler instance > > > + * @param subport_id > > > + * Subport ID > > > + * @param tc_ov_enable > > > + * Boolean flag to enable/disable TC OV > > > + * @return > > > + * 0 upon success, error code otherwise > > > + */ > > > +__rte_experimental > > > +int > > > +rte_sched_subport_tc_ov_config(struct rte_sched_port *port, uint32_t > > > subport_id, bool tc_ov_enable); > > > + > > > #ifdef __cplusplus > > > } > > > #endif > > > diff --git a/lib/sched/version.map b/lib/sched/version.map index > > > d22c07fc9f..c6e994d8df 100644 > > > --- a/lib/sched/version.map > > > +++ b/lib/sched/version.map > > > @@ -34,4 +34,7 @@ EXPERIMENTAL { > > > # added in 21.11 > > > rte_pie_rt_data_init; > > > rte_pie_config_init; > > > + > > > + # added in 22.03 > > > > This is not in 22.03, it will hopefully be in 22.07. > > > > > + rte_sched_subport_tc_ov_config; > > > }; > > > -- > > > 2.25.1 > > > > Regards, > > Cristian Regards, Cristian ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3] sched: enable/disable TC OV at runtime 2022-05-10 9:09 ` Dumitrescu, Cristian @ 2022-05-24 13:33 ` Marcin Danilewicz 2022-05-24 14:52 ` Stephen Hemminger 0 siblings, 1 reply; 12+ messages in thread From: Marcin Danilewicz @ 2022-05-24 13:33 UTC (permalink / raw) To: cristian.dumitrescu Cc: dev, jasvinder.singh, marcinx.danilewicz, megha.ajmera, sham.singh.thakur Hi Megha, Marcin, > Yes, I agree this would be the ideal way to drive this change, but the problem is that modifying the existing subport parameter structure would represent an API change. This would require a deprecation notice, and the patch would be blocked until 22.11 release. Are you willing to wait until 22.11? If not, then adding the configuration function for this flag is the next best thing. Are we making any plans for that? > > Also the name of the feature should not be abbreviated in the patch title. > > In original code, this feature has always been disabled as it impacts >> performance. > > So, in my opinion we should keep it disabled by default and let user enable it > > when required. > > > In the original code, yes, it had to be explicitly enabled through a build-time flag. This was not the best option, and this is precisely what we are trying to fix with this patch. > But on the other hand all the users of these library that I know use it with the TC oversubscription turned on. Functionality is more important for them than performance. Hence my vote now is to enable it by default; those users that prefer performance over functionality can easily turn this feature off with no issues. That was plan for this patch and yet another change to remove API change is awaiting. But when we publish this change. BR, /Marcin -------------------------------------------------------------- Intel Research and Development Ireland Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] sched: enable/disable TC OV at runtime 2022-05-24 13:33 ` Marcin Danilewicz @ 2022-05-24 14:52 ` Stephen Hemminger 2022-05-26 23:12 ` Danilewicz, MarcinX 0 siblings, 1 reply; 12+ messages in thread From: Stephen Hemminger @ 2022-05-24 14:52 UTC (permalink / raw) To: Marcin Danilewicz Cc: cristian.dumitrescu, dev, jasvinder.singh, megha.ajmera, sham.singh.thakur On Tue, 24 May 2022 13:33:31 +0000 Marcin Danilewicz <marcinx.danilewicz@intel.com> wrote: > /Marcin > -------------------------------------------------------------- > Intel Research and Development Ireland Limited > Registered in Ireland > Registered Office: Collinstown Industrial Park, Leixlip, County Kildare > Registered Number: 308263 > > > This e-mail and any attachments may contain confidential material for the sole > use of the intended recipient(s). Any review or distribution by others is > strictly prohibited. If you are not the intended recipient, please contact the > sender and delete all copies. > Please talk to your management/lawyers. This kind of auto-footer violates the required discussion properties of open source. ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v3] sched: enable/disable TC OV at runtime 2022-05-24 14:52 ` Stephen Hemminger @ 2022-05-26 23:12 ` Danilewicz, MarcinX 0 siblings, 0 replies; 12+ messages in thread From: Danilewicz, MarcinX @ 2022-05-26 23:12 UTC (permalink / raw) To: Stephen Hemminger Cc: Dumitrescu, Cristian, dev, Singh, Jasvinder, Ajmera, Megha, Thakur, Sham Singh Hi Stephen, You are absolutely right, but that was not my intention or something I've configured. My e-mail client wasn't sending this alsoe. Perhaps something like email-server or something else was enabled in between. I sent patch as per usual and to my surprise I found that message. My apologies, even if I wasn't source for that mistake. BR, /Marcin > -----Original Message----- > From: Stephen Hemminger <stephen@networkplumber.org> > Sent: Tuesday, May 24, 2022 4:53 PM > To: Danilewicz, MarcinX <marcinx.danilewicz@intel.com> > Cc: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; dev@dpdk.org; > Singh, Jasvinder <jasvinder.singh@intel.com>; Ajmera, Megha > <megha.ajmera@intel.com>; Thakur, Sham Singh > <sham.singh.thakur@intel.com> > Subject: Re: [PATCH v3] sched: enable/disable TC OV at runtime > > On Tue, 24 May 2022 13:33:31 +0000 > Marcin Danilewicz <marcinx.danilewicz@intel.com> wrote: > > > /Marcin > > -------------------------------------------------------------- > > Intel Research and Development Ireland Limited Registered in Ireland > > Registered Office: Collinstown Industrial Park, Leixlip, County > > Kildare Registered Number: 308263 > > > > > > This e-mail and any attachments may contain confidential material for > > the sole use of the intended recipient(s). Any review or distribution > > by others is strictly prohibited. If you are not the intended > > recipient, please contact the sender and delete all copies. > > > > > Please talk to your management/lawyers. This kind of auto-footer violates > the required discussion properties of open source. -------------------------------------------------------------- Intel Research and Development Ireland Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-05-26 23:12 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-27 8:58 [PATCH v3] sched: enable/disable TC OV at runtime Marcin Danilewicz 2022-04-27 9:36 ` Dumitrescu, Cristian 2022-04-27 9:37 ` Dumitrescu, Cristian 2022-04-27 15:53 ` Stephen Hemminger -- strict thread matches above, loose matches on Subject: below -- 2022-04-07 14:51 [dpdk][PATCH 2/2] sched: fix to manage " Marcin Danilewicz 2022-04-27 9:23 ` [PATCH v3] sched: enable/disable " Marcin Danilewicz 2022-04-27 15:48 ` Singh, Jasvinder 2022-05-09 20:05 ` Dumitrescu, Cristian 2022-05-10 6:40 ` Ajmera, Megha 2022-05-10 9:09 ` Dumitrescu, Cristian 2022-05-24 13:33 ` Marcin Danilewicz 2022-05-24 14:52 ` Stephen Hemminger 2022-05-26 23:12 ` Danilewicz, MarcinX
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).