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 5055EA32A4 for ; Fri, 25 Oct 2019 16:00:02 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B588F1C2A4; Fri, 25 Oct 2019 16:00:01 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 9128D1C222 for ; Fri, 25 Oct 2019 15:59:59 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Oct 2019 06:59:58 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,228,1569308400"; d="scan'208";a="373563575" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga005.jf.intel.com with ESMTP; 25 Oct 2019 06:59:58 -0700 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 25 Oct 2019 06:59:57 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx110.amr.corp.intel.com (10.18.116.10) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 25 Oct 2019 06:59:57 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.176]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.96]) with mapi id 14.03.0439.000; Fri, 25 Oct 2019 21:59:55 +0800 From: "Wang, Haiyue" To: Thomas Monjalon , "arybchenko@solarflare.com" , "Yigit, Ferruh" CC: "dev@dpdk.org" , "Ye, Xiaolong" , "Kinsella, Ray" , "Iremonger, Bernard" , "Sun, Chenmin" , "viacheslavo@mellanox.com" , "stephen@networkplumber.org" , "david.marchand@redhat.com" , "jerinj@marvell.com" Thread-Topic: [dpdk-dev] [PATCH v4 1/4] ethdev: add the API for getting burst mode information Thread-Index: AQHVgy4vnIA7MA5UA0Sl3pLNFwDOY6dqovQAgADKh+A= Date: Fri, 25 Oct 2019 13:59:54 +0000 Message-ID: References: <20191015075133.38560-1-haiyue.wang@intel.com> <20191015075133.38560-2-haiyue.wang@intel.com> <1682096.FjkMPolNrE@xps> In-Reply-To: <1682096.FjkMPolNrE@xps> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNDBjNWRkM2UtNWMzMS00MDVmLTkxYTItMmE0MjE1NTY5OGYwIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiQzdZK3VRS0dMTTI4b2psS3dBbjQybTRVUVpyT01cL1lJb2dXSmpzZUtrZ09tVmc1aWF4cTNlSElYWEFPMngxRmwifQ== 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" Hi Thomas & Andrew, > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Friday, October 25, 2019 17:37 > To: Wang, Haiyue ; Yigit, Ferruh > Cc: dev@dpdk.org; Ye, Xiaolong ; Kinsella, Ray ; > Iremonger, Bernard ; Sun, Chenmin ; > arybchenko@solarflare.com; viacheslavo@mellanox.com; stephen@networkplumb= er.org; > david.marchand@redhat.com; jerinj@marvell.com > Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add the API for getting bu= rst mode information >=20 > 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. >=20 > 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 mergi= ng. > Hopefully it is not too late to fix this API before releasing 19.11. >=20 > I think the idea of getting infos from PMD internal mode is not bad. > But I strongly disagree with standardizing the names. More below. >=20 > [...] > > +enum rte_eth_burst_mode_option { > > + RTE_ETH_BURST_SCALAR =3D (1 << 0), > > + RTE_ETH_BURST_VECTOR =3D (1 << 1), >=20 > 2 bits for a boolean value? >=20 They have the Boolean value, but prefer to keep bit opaque, then can loop to access all the bits ... > > + > > + /**< 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), >=20 > What means simple? Looks meaningless. >=20 The below is the 'simple' code, yes, not a good name, will try to extract more generic name. /* Use a simple Tx queue if possible (only fast free is allowed) */ ad->tx_simple_allowed =3D (txq->offloads =3D=3D (txq->offloads & DEV_TX_OFFLOAD_MBUF_FAST_FREE) && txq->tx_rs_thresh >=3D ICE_TX_MAX_BURST); > > + RTE_ETH_BURST_PER_QUEUE =3D (1 << 19), /**< Support per queue burst *= / >=20 > What is per-queue burst? No need to add a comment if not adding any info. > The burst API is *already* per-queue. >=20 Not for this *burst API* per-queue, but for 'dev->rx/tx_pkt_burst', if the PMD can do burst selection per-queue internally, like NOT ALL VECTOR / SCAL= AR. > > +}; >=20 > How can we imagine standardizing the PMD optimizations? > PMD developers are free to have as many burst implementation as they want= . > If we want to report info about what is used, it can be only a free strin= g. >=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; > > +}; >=20 > Why a struct for an integer? >=20 For expanding the information for mode, don't have to change the API parame= ters. > > +/** > > + * Retrieve information about the Rx packet burst mode. > > + * > > + * @param port_id > > + * The port identifier of the Ethernet device. > > + * @param queue_id > > + * The Rx queue on the Ethernet device for which information > > + * will be retrieved. > > + * @param mode > > + * A pointer to a structure of type *rte_eth_burst_mode* to be fille= d > > + * with the information of the packet burst mode. >=20 > No reference to the enum rte_eth_burst_mode_option or RTE_ETH_BURST_ pref= ix? >=20 > > + * > > + * @return > > + * - 0: Success > > + * - -ENOTSUP: routine is not supported by the device PMD. > > + * - -EINVAL: The port_id or the queue_id is out of range. > > + */ > > +__rte_experimental > > +int rte_eth_rx_burst_mode_get(uint16_t port_id, uint16_t queue_id, > > + struct rte_eth_burst_mode *mode); > [...] > > +/** > > + * Retrieve name about burst mode option. > > + * > > + * @param mode > > + * The burst mode option of type *rte_eth_burst_mode_option*. > > + * > > + * @return > > + * - "": Not found > > + * - "xxx": name of the mode option. > > + */ > > +__rte_experimental > > +const char * > > +rte_eth_burst_mode_option_name(uint64_t option); >=20 > rte_eth_burst_mode_name would be a better function name. > But anyway, this function should not exist. > The string should be freely returned by the PMD > in the burst_mode_get functions. >=20 Design like : common part (options: vector, AVX2 etc) + device specific ? struct rte_eth_burst_mode { uint64_t options; char dev_specific[128]; /* PMD has specific burst mode information */ }; Then rte_eth_burst_mode_option_name is good name to keep its small scope on *options* stringification.