From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH v11 1/6] ethdev: add Tx preparation Date: Thu, 27 Oct 2016 18:02:01 +0200 Message-ID: <2078955.d1Aiqtukxu@xps13> References: <1477327917-18564-1-git-send-email-tomaszx.kulasek@intel.com> <1499338.8Le0ABsxDG@xps13> <2601191342CEEE43887BDE71AB9772583F0CD83D@irsmsx105.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org To: "Ananyev, Konstantin" Return-path: Received: from mail-wm0-f54.google.com (mail-wm0-f54.google.com [74.125.82.54]) by dpdk.org (Postfix) with ESMTP id BFA4728FD for ; Thu, 27 Oct 2016 18:02:02 +0200 (CEST) Received: by mail-wm0-f54.google.com with SMTP id n67so54575559wme.1 for ; Thu, 27 Oct 2016 09:02:02 -0700 (PDT) In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0CD83D@irsmsx105.ger.corp.intel.com> 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" 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=y > > > > We cannot enable it until it is implemented in every drivers. > > Not sure why? > If tx_pkt_prep == 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? > > > 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 that 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.