DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] mbuf: fix compile by making sched struct visible
@ 2019-01-10 16:50 Harry van Haaren
  2019-01-10 17:34 ` Thomas Monjalon
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Harry van Haaren @ 2019-01-10 16:50 UTC (permalink / raw)
  To: dev; +Cc: Harry van Haaren, reshma.pattan, cristian.dumitrescu, thomas

Although C compilation works with the struct rte_mbuf_sched
declared inside the struct rte_mbuf namespace, C++ fails to
compile. This fix moves the rte_mbuf_sched struct up to the
global namespace, instead of declaring it inside the struct
mbuf namespace.

The struct rte_mbuf_sched is being used on the stack in
rte_mbuf_sched_get() and as a cast in _set(). For this
reason, it must be exposed as an available type.

Fixes: 5d3f72100904 ("mbuf: implement generic format for sched field")

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

Cc: reshma.pattan@intel.com
Cc: cristian.dumitrescu@intel.com
Cc: thomas@monjalon.net

Hey folks,

Currently the mbuf header will fail to compile with a C++ compiler,
this patch is one possible solution. I'm not particularly happy with
this as a fix as it reduces mbuf struct readability, however it does
resolve the issue.

Regards, -Harry

---
 lib/librte_mbuf/rte_mbuf.h | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index bc562dc8a..a9df0cd00 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -468,6 +468,17 @@ __extension__
 typedef uint64_t MARKER64[0]; /**< marker that allows us to overwrite 8 bytes
                                * with a single assignment */
 
+struct rte_mbuf_sched {
+	uint32_t queue_id;   /**< Queue ID. */
+	uint8_t traffic_class;
+	/**< Traffic class ID. Traffic class 0
+	 * is the highest priority traffic class.
+	 */
+	uint8_t color;
+	/**< Color. @see enum rte_color.*/
+	uint16_t reserved;   /**< Reserved. */
+}; /**< Hierarchical scheduler */
+
 /**
  * The generic rte_mbuf, containing a packet mbuf.
  */
@@ -574,16 +585,7 @@ struct rte_mbuf {
 				 * on PKT_RX_FDIR_* flag in ol_flags.
 				 */
 			} fdir;	/**< Filter identifier if FDIR enabled */
-			struct rte_mbuf_sched {
-				uint32_t queue_id;   /**< Queue ID. */
-				uint8_t traffic_class;
-				/**< Traffic class ID. Traffic class 0
-				 * is the highest priority traffic class.
-				 */
-				uint8_t color;
-				/**< Color. @see enum rte_color.*/
-				uint16_t reserved;   /**< Reserved. */
-			} sched; /**< Hierarchical scheduler */
+			struct rte_mbuf_sched sched; /**< Hierarchical scheduler */
 			struct {
 				uint32_t reserved1;
 				uint16_t reserved2;
-- 
2.17.1

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

* Re: [dpdk-dev] [PATCH] mbuf: fix compile by making sched struct visible
  2019-01-10 16:50 [dpdk-dev] [PATCH] mbuf: fix compile by making sched struct visible Harry van Haaren
@ 2019-01-10 17:34 ` Thomas Monjalon
  2019-01-10 17:57   ` Van Haaren, Harry
  2019-01-10 18:06 ` [dpdk-dev] [PATCH v2] mbuf: fix compile by removing struct from function Harry van Haaren
  2019-01-10 22:05 ` [dpdk-dev] [PATCH] " Stephen Hemminger
  2 siblings, 1 reply; 14+ messages in thread
From: Thomas Monjalon @ 2019-01-10 17:34 UTC (permalink / raw)
  To: Harry van Haaren; +Cc: dev, reshma.pattan, cristian.dumitrescu, olivier.matz

10/01/2019 17:50, Harry van Haaren:
> Although C compilation works with the struct rte_mbuf_sched
> declared inside the struct rte_mbuf namespace, C++ fails to
> compile. This fix moves the rte_mbuf_sched struct up to the
> global namespace, instead of declaring it inside the struct
> mbuf namespace.
> 
> The struct rte_mbuf_sched is being used on the stack in
> rte_mbuf_sched_get() and as a cast in _set(). For this
> reason, it must be exposed as an available type.
> 
> Fixes: 5d3f72100904 ("mbuf: implement generic format for sched field")
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> 
> ---
> 
> Cc: reshma.pattan@intel.com
> Cc: cristian.dumitrescu@intel.com
> Cc: thomas@monjalon.net
> 
> Hey folks,
> 
> Currently the mbuf header will fail to compile with a C++ compiler,
> this patch is one possible solution. I'm not particularly happy with
> this as a fix as it reduces mbuf struct readability, however it does
> resolve the issue.

What are the other possible solutions?

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

* Re: [dpdk-dev] [PATCH] mbuf: fix compile by making sched struct visible
  2019-01-10 17:34 ` Thomas Monjalon
