[dpdk-dev,2/6] rte_sched: expand scheduler hierarchy for more VLAN's

Message ID 1431364071-27298-3-git-send-email-stephen@networkplumber.org (mailing list archive)
State Superseded, archived
Headers

Commit Message

Stephen Hemminger May 11, 2015, 5:07 p.m. UTC
  From: Stephen Hemminger <shemming@brocade.com>

The QoS subport is limited to 8 bits in original code.
But customers demanded ability to support full number of VLAN's (4096)
therefore use the full part of the tag field of mbuf.

Resize the pipe as well to allow for more pipes in future and
avoid expensive bitfield access.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_mbuf/rte_mbuf.h   |  5 ++++-
 lib/librte_sched/rte_sched.h | 38 ++++++++++++++++++++++++--------------
 2 files changed, 28 insertions(+), 15 deletions(-)
  

Comments

Neil Horman May 11, 2015, 5:20 p.m. UTC | #1
On Mon, May 11, 2015 at 10:07:47AM -0700, Stephen Hemminger wrote:
> From: Stephen Hemminger <shemming@brocade.com>
> 
> The QoS subport is limited to 8 bits in original code.
> But customers demanded ability to support full number of VLAN's (4096)
> therefore use the full part of the tag field of mbuf.
> 
> Resize the pipe as well to allow for more pipes in future and
> avoid expensive bitfield access.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/librte_mbuf/rte_mbuf.h   |  5 ++++-
>  lib/librte_sched/rte_sched.h | 38 ++++++++++++++++++++++++--------------
>  2 files changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index ab6de67..cc0658d 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -295,7 +295,10 @@ struct rte_mbuf {
>  			/**< First 4 flexible bytes or FD ID, dependent on
>  			     PKT_RX_FDIR_* flag in ol_flags. */
>  		} fdir;           /**< Filter identifier if FDIR enabled */
> -		uint32_t sched;   /**< Hierarchical scheduler */
> +		struct {
> +			uint32_t lo;
> +			uint32_t hi;
> +		} sched;          /**< Hierarchical scheduler */
>  		uint32_t usr;	  /**< User defined tags. See rte_distributor_process() */
>  	} hash;                   /**< hash information */
>  
> diff --git a/lib/librte_sched/rte_sched.h b/lib/librte_sched/rte_sched.h
> index e6bba22..bf5ef8d 100644
> --- a/lib/librte_sched/rte_sched.h
> +++ b/lib/librte_sched/rte_sched.h
> @@ -195,16 +195,20 @@ struct rte_sched_port_params {
>  #endif
>  };
>  
> -/** Path through the scheduler hierarchy used by the scheduler enqueue operation to
> -identify the destination queue for the current packet. Stored in the field hash.sched
> -of struct rte_mbuf of each packet, typically written by the classification stage and read by
> -scheduler enqueue.*/
> +/*
> + * Path through the scheduler hierarchy used by the scheduler enqueue
> + * operation to identify the destination queue for the current
> + * packet. Stored in the field pkt.hash.sched of struct rte_mbuf of
> + * each packet, typically written by the classification stage and read
> + * by scheduler enqueue.
> + */
>  struct rte_sched_port_hierarchy {
> -	uint32_t queue:2;                /**< Queue ID (0 .. 3) */
> -	uint32_t traffic_class:2;        /**< Traffic class ID (0 .. 3)*/
> -	uint32_t pipe:20;                /**< Pipe ID */
> -	uint32_t subport:6;              /**< Subport ID */
> -	uint32_t color:2;                /**< Color */
> +	uint16_t queue:2;		 /**< Queue ID (0 .. 3) */
> +	uint16_t traffic_class:2;	 /**< Traffic class ID (0 .. 3)*/
> +	uint16_t color:2;		 /**< Color */
> +	uint16_t unused:10;
> +	uint16_t subport;		 /**< Subport ID */
> +	uint32_t pipe;			 /**< Pipe ID */
>  };
Have you run this through the ABI checker?  Seems like this would alter lots of
pointer offsets.
Neil

