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 E0C6A9AF0 for ; Fri, 13 Feb 2015 03:26:05 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP; 12 Feb 2015 18:20:57 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,568,1418112000"; d="scan'208";a="665755128" Received: from pgsmsx102.gar.corp.intel.com ([10.221.44.80]) by fmsmga001.fm.intel.com with ESMTP; 12 Feb 2015 18:26:01 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by PGSMSX102.gar.corp.intel.com (10.221.44.80) with Microsoft SMTP Server (TLS) id 14.3.195.1; Fri, 13 Feb 2015 10:25:41 +0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.161]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.46]) with mapi id 14.03.0195.001; Fri, 13 Feb 2015 10:25:36 +0800 From: "Zhang, Helin" To: Olivier MATZ , "dev@dpdk.org" Thread-Topic: [PATCH v2 03/20] i40e: call i40e_txd_enable_checksum only for offloaded packets Thread-Index: AQHQQFyWjtKsfOOTWkKLI/2HKPDxX5zpbO/ggAA0LgCAAVU6AIAAPvcAgAKpIxA= Date: Fri, 13 Feb 2015 02:25:36 +0000 Message-ID: References: <1422623775-8050-1-git-send-email-olivier.matz@6wind.com> <1423041925-26956-1-git-send-email-olivier.matz@6wind.com> <1423041925-26956-4-git-send-email-olivier.matz@6wind.com> <54DA3AB0.3040500@6wind.com> <54DB8DC0.500@6wind.com> In-Reply-To: <54DB8DC0.500@6wind.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 03/20] i40e: call i40e_txd_enable_checksum only for offloaded packets 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: Fri, 13 Feb 2015 02:26:06 -0000 > -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Thursday, February 12, 2015 1:14 AM > To: Zhang, Helin; dev@dpdk.org > Cc: Ananyev, Konstantin; Liu, Jijiang > Subject: Re: [PATCH v2 03/20] i40e: call i40e_txd_enable_checksum only fo= r > offloaded packets >=20 > Hi Helin, >=20 > On 02/11/2015 06:32 AM, Zhang, Helin wrote: > >> On 02/10/2015 07:03 AM, Zhang, Helin wrote: > >>>> /* Enable checksum offloading */ > >>>> cd_tunneling_params =3D 0; > >>>> - i40e_txd_enable_checksum(ol_flags, &td_cmd, &td_offset, > >>>> - l2_len, l3_len, outer_l2_len, > >>>> - outer_l3_len, > >>>> - &cd_tunneling_params); > >>>> + if (ol_flags & I40E_TX_CKSUM_OFFLOAD_MASK) { > >>> likely should be added. > >> > >> I would say unlikely() instead. I think the non-offload case should > >> be the default one. What do you think? >=20 > Maybe you missed this comment. Any thoughts? Ohh, sorry for the missing! I'd prefer to have likely, as hardware offload is preferred if there is. If= you don't think so, how about to keep nothing (no likely/unlikely) as it is. >=20 >=20 > >>>> + i40e_txd_enable_checksum(ol_flags, &td_cmd, > &td_offset, > >>>> + l2_len, l3_len, outer_l2_len, > >>>> + outer_l3_len, > >>>> + &cd_tunneling_params); > >>>> + } > >>> As this code changes are in fast path, performance regression test > >>> is needed. I would like to see the performance difference with or > >>> without this patch set. Hopefully nothing different. If you need any > >>> helps, just let me > >> know. > >> > >> I'm sorry, I won't have the needed resources to bench this as I would > >> have to setup a performance platform with i40e devices. > >> > >> But I'm pretty sure that the code in non-offload case would be faster > >> with this patch as it will avoid many operations in > i40e_txd_enable_checksum(). > >> > >> For the offload case, as we also removed the if (l2_len =3D=3D 0) and = if > >> (l3_len =3D=3D 0), I think there are also less tests than before my pa= tch series. > >> > >> So in my opinion, adding this test does not really justify to check > >> the performance. > > As 40G is quite sensitive on cpu cycles, we'd better to avoid any > > performance drop during our modifying the code for fast path. > > Performance is what we care about too much. Based on my experiences, > > even minor code changes may result in big performance impact. > > It seems that we may need to help you on performance measurement. >=20 > Thanks, indeed it's helpful if you can check performance non-regression. I have asked our validation guys here to help you on that, but might not in high priority. In addition, we all will take vocation for the coming Chines= e new year. Regards, Helin >=20 > Regards, > Olivier