From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9CF86A04B7; Wed, 14 Oct 2020 11:30:10 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8A6F61DD88; Wed, 14 Oct 2020 11:29:59 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id E02531DD8C for ; Wed, 14 Oct 2020 11:29:57 +0200 (CEST) IronPort-SDR: cJCLE31/HvdI0SPQReWuUJVT+voKL+JfLN2TQDiBqozT17KgZOvWSE56bnp/S80ola19duFc2J uw3cxkgX/KEQ== X-IronPort-AV: E=McAfee;i="6000,8403,9773"; a="162604881" X-IronPort-AV: E=Sophos;i="5.77,374,1596524400"; d="scan'208";a="162604881" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Oct 2020 02:29:56 -0700 IronPort-SDR: TR5KRAccGKXPCv3fhO7NY/EDp7FGhygFH/vPfFC+b1wSYXDkUwgjDiJgZM4iU8ocotkezSHLdH H8S7tRLdybeg== X-IronPort-AV: E=Sophos;i="5.77,374,1596524400"; d="scan'208";a="463818543" Received: from bricha3-mobl.ger.corp.intel.com ([10.254.145.91]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 14 Oct 2020 02:29:52 -0700 Date: Wed, 14 Oct 2020 10:29:46 +0100 From: Bruce Richardson To: Thomas Monjalon Cc: Ferruh Yigit , Morten =?iso-8859-1?Q?Br=F8rup?= , arybchenko@solarflare.com, jia.guo@intel.com, dev@dpdk.org Message-ID: <20201014092946.GD1513@bricha3-MOBL.ger.corp.intel.com> References: <20200914110511.95609-1-mb@smartsharesystems.com> <60d87dbf-167e-d0a6-781a-5f54ee5cb5ac@intel.com> <98CBD80474FA8B44BF855DF32C47DC35C6136A@smartserver.smartshare.dk> <2517144.VUf8PMuDld@thomas> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <2517144.VUf8PMuDld@thomas> Subject: Re: [dpdk-dev] [PATCH] ethdev: rte_eth_rx_burst()nb_pktsrequirements X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Oct 14, 2020 at 10:53:24AM +0200, Thomas Monjalon wrote: > 14/10/2020 10:26, Morten Brørup: > > From: Ferruh Yigit > > > On 9/14/2020 1:42 PM, Morten Brørup wrote: > > > > From: Bruce Richardson > > > >> On Mon, Sep 14, 2020 at 01:05:11PM +0200, Morten Brørup wrote: > > > >>> Updated description of rte_eth_rx_burst() to reflect what drivers, > > > >>> when using vector instructions, expect from nb_pkts. > > > >>> > > > >>> Also discussed on the mailing list here: > > > >>> > > > >> http://inbox.dpdk.org/dev/98CBD80474FA8B44BF855DF32C47DC35C61257@smarts > > > >> erver.smartshare.dk/ > > > >>> > > > >>> --- a/lib/librte_ethdev/rte_ethdev.h > > > >>> +++ b/lib/librte_ethdev/rte_ethdev.h > > > >>> @@ -4469,6 +4469,10 @@ int > > > >> rte_eth_dev_hairpin_capability_get(uint16_t port_id, > > > >>> * burst-oriented optimizations in both synchronous and asynchronous > > > >>> * packet processing environments with no overhead in both cases. > > > >>> * > > > >>> + * @note > > > >>> + * Some drivers using vector instructions require that *nb_pkts* > > > >> is > > > >>> + * divisible by 4 or 8, depending on the driver implementation. > > > >>> + * > > > >> > > > >> Not technically true, in that the drivers will round the value down to > > > >> the > > > >> nearest multiple of 4 or 8. So how about rewording as: > > > >> > > > >> "Some drivers using vector instructions may round the *nb_pkts* driver > > > >> to > > > >> a multiple of 4 or 8 depending upon the driver implementation." > > > >> > > > > > > > > You are correct about the driver behavior. > > > > > > > > However, if you pass nb_pkts=9, the driver will return 8 packets, > > > > and thus it does not conform to the API behavior of returning nb_pkts > > > > if they are there. > > > > > > > > This is why the description in this patch differs from the description we > > > reached in the RFC discussion. > > > > > > > > > > Hi Morten, Bruce, > > > > > > +1 to document the this behavior. > > > > > > But in the patch the wording is more strict: > > > "... require that *nb_pkts* is divisible by 4 or 8 ..." > > > "... The value must be divisible by 8 in order to work with any driver." > > > > > > I am not sure the requirement is that strict. Application still provide any > > > value for 'nb_pkts', so the value doesn't "have to" be divisible 8/4. > > > > > > But for vector PMD case it will return number of packets round down to 8/4. > > > Perhaps can add for vector PMD it must be at least 4/8? > > > > > > Bruce's explanation sound more accurate to me, what do you think? > > > > > > > I aim to keep the explanation in the documentation relatively simple. Keep the parameter description short, and add the details about vector driver behavior as a note to the function. > > > > The reason for all this is the existing documentation describing how to use the rte_eth_rx_burst() function at high level: > > > > The rte_eth_rx_burst() function returns the number of packets actually retrieved [...]. A return value equal to nb_pkts indicates [...] that other received packets remain in the input queue. Applications implementing a "retrieve as much received packets as possible" policy can check this specific case and keep invoking the rte_eth_rx_burst() function until a value less than nb_pkts is returned. > > > > As an alternative to my proposed solution, we could add that vector drivers round down to 4 or 8, and the application's comparison of the nb_pkts and return value must consider this. But that would make the above description strangely complex, rather than just requiring that nb_pkts for vector drivers must be divisible by 4 or 8. > > > > And as a minor detail, keeping my proposed restriction would also eliminate the vector drivers' need to round down. > > > > I don't see a need to be able to call rte_eth_rx_burst() with a value not divisible by 4 or 8 for a vector driver, so my proposed restriction is a tradeoff favoring simplicity over unnecessary flexibility. > > It makes sense to me. > That sounds reasonable for what we have now. We just need to standardize on either 4 or 8 as the required factor of the input size. I would suggest having it as 4, and look to put in fallback paths for the few drivers which don't support less than 8. I think that 8 is too large a min burst size to support, for any apps that want small bursts for lower latency. /Bruce