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 A90A9E72 for ; Wed, 6 May 2015 10:48:42 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga102.jf.intel.com with ESMTP; 06 May 2015 01:48:41 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,378,1427785200"; d="scan'208";a="567036918" Received: from pgsmsx108.gar.corp.intel.com ([10.221.44.103]) by orsmga003.jf.intel.com with ESMTP; 06 May 2015 01:48:40 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by PGSMSX108.gar.corp.intel.com (10.221.44.103) with Microsoft SMTP Server (TLS) id 14.3.224.2; Wed, 6 May 2015 16:48:32 +0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.162]) by shsmsx102.ccr.corp.intel.com ([10.239.4.154]) with mapi id 14.03.0224.002; Wed, 6 May 2015 16:48:31 +0800 From: "Zhang, Helin" To: "Richardson, Bruce" Thread-Topic: [dpdk-dev] [PATCH RFC 1/6] mbuf: update mbuf structure for QinQ support Thread-Index: AQHQhtvREg5FOlOyv0qaMeWU+G5+9J1tMHyggAEju1D//8g5gIAAiDkw Date: Wed, 6 May 2015 08:48:31 +0000 Message-ID: References: <1430793143-3610-1-git-send-email-helin.zhang@intel.com> <1430793143-3610-2-git-send-email-helin.zhang@intel.com> <2601191342CEEE43887BDE71AB97725821424AF9@irsmsx105.ger.corp.intel.com> <20150506083915.GA3824@bricha3-MOBL3> In-Reply-To: <20150506083915.GA3824@bricha3-MOBL3> 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 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH RFC 1/6] mbuf: update mbuf structure for QinQ support 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: Wed, 06 May 2015 08:48:43 -0000 > -----Original Message----- > From: Richardson, Bruce > Sent: Wednesday, May 6, 2015 4:39 PM > To: Zhang, Helin > Cc: Ananyev, Konstantin; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH RFC 1/6] mbuf: update mbuf structure for > QinQ support >=20 > On Wed, May 06, 2015 at 04:06:17AM +0000, Zhang, Helin wrote: > > > > > > > -----Original Message----- > > > From: Ananyev, Konstantin > > > Sent: Tuesday, May 5, 2015 7:05 PM > > > To: Zhang, Helin; dev@dpdk.org > > > Subject: RE: [dpdk-dev] [PATCH RFC 1/6] mbuf: update mbuf structure > > > for QinQ support > > > > > > Hi Helin, > > > > > > > -----Original Message----- > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Helin Zhang > > > > Sent: Tuesday, May 05, 2015 3:32 AM > > > > To: dev@dpdk.org > > > > Subject: [dpdk-dev] [PATCH RFC 1/6] mbuf: update mbuf structure > > > > for QinQ support > > > > > > > > To support QinQ, 'vlan_tci' should be replaced by 'vlan_tci0' and > > > > 'vlan_tci1'. Also new offload flags of 'PKT_RX_QINQ_PKT' and > > > > 'PKT_TX_QINQ_PKT' should be added. > > > > > > > > Signed-off-by: Helin Zhang > > > > --- > > > > app/test-pmd/flowgen.c | 2 +- > > > > app/test-pmd/macfwd.c | 2 +- > > > > app/test-pmd/macswap.c | 2 +- > > > > app/test-pmd/rxonly.c | 2 +- > > > > app/test-pmd/txonly.c | 2 +- > > > > app/test/packet_burst_generator.c | 4 ++-- > > > > lib/librte_ether/rte_ether.h | 4 ++-- > > > > lib/librte_mbuf/rte_mbuf.h | 22 > > > +++++++++++++++++++--- > > > > lib/librte_pmd_e1000/em_rxtx.c | 8 ++++---- > > > > lib/librte_pmd_e1000/igb_rxtx.c | 8 ++++---- > > > > lib/librte_pmd_enic/enic_ethdev.c | 2 +- > > > > lib/librte_pmd_enic/enic_main.c | 2 +- > > > > lib/librte_pmd_fm10k/fm10k_rxtx.c | 2 +- > > > > lib/librte_pmd_i40e/i40e_rxtx.c | 8 ++++---- > > > > lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 11 +++++------ > > > > lib/librte_pmd_vmxnet3/vmxnet3_rxtx.c | 6 +++--- > > > > 16 files changed, 51 insertions(+), 36 deletions(-) > > > > > > > > --- a/lib/librte_mbuf/rte_mbuf.h > > > > +++ b/lib/librte_mbuf/rte_mbuf.h > > > > @@ -101,11 +101,17 @@ extern "C" { #define > PKT_RX_TUNNEL_IPV6_HDR > > > > (1ULL << 12) /**< RX tunnel > > > packet with IPv6 header. */ > > > > #define PKT_RX_FDIR_ID (1ULL << 13) /**< FD id reported > if > > > FDIR match. */ > > > > #define PKT_RX_FDIR_FLX (1ULL << 14) /**< Flexible bytes > > > reported if FDIR match. */ > > > > +#define PKT_RX_QINQ_PKT (1ULL << 15) /**< RX packet > with > > > double VLAN stripped. */ > > > > /* add new RX flags here */ > > > > > > > > /* add new TX flags here */ > > > > > > > > /** > > > > + * Second VLAN insertion (QinQ) flag. > > > > + */ > > > > +#define PKT_TX_QINQ_PKT (1ULL << 49) > > > > + > > > > +/** > > > > * TCP segmentation offload. To enable this offload feature for a > > > > * packet to be transmitted on hardware supporting TSO: > > > > * - set the PKT_TX_TCP_SEG flag in mbuf->ol_flags (this flag > > > > implies @@ -268,7 +274,6 @@ 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; > > > > > > Now here is an implicit 2-bytes whole between 'reserved' and 'rss'. > > > Probably better to make it explicit - make 'reserved' uint32_t. > > Yes, the layout will be changed according to the demands of Vector PMD. > > The vlan structure will be kept the same, but the mbuf structure > > layout will be re-organized a bit. >=20 > Why not just put the extra vlan tag into the reserved space. In the origi= nal > work to restructure the mbuf, that was what the reserved space was put > there for [it was marked as reserved as it was requested that fields not = be > fully dedicated until used, and we did not have double-vlan support at th= at > time]. > However, it seems more sensible to put the vlans there now, unless there > is a good reason to move them to the new location in the mbuf that you > propose below. >=20 > /Bruce Thank you very much for the reminder! The main reason is that we planned to enlarge the packet_type field, so we = have to move the vlan fields down. Hopefully the unified packet type can be= merged before this one. Regards, Helin >=20 > > > > > > > > Another thing - it looks like your change will break ixgbe vector RX= . > > Yes, in the cover-letter, I noted that the vector PMD will be updated > > soon together with the code changes. > > > > > > > > > union { > > > > uint32_t rss; /**< RSS hash result if RSS enabled */ > > > > @@ -289,6 +294,15 @@ struct rte_mbuf { > > > > uint32_t usr; /**< User defined tags. See > > > rte_distributor_process() */ > > > > } hash; /**< hash information */ > > > > > > > > + /* VLAN tags */ > > > > + union { > > > > + uint32_t vlan_tags; > > > > + struct { > > > > + uint16_t vlan_tci0; > > > > + uint16_t vlan_tci1; > > > > > > Do you really need to change vlan_tci to vlan_tci0? > > > Can't you keep 'vlan_tci' for first vlan tag, and add something like > > > 'vlan_tci_ext', or 'vlan_tci_next' for second one? > > > Would save you a lot of changes, again users who use single vlan > > > wouldn't need to update their code for 2.1. > > Yes, good point! The names came from the original mbuf definition done > > by Bruce long long ago. If more guys suggest keeping th old one, and > > just add a new one, I will do like that in the next version of patch se= t. > > Thank you all! > > > > > > > > > + }; > > > > + }; > > > > + > > > > uint32_t seqn; /**< Sequence number. See also > > > > rte_reorder_insert() */ > > > > > > > > /* second cache line - fields only used in slow path or on TX */ > > > > @@ > > > > -766,7 +780,8 @@ static inline void rte_pktmbuf_reset(struct > > > > rte_mbuf > > > *m) > > > > m->next =3D NULL; > > > > m->pkt_len =3D 0; > > > > m->tx_offload =3D 0; > > > > - m->vlan_tci =3D 0; > > > > + m->vlan_tci0 =3D 0; > > > > + m->vlan_tci1 =3D 0; > > > > > > Why just not: > > > m-> vlan_tags =3D 0; > > > ? > > Accepted. Good point! > > > > > > > > > m->nb_segs =3D 1; > > > > m->port =3D 0xff; > > > > > > > > @@ -838,7 +853,8 @@ static inline void rte_pktmbuf_attach(struct > > > rte_mbuf *mi, struct rte_mbuf *m) > > > > mi->data_off =3D m->data_off; > > > > mi->data_len =3D m->data_len; > > > > mi->port =3D m->port; > > > > - mi->vlan_tci =3D m->vlan_tci; > > > > + mi->vlan_tci0 =3D m->vlan_tci0; > > > > + mi->vlan_tci1 =3D m->vlan_tci1; > > > > > > Same thing, why not: > > > mi-> vlan_tags =3D m-> vlan_tags; > > > ? > > Accepted. Good point! > > > > Regards, > > Helin >=20 >