@ 2019-01-10 17:57   ` Van Haaren, Harry
  0 siblings, 0 replies; 14+ messages in thread
From: Van Haaren, Harry @ 2019-01-10 17:57 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, Pattan, Reshma, Dumitrescu, Cristian, olivier.matz

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Thursday, January 10, 2019 5:35 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dev@dpdk.org; Pattan, Reshma <reshma.pattan@intel.com>; Dumitrescu,
> Cristian <cristian.dumitrescu@intel.com>; olivier.matz@6wind.com
> Subject: Re: [PATCH] mbuf: fix compile by making sched struct visible
> 
> 10/01/2019 17:50, Harry van Haaren:
> > Although C compilation works with the struct rte_mbuf_sched
> > declared inside the struct rte_mbuf namespace, C++ fails to
> > compile. This fix moves the rte_mbuf_sched struct up to the
> > global namespace, instead of declaring it inside the struct
> > mbuf namespace.
> >
> > The struct rte_mbuf_sched is being used on the stack in
> > rte_mbuf_sched_get() and as a cast in _set(). For this
> > reason, it must be exposed as an available type.
> >
> > Fixes: 5d3f72100904 ("mbuf: implement generic format for sched field")
> >
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> >
> > ---
> >
> > Cc: reshma.pattan@intel.com
> > Cc: cristian.dumitrescu@intel.com
> > Cc: thomas@monjalon.net
> >
> > Hey folks,
> >
> > Currently the mbuf header will fail to compile with a C++ compiler,
> > this patch is one possible solution. I'm not particularly happy with
> > this as a fix as it reduces mbuf struct readability, however it does
> > resolve the issue.
> 
> What are the other possible solutions?


I guess we could avoid using the struct in inline functions, by reading
or writing from the mbuf struct directly (without pulling out a temporary
rte_mbuf_sched field):

get()
*queue_id = m->hash.sched.queue_id;
...

set()
m->hash.sched.queue_id = queue_id;
...


I believe this was discussed when the patch was being reviewed;
http://patches.dpdk.org/patch/49126/

I have a mild preference to keep all mbuf structs visible inside the unions,
it makes it easier to understand the layout of mbuf, hence I prefer the
above pseudo code suggestion over the v1 patch sent before.

If others have a strong opinion for another solution, I'm ok with it.

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

* [dpdk-dev] [PATCH v2] mbuf: fix compile by removing struct from function
  2019-01-10 16:50 [dpdk-dev] [PATCH] mbuf: fix compile by making sched struct visible Harry van Haaren
  2019-01-10 17:34 ` Thomas Monjalon
@ 2019-01-10 18:06 ` Harry van Haaren
  2019-01-10 18:40   ` Dumitrescu, Cristian
  2019-01-11 11:32   ` [dpdk-dev] [PATCH v3] mbuf: fix compile by making sched struct visible Harry van Haaren
  2019-01-10 22:05 ` [dpdk-dev] [PATCH] " Stephen Hemminger
  2 siblings, 2 replies; 14+ messages in thread
From: Harry van Haaren @ 2019-01-10 18:06 UTC (permalink / raw)
  To: dev
  Cc: Harry van Haaren, reshma.pattan, cristian.dumitrescu, thomas,
	olivier.matz

Although C compilation works with the struct rte_mbuf_sched
declared inside the struct rte_mbuf namespace, C++ fails to
compile.

This fix removes the temporary struct rte_mbuf_sched, instead
reading from the mbuf directly for each struct member. As the
struct is now not used directly, the C++ compiler doesn't need
to know about the struct, resolving the issue.

Fixes: 5d3f72100904 ("mbuf: implement generic format for sched field")

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

Cc: reshma.pattan@intel.com
Cc: cristian.dumitrescu@intel.com
Cc: thomas@monjalon.net
Cc: olivier.matz@6wind.com

See mailing list for v1 discussion, perhaps this solution is more
readable due to leaving sched struct in-line in the mbuf struct.
---
 lib/librte_mbuf/rte_mbuf.h | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index bc562dc8a..1b260efd5 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -2344,11 +2344,9 @@ rte_mbuf_sched_get(const struct rte_mbuf *m, uint32_t *queue_id,
 			uint8_t *traffic_class,
 			uint8_t *color)
 {
-	struct rte_mbuf_sched sched = m->hash.sched;
-
-	*queue_id = sched.queue_id;
-	*traffic_class = sched.traffic_class;
-	*color = sched.color;
+	*queue_id = m->hash.sched.queue_id;
+	*traffic_class = m->hash.sched.traffic_class;
+	*color = m->hash.sched.color;
 }
 
 /**
@@ -2395,11 +2393,9 @@ rte_mbuf_sched_set(struct rte_mbuf *m, uint32_t queue_id,
 			uint8_t traffic_class,
 			uint8_t color)
 {
-	m->hash.sched = (struct rte_mbuf_sched){
-				.queue_id = queue_id,
-				.traffic_class = traffic_class,
-				.color = color,
-			};
+	m->hash.sched.queue_id = queue_id;
+	m->hash.sched.traffic_class = traffic_class;
+	m->hash.sched.color = color;
 }
 
 #ifdef __cplusplus
-- 
2.17.1

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

* Re: [dpdk-dev] [PATCH v2] mbuf: fix compile by removing struct from function
  2019-01-10 18:06 ` [dpdk-dev] [PATCH v2] mbuf: fix compile by removing struct from function Harry van Haaren
@ 2019-01-10 18:40   ` Dumitrescu, Cristian
  2019-01-11  3:01     ` Gavin Hu (Arm Technology China)
  2019-01-11  8:44     ` Olivier Matz
  2019-01-11 11:32   ` [dpdk-dev] [PATCH v3] mbuf: fix compile by making sched struct visible Harry van Haaren
  1 sibling, 2 replies; 14+ messages in thread
From: Dumitrescu, Cristian @ 2019-01-10 18:40 UTC (permalink / raw)
  To: Van Haaren, Harry, dev; +Cc: Pattan, Reshma, thomas, olivier.matz



> -----Original Message-----
> From: Van Haaren, Harry
> Sent: Thursday, January 10, 2019 6:07 PM
> To: dev@dpdk.org
> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Pattan, Reshma
> <reshma.pattan@intel.com>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>; thomas@monjalon.net;
> olivier.matz@6wind.com
> Subject: [PATCH v2] mbuf: fix compile by removing struct from function
> 
> Although C compilation works with the struct rte_mbuf_sched
> declared inside the struct rte_mbuf namespace, C++ fails to
> compile.
> 
> This fix removes the temporary struct rte_mbuf_sched, instead
> reading from the mbuf directly for each struct member. As the
> struct is now not used directly, the C++ compiler doesn't need
> to know about the struct, resolving the issue.
> 
> Fixes: 5d3f72100904 ("mbuf: implement generic format for sched field")
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> 
> ---
> 
> Cc: reshma.pattan@intel.com
> Cc: cristian.dumitrescu@intel.com
> Cc: thomas@monjalon.net
> Cc: olivier.matz@6wind.com
> 
> See mailing list for v1 discussion, perhaps this solution is more
> readable due to leaving sched struct in-line in the mbuf struct.
> ---
>  lib/librte_mbuf/rte_mbuf.h | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index bc562dc8a..1b260efd5 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -2344,11 +2344,9 @@ rte_mbuf_sched_get(const struct rte_mbuf *m,
> uint32_t *queue_id,
>  			uint8_t *traffic_class,
>  			uint8_t *color)
>  {
> -	struct rte_mbuf_sched sched = m->hash.sched;
> -
> -	*queue_id = sched.queue_id;
> -	*traffic_class = sched.traffic_class;
> -	*color = sched.color;
> +	*queue_id = m->hash.sched.queue_id;
> +	*traffic_class = m->hash.sched.traffic_class;
> +	*color = m->hash.sched.color;
>  }
> 
>  /**
> @@ -2395,11 +2393,9 @@ rte_mbuf_sched_set(struct rte_mbuf *m,
> uint32_t queue_id,
>  			uint8_t traffic_class,
>  			uint8_t color)
>  {
> -	m->hash.sched = (struct rte_mbuf_sched){
> -				.queue_id = queue_id,
> -				.traffic_class = traffic_class,
> -				.color = color,
> -			};
> +	m->hash.sched.queue_id = queue_id;
> +	m->hash.sched.traffic_class = traffic_class;
> +	m->hash.sched.color = color;
>  }
> 
>  #ifdef __cplusplus
> --
> 2.17.1

NAK.

I am fine with V1, but against this V2 due to the reasons previously discussed and agreed by Olivier [1] regarding performance. We should not sacrifice performance for the sake of cosmetics criteria that can met some other way.

In order to meet readability requirements from Olivier, I suggest we go back to V1 and we explicitly mention the size of the mbuf->sched field inslide the mbuf as 8 bytes:

struct rte_mbuf {
	...
	struct rte_mbuf_sched sched; /**< Hierarchical scheduler: 8 bytes */
	...
}

