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 4E33458CF for ; Tue, 20 Sep 2016 11:06:50 +0200 (CEST) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga103.fm.intel.com with ESMTP; 20 Sep 2016 02:06:49 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,367,1470726000"; d="scan'208";a="11157624" Received: from irsmsx152.ger.corp.intel.com ([163.33.192.66]) by fmsmga006.fm.intel.com with ESMTP; 20 Sep 2016 02:06:48 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.196]) by IRSMSX152.ger.corp.intel.com ([169.254.6.13]) with mapi id 14.03.0248.002; Tue, 20 Sep 2016 10:06:43 +0100 From: "Ananyev, Konstantin" To: Jerin Jacob , "Kulasek, TomaszX" CC: "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v2 1/6] ethdev: add Tx preparation Thread-Index: AQHSDQSdbAMPwzKA5ECqoX7z1xdg36CAxzYwgAAjHICAAApzgIAAQB+g Date: Tue, 20 Sep 2016 09:06:42 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772583F0B8F33@irsmsx105.ger.corp.intel.com> References: <1472228578-6980-1-git-send-email-tomaszx.kulasek@intel.com> <1473691487-10032-1-git-send-email-tomaszx.kulasek@intel.com> <1473691487-10032-2-git-send-email-tomaszx.kulasek@intel.com> <2601191342CEEE43887BDE71AB9772583F0B583F@irsmsx105.ger.corp.intel.com> <3042915272161B4EB253DA4D77EB373A14F1A294@IRSMSX102.ger.corp.intel.com> <20160919160630.GA18610@localhost.localdomain> In-Reply-To: <20160919160630.GA18610@localhost.localdomain> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2 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: Tue, 20 Sep 2016 09:06:50 -0000 Hi Jerin, > > > > > > > > [...] > > > > > > + > > > > +#ifdef RTE_ETHDEV_TX_PREP > > > > > > Sorry for being a bit late on that discussion, but what the point of > > > having that config macro (RTE_ETHDEV_TX_PREP ) at all? > > > As I can see right now, if driver doesn't setup tx_pkt_prep, then > > > nb_pkts would be return anyway... > > > > > > BTW, there is my another question - should it be that way? > > > Shouldn't we return 0 (and set rte_errno=3DENOTSUP) here if > > > dev->tx_pk_prep =3D=3D NULL? > > > > > > > It's an answer to the Jerin's request discussed here: > > http://dpdk.org/ml/archives/dev/2016-September/046437.html > > > > When driver doesn't support tx_prep, default behavior is "we don't know= requirements, so we have nothing to do here". It will simplify > application logic and improve performance for these drivers, I think. Cat= ching this error with every burst may be problematic. > > > > As for RTE_ETHDEV_TX_PREP macro, suggested by Jerin in the same thread,= I still don't think It's the best solution of the problem > described by him. I have added it here for further discussion. > > > > Jerin, have you something to add? >=20 > Nothing very specific to add here. I think, I have tried to share the rat= ional in, http://dpdk.org/ml/archives/dev/2016- > September/046437.html >=20 Ok, not sure I am fully understand your intention here. I think I understand why you propose rte_eth_tx_prep() to do: if (!dev->tx_pkt_prep) return nb_pkts; That allows drivers to NOOP the tx_prep functionality without paying the price for callback invocation. What I don't understand, why with that in place we also need a NOOP for the whole rte_eth_tx_prep(): +static inline uint16_t +rte_eth_tx_prep(uint8_t port_id __rte_unused, uint16_t queue_id __rte_unus= ed, + struct rte_mbuf **tx_pkts __rte_unused, uint16_t nb_pkts) +{ + return nb_pkts; +} + +#endif What are you trying to save here: just reading ' dev->tx_pkt_prep'? If so, then it seems not that performance critical for me. Something else? >>From my point of view NOOP on the driver level is more than enough. Again I would prefer to introduce new config option, if possible. Konstantin