From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id BB0F2B723 for ; Fri, 20 Feb 2015 19:19:27 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga103.jf.intel.com with ESMTP; 20 Feb 2015 10:13:38 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,616,1418112000"; d="scan'208";a="457330506" Received: from irsmsx104.ger.corp.intel.com ([163.33.3.159]) by FMSMGA003.fm.intel.com with ESMTP; 20 Feb 2015 10:03:52 -0800 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.218]) by IRSMSX104.ger.corp.intel.com ([169.254.5.145]) with mapi id 14.03.0195.001; Fri, 20 Feb 2015 18:18:28 +0000 From: "Dumitrescu, Cristian" To: Stephen Hemminger , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v2 2/7] rte_sched: expand scheduler hierarchy for more VLAN's Thread-Index: AQHQQQm6zpEdcJBlAkmNrvDnEZFPcZz56qCQ Date: Fri, 20 Feb 2015 18:18:27 +0000 Message-ID: <3EB4FA525960D640B5BDFFD6A3D891263231B256@IRSMSX108.ger.corp.intel.com> References: <1423116294-17080-1-git-send-email-stephen@networkplumber.org> <1423116294-17080-2-git-send-email-stephen@networkplumber.org> In-Reply-To: <1423116294-17080-2-git-send-email-stephen@networkplumber.org> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Cc: Stephen Hemminger Subject: Re: [dpdk-dev] [PATCH v2 2/7] rte_sched: expand scheduler hierarchy for more VLAN's X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Feb 2015 18:19:28 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen > Hemminger > Sent: Thursday, February 5, 2015 6:05 AM > To: dev@dpdk.org > Cc: Stephen Hemminger > Subject: [dpdk-dev] [PATCH v2 2/7] rte_sched: expand scheduler hierarchy > for more VLAN's > = > From: Stephen Hemminger > = > 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 > --- > v2 use tag area rather than claiming reserved bit which isn't documented > = > 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 16059c6..8f0c3a4 100644 > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -258,7 +258,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..dda287f 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 fie= ld > hash.sched > -of struct rte_mbuf of each packet, typically written by the classificati= on > 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 */ > }; Extending the number of bits allocated for mbuf->sched makes sense to me. I= agree with this partitioning. > = > /* > @@ -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 =3D (struct > rte_sched_port_hierarchy *) &pkt->hash.sched; > + struct rte_sched_port_hierarchy *sched > + =3D (struct rte_sched_port_hierarchy *) &pkt->hash.sched; > = > - sched->color =3D (uint32_t) color; > sched->subport =3D subport; > + sched->color =3D (uint32_t) color; > sched->pipe =3D pipe; > sched->traffic_class =3D traffic_class; > sched->queue =3D 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 =3D (struct > rte_sched_port_hierarchy *) &pkt->hash.sched; > + struct rte_sched_port_hierarchy *sched > + =3D (struct rte_sched_port_hierarchy *) &pkt->hash.sched; > = > *subport =3D sched->subport; > *pipe =3D sched->pipe; > -- > 2.1.4 The functions used to access the mbuf->sched field are very slow. Functions rte_sched_port_pkt_write(), rte_sched_port_pkt_read_tree_path(), = rte_sched_port_pkt_read_color() are accessing the bitfields directly, and g= cc seems to do a particularly bad job at optimizing the code that makes use= of bitfields. Although less readable, a more performant alternative is to = implement these functions with bit shifting, masking and or-ing operations = as opposed to accessing the bit fields, as it seems to save dozens of cycle= s per packet. Stephen, based on a previous conversation we had a while ago, would you be = OK to do this now and resubmit this patch? -------------------------------------------------------------- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the s= ole use of the intended recipient(s). Any review or distribution by others = is strictly prohibited. If you are not the intended recipient, please conta= ct the sender and delete all copies.