DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Wang, Haiyue" <haiyue.wang@intel.com>
To: "Yigit, Ferruh" <ferruh.yigit@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"Ye, Xiaolong" <xiaolong.ye@intel.com>
Cc: "Kinsella, Ray" <ray.kinsella@intel.com>,
	"Iremonger, Bernard" <bernard.iremonger@intel.com>,
	"Sun, Chenmin" <chenmin.sun@intel.com>
Subject: Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add the API for getting burst mode information
Date: Tue, 15 Oct 2019 01:39:58 +0000	[thread overview]
Message-ID: <E3B9F2FDCB65864C82CD632F23D8AB8773D7B363@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <68913ddd-686c-a2ce-edbf-b5e155191c24@intel.com>

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Tuesday, October 15, 2019 00:39
> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>
> Cc: Kinsella, Ray <ray.kinsella@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>; Sun,
> Chenmin <chenmin.sun@intel.com>
> Subject: Re: [PATCH v3 1/4] ethdev: add the API for getting burst mode information
> 
> On 10/14/2019 4:35 PM, Haiyue Wang wrote:
> > 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.
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
> 
> As far as I can see Bernard has ack only on testpmd patch, 4/4, not for reset of
> the patchset, can you please confirm this offline?
> 
> > Reviewed-by: Xiaolong Ye <xiaolong.ye@intel.com>
> 
> <...>
> 
> > +const char *
> > +rte_eth_burst_mode_option_name(uint64_t option)
> > +{
> > +	switch (option) {
> > +	case RTE_ETH_BURST_SCALAR: return "Scalar";
> > +	case RTE_ETH_BURST_VECTOR: return "Vector";
> > +
> > +	case RTE_ETH_BURST_ALTIVEC: return "AltiVec";
> > +	case RTE_ETH_BURST_NEON: return "Neon";
> > +	case RTE_ETH_BURST_SSE: return "SSE";
> > +	case RTE_ETH_BURST_AVX2: return "AVX2";
> > +	case RTE_ETH_BURST_AVX512: return "AVX512";
> > +
> > +	case RTE_ETH_BURST_SCATTERED: return "Scattered";
> > +	case RTE_ETH_BURST_BULK_ALLOC: return "Bulk Alloc";
> > +	case RTE_ETH_BURST_SIMPLE: return "Simple";
> > +
> > +	case RTE_ETH_BURST_PER_QUEUE: return "Per Queue";
> > +	}
> > +
> > +	return "";
> > +}
> 
> Hi Haiyue,
> 
> The string representation of a vector mode is a data, and I think better to keep
> it separately as an array instead of keeping this information in the function
> and make the function use that data.
> So that when new type are added it won't require to update the function itself.
> 

Hi Ferruh,

Even the vector mode is a data, it still is a bit field option, if we treated them
differently, that will make 'rte_eth_burst_mode_option_name' ugly like:

	switch (option) {
	case RTE_ETH_BURST_SCALAR: return "Scalar";
	case RTE_ETH_BURST_VECTOR: return "Vector";

	case RTE_ETH_BURST_SCATTERED: return "Scattered";
	case RTE_ETH_BURST_BULK_ALLOC: return "Bulk Alloc";
	case RTE_ETH_BURST_SIMPLE: return "Simple";

	case RTE_ETH_BURST_PER_QUEUE: return "Per Queue";
	}


static const struct {
	uint64_t vector;
	const char *name;
} rte_burst_vector_names[] = {
	{ RTE_ETH_BURST_ALTIVEC, "AltiVec" },
	{ RTE_ETH_BURST_NEON, "Neon" },
	{ RTE_ETH_BURST_SSE, "SSE" },
	{ RTE_ETH_BURST_AVX2, "AVX2" },
	{ RTE_ETH_BURST_AVX512, "AVX512" },
};

	for (i = 0; i < RTE_DIM(rte_burst_vector_names); ++i) {
		if (option == rte_burst_ vector _names[i].option)
			return rte_burst_option_names[i].name; 
	}

Why just put them together ?

static const struct {
	uint64_t option;
	const char *name;
} rte_burst_option_names[] = {
	{ RTE_ETH_BURST_SCALAR, "Scalar" },
	{ RTE_ETH_BURST_VECTOR, "Vector" },

	{ RTE_ETH_BURST_ALTIVEC, "AltiVec" },
	{ RTE_ETH_BURST_NEON, "Neon" },
	{ RTE_ETH_BURST_SSE, "SSE" },
	{ RTE_ETH_BURST_AVX2, "AVX2" },
	{ RTE_ETH_BURST_AVX512, "AVX512" },

	{ RTE_ETH_BURST_SCATTERED, "Scattered" },
	{ RTE_ETH_BURST_BULK_ALLOC, "Bulk Alloc" },
	{ RTE_ETH_BURST_SIMPLE, "Simple" },
	{ RTE_ETH_BURST_PER_QUEUE, "Per Queue" },
};

const char *
rte_eth_burst_mode_option_name(uint64_t option)
{
	const char *name = "";
	unsigned int i;

	for (i = 0; i < RTE_DIM(rte_burst_option_names); ++i) {
		if (option == rte_burst_option_names[i].option) {
			name = rte_burst_option_names[i].name;
			break;
		}
	}

	return name;
}

> 'rte_rx_offload_names' and 'rte_eth_dev_rx_offload_name()' is the good sample of
> what I mentioned above.
> 
> Thanks,
> ferruh

  parent reply	other threads:[~2019-10-15  1:40 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-14 15:35 [dpdk-dev] [PATCH v3 0/4] get Rx/Tx packet " Haiyue Wang
2019-10-14 15:35 ` [dpdk-dev] [PATCH v3 1/4] ethdev: add the API for getting " Haiyue Wang
2019-10-14 16:39   ` Ferruh Yigit
2019-10-15  1:02     ` Wang, Haiyue
2019-10-15  1:39     ` Wang, Haiyue [this message]
2019-10-15  7:58       ` Ferruh Yigit
2019-10-14 15:35 ` [dpdk-dev] [PATCH v3 2/4] net/i40e: add Rx/Tx burst mode get callbacks Haiyue Wang
2019-10-14 15:35 ` [dpdk-dev] [PATCH v3 3/4] net/ice: " Haiyue Wang
2019-10-14 15:35 ` [dpdk-dev] [PATCH v3 4/4] app/testpmd: show the Rx/Tx burst mode description Haiyue Wang

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=E3B9F2FDCB65864C82CD632F23D8AB8773D7B363@shsmsx102.ccr.corp.intel.com \
    --to=haiyue.wang@intel.com \
    --cc=bernard.iremonger@intel.com \
    --cc=chenmin.sun@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=ray.kinsella@intel.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).