From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 63F3C5598 for ; Fri, 28 Oct 2016 13:02:29 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP; 28 Oct 2016 04:02:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,557,1473145200"; d="scan'208";a="1060560261" Received: from irsmsx152.ger.corp.intel.com ([163.33.192.66]) by fmsmga001.fm.intel.com with ESMTP; 28 Oct 2016 04:02:10 -0700 Received: from irsmsx103.ger.corp.intel.com ([169.254.3.190]) by IRSMSX152.ger.corp.intel.com ([169.254.6.24]) with mapi id 14.03.0248.002; Fri, 28 Oct 2016 12:02:09 +0100 From: "Richardson, Bruce" To: "Ananyev, Konstantin" , Thomas Monjalon , "Kulasek, TomaszX" Thread-Topic: [dpdk-dev] [PATCH v11 1/6] ethdev: add Tx preparation Thread-Index: AQHSL4isIKY4QwQhKkOuSAhRiz1yIaC8ViAAgAAOXQCAAAKVgIAABkCAgAAESgCAAScPgIAAAdiAgAAByYCAABk5oA== Date: Fri, 28 Oct 2016 11:02:08 +0000 Message-ID: <59AF69C657FD0841A61C55336867B5B035B31CDA@IRSMSX103.ger.corp.intel.com> References: <1477327917-18564-1-git-send-email-tomaszx.kulasek@intel.com> <3042915272161B4EB253DA4D77EB373A14F45162@IRSMSX102.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772583F0CEAEE@irsmsx105.ger.corp.intel.com> <18243372.Y4s77Td6b4@xps13> <2601191342CEEE43887BDE71AB9772583F0CEB79@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0CEB79@irsmsx105.ger.corp.intel.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMGRlODQ3OWMtYzE1Ni00ZGVjLThiZTgtNjc2MWQ0NGFiY2U0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6Ik5NVjNScUppXC9xSG85dSt4a1Q3SWZZbHN3S2prRTFKenNtRE9JZGcrZUhvPSJ9 x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.180] 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 v11 1/6] ethdev: add Tx preparation 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, 28 Oct 2016 11:02:30 -0000 > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin > Sent: Friday, October 28, 2016 11:29 AM > To: Thomas Monjalon ; Kulasek, TomaszX > > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v11 1/6] ethdev: add Tx preparation >=20 >=20 >=20 > > -----Original Message----- > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > Sent: Friday, October 28, 2016 11:22 AM > > To: Ananyev, Konstantin ; Kulasek, > > TomaszX > > Cc: dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH v11 1/6] ethdev: add Tx preparation > > > > 2016-10-28 10:15, Ananyev, Konstantin: > > > > From: Ananyev, Konstantin > > > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > > > > > 2016-10-27 15:52, Ananyev, Konstantin: > > > > > > > > 2016-10-26 14:56, Tomasz Kulasek: > > > > > > > > > --- a/config/common_base > > > > > > > > > +++ b/config/common_base > > > > > > > > > +CONFIG_RTE_ETHDEV_TX_PREP=3Dy > > > > > > > > > > > > > > > > We cannot enable it until it is implemented in every > drivers. > > > > > > > > > > > > > > Not sure why? > > > > > > > If tx_pkt_prep =3D=3D NULL, then rte_eth_tx_prep() would just= act > as noop. > > > > > > > Right now it is not mandatory for the PMD to implement it. > > > > > > > > > > > > If it is not implemented, the application must do the > > > > > > preparation by > > > > > itself. > > > > > > From patch 6: > > > > > > " > > > > > > Removed pseudo header calculation for udp/tcp/tso packets from > > > > > > application and used Tx preparation API for packet preparation > > > > > > and verification. > > > > > > " > > > > > > So how does it behave with other drivers? > > > > > > > > > > Hmm so it seems that we broke testpmd csumonly mode for > > > > > non-intel drivers.. > > > > > My bad, missed that part completely. > > > > > Yes, then I suppose for now we'll need to support both (with and > > > > > without) code paths for testpmd. > > > > > Probably a new fwd mode or just extra parameter for the existing > one? > > > > > Any other suggestions? > > > > > > > > > > > > > I had sent txprep engine in v2 > > > > (http://dpdk.org/dev/patchwork/patch/15775/), but I'm opened on > > > > the suggestions. If you like it I can > > resent > > > > it in place of csumonly modification. > > > > > > I still not sure it is worth to have another version of csum... > > > Can we introduce a new global variable in testpmd and a new command: > > > testpmd> csum tx_prep > > > or so? > > > Looking at current testpmd patch, I suppose the changes will be > minimal. > > > What do you think? > > > > No please no! > > The problem is not in testpmd. > > The problem is in every applications. > > Should we prepare the checksums or let tx_prep do it? >=20 > Not sure, I understood you... > Right now we don't' change other apps. > They would work as before. > If people would like to start to use tx_prep in their apps - they are fre= e > to do that. > If they like to keep doing that manually - that's fine too. > From other side we need an ability to test (and demonstrate) that new > functionality. > So we do need changes in testpmd. > Konstantin >=20 Just my 2c on this: * given this is new functionality, and no apps are currently using it, I'm = not sure I see the harm in having the function available by default. We jus= t need to be clear about the limits of the function and the fact that apps = need to do work themselves if the driver doesn't provide the function. * having it enabled will then allow any apps that want to use it do to so. * however, for our sample apps, and by default in testpmd, we *shouldn't* u= se this functionality, in the absence of any fallback, so that is where I w= ould look to have the enable/disable switch, not in the library. * going forward, I think a SW fallback inside the ethdev API itself would b= e a good addition to make this fully generic. Hope this helps, [and also that I haven't missed some subtlety in the discu= ssion!] /Bruce