DPDK patches and discussions
 help / color / mirror / Atom feed
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
Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add the API for getting burst mode information
Date: Fri, 25 Oct 2019 11:36:44 +0200	[thread overview]
Message-ID: <1682096.FjkMPolNrE@xps> (raw)
In-Reply-To: <20191015075133.38560-2-haiyue.wang@intel.com>

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.



  parent reply	other threads:[~2019-10-25  9:36 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-15  7:51 [dpdk-dev] [PATCH v4 0/4] get Rx/Tx packet " Haiyue Wang
2019-10-15  7:51 ` [dpdk-dev] [PATCH v4 1/4] ethdev: add the API for getting " Haiyue Wang
2019-10-15 10:45   ` Ferruh Yigit
2019-10-15 11:23     ` Wang, Haiyue
2019-10-15 11:13   ` Ferruh Yigit
2019-10-25  9:36   ` Thomas Monjalon [this message]
2019-10-25 10:26     ` Jerin Jacob
2019-10-25 13:59     ` Wang, Haiyue
2019-10-25 14:08     ` Ferruh Yigit
2019-10-25 15:45       ` Thomas Monjalon
2019-10-25 16:02         ` Jerin Jacob
2019-10-25 22:27           ` Thomas Monjalon
2019-10-26  4:40             ` Wang, Haiyue
2019-10-26  6:24               ` Thomas Monjalon
2019-10-26  9:23                 ` Wang, Haiyue
2019-10-26 16:23                   ` Thomas Monjalon
2019-10-29 14:27                     ` Ferruh Yigit
2019-11-03 20:35                       ` Ray Kinsella
2019-11-03 22:41                         ` Thomas Monjalon
2019-11-04  9:49                           ` Ray Kinsella
2019-11-04  9:54                             ` Thomas Monjalon
2019-11-04 10:03                               ` Ray Kinsella
2019-11-04 10:46                                 ` Wang, Haiyue
2019-11-04 11:30                                 ` Thomas Monjalon
2019-11-04 12:07                                   ` Ray Kinsella
2019-11-04 13:09                                     ` Thomas Monjalon
2019-11-04 13:48                                       ` Ray Kinsella
2019-11-04 14:17                                         ` Wang, Haiyue
2019-10-26  6:40             ` Slava Ovsiienko
2019-10-26  9:31               ` Wang, Haiyue
2019-10-26  6:58             ` Jerin Jacob
2019-10-26  9:37               ` Wang, Haiyue
2019-10-29  3:37                 ` Jerin Jacob
2019-10-29  4:44                   ` Wang, Haiyue
2019-10-29  5:19                     ` Jerin Jacob
2019-10-29  5:42                       ` Wang, Haiyue
2019-10-29  5:47                         ` Jerin Jacob
2019-10-29  5:56                           ` Wang, Haiyue
2019-10-29  6:00                           ` Wang, Haiyue
2019-10-29  8:34                             ` Jerin Jacob
2019-10-29 11:26                               ` Wang, Haiyue
2019-10-29 12:56                                 ` Jerin Jacob
2019-10-29 13:51                                   ` Wang, Haiyue
2019-10-29 14:08                                     ` Jerin Jacob
2019-10-29 15:42                                       ` Wang, Haiyue
2019-10-29 15:59                                         ` Jerin Jacob
2019-10-29 16:16                                           ` Wang, Haiyue
2019-10-29 16:59               ` Ferruh Yigit
2019-10-30  4:38                 ` Jerin Jacob
2019-10-30  4:43                   ` Wang, Haiyue
2019-10-30  8:14                 ` Wang, Haiyue
2019-10-31 10:46                   ` Jerin Jacob
2019-10-31 11:15                     ` Ray Kinsella
2019-10-31 11:16                     ` Wang, Haiyue
2019-10-31 14:58                       ` Ferruh Yigit
2019-10-31 15:07                         ` Wang, Haiyue
2019-10-31 15:29                           ` Ferruh Yigit
2019-10-31 15:54                             ` Wang, Haiyue
2019-10-31 11:09                 ` Ray Kinsella
2019-10-15  7:51 ` [dpdk-dev] [PATCH v4 2/4] net/i40e: add Rx/Tx burst mode get callbacks Haiyue Wang
2019-10-15  7:51 ` [dpdk-dev] [PATCH v4 3/4] net/ice: " Haiyue Wang
2019-10-15  7:51 ` [dpdk-dev] [PATCH v4 4/4] app/testpmd: show the Rx/Tx burst mode description Haiyue Wang
2019-10-15 12:11 ` [dpdk-dev] [PATCH v4 0/4] get Rx/Tx packet burst mode information Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1682096.FjkMPolNrE@xps \
    --to=thomas@monjalon.net \
    --cc=arybchenko@solarflare.com \
    --cc=bernard.iremonger@intel.com \
    --cc=chenmin.sun@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=haiyue.wang@intel.com \
    --cc=jerinj@marvell.com \
    --cc=ray.kinsella@intel.com \
    --cc=stephen@networkplumber.org \
    --cc=viacheslavo@mellanox.com \
    --cc=xiaolong.ye@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).