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 BA231A0352; Sun, 3 Nov 2019 23:20:56 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id F3C9B1BE91; Sun, 3 Nov 2019 23:20:55 +0100 (CET) Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by dpdk.org (Postfix) with ESMTP id 3CB631BE8B for ; Sun, 3 Nov 2019 23:20:55 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 7BD8A202F2; Sun, 3 Nov 2019 17:20:54 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Sun, 03 Nov 2019 17:20:54 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=RocySM2U3lM8MCpRJn6tBCGxPcyRUul5d5l/MeR+6j8=; b=Z0GLMqqsHDNj LwiccengH+R4/Pi39cnlEoTVfXwlBqgBJOO8uJwYHyWjXfkahmpGHoS1NJ035yaQ i8EUE03sJc14QjLLeMGhinOXMJcQ3C0wF0vLbP8WAVJzPse7S04jVvhzsd9oI7MX 1puLvz6iYN/eoHE7TSo2jK90Hz1+w80= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=RocySM2U3lM8MCpRJn6tBCGxPcyRUul5d5l/MeR+6 j8=; b=ugwWuduLdK9YpTj3Pt3VJwwZqycH3uBxL2jM+FobSQhC5ZlClaGIu13vj Z93Xs1VVUCnKqiIX1ELqyrV1isbiGAtiT4XNprDIFMXVStr7rlGpBgapAyOQMpav SEoC5ug6VIcIWxGBOPSbfjk3WVu0mAxoffld1xJNOqIH1aZv2MLmdDX7xqE1l8AW RaTHbyF5nqHNKjPhSJuULdW4fRKemzKnlmXU8di58TH4AElBafGc4UaDWzrTFVEw Yyz7ZrjP4rldp2sxHNjEcHeZj3HpCIlnoESAtoestYG0tD1e+f2Kg8bPr9WM0JXR apZYeJ0ByAZ2k125sJJKKFjfwdscw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedufedrudduuddgudeigecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc fkphepleefrddvfedrvdegkedrvddtkeenucfrrghrrghmpehmrghilhhfrhhomhepthhh ohhmrghssehmohhnjhgrlhhonhdrnhgvthenucevlhhushhtvghrufhiiigvpedt X-ME-Proxy: Received: from xps.localnet (208.248.23.93.rev.sfr.net [93.23.248.208]) by mail.messagingengine.com (Postfix) with ESMTPA id A7030306005B; Sun, 3 Nov 2019 17:20:51 -0500 (EST) From: Thomas Monjalon To: "Wang, Haiyue" Cc: "dev@dpdk.org" , "arybchenko@solarflare.com" , "Yigit, Ferruh" , "jerinjacobk@gmail.com" , "Ye, Xiaolong" , "Kinsella, Ray" , "Sun, Chenmin" , Slava Ovsiienko Date: Sun, 03 Nov 2019 23:20:49 +0100 Message-ID: <65534969.fODW5VeCLa@xps> In-Reply-To: References: <20191031171139.105110-1-haiyue.wang@intel.com> <20693558.VL3dRorq05@xps> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v1 3/3] ethdev: enhance the API for getting burst mode information 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" 03/11/2019 04:52, Wang, Haiyue: > Hi Thomas, > > Reply the missed: > > From: Thomas Monjalon > > > > Thank you for trying to address comments done late. > > > > 31/10/2019 18:11, Haiyue Wang: > > > --- a/lib/librte_ethdev/rte_ethdev.h > > > +++ b/lib/librte_ethdev/rte_ethdev.h > > > -enum rte_eth_burst_mode_option { > > > - RTE_ETH_BURST_SCALAR = (1 << 0), > > > - RTE_ETH_BURST_VECTOR = (1 << 1), > > > - > > > - /**< bits[15:2] are reserved for each vector type */ > > > - RTE_ETH_BURST_ALTIVEC = (1 << 2), > > > - RTE_ETH_BURST_NEON = (1 << 3), > > > - RTE_ETH_BURST_SSE = (1 << 4), > > > - RTE_ETH_BURST_AVX2 = (1 << 5), > > > - RTE_ETH_BURST_AVX512 = (1 << 6), > > > - > > > - RTE_ETH_BURST_SCATTERED = (1 << 16), /**< Support scattered packets */ > > > - RTE_ETH_BURST_BULK_ALLOC = (1 << 17), /**< Support mbuf bulk alloc */ > > > - RTE_ETH_BURST_SIMPLE = (1 << 18), > > > - > > > - RTE_ETH_BURST_PER_QUEUE = (1 << 19), /**< Support per queue burst */ > > > -}; > > > +#define RTE_ETH_BURST_SCALAR (1ULL << 0) > > > +#define RTE_ETH_BURST_VECTOR (1ULL << 1) > > > > Only one bit is needed: if it is not vector, it is scalar. > > I think you can remove the scalar bit. > > > > 1. 'options' can be all zero, so we can't assume it is 'scalar', so let's the PMD > set it explicitly. So what means zero? neither scalar nor vector? > 2. As replied before, it seems a little redundant, but a bit-opaque definition > will make string format easily, like a direct 'for' loop: > ---------------------------------- > static const struct { > uint64_t option; > const char *name; > } rte_burst_option_names[] = { > { RTE_ETH_BURST_SCALAR, "Scalar" }, > { RTE_ETH_BURST_VECTOR, "Vector" }, > > { RTE_ETH_BURST_ALTIVEC, "AltiVec" }, > { RTE_ETH_BURST_NEON, "Neon" }, > ---------------------------------- > > And in fact, only one vector type will be returned, but if use the bit save definition > like register, there will be something like 'RTE_ETH_BURST_VERTOR_TYPE_MASK', this will > make PMD and string format harder ... I don't understand what is hard in parsing a bit. But I'm probably not skilled enough. Also I would be interested to understand what means scalar exactly here? It is processing packets one by one? And vector, does it mean four packets at once? How can I differentiate a function which is processing packets two packets at once? > > > +/**< bits[15:2] are reserved for each vector type */ > > > > Why 15:2 instead of 2:15? > > Changed as: > /**< bits 2 ~ 15 are reserved for each vector type */ > > > > +#define RTE_ETH_BURST_ALTIVEC (1ULL << 2) > > > +#define RTE_ETH_BURST_NEON (1ULL << 3) > > > +#define RTE_ETH_BURST_SSE (1ULL << 4) > > > +#define RTE_ETH_BURST_AVX2 (1ULL << 5) > > > +#define RTE_ETH_BURST_AVX512 (1ULL << 6) > > > > Of course, I still believe that giving a special treatment > > to vector instructions is wrong. > > You did not justify why it needs to be defined in bits > > instead of string. I am not asking again because anyway you > > don't really reply. I think you are executing an order you received > > and I don't want to blame you more. > > I suspect a real hidden issue in Intel CPUs that you try to mitigate. > > No need to reply to this comment. > > Anyway I will propose to replace this API in the next release. > > > > > +/**< Support per queue burst */ > > > +#define RTE_ETH_BURST_PER_QUEUE (1ULL << 63) > > > > This comment is meaningless. > > If this bit has a usage, please explain how to use this bit > > in the comment. > > > > Changes as: > > /**< If the Rx/Tx queues have different burst mode description, the bit will be > * set by PMD, then the application can enumerate to retrive burst description > * for all PMD queues. > */ OK this is a better description, thanks. Indeed this parameter looks to be a good idea. If we would drop this bit-field, the per-queue bit could be a parameter of the function.