Olivier, is this a good compromise?

Regards,
Cristian

[1] https://mails.dpdk.org/archives/dev/2018-December/121806.html

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

* Re: [dpdk-dev] [PATCH] mbuf: fix compile by making sched struct visible
  2019-01-10 16:50 [dpdk-dev] [PATCH] mbuf: fix compile by making sched struct visible Harry van Haaren
  2019-01-10 17:34 ` Thomas Monjalon
  2019-01-10 18:06 ` [dpdk-dev] [PATCH v2] mbuf: fix compile by removing struct from function Harry van Haaren
@ 2019-01-10 22:05 ` Stephen Hemminger
  2 siblings, 0 replies; 14+ messages in thread
From: Stephen Hemminger @ 2019-01-10 22:05 UTC (permalink / raw)
  To: Harry van Haaren; +Cc: dev, reshma.pattan, cristian.dumitrescu, thomas

On Thu, 10 Jan 2019 16:50:51 +0000
Harry van Haaren <harry.van.haaren@intel.com> wrote:

> Although C compilation works with the struct rte_mbuf_sched
> declared inside the struct rte_mbuf namespace, C++ fails to
> compile. This fix moves the rte_mbuf_sched struct up to the
> global namespace, instead of declaring it inside the struct
> mbuf namespace.
> 
> The struct rte_mbuf_sched is being used on the stack in
> rte_mbuf_sched_get() and as a cast in _set(). For this
> reason, it must be exposed as an available type.
> 
> Fixes: 5d3f72100904 ("mbuf: implement generic format for sched field")
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> 
I believe this was done so that the compiler doesn't generate
bad code.

If you reference the mbuf to get the fields then each operation becomes
a load shift and mask operation to get to the bitfield.  But if they
are local then this is all done on a single register value.

Check the generated code.

One solution would be to move this into a private header file
where C++ won't find it.

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

* Re: [dpdk-dev] [PATCH v2] mbuf: fix compile by removing struct from function
  2019-01-10 18:40   ` Dumitrescu, Cristian
