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 B3B37A00BE; Tue, 29 Oct 2019 15:27:54 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 39E931BEB7; Tue, 29 Oct 2019 15:27:54 +0100 (CET) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 2AB441BEB6 for ; Tue, 29 Oct 2019 15:27:52 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 Oct 2019 07:27:51 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,244,1569308400"; d="scan'208";a="202859766" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.9]) ([10.237.221.9]) by orsmga003.jf.intel.com with ESMTP; 29 Oct 2019 07:27:48 -0700 To: Thomas Monjalon , "Wang, Haiyue" Cc: Jerin Jacob , dpdk-dev , "Ye, Xiaolong" , "Kinsella, Ray" , "Iremonger, Bernard" , "Sun, Chenmin" , Andrew Rybchenko , Slava Ovsiienko , Stephen Hemminger , David Marchand , Jerin Jacob , edwin.verplanke@intel.com References: <20191015075133.38560-1-haiyue.wang@intel.com> <4392076.GEpbuJ9hXI@xps> <14832014.5GoLVrWRzY@xps> From: Ferruh Yigit Openpgp: preference=signencrypt Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata= mQINBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABtCVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+iQJUBBMBCgA+AhsDAh4BAheABQsJCAcDBRUK CQgLBRYCAwEAFiEE0jZTh0IuwoTjmYHH+TPrQ98TYR8FAl1meboFCQlupOoACgkQ+TPrQ98T YR9ACBAAv2tomhyxY0Tp9Up7mNGLfEdBu/7joB/vIdqMRv63ojkwr9orQq5V16V/25+JEAD0 60cKodBDM6HdUvqLHatS8fooWRueSXHKYwJ3vxyB2tWDyZrLzLI1jxEvunGodoIzUOtum0Ce gPynnfQCelXBja0BwLXJMplM6TY1wXX22ap0ZViC0m714U5U4LQpzjabtFtjT8qOUR6L7hfy YQ72PBuktGb00UR/N5UrR6GqB0x4W41aZBHXfUQnvWIMmmCrRUJX36hOTYBzh+x86ULgg7H2 1499tA4o6rvE13FiGccplBNWCAIroAe/G11rdoN5NBgYVXu++38gTa/MBmIt6zRi6ch15oLA Ln2vHOdqhrgDuxjhMpG2bpNE36DG/V9WWyWdIRlz3NYPCDM/S3anbHlhjStXHOz1uHOnerXM 1jEjcsvmj1vSyYoQMyRcRJmBZLrekvgZeh7nJzbPHxtth8M7AoqiZ/o/BpYU+0xZ+J5/szWZ aYxxmIRu5ejFf+Wn9s5eXNHmyqxBidpCWvcbKYDBnkw2+Y9E5YTpL0mS0dCCOlrO7gca27ux ybtbj84aaW1g0CfIlUnOtHgMCmz6zPXThb+A8H8j3O6qmPoVqT3qnq3Uhy6GOoH8Fdu2Vchh TWiF5yo+pvUagQP6LpslffufSnu+RKAagkj7/RSuZV25Ag0EV9ZMvgEQAKc0Db17xNqtSwEv mfp4tkddwW9XA0tWWKtY4KUdd/jijYqc3fDD54ESYpV8QWj0xK4YM0dLxnDU2IYxjEshSB1T qAatVWz9WtBYvzalsyTqMKP3w34FciuL7orXP4AibPtrHuIXWQOBECcVZTTOdZYGAzaYzxiA ONzF9eTiwIqe9/oaOjTwTLnOarHt16QApTYQSnxDUQljeNvKYt1lZE/gAUUxNLWsYyTT+22/ vU0GDUahsJxs1+f1yEr+OGrFiEAmqrzpF0lCS3f/3HVTU6rS9cK3glVUeaTF4+1SK5ZNO35p iVQCwphmxa+dwTG/DvvHYCtgOZorTJ+OHfvCnSVjsM4kcXGjJPy3JZmUtyL9UxEbYlrffGPQ I3gLXIGD5AN5XdAXFCjjaID/KR1c9RHd7Oaw0Pdcq9UtMLgM1vdX8RlDuMGPrj5sQrRVbgYH fVU/TQCk1C9KhzOwg4Ap2T3tE1umY/DqrXQgsgH71PXFucVjOyHMYXXugLT8YQ0gcBPHy9mZ qw5mgOI5lCl6d4uCcUT0l/OEtPG/rA1lxz8ctdFBVOQOxCvwRG2QCgcJ/UTn5vlivul+cThi 6ERPvjqjblLncQtRg8izj2qgmwQkvfj+h7Ex88bI8iWtu5+I3K3LmNz/UxHBSWEmUnkg4fJl Rr7oItHsZ0ia6wWQ8lQnABEBAAGJAjwEGAEKACYCGwwWIQTSNlOHQi7ChOOZgcf5M+tD3xNh HwUCXWZ5wAUJB3FgggAKCRD5M+tD3xNhH2O+D/9OEz62YuJQLuIuOfL67eFTIB5/1+0j8Tsu o2psca1PUQ61SZJZOMl6VwNxpdvEaolVdrpnSxUF31kPEvR0Igy8HysQ11pj8AcgH0a9FrvU /8k2Roccd2ZIdpNLkirGFZR7LtRw41Kt1Jg+lafI0efkiHKMT/6D/P1EUp1RxOBNtWGV2hrd 0Yg9ds+VMphHHU69fDH02SwgpvXwG8Qm14Zi5WQ66R4CtTkHuYtA63sS17vMl8fDuTCtvfPF HzvdJLIhDYN3Mm1oMjKLlq4PUdYh68Fiwm+boJoBUFGuregJFlO3hM7uHBDhSEnXQr5mqpPM 6R/7Q5BjAxrwVBisH0yQGjsWlnysRWNfExAE2sRePSl0or9q19ddkRYltl6X4FDUXy2DTXa9 a+Fw4e1EvmcF3PjmTYs9IE3Vc64CRQXkhujcN4ZZh5lvOpU8WgyDxFq7bavFnSS6kx7Tk29/ wNJBp+cf9qsQxLbqhW5kfORuZGecus0TLcmpZEFKKjTJBK9gELRBB/zoN3j41hlEl7uTUXTI JQFLhpsFlEdKLujyvT/aCwP3XWT+B2uZDKrMAElF6ltpTxI53JYi22WO7NH7MR16Fhi4R6vh FHNBOkiAhUpoXRZXaCR6+X4qwA8CwHGqHRBfYFSU/Ulq1ZLR+S3hNj2mbnSx0lBs1eEqe2vh cA== Message-ID: Date: Tue, 29 Oct 2019 14:27:47 +0000 MIME-Version: 1.0 In-Reply-To: <14832014.5GoLVrWRzY@xps> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add 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" On 10/26/2019 5:23 PM, Thomas Monjalon wrote: > 26/10/2019 11:23, Wang, Haiyue: >> From: Thomas Monjalon [mailto:thomas@monjalon.net] >>> 26/10/2019 06:40, Wang, Haiyue: >>>> From: Thomas Monjalon [mailto:thomas@monjalon.net] >>>>> 25/10/2019 18:02, Jerin Jacob: >>>>>> On Fri, Oct 25, 2019 at 9:15 PM Thomas Monjalon wrote: >>>>>>> 25/10/2019 16:08, Ferruh Yigit: >>>>>>>> On 10/25/2019 10:36 AM, Thomas Monjalon wrote: >>>>>>>>> 15/10/2019 09:51, Haiyue Wang: >>>>>>>>>> Some PMDs have more than one RX/TX burst paths, add the ethdev API >>>>>>>>>> that allows an application to retrieve the mode information about >>>>>>>>>> Rx/Tx packet burst such as Scalar or Vector, and Vector technology >>>>>>>>>> like AVX2. >>>>>>>>> >>>>>>>>> I missed this patch. I and Andrew, maintainers of ethdev, were not CC'ed. >>>>>>>>> Ferruh, I would expect to be Cc'ed and/or get a notification before merging. >>>>>>>> >>>>>>>> It has been discussed in the mail list and went through multiple discussions, >>>>>>>> patch is out since the August, +1 to cc all maintainers I missed that part, >>>>>>>> but when the patch is reviewed and there is no objection, why block the merge? >>>>>>> >>>>>>> I'm not saying blocking the merge. >>>>>>> My bad is that I missed the patch and I am asking for help with a notification >>>>>>> in this case. Same for Andrew I guess. >>>>>>> Note: it is merged in master and I am looking to improve this feature. >>>>>> >>>>>>>>>> +/** >>>>>>>>>> + * Ethernet device RX/TX queue packet burst mode information structure. >>>>>>>>>> + * Used to retrieve information about packet burst mode setting. >>>>>>>>>> + */ >>>>>>>>>> +struct rte_eth_burst_mode { >>>>>>>>>> + uint64_t options; >>>>>>>>>> +}; >>>>>>>>> >>>>>>>>> Why a struct for an integer? >>>>>>>> >>>>>>>> Again by a request from me, to not need to break the API if we need to add more >>>>>>>> thing in the future. >>>>>>> >>>>>>> I would replace it with a string. This is the most flexible API. >>>>>> >>>>>> IMO, Probably, best of both worlds make a good option here, >>>>>> as Haiyue suggested if we have an additional dev_specific[1] in structure. >>>>>> and when a pass to the application, let common code make final string as >>>>>> (options flags to string + dev_specific) >>>>>> >>>>>> options flag can be zero if PMD does not have any generic flags nor >>>>>> interested in such a scheme. >>>>>> Generic flags will help at least to have some common code. >>>>>> >>>>>> [1] >>>>>> struct rte_eth_burst_mode { >>>>>> uint64_t options; >>>>>> char dev_specific[128]; /* PMD has specific burst mode information */ >>>>>> }; >>>>> >>>>> I really don't see how we can have generic flags. >>>>> The flags which are proposed are just matching >>>>> the functions implemented in Intel PMDs. >>>>> And this is a complicate solution. >>>>> Why not just returning a name for the selected Rx/Tx mode? >>>> >>>> Intel PMDs use the *generic* methods like x86 SSE, AVX2, ARM NEON, PPC ALTIVEC, >>>> 'dev->data->scattered_rx' etc for the target : "DPDK is the Data Plane Development Kit >>>> that consists of libraries to accelerate packet processing workloads running on a wide >>>> variety of CPU architectures." >>> >>> How RTE_ETH_BURST_SCATTERED and RTE_ETH_BURST_BULK_ALLOC are generic? >>> They just match some features of the Intel PMDs. >>> Why not exposing other optimizations of the Rx/Tx implementations? >>> You totally missed the point of generic burst mode description. >>> >>>> If understand these new experimental APIs from above, then bit options is the best, >>>> and we didn't invent new words to describe them, just from the CPU & other *generic* >>>> technology. And the application can loop to check which kind of burst is running by >>>> just simple bit test. >>>> >>>> If PMDs missed these, they can update them in future roadmaps to enhance their PMDs, >>>> like MLX5 supports ARM NEON, x86 SSE. >>> >>> I have no word! >>> You really think other PMDs should learn from Intel how to "enhance" their PMD? >>> You talk about mlx5, did you look at its code? Did you see the burst modes >>> depending on which specific hardware path is used (MPRQ, EMPW, inline)? >>> Or depending on which offloads are handled? >>> >>> Again, the instruction set used by the function is a small part >>> of the burst mode optimization. >>> >>> So you did not reply to my question: >>> Why not just returning a name for the selected Rx/Tx mode? >> >> In fact, RFC v1/v2 returns the *name*, but the *name* is hard for >> application to do further processing, strcmp, strstr ? Not so nice >> for C code, and it is not so standard, So switch it to bit definition. > > Again, please answer my question: why do you need it? > I think it is just informative, that's why a string should be enough. > I am clearly against the bitmap because it is way too much restrictive. > I disagree that knowing it is using AVX2 or AVX512 is so interesting. > What you would like to know is whether it is processing packets 4 by 4, > for instance, or to know which offload is supported, or what hardware trick > is used in the datapath design. > There are so many options in a datapath design that it cannot be > represented with a bitmap. And it makes no sense to have some design > criterias more important than others. > I Cc an Intel architect (Edwin) who could explain you how much > a datapath design is more complicate than just using AVX instructions. As I understand this is to let applications to give informed decision based on what vectorization is used in the driver, currently this is not know by the application. And as previously replied, the main target of the API is to define the vector path, not all optimizations, so the number is limited. There are many optimization in the data path, I agree we may not represent all of them, and agreed existing enum having "RTE_ETH_BURST_BULK_ALLOC" and similar causing this confusion, perhaps we can remove them. And if the requirement from the application is just informative, I would agree that free text string will be better, right now 'rte_eth_rx/tx_burst_mode_get()' is the main API to provide the information and 'rte_eth_burst_mode_option_name()' is a helper for application/driver to log this information.