[v3] mbuf: fix compile by making sched struct visible

Message ID 20190111113239.61275-1-harry.van.haaren@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v3] mbuf: fix compile by making sched struct visible |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing fail Performance Testing issues
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

Van Haaren, Harry Jan. 11, 2019, 11:32 a.m. UTC
  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(-)
  

Comments

Cristian Dumitrescu Jan. 11, 2019, 2:33 p.m. UTC | #1
> -----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.
  
Olivier Matz Jan. 14, 2019, 2:58 p.m. UTC | #2
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>
  
Thomas Monjalon Jan. 14, 2019, 3:28 p.m. UTC | #3
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
  

Patch

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;