@ 2019-01-11  3:01     ` Gavin Hu (Arm Technology China)
  2019-01-11  6:03       ` Stephen Hemminger
  2019-01-11  8:44     ` Olivier Matz
  1 sibling, 1 reply; 14+ messages in thread
From: Gavin Hu (Arm Technology China) @ 2019-01-11  3:01 UTC (permalink / raw)
  To: Dumitrescu, Cristian, Van Haaren, Harry, dev
  Cc: Pattan, Reshma, thomas, olivier.matz, Honnappa Nagarahalli



> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dumitrescu, Cristian
> Sent: Friday, January 11, 2019 2:40 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org
> Cc: Pattan, Reshma <reshma.pattan@intel.com>; thomas@monjalon.net;
> olivier.matz@6wind.com
> Subject: Re: [dpdk-dev] [PATCH v2] mbuf: fix compile by removing struct
> from function
>
>
>
> > -----Original Message-----
> > From: Van Haaren, Harry
> > Sent: Thursday, January 10, 2019 6:07 PM
> > To: dev@dpdk.org
> > Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Pattan, Reshma
> > <reshma.pattan@intel.com>; Dumitrescu, Cristian
> > <cristian.dumitrescu@intel.com>; thomas@monjalon.net;
> > olivier.matz@6wind.com
> > Subject: [PATCH v2] mbuf: fix compile by removing struct from function
> >
> > Although C compilation works with the struct rte_mbuf_sched
> > declared inside the struct rte_mbuf namespace, C++ fails to
> > compile.
> >
> > This fix removes the temporary struct rte_mbuf_sched, instead
> > reading from the mbuf directly for each struct member. As the
> > struct is now not used directly, the C++ compiler doesn't need
> > to know about the struct, resolving the issue.
> >
> > Fixes: 5d3f72100904 ("mbuf: implement generic format for sched field")
> >
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> >
> > ---
> >
> > Cc: reshma.pattan@intel.com
> > Cc: cristian.dumitrescu@intel.com
> > Cc: thomas@monjalon.net
> > Cc: olivier.matz@6wind.com
> >
> > See mailing list for v1 discussion, perhaps this solution is more
> > readable due to leaving sched struct in-line in the mbuf struct.
> > ---
> >  lib/librte_mbuf/rte_mbuf.h | 16 ++++++----------
> >  1 file changed, 6 insertions(+), 10 deletions(-)
> >
> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > index bc562dc8a..1b260efd5 100644
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > @@ -2344,11 +2344,9 @@ rte_mbuf_sched_get(const struct rte_mbuf
> *m,
> > uint32_t *queue_id,
> >  uint8_t *traffic_class,
> >  uint8_t *color)
> >  {
> > -struct rte_mbuf_sched sched = m->hash.sched;

Did C++ complain *sched? *sched is better with less stack mem footprint.
If the pointer works, the code looks better.

> > -
> > -*queue_id = sched.queue_id;
> > -*traffic_class = sched.traffic_class;
> > -*color = sched.color;
> > +*queue_id = m->hash.sched.queue_id;
> > +*traffic_class = m->hash.sched.traffic_class;
> > +*color = m->hash.sched.color;
> >  }
> >
> >  /**
> > @@ -2395,11 +2393,9 @@ rte_mbuf_sched_set(struct rte_mbuf *m,
> > uint32_t queue_id,
> >  uint8_t traffic_class,
> >  uint8_t color)
> >  {
> > -m->hash.sched = (struct rte_mbuf_sched){
> > -.queue_id = queue_id,
> > -.traffic_class = traffic_class,
> > -.color = color,
> > -};
> > +m->hash.sched.queue_id = queue_id;
> > +m->hash.sched.traffic_class = traffic_class;
> > +m->hash.sched.color = color;
> >  }
> >
> >  #ifdef __cplusplus
> > --
> > 2.17.1
>
> NAK.
>
> I am fine with V1, but against this V2 due to the reasons previously discussed
> and agreed by Olivier [1] regarding performance. We should not sacrifice
> performance for the sake of cosmetics criteria that can met some other way.
>
> In order to meet readability requirements from Olivier, I suggest we go back
> to V1 and we explicitly mention the size of the mbuf->sched field inslide the
> mbuf as 8 bytes:
>
> struct rte_mbuf {
> ...
> struct rte_mbuf_sched sched; /**< Hierarchical scheduler: 8 bytes
> */
> ...
> }
>
> Olivier, is this a good compromise?
>
> Regards,
> Cristian
>
> [1] https://mails.dpdk.org/archives/dev/2018-December/121806.html

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

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

* Re: [dpdk-dev] [PATCH v2] mbuf: fix compile by removing struct from function
  2019-01-11  3:01     ` Gavin Hu (Arm Technology China)
