From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 7ECA1A32A2
	for <public@inbox.dpdk.org>; Fri, 25 Oct 2019 11:36:51 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id C1A481C1A8;
	Fri, 25 Oct 2019 11:36:50 +0200 (CEST)
Received: from new3-smtp.messagingengine.com (new3-smtp.messagingengine.com
 [66.111.4.229]) by dpdk.org (Postfix) with ESMTP id 78AEA1C1A5
 for <dev@dpdk.org>; Fri, 25 Oct 2019 11:36:49 +0200 (CEST)
Received: from compute1.internal (compute1.nyi.internal [10.202.2.41])
 by mailnew.nyi.internal (Postfix) with ESMTP id EAD2F5DB2;
 Fri, 25 Oct 2019 05:36:47 -0400 (EDT)
Received: from mailfrontend1 ([10.202.2.162])
 by compute1.internal (MEProxy); Fri, 25 Oct 2019 05:36:47 -0400
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=jHf5wCjJOvKDCBkNFi8jxssNFnvAPKRQoIasAXsdAVg=; b=FG6kp8reczQ4
 /C32Ch2X5nI9JBUhJ7h2yzSWA1DjFcj7eG2qqgelkRpfWKynuOyHdjxjxorP/KEb
 DzEA5jJ1nRhoyYiBvkmPJzQVQ6W6oddDi61YU1wuApi8RzstmNnGotG3pBAsMsWB
 My3CTa6dXVgVr+UxcsmE3aKHYBQpcF0=
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=jHf5wCjJOvKDCBkNFi8jxssNFnvAPKRQoIasAXsdA
 Vg=; b=eQpHSPX/2LFgJjQBYQjqU47JHnOjebaRsP3ADUJ3oI41LhOkuyfsSK8o+
 VkK5ASTXs5SnNQhfDvq57yqPHkWwDVIGPddFw5+JTLD9SEGAS2WcBMjNA2mZJe1e
 cQybFTYoIR5I/BuSDUgrwLlyKI+Zt831EpUesprcVEVt9CWlLSxtxj7juItmGCu+
 TB9rSO08Y3sCt0cbKp44qqVg68RY7Psr1b1XAZGY9FEJVCTk9oQxFS7LfNbKwIQa
 69vRHbXz0UEBDZHsi+lzIAoHvRj0YXV0LCuaG4nb/DjqZokkFcdQWiLYNu7vKUJH
 D1uBMheBrxQkNEgd0OBpmrr4+lJ9A==
X-ME-Sender: <xms:L8KyXTTFf0pIq4AvYKBfmd9z6e_YOcSWGuURfMms1mqKaJOOsrb-jg>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedufedrleefgddulecutefuodetggdotefrodftvf
 curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu
 uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc
 fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs
 ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucfkph
 epjeejrddufeegrddvtdefrddukeegnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthhho
 mhgrshesmhhonhhjrghlohhnrdhnvghtnecuvehluhhsthgvrhfuihiivgeptd
X-ME-Proxy: <xmx:L8KyXcYnO19bgoXdFBbbhBbIgBrgYmhAjiWwm_PNdhw8ZR0AScN7WQ>
 <xmx:L8KyXZDaPCYVbKD9LYNLNBwlNBfCYIOERtoOCxpdOgmdNx5U2epS0g>
 <xmx:L8KyXbBUyLcms71i5lXCEU9W-KqXmRfZJRAjZbQmzpIi0O_pYaGSug>
 <xmx:L8KyXUSsOQNrZJgU-1wcMzVFWKOQAzzpJ4Vz96CFrEaNukWLT8Xmdg>
Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184])
 by mail.messagingengine.com (Postfix) with ESMTPA id 963C08005B;
 Fri, 25 Oct 2019 05:36:45 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: Haiyue Wang <haiyue.wang@intel.com>, ferruh.yigit@intel.com
Cc: dev@dpdk.org, xiaolong.ye@intel.com, ray.kinsella@intel.com,
 bernard.iremonger@intel.com, chenmin.sun@intel.com, arybchenko@solarflare.com,
 viacheslavo@mellanox.com, stephen@networkplumber.org,
 david.marchand@redhat.com, jerinj@marvell.com
Date: Fri, 25 Oct 2019 11:36:44 +0200
Message-ID: <1682096.FjkMPolNrE@xps>
In-Reply-To: <20191015075133.38560-2-haiyue.wang@intel.com>
References: <20191015075133.38560-1-haiyue.wang@intel.com>
 <20191015075133.38560-2-haiyue.wang@intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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.
Hopefully it is not too late to fix this API before releasing 19.11.

I think the idea of getting infos from PMD internal mode is not bad.
But I strongly disagree with standardizing the names. More below.

[...]
> +enum rte_eth_burst_mode_option {
> +	RTE_ETH_BURST_SCALAR = (1 << 0),
> +	RTE_ETH_BURST_VECTOR = (1 << 1),

2 bits for a boolean value?

> +
> +	/**< 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),

What means simple? Looks meaningless.

> +	RTE_ETH_BURST_PER_QUEUE = (1 << 19), /**< Support per queue burst */

What is per-queue burst? No need to add a comment if not adding any info.
The burst API is *already* per-queue.

> +};

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 string.

> +/**
> + * 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?

> +/**
> + * 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 filled
> + *   with the information of the packet burst mode.

No reference to the enum rte_eth_burst_mode_option or RTE_ETH_BURST_ prefix?

> + *
> + * @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);

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.