From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id F21AF5A74 for ; Thu, 5 Mar 2015 02:01:48 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP; 04 Mar 2015 17:00:20 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,344,1422950400"; d="scan'208";a="675325528" Received: from pgsmsx103.gar.corp.intel.com ([10.221.44.82]) by fmsmga001.fm.intel.com with ESMTP; 04 Mar 2015 17:01:47 -0800 Received: from kmsmsx154.gar.corp.intel.com (172.21.73.14) by PGSMSX103.gar.corp.intel.com (10.221.44.82) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 5 Mar 2015 08:57:04 +0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.110.14) by KMSMSX154.gar.corp.intel.com (172.21.73.14) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 5 Mar 2015 08:56:59 +0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.161]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.197]) with mapi id 14.03.0195.001; Thu, 5 Mar 2015 08:55:45 +0800 From: "Zhang, Helin" To: "Chilikin, Andrey" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v4 01/18] mbuf: redefinition of packet_type in rte_mbuf Thread-Index: AQHQUo8NCbApITdTRkSFMj6AcoYVZJ0JEt8QgALzGDCAACdqAIAA6krw Date: Thu, 5 Mar 2015 00:55:45 +0000 Message-ID: References: <1424156374-21768-1-git-send-email-helin.zhang@intel.com> <1425042696-23162-1-git-send-email-helin.zhang@intel.com> <1425042696-23162-2-git-send-email-helin.zhang@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4 01/18] mbuf: redefinition of packet_type in rte_mbuf 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: Thu, 05 Mar 2015 01:01:49 -0000 > -----Original Message----- > From: Chilikin, Andrey > Sent: Wednesday, March 4, 2015 6:59 PM > To: Zhang, Helin; dev@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v4 01/18] mbuf: redefinition of packet_typ= e in > rte_mbuf >=20 > > -----Original Message----- > > From: Zhang, Helin > > Sent: Wednesday, March 4, 2015 8:34 AM > > To: Chilikin, Andrey; dev@dpdk.org > > Subject: RE: [dpdk-dev] [PATCH v4 01/18] mbuf: redefinition of > > packet_type in rte_mbuf > > > > > > > > > -----Original Message----- > > > From: Chilikin, Andrey > > > Sent: Monday, March 2, 2015 7:48 PM > > > To: Zhang, Helin; dev@dpdk.org > > > Subject: RE: [dpdk-dev] [PATCH v4 01/18] mbuf: redefinition of > > > packet_type in rte_mbuf > > > > > > Hi Helin, > > > > > > I see that you have removed "uint16_t reserved" member from rte_mbuf: > > > > > > > + uint16_t data_len; /**< Amount of data in segment buffer. > */ > > > > uint16_t vlan_tci; /**< VLAN Tag Control Identifier (CPU > order) > > > */ > > > > - uint16_t reserved; > > > > union { > > > > uint32_t rss; /**< RSS hash result if RSS enabled */ > > > > > > This reserved field was kept next to vlan_tci as a placeholder for > > > the second VLAN label for QinQ support so if need be vlan_tci + > > > reserved could be casted to > > > 32 bit QinQ value or one 32bit VNTAG label. Without keeping two > > > label adjusted to each other casting to 32 bit will not be possible > > > and will affect QinQ performance. > > Yes, but packet type is quite important which needs to be extended > > from 16 bits to 32 bits. > > For FVL, the vlan tags are in different fields. > I do not see how FVL internal descriptor can affect DPDK mbuf structure. FVL rx descriptor plays key role of the mbuf structure definition, as we ha= ve Vector PMD. >=20 > > We can think of putting them > > together in mbuf, Possibly move current vlan tag down, and add one > > more 16 bits. Let's see what is the best then. > But if we know that we would need this change for QinQ anyway should we > move vlan_tci +reserved 16bits now in this patch instead of testing > performance twice - for this and for future patch when we move > vlan_tci+reserved? Good idea, and I will discuss it with team members to see if there is any o= bjection. Generally we modify things as needed, but not modify things by prediction. = This was indicated by Thomas and other reviewers several times. Regards, Helin >=20 > Regards, > Andrey >=20 > > Thanks for the notes! > > > > Regards, > > Helin > > > > > > > > Regards, > > > Andrey > > > > > > > > > > -----Original Message----- > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Helin Zhang > > > > Sent: Friday, February 27, 2015 1:11 PM > > > > To: dev@dpdk.org > > > > Subject: [dpdk-dev] [PATCH v4 01/18] mbuf: redefinition of > > > > packet_type in rte_mbuf > > > > > > > > In order to unify the packet type, the field of 'packet_type' in > > > > 'struct rte_mbuf' needs to be extended from 16 to 32 bits. > > > > Accordingly, some fields in 'struct rte_mbuf' are re-organized to > > > > support this change for Vector PMD. As 'struct rte_kni_mbuf' for > > > > KNI should be right mapped to 'struct rte_mbuf', it should be > > > > modified accordingly. In addition, Vector PMD of ixgbe is disabled > > > > by default, as 'struct > > > rte_mbuf' changed. > > > > > > > > Signed-off-by: Helin Zhang > > > > Signed-off-by: Cunming Liang > > > > --- > > > > config/common_linuxapp | 2 +- > > > > .../linuxapp/eal/include/exec-env/rte_kni_common.h | 4 ++-- > > > > lib/librte_mbuf/rte_mbuf.h | 23 > > > +++++++++++++++------- > > > > 3 files changed, 19 insertions(+), 10 deletions(-) > > > > > > > > v2 changes: > > > > * Enlarged the packet_type field from 16 bits to 32 bits. > > > > * Redefined the packet type sub-fields. > > > > * Updated the 'struct rte_kni_mbuf' for KNI according to the mbuf > > changes. > > > > > > > > v3 changes: > > > > * Put the mbuf layout changes into a single patch. > > > > * Disabled vector ixgbe PMD by default, as mbuf layout changed. > > > > > > > > diff --git a/config/common_linuxapp b/config/common_linuxapp index > > > > 97f1c9e..97d7bae 100644 > > > > --- a/config/common_linuxapp > > > > +++ b/config/common_linuxapp > > > > @@ -166,7 +166,7 @@ > CONFIG_RTE_LIBRTE_IXGBE_DEBUG_TX_FREE=3Dn > > > > CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=3Dn > > > > CONFIG_RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC=3Dn > > > > CONFIG_RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC=3Dy > > > > -CONFIG_RTE_IXGBE_INC_VECTOR=3Dy > > > > +CONFIG_RTE_IXGBE_INC_VECTOR=3Dn > > > > CONFIG_RTE_IXGBE_RX_OLFLAGS_ENABLE=3Dy > > > > > > > > # > > > > diff --git > > > > a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > > > b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > > > index 1e55c2d..bd1cc09 100644 > > > > --- > > > > a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h > > > > +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common. > > > > +++ h > > > > @@ -117,9 +117,9 @@ struct rte_kni_mbuf { > > > > uint16_t data_off; /**< Start address of data in segment > buffer. */ > > > > char pad1[4]; > > > > uint64_t ol_flags; /**< Offload features. */ > > > > - char pad2[2]; > > > > - uint16_t data_len; /**< Amount of data in segment buffer. */ > > > > + char pad2[4]; > > > > uint32_t pkt_len; /**< Total pkt len: sum of all segment > data_len. > > > > */ > > > > + uint16_t data_len; /**< Amount of data in segment buffer. */ > > > > > > > > /* fields on second cache line */ > > > > char pad3[8] __attribute__((__aligned__(RTE_CACHE_LINE_SIZE))); > > > > diff --git a/lib/librte_mbuf/rte_mbuf.h > > > > b/lib/librte_mbuf/rte_mbuf.h index 17ba791..f5b7a8b 100644 > > > > --- a/lib/librte_mbuf/rte_mbuf.h > > > > +++ b/lib/librte_mbuf/rte_mbuf.h > > > > @@ -258,17 +258,26 @@ struct rte_mbuf { > > > > /* remaining bytes are set on RX when pulling packet from > > > > descriptor */ > > > > MARKER rx_descriptor_fields1; > > > > > > > > - /** > > > > - * The packet type, which is used to indicate ordinary packet and= also > > > > - * tunneled packet format, i.e. each number is represented a type= of > > > > - * packet. > > > > + /* > > > > + * The packet type, which is the combination of outer/inner L2, L= 3, L4 > > > > + * and tunnel types. > > > > */ > > > > - uint16_t packet_type; > > > > + union { > > > > + uint32_t packet_type; /**< L2/L3/L4 and tunnel information. > > > > */ > > > > + struct { > > > > + uint32_t l2_type:4; /**< (Outer) L2 type. */ > > > > + uint32_t l3_type:4; /**< (Outer) L3 type. */ > > > > + uint32_t l4_type:4; /**< (Outer) L4 type. */ > > > > + uint32_t tun_type:4; /**< Tunnel type. */ > > > > + uint32_t inner_l2_type:4; /**< Inner L2 type. */ > > > > + uint32_t inner_l3_type:4; /**< Inner L3 type. */ > > > > + uint32_t inner_l4_type:4; /**< Inner L4 type. */ > > > > + }; > > > > + }; > > > > > > > > - uint16_t data_len; /**< Amount of data in segment buffer. > */ > > > > uint32_t pkt_len; /**< Total pkt len: sum of all segments= . > */ > > > > + uint16_t data_len; /**< Amount of data in segment buffer. > */ > > > > uint16_t vlan_tci; /**< VLAN Tag Control Identifier (CPU > order) > > > */ > > > > - uint16_t reserved; > > > > union { > > > > uint32_t rss; /**< RSS hash result if RSS enabled */ > > > > struct { > > > > -- > > > > 1.9.3