>  
>  /*
> @@ -350,12 +354,15 @@ rte_sched_queue_read_stats(struct rte_sched_port *port,
>   */
>  static inline void
>  rte_sched_port_pkt_write(struct rte_mbuf *pkt,
> -	uint32_t subport, uint32_t pipe, uint32_t traffic_class, uint32_t queue, enum rte_meter_color color)
> +			 uint32_t subport, uint32_t pipe,
> +			 uint32_t traffic_class,
> +			 uint32_t queue, enum rte_meter_color color)
>  {
> -	struct rte_sched_port_hierarchy *sched = (struct rte_sched_port_hierarchy *) &pkt->hash.sched;
> +	struct rte_sched_port_hierarchy *sched
> +		= (struct rte_sched_port_hierarchy *) &pkt->hash.sched;
>  
> -	sched->color = (uint32_t) color;
>  	sched->subport = subport;
> +	sched->color = (uint32_t) color;
>  	sched->pipe = pipe;
>  	sched->traffic_class = traffic_class;
>  	sched->queue = queue;
> @@ -379,9 +386,12 @@ rte_sched_port_pkt_write(struct rte_mbuf *pkt,
>   *
>   */
>  static inline void
> -rte_sched_port_pkt_read_tree_path(struct rte_mbuf *pkt, uint32_t *subport, uint32_t *pipe, uint32_t *traffic_class, uint32_t *queue)
> +rte_sched_port_pkt_read_tree_path(struct rte_mbuf *pkt, uint32_t *subport,
> +				  uint32_t *pipe, uint32_t *traffic_class,
> +				  uint32_t *queue)
>  {
> -	struct rte_sched_port_hierarchy *sched = (struct rte_sched_port_hierarchy *) &pkt->hash.sched;
> +	struct rte_sched_port_hierarchy *sched
> +		= (struct rte_sched_port_hierarchy *) &pkt->hash.sched;
>  
>  	*subport = sched->subport;
>  	*pipe = sched->pipe;
> -- 
> 2.1.4
> 
>
  
Stephen Hemminger May 11, 2015, 5:32 p.m. UTC | #2
On Mon, 11 May 2015 17:20:07 +0000
Neil Horman <nhorman@tuxdriver.com> wrote:

> Have you run this through the ABI checker?  Seems like this would alter lots of
> pointer offsets.
> Neil

No, I have not run it through ABI checker.
It would change the ABI for applications using qos_sched but will not
change layout of mbuf.

But my assumption was that as part of release process the ABI version
would change rather than doing for each patch that gets merged.
  
Neil Horman May 11, 2015, 5:43 p.m. UTC | #3
On Mon, May 11, 2015 at 10:32:59AM -0700, Stephen Hemminger wrote:
> On Mon, 11 May 2015 17:20:07 +0000
> Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> > Have you run this through the ABI checker?  Seems like this would alter lots of
> > pointer offsets.
> > Neil
> 
> No, I have not run it through ABI checker.
> It would change the ABI for applications using qos_sched but will not
> change layout of mbuf.
> 
> But my assumption was that as part of release process the ABI version
> would change rather than doing for each patch that gets merged.
> 

You're correct that the ABI version can change, but the process is to make an
update to doc/guides/rel_notes/abi.rst documenting the proposed changed, wait
for that to be published in an official release, then make the change for the
following release.  That way downstream adopters have some lead time to prep for
upstream changes.

Neil
  

Patch

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index ab6de67..cc0658d 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -295,7 +295,10 @@  struct rte_mbuf {
 			/**< First 4 flexible bytes or FD ID, dependent on
 			     PKT_RX_FDIR_* flag in ol_flags. */
 		} fdir;           /**< Filter identifier if FDIR enabled */
-		uint32_t sched;   /**< Hierarchical scheduler */
+		struct {
+			uint32_t lo;
+			uint32_t hi;
+		} sched;          /**< Hierarchical scheduler */
 		uint32_t usr;	  /**< User defined tags. See rte_distributor_process() */
 	} hash;                   /**< hash information */
 
diff --git a/lib/librte_sched/rte_sched.h b/lib/librte_sched/rte_sched.h
index e6bba22..bf5ef8d 100644
--- a/lib/librte_sched/rte_sched.h
+++ b/lib/librte_sched/rte_sched.h
@@ -195,16 +195,20 @@  struct rte_sched_port_params {
 #endif
 };
 
-/** Path through the scheduler hierarchy used by the scheduler enqueue operation to
-identify the destination queue for the current packet. Stored in the field hash.sched
-of struct rte_mbuf of each packet, typically written by the classification stage and read by
-scheduler enqueue.*/
+/*
+ * Path through the scheduler hierarchy used by the scheduler enqueue
+ * operation to identify the destination queue for the current
+ * packet. Stored in the field pkt.hash.sched of struct rte_mbuf of
+ * each packet, typically written by the classification stage and read
+ * by scheduler enqueue.
+ */
 struct rte_sched_port_hierarchy {
-	uint32_t queue:2;                /**< Queue ID (0 .. 3) */
-	uint32_t traffic_class:2;        /**< Traffic class ID (0 .. 3)*/
-	uint32_t pipe:20;                /**< Pipe ID */
-	uint32_t subport:6;              /**< Subport ID */
-	uint32_t color:2;                /**< Color */
+	uint16_t queue:2;		 /**< Queue ID (0 .. 3) */
+	uint16_t traffic_class:2;	 /**< Traffic class ID (0 .. 3)*/
+	uint16_t color:2;		 /**< Color */
+	uint16_t unused:10;
+	uint16_t subport;		 /**< Subport ID */
+	uint32_t pipe;			 /**< Pipe ID */
 };
 
 /*
@@ -350,12 +354,15 @@  rte_sched_queue_read_stats(struct rte_sched_port *port,
  */
 static inline void
 rte_sched_port_pkt_write(struct rte_mbuf *pkt,
-	uint32_t subport, uint32_t pipe, uint32_t traffic_class, uint32_t queue, enum rte_meter_color color)
+			 uint32_t subport, uint32_t pipe,
+			 uint32_t traffic_class,
+			 uint32_t queue, enum rte_meter_color color)
 {
-	struct rte_sched_port_hierarchy *sched = (struct rte_sched_port_hierarchy *) &pkt->hash.sched;
+	struct rte_sched_port_hierarchy *sched
+		= (struct rte_sched_port_hierarchy *) &pkt->hash.sched;
 
-	sched->color = (uint32_t) color;
 	sched->subport = subport;
+	sched->color = (uint32_t) color;
 	sched->pipe = pipe;
 	sched->traffic_class = traffic_class;
 	sched->queue = queue;
@@ -379,9 +386,12 @@  rte_sched_port_pkt_write(struct rte_mbuf *pkt,
  *
  */
 static inline void
-rte_sched_port_pkt_read_tree_path(struct rte_mbuf *pkt, uint32_t *subport, uint32_t *pipe, uint32_t *traffic_class, uint32_t *queue)
+rte_sched_port_pkt_read_tree_path(struct rte_mbuf *pkt, uint32_t *subport,
+				  uint32_t *pipe, uint32_t *traffic_class,
+				  uint32_t *queue)
 {
-	struct rte_sched_port_hierarchy *sched = (struct rte_sched_port_hierarchy *) &pkt->hash.sched;
+	struct rte_sched_port_hierarchy *sched
+		= (struct rte_sched_port_hierarchy *) &pkt->hash.sched;
 
 	*subport = sched->subport;
 	*pipe = sched->pipe;