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 03815A32A8 for ; Sat, 26 Oct 2019 11:23:20 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 254641BEEA; Sat, 26 Oct 2019 11:23:20 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 0B8871BEDD for ; Sat, 26 Oct 2019 11:23:18 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Oct 2019 02:23:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,231,1569308400"; d="scan'208";a="202043981" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga003.jf.intel.com with ESMTP; 26 Oct 2019 02:23:17 -0700 Received: from fmsmsx605.amr.corp.intel.com (10.18.126.85) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sat, 26 Oct 2019 02:23:17 -0700 Received: from fmsmsx605.amr.corp.intel.com (10.18.126.85) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Sat, 26 Oct 2019 02:23:17 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Sat, 26 Oct 2019 02:23:16 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.176]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.166]) with mapi id 14.03.0439.000; Sat, 26 Oct 2019 17:23:15 +0800 From: "Wang, Haiyue" To: Thomas Monjalon CC: Jerin Jacob , "Yigit, Ferruh" , dpdk-dev , "Ye, Xiaolong" , "Kinsella, Ray" , "Iremonger, Bernard" , "Sun, Chenmin" , Andrew Rybchenko , "Slava Ovsiienko" , Stephen Hemminger , David Marchand , Jerin Jacob Thread-Topic: [dpdk-dev] [PATCH v4 1/4] ethdev: add the API for getting burst mode information Thread-Index: AQHVgy4vnIA7MA5UA0Sl3pLNFwDOY6dqovQAgABMCACAABr4gIAABOkAgABrZQCAAOrKcP//moGAgAC0blA= Date: Sat, 26 Oct 2019 09:23:14 +0000 Message-ID: References: <20191015075133.38560-1-haiyue.wang@intel.com> <12001140.UMXFOKstgs@xps> <4392076.GEpbuJ9hXI@xps> In-Reply-To: <4392076.GEpbuJ9hXI@xps> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiODcwYzU3YWMtNWZhMy00OTY2LTg0YWItNzFmNDA2MGFmZDJjIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoid25CUXo3YnZPVXVRNmE0Qk5WMVpzKzBLbDBMbFAyVnBjOFpXXC9IdUlJOXlLOURxZGF3VWNYRmUyd0pqaW5hQXcifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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" > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Saturday, October 26, 2019 14:24 > To: Wang, Haiyue > Cc: Jerin Jacob ; Yigit, Ferruh ; dpdk-dev > ; Ye, Xiaolong ; Kinsella, Ray ; > Iremonger, Bernard ; Sun, Chenmin ; Andrew > Rybchenko ; Slava Ovsiienko ; Stephen Hemminger > ; David Marchand ;= Jerin Jacob > > Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add the API for getting bu= rst mode information >=20 > 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 ethd= ev API > > > > > > >> that allows an application to retrieve the mode information = about > > > > > > >> Rx/Tx packet burst such as Scalar or Vector, and Vector tech= nology > > > > > > >> like AVX2. > > > > > > > > > > > > > > I missed this patch. I and Andrew, maintainers of ethdev, wer= e 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 multipl= e discussions, > > > > > > patch is out since the August, +1 to cc all maintainers I misse= d that part, > > > > > > but when the patch is reviewed and there is no objection, why b= lock 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 fea= ture. > > > > > > > > > > >> +/** > > > > > > >> + * Ethernet device RX/TX queue packet burst mode informatio= n structure. > > > > > > >> + * Used to retrieve information about packet burst mode set= ting. > > > > > > >> + */ > > > > > > >> +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 str= ucture. > > > > and when a pass to the application, let common code make final stri= ng 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 info= rmation */ > > > > }; > > > > > > 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 ru= nning on a wide > > variety of CPU architectures." >=20 > 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. >=20 > > 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 & ot= her *generic* > > technology. And the application can loop to check which kind of burst i= s running by > > just simple bit test. > > > > If PMDs missed these, they can update them in future roadmaps to enhanc= e their PMDs, > > like MLX5 supports ARM NEON, x86 SSE. >=20 > I have no word! > You really think other PMDs should learn from Intel how to "enhance" thei= r PMD? > You talk about mlx5, did you look at its code? Did you see the burst mode= s > depending on which specific hardware path is used (MPRQ, EMPW, inline)? > Or depending on which offloads are handled? >=20 > Again, the instruction set used by the function is a small part > of the burst mode optimization. >=20 > So you did not reply to my question: > Why not just returning a name for the selected Rx/Tx mode? >=20 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. Well, the *name* here we used is CPU related, at least, it is common. But not "specific hardware path is used (MPRQ, EMPW, inline)". If the API is not bad, but the data is not so good, then accept below ? ;-) struct eth_pmd_burst_mode { uint64_t options; /* Generic vector etc. */ char name[128]; /* PMD specific burst mode information */ }; =3D=3D=3D https://patchwork.dpdk.org/patch/57626/ https://patchwork.dpdk.org/patch/57642/ https://patchwork.dpdk.org/patch/57644/ +int +ice_rx_burst_info_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue= _id, + char *buf, int sz) +{ + int len =3D 0; + + if (dev->rx_pkt_burst =3D=3D ice_recv_scattered_pkts) + len =3D snprintf(buf, sz, "Scattered Rx"); + else if (dev->rx_pkt_burst =3D=3D ice_recv_pkts_bulk_alloc) + len =3D snprintf(buf, sz, "Bulk Rx"); + else if (dev->rx_pkt_burst =3D=3D ice_recv_pkts) + len =3D snprintf(buf, sz, "Normal Rx"); +#ifdef RTE_ARCH_X86 + else if (dev->rx_pkt_burst =3D=3D ice_recv_scattered_pkts_vec_avx2) + len =3D snprintf(buf, sz, "AVX2 Vector Scattered Rx"); + else if (dev->rx_pkt_burst =3D=3D ice_recv_scattered_pkts_vec) + len =3D snprintf(buf, sz, "Vector Scattered Rx"); + else if (dev->rx_pkt_burst =3D=3D ice_recv_pkts_vec_avx2) + len =3D snprintf(buf, sz, "AVX2 Vector Rx"); + else if (dev->rx_pkt_burst =3D=3D ice_recv_pkts_vec) + len =3D snprintf(buf, sz, "Vector Rx"); +#endif + + if (len >=3D sz) + len =3D -ENOSPC; /* The output was truncated */ + + return len; +}