@ 2019-01-11  6:03       ` Stephen Hemminger
  0 siblings, 0 replies; 14+ messages in thread
From: Stephen Hemminger @ 2019-01-11  6:03 UTC (permalink / raw)
  To: Gavin Hu (Arm Technology China)
  Cc: Dumitrescu, Cristian, Van Haaren, Harry, dev, Pattan, Reshma,
	thomas, olivier.matz, Honnappa Nagarahalli

On Fri, 11 Jan 2019 03:01:43 +0000
"Gavin Hu (Arm Technology China)" <Gavin.Hu@arm.com> wrote:

> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Dumitrescu, Cristian
> > Sent: Friday, January 11, 2019 2:40 AM
> > To: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org
> > Cc: Pattan, Reshma <reshma.pattan@intel.com>; thomas@monjalon.net;
> > olivier.matz@6wind.com
> > Subject: Re: [dpdk-dev] [PATCH v2] mbuf: fix compile by removing struct
> > from function
> >
> >
> >  
> > > -----Original Message-----
> > > From: Van Haaren, Harry
> > > Sent: Thursday, January 10, 2019 6:07 PM
> > > To: dev@dpdk.org
> > > Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Pattan, Reshma
> > > <reshma.pattan@intel.com>; Dumitrescu, Cristian
> > > <cristian.dumitrescu@intel.com>; thomas@monjalon.net;
> > > olivier.matz@6wind.com
> > > Subject: [PATCH v2] mbuf: fix compile by removing struct from function
> > >
> > > Although C compilation works with the struct rte_mbuf_sched
> > > declared inside the struct rte_mbuf namespace, C++ fails to
> > > compile.
> > >
> > > This fix removes the temporary struct rte_mbuf_sched, instead
> > > reading from the mbuf directly for each struct member. As the
> > > struct is now not used directly, the C++ compiler doesn't need
> > > to know about the struct, resolving the issue.
> > >
> > > Fixes: 5d3f72100904 ("mbuf: implement generic format for sched field")
> > >
> > > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> > >
> > > ---
> > >
> > > Cc: reshma.pattan@intel.com
> > > Cc: cristian.dumitrescu@intel.com
> > > Cc: thomas@monjalon.net
> > > Cc: olivier.matz@6wind.com
> > >
> > > See mailing list for v1 discussion, perhaps this solution is more
> > > readable due to leaving sched struct in-line in the mbuf struct.
> > > ---
> > >  lib/librte_mbuf/rte_mbuf.h | 16 ++++++----------
> > >  1 file changed, 6 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > > index bc562dc8a..1b260efd5 100644
> > > --- a/lib/librte_mbuf/rte_mbuf.h
> > > +++ b/lib/librte_mbuf/rte_mbuf.h
> > > @@ -2344,11 +2344,9 @@ rte_mbuf_sched_get(const struct rte_mbuf  
> > *m,  
> > > uint32_t *queue_id,
> > >  uint8_t *traffic_class,
> > >  uint8_t *color)
> > >  {
> > > -struct rte_mbuf_sched sched = m->hash.sched;  
> 
> Did C++ complain *sched? *sched is better with less stack mem footprint.
> If the pointer works, the code looks better.

Using *sched will cause compiler to generate multiple references which
is the real performance issue here.

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

* Re: [dpdk-dev] [PATCH v2] mbuf: fix compile by removing struct from function
  2019-01-10 18:40   ` Dumitrescu, Cristian
  2019-01-11  3:01     ` Gavin Hu (Arm Technology China)
@ 2019-01-11  8:44     ` Olivier Matz
  2019-01-11 11:20       ` Van Haaren, Harry
  1 sibling, 1 reply; 14+ messages in thread
From: Olivier Matz @ 2019-01-11  8:44 UTC (permalink / raw)
  To: Dumitrescu, Cristian; +Cc: Van Haaren, Harry, dev, Pattan, Reshma, thomas

Hi,

On Thu, Jan 10, 2019 at 06:40:06PM +0000, Dumitrescu, Cristian wrote:
> 
> 
> > -----Original Message-----
> > From: Van Haaren, Harry
> > Sent: Thursday, January 10, 2019 6:07 PM
> > To: dev@dpdk.org
> > Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Pattan, Reshma
> > <reshma.pattan@intel.com>; Dumitrescu, Cristian
> > <cristian.dumitrescu@intel.com>; thomas@monjalon.net;
> > olivier.matz@6wind.com
> > Subject: [PATCH v2] mbuf: fix compile by removing struct from function
> > 
> > Although C compilation works with the struct rte_mbuf_sched
> > declared inside the struct rte_mbuf namespace, C++ fails to
> > compile.
> > 
> > This fix removes the temporary struct rte_mbuf_sched, instead
> > reading from the mbuf directly for each struct member. As the
> > struct is now not used directly, the C++ compiler doesn't need
> > to know about the struct, resolving the issue.
> > 
> > Fixes: 5d3f72100904 ("mbuf: implement generic format for sched field")
> > 
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> > 
> > ---
> > 
> > Cc: reshma.pattan@intel.com
> > Cc: cristian.dumitrescu@intel.com
> > Cc: thomas@monjalon.net
> > Cc: olivier.matz@6wind.com
> > 
> > See mailing list for v1 discussion, perhaps this solution is more
> > readable due to leaving sched struct in-line in the mbuf struct.
> > ---
> >  lib/librte_mbuf/rte_mbuf.h | 16 ++++++----------
> >  1 file changed, 6 insertions(+), 10 deletions(-)
> > 
> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > index bc562dc8a..1b260efd5 100644
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > @@ -2344,11 +2344,9 @@ rte_mbuf_sched_get(const struct rte_mbuf *m,
> > uint32_t *queue_id,
> >  			uint8_t *traffic_class,
> >  			uint8_t *color)
> >  {
> > -	struct rte_mbuf_sched sched = m->hash.sched;
> > -
> > -	*queue_id = sched.queue_id;
> > -	*traffic_class = sched.traffic_class;
> > -	*color = sched.color;
> > +	*queue_id = m->hash.sched.queue_id;
> > +	*traffic_class = m->hash.sched.traffic_class;
> > +	*color = m->hash.sched.color;
> >  }
> > 
> >  /**
> > @@ -2395,11 +2393,9 @@ rte_mbuf_sched_set(struct rte_mbuf *m,
> > uint32_t queue_id,
> >  			uint8_t traffic_class,
> >  			uint8_t color)
> >  {
> > -	m->hash.sched = (struct rte_mbuf_sched){
> > -				.queue_id = queue_id,
> > -				.traffic_class = traffic_class,
> > -				.color = color,
> > -			};
> > +	m->hash.sched.queue_id = queue_id;
> > +	m->hash.sched.traffic_class = traffic_class;
> > +	m->hash.sched.color = color;
> >  }
> > 
> >  #ifdef __cplusplus
> > --
> > 2.17.1
> 
> NAK.
> 
> I am fine with V1, but against this V2 due to the reasons previously discussed and agreed by Olivier [1] regarding performance. We should not sacrifice performance for the sake of cosmetics criteria that can met some other way.
> 
> In order to meet readability requirements from Olivier, I suggest we go back to V1 and we explicitly mention the size of the mbuf->sched field inslide the mbuf as 8 bytes:
> 
> struct rte_mbuf {
> 	...
> 	struct rte_mbuf_sched sched; /**< Hierarchical scheduler: 8 bytes */
> 	...
> }
> 
> Olivier, is this a good compromise?

Looks good to me, yes.


Thanks

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

* Re: [dpdk-dev] [PATCH v2] mbuf: fix compile by removing struct from function
  2019-01-11  8:44     ` Olivier Matz
