Re: [dpdk-dev] [PATCH] mbuf: implement generic format for sched field

["Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>]

> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Saturday, December 1, 2018 2:23 PM
> To: Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> Pattan, Reshma <reshma.pattan@intel.com>; Rao, Nikhil
> <nikhil.rao@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] mbuf: implement generic format for sched
> field
> 
> -----Original Message-----
> > Date: Fri, 23 Nov 2018 16:54:23 +0000
> > From: Jasvinder Singh <jasvinder.singh@intel.com>
> > To: dev@dpdk.org
> > CC: cristian.dumitrescu@intel.com, Reshma Pattan
> <reshma.pattan@intel.com>
> > Subject: [dpdk-dev] [PATCH] mbuf: implement generic format for sched
> field
> > X-Mailer: git-send-email 2.17.1
> >
> > This patch implements the changes proposed in the deprecation
> > notes [1][2].
> >
> > The opaque mbuf->hash.sched field is updated to support generic
> > definition in line with the ethdev TM and MTR APIs. The new generic
> > format contains: queue ID, traffic class, color.
> >
> > In addtion, following API functions of the sched library have
> > been modified with an additional parameter of type struct
> > rte_sched_port to accomodate the changes made to mbuf sched field.
> > (i)  rte_sched_port_pkt_write()
> > (ii) rte_sched_port_pkt_read()
> >
> > The other libraries, sample applications and tests which use mbuf
> > sched field have been updated as well.
> >
> > [1] http://mails.dpdk.org/archives/dev/2018-February/090651.html
> > [2] https://mails.dpdk.org/archives/dev/2018-November/119051.html
> >
> > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> > ---
> > @@ -575,12 +575,10 @@ struct rte_mbuf {
> >                                  */
> >                         } fdir; /**< Filter identifier if FDIR enabled */
> >                         struct {
> > -                               uint32_t lo;
> > -                               uint32_t hi;
> > -                               /**< The event eth Tx adapter uses this field
> > -                                * to store Tx queue id.
> > -                                * @see rte_event_eth_tx_adapter_txq_set()
> > -                                */
> > +                               uint32_t queue_id;   /**< Queue ID. */
> > +                               uint8_t traffic_class;   /**< Traffic class ID. */
> > +                               uint8_t color;   /**< Color. */
> > +                               uint16_t reserved;   /**< Reserved. */
> >                         } sched;          /**< Hierarchical scheduler */
> 
> +Nikhil.
> 
> Currently rte_event_eth_tx_adapter_txq_set() and
> rte_event_eth_tx_adapter_txq_get() implemented using
> hash.sched.queue_id. How about moving out from "sched" to "txadapter"?
> Something like below,
> 
> $ git diff
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 3dbc6695e..b73bbef93 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -575,13 +575,20 @@ struct rte_mbuf {
>                                  */
>                         } fdir; /**< Filter identifier if FDIR enabled
> */
>                         struct {
> -                               uint32_t lo;
> -                               uint32_t hi;
> +                               uint32_t queue_id;   /**< Queue ID. */
> +                               uint8_t traffic_class;   /**< Traffic class ID. */
> +                               uint8_t color;   /**< Color. */
> +                               uint16_t reserved;   /**< Reserved. */
> +                       } sched;          /**< Hierarchical scheduler */
> +                       struct {
> +                               uint32_t reserved1;
> +                               uint16_t reserved2;
> +                               uint16_t txq;
>                                 /**< The event eth Tx adapter uses this field
>                                  * to store Tx queue id.
>                                  * @see rte_event_eth_tx_adapter_txq_set()
>                                  */
> -                       } sched;          /**< Hierarchical scheduler */
> +                       } txadapter; /**< Eventdev ethdev Tx adapter */
>                         /**< User defined tags. See rte_distributor_process() */
>                         uint32_t usr;
>                 } hash;                   /**< hash information */
> 
> 
> >  rte_event_eth_tx_adapter_txq_set(struct rte_mbuf *pkt, uint16_t
> queue)
> >  {
> > -       uint16_t *p = (uint16_t *)&pkt->hash.sched.hi;
> > +       uint16_t *p = (uint16_t *)&pkt->hash.sched.queue_id;
> >         p[1] = queue;
> >  }
> >
> > @@ -320,7 +320,7 @@ rte_event_eth_tx_adapter_txq_set(struct
> rte_mbuf *pkt, uint16_t queue)
> >  static __rte_always_inline uint16_t __rte_experimental
> >  rte_event_eth_tx_adapter_txq_get(struct rte_mbuf *pkt)
> >  {
> > -       uint16_t *p = (uint16_t *)&pkt->hash.sched.hi;
> > +       uint16_t *p = (uint16_t *)&pkt->hash.sched.queue_id;
> >         return p[1];
> >  }
> >

Hi Jerin,

Is there a reason why eventdev cannot use the generic 32-bit queue_id field in mbuf->hash.sched?

We can definitely do it the way you suggest, but want to check with you first.

Regards,
Cristian