From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Ananyev, Konstantin" Subject: Re: [PATCH v11 1/6] ethdev: add Tx preparation Date: Fri, 28 Oct 2016 10:15:47 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772583F0CEAEE@irsmsx105.ger.corp.intel.com> References: <1477327917-18564-1-git-send-email-tomaszx.kulasek@intel.com> <1499338.8Le0ABsxDG@xps13> <2601191342CEEE43887BDE71AB9772583F0CD83D@irsmsx105.ger.corp.intel.com> <2078955.d1Aiqtukxu@xps13> <2601191342CEEE43887BDE71AB9772583F0CE8E3@irsmsx105.ger.corp.intel.com> <3042915272161B4EB253DA4D77EB373A14F45162@IRSMSX102.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" To: "Kulasek, TomaszX" , Thomas Monjalon Return-path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 507B23772 for ; Fri, 28 Oct 2016 12:15:52 +0200 (CEST) In-Reply-To: <3042915272161B4EB253DA4D77EB373A14F45162@IRSMSX102.ger.corp.intel.com> Content-Language: en-US List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Tomasz, >=20 > Hi >=20 > > -----Original Message----- > > From: Ananyev, Konstantin > > Sent: Thursday, October 27, 2016 18:24 > > To: Thomas Monjalon > > Cc: Kulasek, TomaszX ; dev@dpdk.org > > Subject: RE: [dpdk-dev] [PATCH v11 1/6] ethdev: add Tx preparation > > > > > > > > > -----Original Message----- > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > > Sent: Thursday, October 27, 2016 5:02 PM > > > To: Ananyev, Konstantin > > > Cc: Kulasek, TomaszX ; dev@dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v11 1/6] ethdev: add Tx preparation > > > > > > 2016-10-27 15:52, Ananyev, Konstantin: > > > > > > > > > > > > > > Hi Tomasz, > > > > > > > > > > This is a major new function in the API and I still have some > > comments. > > > > > > > > > > 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 a= s 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 withou= t) > > code paths for testpmd. > > Probably a new fwd mode or just extra parameter for the existing one? > > Any other suggestions? > > >=20 > 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?=20 Looking at current testpmd patch, I suppose the changes will be minimal. What do you think? Konstantin=20 >=20 > Tomasz >=20 > > > > > > > > > struct rte_eth_dev { > > > > > > eth_rx_burst_t rx_pkt_burst; /**< Pointer to PMD receive > > function. */ > > > > > > eth_tx_burst_t tx_pkt_burst; /**< Pointer to PMD transmit > > > > > > function. */ > > > > > > + eth_tx_prep_t tx_pkt_prep; /**< Pointer to PMD transmit > > > > > > +prepare function. */ > > > > > > struct rte_eth_dev_data *data; /**< Pointer to device data *= / > > > > > > const struct eth_driver *driver;/**< Driver for this device *= / > > > > > > const struct eth_dev_ops *dev_ops; /**< Functions exported by > > > > > > PMD */ > > > > > > > > > > Could you confirm why tx_pkt_prep is not in dev_ops? > > > > > I guess we want to have several implementations? > > > > > > > > Yes, it depends on configuration options, same as tx_pkt_burst. > > > > > > > > > > > > > > Shouldn't we have a const struct control_dev_ops and a struct > > datapath_dev_ops? > > > > > > > > That's probably a good idea, but I suppose it is out of scope for t= hat > > patch. > > > > > > No it's not out of scope. > > > It answers to the question "why is it added in this structure and not > > dev_ops". > > > We won't do this change when nothing else is changed in the struct. > > > > Not sure I understood you here: > > Are you saying datapath_dev_ops/controlpath_dev_ops have to be introduc= ed > > as part of that patch? > > But that's a lot of changes all over rte_ethdev.[h,c]. > > It definitely worse a separate patch (might be some discussion) for me. > > Konstantin > > > >