[dpdk-dev] [PATCH 2/7] rte_sched: use reserved field to allow more VLAN's

[Ananyev, Konstantin]

Hi Stephen,

> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Stephen Hemminger
> Sent: Sunday, February 01, 2015 10:04 AM
> To: dev at dpdk.org
> Cc: Stephen Hemminger
> Subject: [dpdk-dev] [PATCH 2/7] rte_sched: use reserved field to allow more VLAN's
> 
> From: Stephen Hemminger <shemming at 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 reserved field of mbuf for this field instead
> of packing inside other classify portions.
> 
> Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>
> ---
>  lib/librte_mbuf/rte_mbuf.h   |  2 +-
>  lib/librte_sched/rte_sched.h | 31 ++++++++++++++++++++-----------
>  2 files changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 16059c6..b6b08f4 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -242,7 +242,7 @@ struct rte_mbuf {
>  	uint16_t data_len;        /**< Amount of data in segment buffer. */
>  	uint32_t pkt_len;         /**< Total pkt len: sum of all segments. */
>  	uint16_t vlan_tci;        /**< VLAN Tag Control Identifier (CPU order) */
> -	uint16_t reserved;
> +	uint16_t subport;	  /**< SCHED Subport ID */

As I remember, we keep these reserved 2 bytes for RX 2 double vlan tag offload.
So probably not a good idea to use it for something that is rte_sched specific.
If you really need extra space fo rte_sched fields inside mbuf, can't you move it into second cache line?
Or might be you can use userdata, to either store sched information directly, or as a pointer to some external memory  location? 
Another possibility - union mbuf.hash is 64bit now, while sched uses only 32bits.
So might be you can rearrange it to make sched 64bits too?
Something like:

union {
                uint32_t rss;     /**< RSS hash result if RSS enabled */
                struct {
                        union {
                                struct {
                                        uint16_t hash;
                                        uint16_t id;
                                };
                                uint32_t lo;
                                /**< Second 4 flexible bytes */
                        };
                        uint32_t hi;
                        /**< 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 */
+               uint64_t sched;   /**< Hierarchical scheduler */
                uint32_t usr;     /**< User defined tags. See @rte_distributor_p
rocess */
} hash;                   /**< hash information */

Konstantin


>  	union {
>  		uint32_t rss;     /**< RSS hash result if RSS enabled */
>  		struct {
> diff --git a/lib/librte_sched/rte_sched.h b/lib/librte_sched/rte_sched.h
> index e6bba22..0688422 100644
> --- a/lib/librte_sched/rte_sched.h
> +++ b/lib/librte_sched/rte_sched.h
> @@ -195,15 +195,18 @@ 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 extra:6;		 /**< currently unused */
>  	uint32_t color:2;                /**< Color */
>  };
> 
> @@ -350,12 +353,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;
> 
> +	pkt->subport = subport;
>  	sched->color = (uint32_t) color;
> -	sched->subport = subport;
>  	sched->pipe = pipe;
>  	sched->traffic_class = traffic_class;
>  	sched->queue = queue;
> @@ -379,11 +385,14 @@ 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;
> +	*subport = pkt->subport;
>  	*pipe = sched->pipe;
>  	*traffic_class = sched->traffic_class;
>  	*queue = sched->queue;
> --
> 2.1.4