DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] sched: fix for tc_ov_enable flag position in subport structure.
@ 2023-01-09 14:54 Megha Ajmera
  2023-01-10 11:12 ` Dumitrescu, Cristian
  2023-01-10 11:27 ` David Marchand
  0 siblings, 2 replies; 9+ messages in thread
From: Megha Ajmera @ 2023-01-09 14:54 UTC (permalink / raw)
  To: dev, jasvinder.singh, cristian.dumitrescu; +Cc: stable, marcinx.danilewicz

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 {
 	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 {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] sched: fix for tc_ov_enable flag position in subport structure.
  2023-01-09 14:54 [PATCH] sched: fix for tc_ov_enable flag position in subport structure Megha Ajmera
@ 2023-01-10 11:12 ` Dumitrescu, Cristian
  2023-01-10 11:27 ` David Marchand
  1 sibling, 0 replies; 9+ messages in thread
From: Dumitrescu, Cristian @ 2023-01-10 11:12 UTC (permalink / raw)
  To: Ajmera, Megha, dev, Singh, Jasvinder; +Cc: stable, marcinx.danilewicz



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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] sched: fix for tc_ov_enable flag position in subport structure.
  2023-01-09 14:54 [PATCH] sched: fix for tc_ov_enable flag position in subport structure Megha Ajmera
  2023-01-10 11:12 ` Dumitrescu, Cristian
@ 2023-01-10 11:27 ` David Marchand
  2023-02-06  7:43   ` Thomas Monjalon
  2023-02-07  6:10   ` [PATCH v2] sched: fix for incorrect alignment of bitmap, pipe and queue structs in subport Megha Ajmera
  1 sibling, 2 replies; 9+ messages in thread
From: David Marchand @ 2023-01-10 11:27 UTC (permalink / raw)
  To: Megha Ajmera
  Cc: dev, jasvinder.singh, cristian.dumitrescu, stable, marcinx.danilewicz

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 {



-- 
David Marchand


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] sched: fix for tc_ov_enable flag position in subport structure.
  2023-01-10 11:27 ` David Marchand
@ 2023-02-06  7:43   ` Thomas Monjalon
  2023-02-06  9:32     ` Dumitrescu, Cristian
  2023-02-07  6:10   ` [PATCH v2] sched: fix for incorrect alignment of bitmap, pipe and queue structs in subport Megha Ajmera
  1 sibling, 1 reply; 9+ messages in thread
From: Thomas Monjalon @ 2023-02-06  7:43 UTC (permalink / raw)
  To: Megha Ajmera
  Cc: stable, dev, jasvinder.singh, cristian.dumitrescu,
	marcinx.danilewicz, David Marchand

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?





^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] sched: fix for tc_ov_enable flag position in subport structure.
  2023-02-06  7:43   ` Thomas Monjalon
@ 2023-02-06  9:32     ` Dumitrescu, Cristian
  2023-02-07  9:09       ` Ajmera, Megha
  0 siblings, 1 reply; 9+ messages in thread
From: Dumitrescu, Cristian @ 2023-02-06  9:32 UTC (permalink / raw)
  To: Thomas Monjalon, Ajmera, Megha
  Cc: stable, dev, Singh, Jasvinder, marcinx.danilewicz, David Marchand



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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] sched: fix for incorrect alignment of bitmap, pipe and queue structs in subport
  2023-01-10 11:27 ` David Marchand
  2023-02-06  7:43   ` Thomas Monjalon
@ 2023-02-07  6:10   ` Megha Ajmera
  2023-02-19 22:40     ` Thomas Monjalon
  1 sibling, 1 reply; 9+ messages in thread
From: Megha Ajmera @ 2023-02-07  6:10 UTC (permalink / raw)
  To: dev, jasvinder.singh, cristian.dumitrescu; +Cc: stable

Big structures like bitmap, pipes and queues in subport are addressed
using offset of 'memory' field in subport structures. This means no other
variable should be added after 'memory' variable or else addressing of such
structs like bitmap etc. become incorrect.

Realigned tc_ov_enabled variable in subport structure.

Fixes: f5e60154ade ("sched: enable traffic class oversubscription conditionally")

Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@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 {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] sched: fix for tc_ov_enable flag position in subport structure.
  2023-02-06  9:32     ` Dumitrescu, Cristian
@ 2023-02-07  9:09       ` Ajmera, Megha
  0 siblings, 0 replies; 9+ messages in thread
From: Ajmera, Megha @ 2023-02-07  9:09 UTC (permalink / raw)
  To: Dumitrescu, Cristian, Thomas Monjalon
  Cc: stable, dev, Singh, Jasvinder, marcinx.danilewicz, David Marchand

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] sched: fix for incorrect alignment of bitmap, pipe and queue structs in subport
  2023-02-07  6:10   ` [PATCH v2] sched: fix for incorrect alignment of bitmap, pipe and queue structs in subport Megha Ajmera
@ 2023-02-19 22:40     ` Thomas Monjalon
  2023-02-19 22:43       ` Thomas Monjalon
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Monjalon @ 2023-02-19 22:40 UTC (permalink / raw)
  To: Megha Ajmera; +Cc: dev, jasvinder.singh, cristian.dumitrescu, stable

07/02/2023 07:10, Megha Ajmera:
> Big structures like bitmap, pipes and queues in subport are addressed
> using offset of 'memory' field in subport structures. This means no other
> variable should be added after 'memory' variable or else addressing of such
> structs like bitmap etc. become incorrect.
> 
> Realigned tc_ov_enabled variable in subport structure.
> 
> Fixes: f5e60154ade ("sched: enable traffic class oversubscription conditionally")
> 
> Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

I bet the impact is wrongly explained, but I don't want to look deeper.
Applied.




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] sched: fix for incorrect alignment of bitmap, pipe and queue structs in subport
  2023-02-19 22:40     ` Thomas Monjalon
@ 2023-02-19 22:43       ` Thomas Monjalon
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Monjalon @ 2023-02-19 22:43 UTC (permalink / raw)
  To: Megha Ajmera
  Cc: stable, dev, jasvinder.singh, cristian.dumitrescu, david.marchand

19/02/2023 23:40, Thomas Monjalon:
> 07/02/2023 07:10, Megha Ajmera:
> > Big structures like bitmap, pipes and queues in subport are addressed
> > using offset of 'memory' field in subport structures. This means no other
> > variable should be added after 'memory' variable or else addressing of such
> > structs like bitmap etc. become incorrect.
> > 
> > Realigned tc_ov_enabled variable in subport structure.
> > 
> > Fixes: f5e60154ade ("sched: enable traffic class oversubscription conditionally")
> > 
> > Signed-off-by: Megha Ajmera <megha.ajmera@intel.com>
> > Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> 
> I bet the impact is wrongly explained, but I don't want to look deeper.
> Applied.

And I've added Cc: stable@dpdk.org that David and Cristian asked for.




^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-02-19 22:43 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-09 14:54 [PATCH] sched: fix for tc_ov_enable flag position in subport structure Megha Ajmera
2023-01-10 11:12 ` Dumitrescu, Cristian
2023-01-10 11:27 ` David Marchand
2023-02-06  7:43   ` Thomas Monjalon
2023-02-06  9:32     ` Dumitrescu, Cristian
2023-02-07  9:09       ` Ajmera, Megha
2023-02-07  6:10   ` [PATCH v2] sched: fix for incorrect alignment of bitmap, pipe and queue structs in subport Megha Ajmera
2023-02-19 22:40     ` Thomas Monjalon
2023-02-19 22:43       ` Thomas Monjalon

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