From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by dpdk.org (Postfix) with ESMTP id CE52558F4 for ; Mon, 26 May 2014 17:20:08 +0200 (CEST) Received: from azsmga001.ch.intel.com ([10.2.17.19]) by azsmga101.ch.intel.com with ESMTP; 26 May 2014 08:20:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.98,913,1392192000"; d="scan'208";a="436983588" Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by azsmga001.ch.intel.com with ESMTP; 26 May 2014 08:20:17 -0700 Received: from irsmsx153.ger.corp.intel.com (163.33.192.75) by IRSMSX101.ger.corp.intel.com (163.33.3.153) with Microsoft SMTP Server (TLS) id 14.3.123.3; Mon, 26 May 2014 16:20:16 +0100 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.239]) by IRSMSX153.ger.corp.intel.com ([169.254.9.252]) with mapi id 14.03.0123.003; Mon, 26 May 2014 16:20:05 +0100 From: "Ananyev, Konstantin" To: Olivier MATZ , Thomas Monjalon , "Shaw, Jeffrey B" , "Richardson, Bruce" , "Venkatesan, Venky" , "nhorman@tuxdriver.com" , "stephen@networkplumber.org" Thread-Topic: [PATCH v2 00/17] add TSO support Thread-Index: AQHPdc8XoWzTaNFHzkqBPEPDa5Op0ptOHANggAAPYQCABMBQoA== Date: Mon, 26 May 2014 15:20:05 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772580EFB0AD6@IRSMSX105.ger.corp.intel.com> References: <1400507789-18453-1-git-send-email-olivier.matz@6wind.com> <466924555.DZ26nc55Es@xps13> <2601191342CEEE43887BDE71AB9772580EFA84CD@IRSMSX105.ger.corp.intel.com> <537F5C18.7030603@6wind.com> In-Reply-To: <537F5C18.7030603@6wind.com> Accept-Language: en-IE, 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 v2 00/17] add TSO 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: Mon, 26 May 2014 15:20:09 -0000 Hi Oliver, >> I don't see any big changes in the v2 of that patch. >> >> At least both things that I have concerns about, stay unchanged in >> the v2: >> >> 1) merge physaddr and buf_len in a bitfield - I still think we better >> keep physaddr as 64bit field (PATCH 5). >As nobody reacted to our discussion thread [1] with other arguments, I >stayed on my initial position: >- 2 bytes more in the mbuf structure is always good to have >- no performance impact detected (see my test reports) >- the 48 bits physical address limit will be reached in several years, > and at this time, maybe the cache lines will be 128 bytes? (solving > our initial rte_mbuf issue). So let's just deal with current or near > future hardware. As, I understand right now, you don't need these 2 bytes to enable TSO supp= ort. >>From other side, if we'll occupy them by some fields now, then in future we wouldn't be able to expand our phys_addr without re-working mbuf structu= re again and breaking backward compatibility. No doubt, that there are a lot of extra things that needed(wanted) to be pu= t into the mbuf. And as new HW with extra offload capabilities will come out - that demand w= ill only increase. That's why I think we wouldn't be able to squeeze all future fields in 64B = anyway. So it seems to me that we would have to expand mbuf to 128B sooner or later= and like we it or not. So I still think we better keep phys_addr 64bits long. At least for now.=20 >> 2) fix_tcp_phdr_cksum() is inside ixgbe TX function, while I think it >> should be moved out of it to the upper layer (PATCH 11). >Thomas's answer on this question [2] was to do that way, so I did not >modify the v1 patches. > Thomas Monjalon wrote: >We know at least 2 checksum methods: >- the standard one >- the special one for ixgbe TSO >In Linux ixgbe, checksum is redone in the driver for TSO case. I don't think we should use linux ixgbe driver as an etalon here. >We want to compute checksum in the application/stack in order to prevent=20 >driver from modifying packet data, that could cause cache miss. >But the application cannot always know which checksum method to use becaus= e it=20 >doesn't have to know which driver will process the packet. >So we have to choose which checksum method can be done in the application= =20 >without driver processing. It's not an easy choice. >It seems simpler and reasonable to choose the standard pseudo-header check= sum=20 >method as it is done in Linux. >Having a stable and generic API is something important we must target. As I said: with the current DPDK implementation the upper code would still = be different for packets with TCP checksum (without segmentation) and TCP s= egmentation: you need to setup different flags in mbuf, with TSO you need to setup l4_le= n and mss fields inside mbuf, with just checksum - you don't. So right now upper layer code has to know is it doing TSO or just checksum = anyway. If that's the case why it also can't compute pseudo-checksum in a 2 differe= nt ways too? Also, the patch introduces support TSO only for ixgbe. I suppose that TSO support for igb can be added in the same way.=20 Let's talk about generic API, when we'll have to expand it into different d= evices. Konstantin