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(-)
  

> -----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.