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 43324A04A2; Wed, 6 Nov 2019 02:22:02 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 833D51BFC3; Wed, 6 Nov 2019 02:22:01 +0100 (CET) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 0DA501BFC0 for ; Wed, 6 Nov 2019 02:21:58 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Nov 2019 17:21:57 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,272,1569308400"; d="scan'208";a="227329300" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga004.fm.intel.com with ESMTP; 05 Nov 2019 17:21:57 -0800 Received: from fmsmsx602.amr.corp.intel.com (10.18.126.82) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 5 Nov 2019 17:21:57 -0800 Received: from fmsmsx602.amr.corp.intel.com (10.18.126.82) by fmsmsx602.amr.corp.intel.com (10.18.126.82) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Tue, 5 Nov 2019 17:21:56 -0800 Received: from shsmsx153.ccr.corp.intel.com (10.239.6.53) by fmsmsx602.amr.corp.intel.com (10.18.126.82) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Tue, 5 Nov 2019 17:21:56 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.108]) by SHSMSX153.ccr.corp.intel.com ([169.254.12.215]) with mapi id 14.03.0439.000; Wed, 6 Nov 2019 09:21:54 +0800 From: "Wang, Haiyue" To: Thomas Monjalon CC: "dev@dpdk.org" , "jerinjacobk@gmail.com" , "Yigit, Ferruh" , "arybchenko@solarflare.com" , "viacheslavo@mellanox.com" , "damarion@cisco.com" , "Ye, Xiaolong" , "Sun, Chenmin" , "Kinsella, Ray" , "Liu, Yu Y" Thread-Topic: [dpdk-dev] [PATCH v2] ethdev: enhance the API for getting burst mode information Thread-Index: AQHVkv0KxvY/V6FXTUaxieDROzbUpad8x5cAgACMNeA= Date: Wed, 6 Nov 2019 01:21:54 +0000 Message-ID: References: <20191104103920.64907-1-haiyue.wang@intel.com> <2399300.Xf6G4o05WB@xps> In-Reply-To: <2399300.Xf6G4o05WB@xps> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiY2YzNzRmMGEtNzc0Mi00ZGE1LWJkY2UtZDU5YjE5YTgxYWY0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoicFhPTGVLQlhWZVJUU1JIb3pKZHlEUUp5dlRTaWV0NU9wRmgrMmpvd295YVpuVjdZd3BjSVwvNFk0TnRBdUk1dUYifQ== 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 v2] 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" Hi Thomas, > -----Original Message----- > From: Thomas Monjalon > Sent: Wednesday, November 6, 2019 08:34 > To: Wang, Haiyue > Cc: dev@dpdk.org; jerinjacobk@gmail.com; Yigit, Ferruh ; > arybchenko@solarflare.com; viacheslavo@mellanox.com; damarion@cisco.com; = Ye, Xiaolong > ; Sun, Chenmin ; Kinsella, = Ray ; > Liu, Yu Y > Subject: Re: [dpdk-dev] [PATCH v2] ethdev: enhance the API for getting bu= rst mode information >=20 > 04/11/2019 11:39, Haiyue Wang: > > Change the type of burst mode information from bit field to free string > > data, so that each PMD can describe the Rx/Tx busrt functions flexibly. > > > > Signed-off-by: Haiyue Wang > > --- > > > > v2: - Drop the bit field for burst mode information handling. >=20 > Please use --in-reply-to, so the versions of a patch can be in the same t= hread. >=20 Will take care next time. > > --- a/lib/librte_ethdev/rte_ethdev.h > > +++ b/lib/librte_ethdev/rte_ethdev.h > > /** > > - * Burst mode types, values can be ORed to define the burst mode of a = driver. > > + * Generic Burst mode flag definition, values can be ORed. > > + */ > > +#define RTE_ETH_BURST_FLAG_PER_QUEUE (1ULL << 0) > > +/**< If the queues have different burst mode description, this bit wil= l be set > > + * by PMD, then the application can iterate to retrieve burst descript= ion for > > + * all other queues. > > */ >=20 > I am not sure you can have a doxygen comment before and after the same it= em. >=20 The first is for all flags, but only one now, so looks like for the same it= em. The second is just for RTE_ETH_BURST_FLAG_PER_QUEUE flag. > > -enum rte_eth_burst_mode_option { > > - RTE_ETH_BURST_SCALAR =3D (1 << 0), > > - RTE_ETH_BURST_VECTOR =3D (1 << 1), > > - > > - /**< bits[15:2] are reserved for each vector type */ > > - RTE_ETH_BURST_ALTIVEC =3D (1 << 2), > > - RTE_ETH_BURST_NEON =3D (1 << 3), > > - RTE_ETH_BURST_SSE =3D (1 << 4), > > - RTE_ETH_BURST_AVX2 =3D (1 << 5), > > - RTE_ETH_BURST_AVX512 =3D (1 << 6), > > - > > - RTE_ETH_BURST_SCATTERED =3D (1 << 16), /**< Support scattered packets= */ > > - RTE_ETH_BURST_BULK_ALLOC =3D (1 << 17), /**< Support mbuf bulk alloc = */ > > - RTE_ETH_BURST_SIMPLE =3D (1 << 18), > > - > > - RTE_ETH_BURST_PER_QUEUE =3D (1 << 19), /**< Support per queue burst *= / > > -}; >=20 > Thank you >=20 > > /** > > * 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; > > + uint64_t flags; /**< The ORed values of RTE_ETH_BURST_FLAG_xxx */ > > + > > +#define RTE_ETH_BURST_MODE_INFO_SIZE 1024 /**< Maximum size for inform= ation */ > > + char info[RTE_ETH_BURST_MODE_INFO_SIZE]; /**< burst mode information = */ > > }; >=20 > I think the API can be simpler by passing the flags as function parameter= . >=20 > In my understanding the burst mode name is fixed per Rx/Tx function, > so it can be a constant string referenced with a simple char*. >=20 > This is the current API: >=20 > int rte_eth_rx_burst_mode_get(uint16_t port_id, uint16_t queue_id, > struct rte_eth_burst_mode *mode); >=20 > I wonder what do you think of such API? (just a proposal for comments): >=20 > char *rte_eth_rx_burst_mode_get(uint16_t port_id, uint16_t queue_id, uint= 64_t flags); >=20 > Or is there some cases where you want to build the string with snprintf? > (I cannot think about a case, given it should mapped to a C-function) >=20 1. 'a constant string' is hard for PMD expanding if it wants to make the st= ring dynamic according to the setting, like: http://patchwork.dpdk.org/patch/= 62352/ (although based on bit options design). 2. And for dynamic string, if it is *return type*, then the PMD needs to handle the memory allocation, and the application frees it. And 'uint64_= t flags' is output parameter, so it should be like 'uint64_t *flags', but this ne= eds the application to declare it or not, and needs PMDs to check whether it is = passed or not, then set it. So for making things easy, the 'struct rte_eth_burst_mode' may be nice, = then the application just declares one line : 'struct rte_eth_burst_mode mode', t= hen all things are filled by PMD in one place. >=20