sched: fix for tc_ov_enable flag position in subport structure.
Checks
Commit Message
Current position of "tv_ov_enable" variable in
rte_sched_subport structure makes the "memory" variable unused.
Fixes: f5e60154ade ("sched: enable traffic class oversubscription conditionally")
CC: marcinx.danilewicz@intel.com
Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
---
lib/sched/rte_sched.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Comments
> -----Original Message-----
> From: Ajmera, Megha <megha.ajmera@intel.com>
> Sent: Monday, January 9, 2023 2:55 PM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu,
> Cristian <cristian.dumitrescu@intel.com>
> Cc: stable@dpdk.org; marcinx.danilewicz@intel.com
> Subject: [PATCH] sched: fix for tc_ov_enable flag position in subport structure.
>
> Current position of "tv_ov_enable" variable in
> rte_sched_subport structure makes the "memory" variable unused.
>
> Fixes: f5e60154ade ("sched: enable traffic class oversubscription conditionally")
> CC: marcinx.danilewicz@intel.com
> Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
> ---
> lib/sched/rte_sched.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
> index c91697131d..eaecd7ceb4 100644
> --- a/lib/sched/rte_sched.c
> +++ b/lib/sched/rte_sched.c
> @@ -202,6 +202,9 @@ struct rte_sched_subport {
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
On Mon, Jan 9, 2023 at 3:59 PM Megha Ajmera <megha.ajmera@intel.com> wrote:
>
> Current position of "tv_ov_enable" variable in
tc_ov_enabled*
> rte_sched_subport structure makes the "memory" variable unused.
I did not enter the beast... but my understanding is that some object
internal to rte_sched_subport currently shares location with this
tc_ov_enabled field.
So please find a better title and describe the impact.
>
> Fixes: f5e60154ade ("sched: enable traffic class oversubscription conditionally")
> CC: marcinx.danilewicz@intel.com
This is stable@dpdk.org material, isn't it?
> Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
> ---
> lib/sched/rte_sched.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/sched/rte_sched.c b/lib/sched/rte_sched.c
> index c91697131d..eaecd7ceb4 100644
> --- a/lib/sched/rte_sched.c
> +++ b/lib/sched/rte_sched.c
> @@ -202,6 +202,9 @@ struct rte_sched_subport {
> uint32_t qsize_add[RTE_SCHED_QUEUES_PER_PIPE];
> uint32_t qsize_sum;
>
> + /* TC oversubscription activation */
> + int tc_ov_enabled;
> +
> struct rte_sched_pipe *pipe;
> struct rte_sched_queue *queue;
> struct rte_sched_queue_extra *queue_extra;
> @@ -210,8 +213,6 @@ struct rte_sched_subport {
> struct rte_mbuf **queue_array;
> uint8_t memory[0] __rte_cache_aligned;
>
> - /* TC oversubscription activation */
> - int tc_ov_enabled;
> } __rte_cache_aligned;
>
> struct rte_sched_port {
10/01/2023 12:27, David Marchand:
> On Mon, Jan 9, 2023 at 3:59 PM Megha Ajmera <megha.ajmera@intel.com> wrote:
> >
> > Current position of "tv_ov_enable" variable in
>
> tc_ov_enabled*
>
>
> > rte_sched_subport structure makes the "memory" variable unused.
>
> I did not enter the beast... but my understanding is that some object
> internal to rte_sched_subport currently shares location with this
> tc_ov_enabled field.
> So please find a better title and describe the impact.
>
>
> >
> > Fixes: f5e60154ade ("sched: enable traffic class oversubscription conditionally")
> > CC: marcinx.danilewicz@intel.com
>
> This is stable@dpdk.org material, isn't it?
>
>
> > Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
Ping
Please update or the patch will be abandoned.
Jasvinder, Cristian, can you help?
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, February 6, 2023 7:43 AM
> To: Ajmera, Megha <megha.ajmera@intel.com>
> Cc: stable@dpdk.org; dev@dpdk.org; Singh, Jasvinder
> <jasvinder.singh@intel.com>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>; marcinx.danilewicz@intel.com; David
> Marchand <david.marchand@redhat.com>
> Subject: Re: [PATCH] sched: fix for tc_ov_enable flag position in subport
> structure.
>
> 10/01/2023 12:27, David Marchand:
> > On Mon, Jan 9, 2023 at 3:59 PM Megha Ajmera <megha.ajmera@intel.com>
> wrote:
> > >
> > > Current position of "tv_ov_enable" variable in
> >
> > tc_ov_enabled*
> >
> >
> > > rte_sched_subport structure makes the "memory" variable unused.
> >
> > I did not enter the beast... but my understanding is that some object
> > internal to rte_sched_subport currently shares location with this
> > tc_ov_enabled field.
> > So please find a better title and describe the impact.
> >
> >
> > >
> > > Fixes: f5e60154ade ("sched: enable traffic class oversubscription
> conditionally")
> > > CC: marcinx.danilewicz@intel.com
> >
> > This is stable@dpdk.org material, isn't it?
> >
> >
> > > Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
>
> Ping
> Please update or the patch will be abandoned.
> Jasvinder, Cristian, can you help?
>
>
>
Megha,
David asked for a better description of the fix and CC stable about a month ago, did you miss his email? These should be quick to handle, are you able to send V2?
Regards,
Cristian
> >
> > 10/01/2023 12:27, David Marchand:
> > > On Mon, Jan 9, 2023 at 3:59 PM Megha Ajmera <megha.ajmera@intel.com>
> > wrote:
> > > >
> > > > Current position of "tv_ov_enable" variable in
> > >
> > > tc_ov_enabled*
> > >
> > >
> > > > rte_sched_subport structure makes the "memory" variable unused.
> > >
> > > I did not enter the beast... but my understanding is that some
> > > object internal to rte_sched_subport currently shares location with
> > > this tc_ov_enabled field.
> > > So please find a better title and describe the impact.
> > >
> > >
> > >
> > > This is stable@dpdk.org material, isn't it?
> > >
> > >
Yes. Applicable to stable branch as well.
>
> Megha,
>
> David asked for a better description of the fix and CC stable about a month ago,
> did you miss his email? These should be quick to handle, are you able to send
> V2?
Hi Cristian,
I missed earlier email on this thread. Sent v2 version of patch with updated title and description.
Please have a look and review.
@@ -202,6 +202,9 @@ struct rte_sched_subport {
uint32_t qsize_add[RTE_SCHED_QUEUES_PER_PIPE];
uint32_t qsize_sum;
+ /* TC oversubscription activation */
+ int tc_ov_enabled;
+
struct rte_sched_pipe *pipe;
struct rte_sched_queue *queue;
struct rte_sched_queue_extra *queue_extra;
@@ -210,8 +213,6 @@ struct rte_sched_subport {
struct rte_mbuf **queue_array;
uint8_t memory[0] __rte_cache_aligned;
- /* TC oversubscription activation */
- int tc_ov_enabled;
} __rte_cache_aligned;
struct rte_sched_port {