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 AEB282170 for ; Thu, 13 Oct 2016 12:47:27 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga103.jf.intel.com with ESMTP; 13 Oct 2016 03:47:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,339,1473145200"; d="scan'208";a="1069879568" Received: from irsmsx152.ger.corp.intel.com ([163.33.192.66]) by fmsmga002.fm.intel.com with ESMTP; 13 Oct 2016 03:47:25 -0700 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.198]) by IRSMSX152.ger.corp.intel.com ([169.254.6.13]) with mapi id 14.03.0248.002; Thu, 13 Oct 2016 11:47:25 +0100 From: "Kulasek, TomaszX" To: Thomas Monjalon CC: "dev@dpdk.org" , "Ananyev, Konstantin" Thread-Topic: [dpdk-dev] [PATCH v4 1/6] ethdev: add Tx preparation Thread-Index: AQHSGvk8XABZJcdomUWeVrtqVYv78qChuMiAgARB0ACAAC9EoA== Date: Thu, 13 Oct 2016 10:47:24 +0000 Message-ID: <3042915272161B4EB253DA4D77EB373A14F35C5C@IRSMSX102.ger.corp.intel.com> References: <20160928111052.9968-1-tomaszx.kulasek@intel.com> <20160930090039.10164-2-tomaszx.kulasek@intel.com> <1995417.Vz0qMdTBBe@xps13> <1506813.gKiMgNqmq7@xps13> In-Reply-To: <1506813.gKiMgNqmq7@xps13> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4 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: Thu, 13 Oct 2016 10:47:28 -0000 Hi Thomas, > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Thursday, October 13, 2016 09:09 > To: Kulasek, TomaszX > Cc: dev@dpdk.org; Ananyev, Konstantin > Subject: Re: [dpdk-dev] [PATCH v4 1/6] ethdev: add Tx preparation >=20 > Hi Tomasz, >=20 > Any news? > Sorry to speed up, we are very very late for RC1. >=20 > 2016-10-10 16:08, Thomas Monjalon: > > Hi, > > > > Now that the feature seems to meet a consensus, I've looked at it more > > closely before integrating. Sorry if it appears like a late review. > > > > 2016-09-30 11:00, Tomasz Kulasek: > > > Added API for `rte_eth_tx_prep` > > > > I would love to read the usability and performance considerations here. > > No need for something as long as the cover letter. Just few lines > > about why it is needed and why it is a good choice for performance. > > > > > uint16_t rte_eth_tx_prep(uint8_t port_id, uint16_t queue_id, > > > struct rte_mbuf **tx_pkts, uint16_t nb_pkts) > > > > > > Added fields to the `struct rte_eth_desc_lim`: > > > > > > uint16_t nb_seg_max; > > > /**< Max number of segments per whole packet. */ > > > > > > uint16_t nb_mtu_seg_max; > > > /**< Max number of segments per one MTU */ > > [...] > > > +#else > > > + > > > +static inline uint16_t > > > +rte_eth_tx_prep(uint8_t port_id __rte_unused, uint16_t queue_id > __rte_unused, > > > + struct rte_mbuf **tx_pkts __rte_unused, uint16_t nb_pkts) > > > > Doxygen is failing here. > > Have you tried to move __rte_unused before the identifier? > > > > [...] > > > +#define PKT_TX_OFFLOAD_MASK ( \ > > > + PKT_TX_IP_CKSUM | \ > > > + PKT_TX_L4_MASK | \ > > > + PKT_TX_OUTER_IP_CKSUM | \ > > > + PKT_TX_TCP_SEG | \ > > > + PKT_TX_QINQ_PKT | \ > > > + PKT_TX_VLAN_PKT) > > > > We should really stop adding some public constants without the proper > > RTE prefix. > > And by the way, should not we move such flags into rte_net? >=20 > Do not worry with this comment. It was just a thought which could be > addressed in a separate patch by someone else. I've used this name convention to be consistent with other offload flag nam= es, and this is the only reason. The place of these flags was chosen for ea= sier management (flags and mask in one place). I will leave it as is. >=20 > > [...] > > > -SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include :=3D rte_ip.h rte_tcp.h > > > rte_udp.h rte_sctp.h rte_icmp.h rte_arp.h > > > +SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include :=3D rte_ip.h rte_tcp.h > > > +rte_udp.h rte_sctp.h rte_icmp.h rte_arp.h rte_pkt.h > > > > You can use the +=3D operator on a new line for free :) > > > > No more comments, the rest seems OK. Thanks >=20 Some additional work was needed due to the new tx tunnel type flags and cha= nges in csum engine. I will send v5 today. Tomasz