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 3BB377E0B for ; Mon, 20 Oct 2014 14:38:48 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP; 20 Oct 2014 05:43:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,756,1406617200"; d="scan'208";a="621843545" Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by orsmga002.jf.intel.com with ESMTP; 20 Oct 2014 05:46:58 -0700 Received: from irsmsx108.ger.corp.intel.com (163.33.3.3) by IRSMSX101.ger.corp.intel.com (163.33.3.153) with Microsoft SMTP Server (TLS) id 14.3.195.1; Mon, 20 Oct 2014 13:45:57 +0100 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.248]) by IRSMSX108.ger.corp.intel.com ([169.254.11.21]) with mapi id 14.03.0195.001; Mon, 20 Oct 2014 13:45:57 +0100 From: "Walukiewicz, Miroslaw" To: Thomas Monjalon Thread-Topic: [dpdk-dev] [PATCH] pmd: Add generic support for TCP TSO (Transmit Segmentation Offload) Thread-Index: AQHP7Epr9YF+GxESYUWki811ObjU9Jw4yQ2AgAAWpWA= Date: Mon, 20 Oct 2014 12:45:57 +0000 Message-ID: <7C4248CAE043B144B1CD242D275626532FDE0709@IRSMSX104.ger.corp.intel.com> References: <20141020094252.14456.58891.stgit@gklab-18-011.igk.intel.com> <1675767.ObibVazTHA@xps13> In-Reply-To: <1675767.ObibVazTHA@xps13> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] 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] pmd: Add generic support for TCP TSO (Transmit Segmentation Offload) 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, 20 Oct 2014 12:38:49 -0000 Hi Thomas,=20 Thank for your comments. My responses are inline. > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Monday, October 20, 2014 1:30 PM > To: Walukiewicz, Miroslaw > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] pmd: Add generic support for TCP TSO > (Transmit Segmentation Offload) >=20 > Hi Miroslaw, >=20 > I'll try to comment your patch, but I don't know if you'll receive it. > Indeed, you didn't reply to the previous comments. > Please configure your email client to receive these emails. > This is not a write-only list. >=20 > 2014-10-20 05:42, miroslaw.walukiewicz@intel.com: > > Add new PKT_TX_TCP_SEG flag > > Add new fields in the tx offload fields indicating MSS and L4 len >=20 > You should explain why these additions are needed. I will resend a patch with better description of new fields.=20 >=20 > > /* fields to support TX offloads */ > > - union { > > - uint16_t l2_l3_len; /**< combined l2/l3 lengths as single var > */ > > - struct { > > - uint16_t l3_len:9; /**< L3 (IP) Header Length. */ > > - uint16_t l2_len:7; /**< L2 (MAC) Header Length. > */ > > + /* two bytes - l2/l3 len for compatibility (endian issues) > > + * two bytes - reseved for alignment > > + * two bytes - l4 len (TCP/UDP) header len > > + * two bytes - TCP tso segment size > > + */ > > + struct { > > + union { > > + uint16_t l2_l3_len; /**< combined l2/l3 len */ > > + struct { > > + uint16_t l3_len:9; /**< L3 (IP) Header */ > > + uint16_t l2_len:7; /**< L2 (MAC) Header */ > > + }; > > }; >=20 > Why nesting these fields in an anonymous structure? I want to keep a source compatibility with non-TSO applications using that = field for example IP checksum computing by NIC.=20 Keeping this structure anonymous I do not require changes in old applicatio= ns that do not need TSO support. The second argument is that in original patch extending the rte_mbuf to 128= bytes made by Bruce the author made this structure anonymous and I follow = this assumption too. >=20 > > + uint16_t reserved_tx_offload; > > + uint16_t l4_len; /**< TCP/UDP header len */ > > + uint16_t tso_segsz; /**< TCP TSO segment size */ > > }; >=20 > What means reserved_tx_offload? It is really for alignment. I want to keep all this structure 8 byte long. Really I found an issue in my patch. I think that all tx offload fields sho= uld be available in single 64-bit dword to make correct operation on in pkt= _mbuf_reset and pkt_mbuf_attach. Today these macros use only first 32-bits from structure and keeps l4_len a= nd tso_segsz untouched. I will modify my patch also in this direction.=20 >=20 > Is there an impact on performance of actual drivers ? >=20 I did not observed on my machine any significant differences when aligned a= nd non-aligned structure is used.=20 I agree that alignment is important for small packets. The TSO is used for= using very long TCP segments usually. > How this patch is related with previous work in progress about TSO? >=20 As the original Bruce's patch defining a new rte_mbuf structure did not fol= low exactly the concept proposed by Olivier Matz I made the closest approxi= mation. I defined PKT_TX_TCP_SEG, l4_len, mss =3D tso_segsz=20 Using mss could be misinterpreted. I think tso_segsz much better describes = this field meaning. I completely agree that the pseudo header checksum could be computed outs= ide driver and I also followed this assumption. Mirek > -- > Thomas