From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 89F53234 for ; Wed, 11 Feb 2015 06:32:42 +0100 (CET) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga101.fm.intel.com with ESMTP; 10 Feb 2015 21:32:40 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,556,1418112000"; d="scan'208";a="650438655" Received: from pgsmsx104.gar.corp.intel.com ([10.221.44.91]) by orsmga001.jf.intel.com with ESMTP; 10 Feb 2015 21:32:38 -0800 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by PGSMSX104.gar.corp.intel.com (10.221.44.91) with Microsoft SMTP Server (TLS) id 14.3.195.1; Wed, 11 Feb 2015 13:32:37 +0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.161]) by shsmsx102.ccr.corp.intel.com ([169.254.2.62]) with mapi id 14.03.0195.001; Wed, 11 Feb 2015 13:32: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/ggAA0LgCAAVU6AA== Date: Wed, 11 Feb 2015 05:32:35 +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> In-Reply-To: <54DA3AB0.3040500@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: Wed, 11 Feb 2015 05:32:43 -0000 > -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Wednesday, February 11, 2015 1:07 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/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. >=20 > I would say unlikely() instead. I think the non-offload case should be th= e default > one. What do you think? >=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, jus= t let me > know. >=20 > I'm sorry, I won't have the needed resources to bench this as I would hav= e to > setup a performance platform with i40e devices. >=20 > But I'm pretty sure that the code in non-offload case would be faster wit= h this > patch as it will avoid many operations in i40e_txd_enable_checksum(). >=20 > 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 patch series. >=20 > 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 performan= ce drop during our modifying the code for fast path. Performance is what we care ab= out 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. Regards, Helin >=20 > Regards, > Olivier