From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 6F1B55680 for ; Mon, 15 Feb 2016 06:32:38 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP; 14 Feb 2016 21:32:37 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,449,1449561600"; d="scan'208";a="902684727" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga001.fm.intel.com with ESMTP; 14 Feb 2016 21:32:36 -0800 Received: from fmsmsx123.amr.corp.intel.com (10.18.125.38) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 14 Feb 2016 21:32:36 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx123.amr.corp.intel.com (10.18.125.38) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 14 Feb 2016 21:32:36 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.172]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.249]) with mapi id 14.03.0248.002; Mon, 15 Feb 2016 13:32:33 +0800 From: "Lu, Wenzhuo" To: "Ananyev, Konstantin" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v2 5/6] ixgbe: support VxLAN & NVGRE TX checksum off-load Thread-Index: AQHRX32U8uhWbDQ7Rku34RjjFnJfjJ8sdFhQ Date: Mon, 15 Feb 2016 05:32:33 +0000 Message-ID: <6A0DE07E22DDAD4C9103DF62FEBC09090342A92F@shsmsx102.ccr.corp.intel.com> References: <1452496044-17524-1-git-send-email-wenzhuo.lu@intel.com> <1452735516-4527-1-git-send-email-wenzhuo.lu@intel.com> <1452735516-4527-6-git-send-email-wenzhuo.lu@intel.com> <2601191342CEEE43887BDE71AB97725836B03304@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB97725836B03304@irsmsx105.ger.corp.intel.com> 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 v2 5/6] ixgbe: support VxLAN & NVGRE TX checksum off-load 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: Mon, 15 Feb 2016 05:32:39 -0000 Hi Konstantin, > -----Original Message----- > From: Ananyev, Konstantin > Sent: Friday, February 5, 2016 2:55 AM > To: Lu, Wenzhuo; dev@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v2 5/6] ixgbe: support VxLAN & NVGRE TX > checksum off-load >=20 > Hi Wenzhuo, >=20 > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu > > Sent: Thursday, January 14, 2016 1:39 AM > > To: dev@dpdk.org > > Subject: [dpdk-dev] [PATCH v2 5/6] ixgbe: support VxLAN & NVGRE TX > > checksum off-load > > > > The patch add VxLAN & NVGRE TX checksum off-load. When the flag of > > outer IP header checksum offload is set, we'll set the context > > descriptor to enable this checksum off-load. > > > > Signed-off-by: Wenzhuo Lu > > --- > > drivers/net/ixgbe/ixgbe_rxtx.c | 52 > > ++++++++++++++++++++++++++++++++++-------- > > drivers/net/ixgbe/ixgbe_rxtx.h | 6 ++++- > > lib/librte_mbuf/rte_mbuf.h | 2 ++ > > 3 files changed, 49 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c > > b/drivers/net/ixgbe/ixgbe_rxtx.c index 512ac3a..fea2495 100644 > > --- a/drivers/net/ixgbe/ixgbe_rxtx.c > > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c > > @@ -85,7 +85,8 @@ > > PKT_TX_VLAN_PKT | \ > > PKT_TX_IP_CKSUM | \ > > PKT_TX_L4_MASK | \ > > - PKT_TX_TCP_SEG) > > + PKT_TX_TCP_SEG | \ > > + PKT_TX_OUTER_IP_CKSUM) >=20 >=20 > I think you also need to update dev_info.tx_offload_capa, couldn't find w= here > you doing it. My bad. I'll add it. Thanks. >=20 > > > > static inline struct rte_mbuf * > > rte_rxmbuf_alloc(struct rte_mempool *mp) @@ -364,9 +365,11 @@ > > ixgbe_set_xmit_ctx(struct ixgbe_tx_queue *txq, > > uint32_t ctx_idx; > > uint32_t vlan_macip_lens; > > union ixgbe_tx_offload tx_offload_mask; > > + uint32_t seqnum_seed =3D 0; > > > > ctx_idx =3D txq->ctx_curr; > > - tx_offload_mask.data =3D 0; > > + tx_offload_mask.data[0] =3D 0; > > + tx_offload_mask.data[1] =3D 0; > > type_tucmd_mlhl =3D 0; > > > > /* Specify which HW CTX to upload. */ @@ -430,9 +433,20 @@ > > ixgbe_set_xmit_ctx(struct ixgbe_tx_queue *txq, > > } > > } > > > > + if (ol_flags & PKT_TX_OUTER_IP_CKSUM) { > > + tx_offload_mask.outer_l3_len |=3D ~0; > > + tx_offload_mask.outer_l2_len |=3D ~0; > > + seqnum_seed |=3D tx_offload.outer_l3_len > > + << IXGBE_ADVTXD_OUTER_IPLEN; > > + seqnum_seed |=3D tx_offload.outer_l2_len > > + << IXGBE_ADVTXD_TUNNEL_LEN; > > + } >=20 > I don't have an X550 card off-hand, but reading through datasheet - it do= esn't > seem right. > When OUTER_IP_CKSUM is enabled MACLEN becomes outer_l2_len and > TUNNEL_LEN should be: outer_l4_len + tunnel_hdr_len + inner_l2_len. > So I think that in our case TUNNEL_LEN should be set to l2_len. Yes, you're right. I made a mistake here. Will correct it. Thanks. >=20 > > + > > txq->ctx_cache[ctx_idx].flags =3D ol_flags; > > - txq->ctx_cache[ctx_idx].tx_offload.data =3D > > - tx_offload_mask.data & tx_offload.data; > > + txq->ctx_cache[ctx_idx].tx_offload.data[0] =3D > > + tx_offload_mask.data[0] & tx_offload.data[0]; > > + txq->ctx_cache[ctx_idx].tx_offload.data[1] =3D > > + tx_offload_mask.data[1] & tx_offload.data[1]; > > txq->ctx_cache[ctx_idx].tx_offload_mask =3D tx_offload_mask; > > > > ctx_txd->type_tucmd_mlhl =3D rte_cpu_to_le_32(type_tucmd_mlhl); > > @@ -441,7 +455,7 @@ ixgbe_set_xmit_ctx(struct ixgbe_tx_queue *txq, > > vlan_macip_lens |=3D ((uint32_t)tx_offload.vlan_tci << > IXGBE_ADVTXD_VLAN_SHIFT); > > ctx_txd->vlan_macip_lens =3D rte_cpu_to_le_32(vlan_macip_lens); > > ctx_txd->mss_l4len_idx =3D rte_cpu_to_le_32(mss_l4len_idx); > > - ctx_txd->seqnum_seed =3D 0; > > + ctx_txd->seqnum_seed =3D seqnum_seed; > > } > > > > /* > > @@ -454,16 +468,24 @@ what_advctx_update(struct ixgbe_tx_queue *txq, > > uint64_t flags, { > > /* If match with the current used context */ > > if (likely((txq->ctx_cache[txq->ctx_curr].flags =3D=3D flags) && > > - (txq->ctx_cache[txq->ctx_curr].tx_offload.data =3D=3D > > - (txq->ctx_cache[txq->ctx_curr].tx_offload_mask.data & > tx_offload.data)))) { > > + (txq->ctx_cache[txq->ctx_curr].tx_offload.data[0] =3D=3D > > + (txq->ctx_cache[txq->ctx_curr].tx_offload_mask.data[0] > > + & tx_offload.data[0])) && > > + (txq->ctx_cache[txq->ctx_curr].tx_offload.data[1] =3D=3D > > + (txq->ctx_cache[txq->ctx_curr].tx_offload_mask.data[1] > > + & tx_offload.data[1])))) { > > return txq->ctx_curr; > > } > > > > /* What if match with the next context */ > > txq->ctx_curr ^=3D 1; > > if (likely((txq->ctx_cache[txq->ctx_curr].flags =3D=3D flags) && > > - (txq->ctx_cache[txq->ctx_curr].tx_offload.data =3D=3D > > - (txq->ctx_cache[txq->ctx_curr].tx_offload_mask.data & > tx_offload.data)))) { > > + (txq->ctx_cache[txq->ctx_curr].tx_offload.data[0] =3D=3D > > + (txq->ctx_cache[txq->ctx_curr].tx_offload_mask.data[0] > > + & tx_offload.data[0])) && > > + (txq->ctx_cache[txq->ctx_curr].tx_offload.data[1] =3D=3D > > + (txq->ctx_cache[txq->ctx_curr].tx_offload_mask.data[1] > > + & tx_offload.data[1])))) { > > return txq->ctx_curr; > > } > > > > @@ -492,6 +514,12 @@ tx_desc_ol_flags_to_cmdtype(uint64_t ol_flags) > > cmdtype |=3D IXGBE_ADVTXD_DCMD_VLE; > > if (ol_flags & PKT_TX_TCP_SEG) > > cmdtype |=3D IXGBE_ADVTXD_DCMD_TSE; > > + if (ol_flags & PKT_TX_OUTER_IP_CKSUM) > > + cmdtype |=3D (1 << IXGBE_ADVTXD_OUTERIPCS_SHIFT); > > + if (ol_flags & PKT_TX_VXLAN_PKT) > > + cmdtype &=3D ~(1 << IXGBE_ADVTXD_TUNNEL_TYPE_NVGRE); > > + else > > + cmdtype |=3D (1 << IXGBE_ADVTXD_TUNNEL_TYPE_NVGRE); > > return cmdtype; > > } > > > > @@ -588,8 +616,10 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf > **tx_pkts, > > uint64_t tx_ol_req; > > uint32_t ctx =3D 0; > > uint32_t new_ctx; > > - union ixgbe_tx_offload tx_offload =3D {0}; > > + union ixgbe_tx_offload tx_offload; > > > > + tx_offload.data[0] =3D 0; > > + tx_offload.data[1] =3D 0; > > txq =3D tx_queue; > > sw_ring =3D txq->sw_ring; > > txr =3D txq->tx_ring; > > @@ -623,6 +653,8 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf > **tx_pkts, > > tx_offload.l4_len =3D tx_pkt->l4_len; > > tx_offload.vlan_tci =3D tx_pkt->vlan_tci; > > tx_offload.tso_segsz =3D tx_pkt->tso_segsz; > > + tx_offload.outer_l2_len =3D tx_pkt->outer_l2_len; > > + tx_offload.outer_l3_len =3D tx_pkt->outer_l3_len; > > > > /* If new context need be built or reuse the exist ctx. */ > > ctx =3D what_advctx_update(txq, tx_ol_req, diff --git > > a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h > > index 475a800..c15f9fa 100644 > > --- a/drivers/net/ixgbe/ixgbe_rxtx.h > > +++ b/drivers/net/ixgbe/ixgbe_rxtx.h > > @@ -163,7 +163,7 @@ enum ixgbe_advctx_num { > > > > /** Offload features */ > > union ixgbe_tx_offload { > > - uint64_t data; > > + uint64_t data[2]; >=20 > I wonder what is there any performance impact of increasing size of tx_of= fload? I believe there's some impact. But it's too little and can be ignored. As I= tested there's no performance drop:) >=20 > > struct { > > uint64_t l2_len:7; /**< L2 (MAC) Header Length. */ > > uint64_t l3_len:9; /**< L3 (IP) Header Length. */ @@ -171,6 > +171,10 > > @@ union ixgbe_tx_offload { > > uint64_t tso_segsz:16; /**< TCP TSO segment size */ > > uint64_t vlan_tci:16; > > /**< VLAN Tag Control Identifier (CPU order). */ > > + > > + /* fields for TX offloading of tunnels */ > > + uint64_t outer_l3_len:8; /**< Outer L3 (IP) Hdr Length. */ > > + uint64_t outer_l2_len:8; /**< Outer L2 (MAC) Hdr Length. */ > > }; > > }; > > > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > > index 5ad5e59..1bda00e 100644 > > --- a/lib/librte_mbuf/rte_mbuf.h > > +++ b/lib/librte_mbuf/rte_mbuf.h > > @@ -103,6 +103,8 @@ extern "C" { > > > > /* add new TX flags here */ > > > > +#define PKT_TX_VXLAN_PKT (1ULL << 48) /**< TX packet is a VxLAN > packet. */ > > + >=20 > From reading X550 spec, I don't really understand what for we need to spe= cify is > it GRE or VXLAN packet, so probably we don't need that flag for now at al= l? The reason is we need to set the tunnel type in the Advanced Transmit Data = Descriptor. > If we really do, might bw worth to organise it like KT_TX_L4_CKSUM (as en= um) > and reserve few values for future expansion (2 or 3 bits?). Now there're only VxLAN and NVGRE supported. So, only 1 bit is occupied for= that. Not sure if more types will be supported in the future. Any suggesti= ons? Thanks. >=20 > Konstantin