@ 2019-01-11 11:20       ` Van Haaren, Harry
  0 siblings, 0 replies; 14+ messages in thread
From: Van Haaren, Harry @ 2019-01-11 11:20 UTC (permalink / raw)
  To: Olivier Matz, Dumitrescu, Cristian, Stephen Hemminger
  Cc: dev, Pattan, Reshma, thomas

Converging discussion, +Stephen Hemminger wrote:

> I believe this was done so that the compiler doesn't generate
> bad code.
>
> If you reference the mbuf to get the fields then each operation becomes
> a load shift and mask operation to get to the bitfield.  But if they
> are local then this is all done on a single register value.
>
> Check the generated code.

I did :) I see no difference in generated assembly here, system has
GCC 7.3.0 with Meson release build and -g for debug symbols.


Anyway, as per below the consensus seems to be to just lift the struct
as per V1 with a note that the sched field is 8 bytes. I'll send v3 with
the comment updated so we can close this ASAP.



> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Friday, January 11, 2019 8:45 AM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org; Pattan,
> Reshma <reshma.pattan@intel.com>; thomas@monjalon.net
> Subject: Re: [PATCH v2] mbuf: fix compile by removing struct from function

<snip>

> > >  /**
> > > @@ -2395,11 +2393,9 @@ rte_mbuf_sched_set(struct rte_mbuf *m,
> > > uint32_t queue_id,
> > >  			uint8_t traffic_class,
> > >  			uint8_t color)
> > >  {
> > > -	m->hash.sched = (struct rte_mbuf_sched){
> > > -				.queue_id = queue_id,
> > > -				.traffic_class = traffic_class,
> > > -				.color = color,
> > > -			};
> > > +	m->hash.sched.queue_id = queue_id;
> > > +	m->hash.sched.traffic_class = traffic_class;
> > > +	m->hash.sched.color = color;
> > >  }
> > >
> > >  #ifdef __cplusplus
> > > --
> > > 2.17.1
> >
> > NAK.
> >
> > I am fine with V1, but against this V2 due to the reasons previously
> discussed and agreed by Olivier [1] regarding performance. We should not
> sacrifice performance for the sake of cosmetics criteria that can met some
> other way.
> >
> > In order to meet readability requirements from Olivier, I suggest we go
> back to V1 and we explicitly mention the size of the mbuf->sched field
> inslide the mbuf as 8 bytes:
> >
> > struct rte_mbuf {
> > 	...
> > 	struct rte_mbuf_sched sched; /**< Hierarchical scheduler: 8 bytes */
> > 	...
> > }
> >
> > Olivier, is this a good compromise?
> 
> Looks good to me, yes.
> 
> 
> Thanks

See above, I'll send v3 with updated comment.

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

* [dpdk-dev] [PATCH v3] mbuf: fix compile by making sched struct visible
  2019-01-10 18:06 ` [dpdk-dev] [PATCH v2] mbuf: fix compile by removing struct from function Harry van Haaren
  2019-01-10 18:40   ` Dumitrescu, Cristian
@ 2019-01-11 11:32   ` Harry van Haaren
  2019-01-11 14:33     ` Dumitrescu, Cristian
  1 sibling, 1 reply; 14+ messages in thread
From: Harry van Haaren @ 2019-01-11 11:32 UTC (permalink / raw)
  To: dev
  Cc: Harry van Haaren, reshma.pattan, cristian.dumitrescu,
	olivier.matz, thomas, stephen, gavin.hu, honnappa.nagarahalli

Although C compilation works with the struct rte_mbuf_sched
declared inside the struct rte_mbuf namespace, C++ fails to
compile. This fix moves the rte_mbuf_sched struct up to the
global namespace, instead of declaring it inside the struct
mbuf namespace.

The struct rte_mbuf_sched is being used on the stack in
rte_mbuf_sched_get() and as a cast in _set(). For this
reason, it must be exposed as an available type.

Fixes: 5d3f72100904 ("mbuf: implement generic format for sched field")

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

v3:
 - Update comment in mbuf to state size of struct sched (Crisitian)

v2:
 - Different solution, not applicable, v3 based on v1 (ML discussion)

Cc: reshma.pattan@intel.com
Cc: cristian.dumitrescu@intel.com
Cc: olivier.matz@6wind.com
Cc: thomas@monjalon.net
Cc: stephen@networkplumber.org
Cc: gavin.hu@arm.com
Cc: honnappa.nagarahalli@arm.com
---
 lib/librte_mbuf/rte_mbuf.h | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index bc562dc8a..d53b68253 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -468,6 +468,17 @@ __extension__
 typedef uint64_t MARKER64[0]; /**< marker that allows us to overwrite 8 bytes
                                * with a single assignment */
 
+struct rte_mbuf_sched {
+	uint32_t queue_id;   /**< Queue ID. */
+	uint8_t traffic_class;
+	/**< Traffic class ID. Traffic class 0
+	 * is the highest priority traffic class.
+	 */
+	uint8_t color;
+	/**< Color. @see enum rte_color.*/
+	uint16_t reserved;   /**< Reserved. */
+}; /**< Hierarchical scheduler */
+
 /**
  * The generic rte_mbuf, containing a packet mbuf.
  */
@@ -574,16 +585,8 @@ struct rte_mbuf {
 				 * on PKT_RX_FDIR_* flag in ol_flags.
 				 */
 			} fdir;	/**< Filter identifier if FDIR enabled */
-			struct rte_mbuf_sched {
-				uint32_t queue_id;   /**< Queue ID. */
-				uint8_t traffic_class;
-				/**< Traffic class ID. Traffic class 0
-				 * is the highest priority traffic class.
-				 */
-				uint8_t color;
-				/**< Color. @see enum rte_color.*/
-				uint16_t reserved;   /**< Reserved. */
-			} sched; /**< Hierarchical scheduler */
+			struct rte_mbuf_sched sched;
+			/**< Hierarchical scheduler : 8 bytes */
 			struct {
 				uint32_t reserved1;
 				uint16_t reserved2;
-- 
2.17.1

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

* Re: [dpdk-dev] [PATCH v3] mbuf: fix compile by making sched struct visible
  2019-01-11 11:32   ` [dpdk-dev] [PATCH v3] mbuf: fix compile by making sched struct visible Harry van Haaren
@ 2019-01-11 14:33     ` Dumitrescu, Cristian
  2019-01-14 14:58       ` Olivier Matz
  0 siblings, 1 reply; 14+ messages in thread
From: Dumitrescu, Cristian @ 2019-01-11 14:33 UTC (permalink / raw)
  To: Van Haaren, Harry, dev
  Cc: Pattan, Reshma, olivier.matz, thomas, stephen, gavin.hu,
	honnappa.nagarahalli



> -----Original Message-----
> From: Van Haaren, Harry
> Sent: Friday, January 11, 2019 11:33 AM
> To: dev@dpdk.org
> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Pattan, Reshma
> <reshma.pattan@intel.com>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>; olivier.matz@6wind.com;
> thomas@monjalon.net; stephen@networkplumber.org;
> gavin.hu@arm.com; honnappa.nagarahalli@arm.com
> Subject: [PATCH v3] mbuf: fix compile by making sched struct visible
> 
> Although C compilation works with the struct rte_mbuf_sched
> declared inside the struct rte_mbuf namespace, C++ fails to
> compile. This fix moves the rte_mbuf_sched struct up to the
> global namespace, instead of declaring it inside the struct
> mbuf namespace.
> 
> The struct rte_mbuf_sched is being used on the stack in
> rte_mbuf_sched_get() and as a cast in _set(). For this
> reason, it must be exposed as an available type.
> 
> Fixes: 5d3f72100904 ("mbuf: implement generic format for sched field")
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> 
> ---
> 
> v3:
>  - Update comment in mbuf to state size of struct sched (Crisitian)
> 
> v2:
>  - Different solution, not applicable, v3 based on v1 (ML discussion)
> 
> Cc: reshma.pattan@intel.com
> Cc: cristian.dumitrescu@intel.com
> Cc: olivier.matz@6wind.com
> Cc: thomas@monjalon.net
> Cc: stephen@networkplumber.org
> Cc: gavin.hu@arm.com
> Cc: honnappa.nagarahalli@arm.com
> ---
>  lib/librte_mbuf/rte_mbuf.h | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index bc562dc8a..d53b68253 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -468,6 +468,17 @@ __extension__
>  typedef uint64_t MARKER64[0]; /**< marker that allows us to overwrite 8
> bytes
>                                 * with a single assignment */
> 
> +struct rte_mbuf_sched {
> +	uint32_t queue_id;   /**< Queue ID. */
> +	uint8_t traffic_class;
> +	/**< Traffic class ID. Traffic class 0
> +	 * is the highest priority traffic class.
> +	 */
> +	uint8_t color;
> +	/**< Color. @see enum rte_color.*/
> +	uint16_t reserved;   /**< Reserved. */
> +}; /**< Hierarchical scheduler */
> +
>  /**
>   * The generic rte_mbuf, containing a packet mbuf.
>   */
> @@ -574,16 +585,8 @@ struct rte_mbuf {
>  				 * on PKT_RX_FDIR_* flag in ol_flags.
>  				 */
>  			} fdir;	/**< Filter identifier if FDIR enabled */
> -			struct rte_mbuf_sched {
> -				uint32_t queue_id;   /**< Queue ID. */
> -				uint8_t traffic_class;
> -				/**< Traffic class ID. Traffic class 0
> -				 * is the highest priority traffic class.
> -				 */
> -				uint8_t color;
> -				/**< Color. @see enum rte_color.*/
> -				uint16_t reserved;   /**< Reserved. */
> -			} sched; /**< Hierarchical scheduler */
> +			struct rte_mbuf_sched sched;
> +			/**< Hierarchical scheduler : 8 bytes */
>  			struct {
>  				uint32_t reserved1;
>  				uint16_t reserved2;
> --
> 2.17.1

Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

Of course, final say belongs to Olivier, so Olivier please let un know your vote.

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

* Re: [dpdk-dev] [PATCH v3] mbuf: fix compile by making sched struct visible
  2019-01-11 14:33     ` Dumitrescu, Cristian
@ 2019-01-14 14:58       ` Olivier Matz
  2019-01-14 15:28         ` Thomas Monjalon
  0 siblings, 1 reply; 14+ messages in thread
From: Olivier Matz @ 2019-01-14 14:58 UTC (permalink / raw)
  To: Dumitrescu, Cristian
  Cc: Van Haaren, Harry, dev, Pattan, Reshma, thomas, stephen,
	gavin.hu, honnappa.nagarahalli

On Fri, Jan 11, 2019 at 02:33:16PM +0000, Dumitrescu, Cristian wrote:
> 
> 
> > -----Original Message-----
> > From: Van Haaren, Harry
> > Sent: Friday, January 11, 2019 11:33 AM
> > To: dev@dpdk.org
> > Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Pattan, Reshma
> > <reshma.pattan@intel.com>; Dumitrescu, Cristian
> > <cristian.dumitrescu@intel.com>; olivier.matz@6wind.com;
> > thomas@monjalon.net; stephen@networkplumber.org;
> > gavin.hu@arm.com; honnappa.nagarahalli@arm.com
> > Subject: [PATCH v3] mbuf: fix compile by making sched struct visible
> > 
> > Although C compilation works with the struct rte_mbuf_sched
> > declared inside the struct rte_mbuf namespace, C++ fails to
> > compile. This fix moves the rte_mbuf_sched struct up to the
> > global namespace, instead of declaring it inside the struct
> > mbuf namespace.
> > 
> > The struct rte_mbuf_sched is being used on the stack in
> > rte_mbuf_sched_get() and as a cast in _set(). For this
> > reason, it must be exposed as an available type.
> > 
> > Fixes: 5d3f72100904 ("mbuf: implement generic format for sched field")
> > 
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> > 
> > ---
> > 
> > v3:
> >  - Update comment in mbuf to state size of struct sched (Crisitian)
> > 
> > v2:
> >  - Different solution, not applicable, v3 based on v1 (ML discussion)
> > 
> > Cc: reshma.pattan@intel.com
> > Cc: cristian.dumitrescu@intel.com
> > Cc: olivier.matz@6wind.com
> > Cc: thomas@monjalon.net
> > Cc: stephen@networkplumber.org
> > Cc: gavin.hu@arm.com
> > Cc: honnappa.nagarahalli@arm.com
> > ---
> >  lib/librte_mbuf/rte_mbuf.h | 23 +++++++++++++----------
> >  1 file changed, 13 insertions(+), 10 deletions(-)
> > 
> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > index bc562dc8a..d53b68253 100644
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > @@ -468,6 +468,17 @@ __extension__
> >  typedef uint64_t MARKER64[0]; /**< marker that allows us to overwrite 8
> > bytes
> >                                 * with a single assignment */
> > 
> > +struct rte_mbuf_sched {
> > +	uint32_t queue_id;   /**< Queue ID. */
> > +	uint8_t traffic_class;
> > +	/**< Traffic class ID. Traffic class 0
> > +	 * is the highest priority traffic class.
> > +	 */
> > +	uint8_t color;
> > +	/**< Color. @see enum rte_color.*/
> > +	uint16_t reserved;   /**< Reserved. */
> > +}; /**< Hierarchical scheduler */
> > +
> >  /**
> >   * The generic rte_mbuf, containing a packet mbuf.
> >   */
> > @@ -574,16 +585,8 @@ struct rte_mbuf {
> >  				 * on PKT_RX_FDIR_* flag in ol_flags.
> >  				 */
> >  			} fdir;	/**< Filter identifier if FDIR enabled */
> > -			struct rte_mbuf_sched {
> > -				uint32_t queue_id;   /**< Queue ID. */
> > -				uint8_t traffic_class;
> > -				/**< Traffic class ID. Traffic class 0
> > -				 * is the highest priority traffic class.
> > -				 */
> > -				uint8_t color;
> > -				/**< Color. @see enum rte_color.*/
> > -				uint16_t reserved;   /**< Reserved. */
> > -			} sched; /**< Hierarchical scheduler */
> > +			struct rte_mbuf_sched sched;
> > +			/**< Hierarchical scheduler : 8 bytes */
> >  			struct {
> >  				uint32_t reserved1;
> >  				uint16_t reserved2;
> > --
> > 2.17.1
> 
> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> 
> Of course, final say belongs to Olivier, so Olivier please let un know your vote.

Acked-by: Olivier Matz <olivier.matz@6wind.com>

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

* Re: [dpdk-dev] [PATCH v3] mbuf: fix compile by making sched struct visible
  2019-01-14 14:58       ` Olivier Matz
@ 2019-01-14 15:28         ` Thomas Monjalon
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Monjalon @ 2019-01-14 15:28 UTC (permalink / raw)
  To: Van Haaren, Harry
  Cc: dev, Olivier Matz, Dumitrescu, Cristian, Pattan, Reshma, stephen,
	gavin.hu, honnappa.nagarahalli

14/01/2019 15:58, Olivier Matz:
> On Fri, Jan 11, 2019 at 02:33:16PM +0000, Dumitrescu, Cristian wrote:
> > From: Van Haaren, Harry
> > > 
> > > Although C compilation works with the struct rte_mbuf_sched
> > > declared inside the struct rte_mbuf namespace, C++ fails to
> > > compile. This fix moves the rte_mbuf_sched struct up to the
> > > global namespace, instead of declaring it inside the struct
> > > mbuf namespace.
> > > 
> > > The struct rte_mbuf_sched is being used on the stack in
> > > rte_mbuf_sched_get() and as a cast in _set(). For this
> > > reason, it must be exposed as an available type.
> > > 
> > > Fixes: 5d3f72100904 ("mbuf: implement generic format for sched field")
> > > 
> > > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> > > 
> > > ---
> > > 
> > > v3:
> > >  - Update comment in mbuf to state size of struct sched (Crisitian)
> > > 
> > > v2:
> > >  - Different solution, not applicable, v3 based on v1 (ML discussion)
> > 
> > Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > 
> > Of course, final say belongs to Olivier, so Olivier please let un know your vote.
> 
> Acked-by: Olivier Matz <olivier.matz@6wind.com>

Applied, thanks

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

end of thread, other threads:[~2019-01-14 15:28 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-10 16:50 [dpdk-dev] [PATCH] mbuf: fix compile by making sched struct visible Harry van Haaren
2019-01-10 17:34 ` Thomas Monjalon
2019-01-10 17:57   ` Van Haaren, Harry
2019-01-10 18:06 ` [dpdk-dev] [PATCH v2] mbuf: fix compile by removing struct from function Harry van Haaren
2019-01-10 18:40   ` Dumitrescu, Cristian
2019-01-11  3:01     ` Gavin Hu (Arm Technology China)
2019-01-11  6:03       ` Stephen Hemminger
2019-01-11  8:44     ` Olivier Matz
2019-01-11 11:20       ` Van Haaren, Harry
2019-01-11 11:32   ` [dpdk-dev] [PATCH v3] mbuf: fix compile by making sched struct visible Harry van Haaren
2019-01-11 14:33     ` Dumitrescu, Cristian
2019-01-14 14:58       ` Olivier Matz
2019-01-14 15:28         ` Thomas Monjalon
2019-01-10 22:05 ` [dpdk-dev] [PATCH] " Stephen Hemminger

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