Re: [dpdk-dev] [PATCH v2 1/2] app/testpmd: add commands to test new Rx offload API

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andrew Rybchenko
> Sent: Tuesday, March 13, 2018 7:21 AM
> To: Dai, Wei <wei.dai@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 1/2] app/testpmd: add commands to test new Rx offload API
> 
> On 03/13/2018 09:42 AM, Wei Dai wrote:
> > Add following testpmd run-time commands to support test of
> > new Rx offload API:
> > rx_offload get capability <port_id>
> > rx_offload get configuration <port_id>
> > rx_offload enable|disable per_port <offload> <port_id>
> > rx_offload enable|disable per_queue <offload> <port_id> <queue_id>
> >
> > Above last 2 commands should be run when the port is stopped.
> > And <offload> can be one of "vlan_strip", "ipv4_cksum", ...
> >
> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > ---
> >   app/test-pmd/cmdline.c | 424 +++++++++++++++++++++++++++++++++++++++++++++++++
> >   app/test-pmd/testpmd.c |  15 +-
> >   app/test-pmd/testpmd.h |   1 +
> >   3 files changed, 438 insertions(+), 2 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> > index d1dc1de..dfd0ca6 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -15996,6 +15996,426 @@ cmdline_parse_inst_t cmd_ptype_mapping_update = {
> >   	},
> >   };
> >
> > +/* Get Rx offloads capability */
> > +struct cmd_rx_offload_get_capa_result {
> > +	cmdline_fixed_string_t rx_offload;
> > +	cmdline_fixed_string_t get;
> > +	cmdline_fixed_string_t capability;
> > +	portid_t port_id;
> > +};
> > +
> > +cmdline_parse_token_string_t cmd_rx_offload_get_capa_rx_offload =
> > +	TOKEN_STRING_INITIALIZER
> > +		(struct cmd_rx_offload_get_capa_result,
> > +		 rx_offload, "rx_offload");
> > +cmdline_parse_token_string_t cmd_rx_offload_get_capa_get =
> > +	TOKEN_STRING_INITIALIZER
> > +		(struct cmd_rx_offload_get_capa_result,
> > +		 get, "get");
> > +cmdline_parse_token_string_t cmd_rx_offload_get_capa_capability =
> > +	TOKEN_STRING_INITIALIZER
> > +		(struct cmd_rx_offload_get_capa_result,
> > +		 capability, "capability");
> > +cmdline_parse_token_num_t cmd_rx_offload_get_capa_port_id =
> > +	TOKEN_NUM_INITIALIZER
> > +		(struct cmd_rx_offload_get_capa_result,
> > +		 port_id, UINT16);
> > +
> > +static void
> > +print_rx_offloads(uint64_t offloads)
> > +{
> > +	uint64_t single_offload;
> > +
> > +	for (single_offload = DEV_RX_OFFLOAD_VLAN_STRIP;
> > +	     single_offload <= DEV_RX_OFFLOAD_SECURITY;
> 
> It requires attention when a new offload is added.
> Please, consider to use something like __builtin_ffsll(), print name of
> the found bit, clear it and retry. It allows to avoid boundaries
> specification.
> 
> <snip>
> 
> 
> > +static int
> > +config_rx_offload(const char *name, uint64_t *offload, int on)
> > +{
> > +	uint64_t local = *offload;
> > +
> > +	if (!strcmp(name, "vlan_strip")) {
> > +		if (on)
> > +			local |= DEV_RX_OFFLOAD_VLAN_STRIP;
> > +		else
> > +			local &= ~DEV_RX_OFFLOAD_VLAN_STRIP;
> > +	} else if (!strcmp(name, "ipv4_cksum")) {
> > +		if (on)
> > +			local |= DEV_RX_OFFLOAD_IPV4_CKSUM;
> > +		else
> > +			local &= ~DEV_RX_OFFLOAD_IPV4_CKSUM;
> > +	} else if (!strcmp(name, "udp_cksum")) {
> > +		if (on)
> > +			local |= DEV_RX_OFFLOAD_UDP_CKSUM;
> > +		else
> > +			local &= ~DEV_RX_OFFLOAD_UDP_CKSUM;
> > +	} else if (!strcmp(name, "tcp_cksum")) {
> > +		if (on)
> > +			local |= DEV_RX_OFFLOAD_TCP_CKSUM;
> > +		else
> > +			local &= ~DEV_RX_OFFLOAD_TCP_CKSUM;
> > +	} else if (!strcmp(name, "tcp_lro")) {
> > +		if (on)
> > +			local |= DEV_RX_OFFLOAD_TCP_LRO;
> > +		else
> > +			local &= ~DEV_RX_OFFLOAD_TCP_LRO;
> > +	} else if (!strcmp(name, "qinq_strip")) {
> > +		if (on)
> > +			local |= DEV_RX_OFFLOAD_QINQ_STRIP;
> > +		else
> > +			local &= ~DEV_RX_OFFLOAD_QINQ_STRIP;
> > +	} else if (!strcmp(name, "outer_ipv4_cksum")) {
> > +		if (on)
> > +			local |= DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM;
> > +		else
> > +			local &= ~DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM;
> > +	} else if (!strcmp(name, "macsec_strip")) {
> > +		if (on)
> > +			local |= DEV_RX_OFFLOAD_MACSEC_STRIP;
> > +		else
> > +			local &= ~DEV_RX_OFFLOAD_MACSEC_STRIP;
> > +	} else if (!strcmp(name, "header_split")) {
> > +		if (on)
> > +			local |= DEV_RX_OFFLOAD_HEADER_SPLIT;
> > +		else
> > +			local &= ~DEV_RX_OFFLOAD_HEADER_SPLIT;
> > +	} else if (!strcmp(name, "vlan_filter")) {
> > +		if (on)
> > +			local |= DEV_RX_OFFLOAD_VLAN_FILTER;
> > +		else
> > +			local &= ~DEV_RX_OFFLOAD_VLAN_FILTER;
> > +	} else if (!strcmp(name, "vlan_extend")) {
> > +		if (on)
> > +			local |= DEV_RX_OFFLOAD_VLAN_EXTEND;
> > +		else
> > +			local &= ~DEV_RX_OFFLOAD_VLAN_EXTEND;
> > +	} else if (!strcmp(name, "jumbo_frame")) {
> > +		if (on)
> > +			local |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> > +		else
> > +			local &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> > +	} else if (!strcmp(name, "crc_strip")) {
> > +		if (on)
> > +			local |= DEV_RX_OFFLOAD_CRC_STRIP;
> > +		else
> > +			local &= ~DEV_RX_OFFLOAD_CRC_STRIP;
> > +	} else if (!strcmp(name, "scatter")) {
> > +		if (on)
> > +			local |= DEV_RX_OFFLOAD_SCATTER;
> > +		else
> > +			local &= ~DEV_RX_OFFLOAD_SCATTER;
> > +	} else if (!strcmp(name, "timestamp")) {
> > +		if (on)
> > +			local |= DEV_RX_OFFLOAD_TIMESTAMP;
> > +		else
> > +			local &= ~DEV_RX_OFFLOAD_TIMESTAMP;
> > +	} else if (!strcmp(name, "security")) {
> > +		if (on)
> > +			local |= DEV_RX_OFFLOAD_SECURITY;
> > +		else
> > +			local &= ~DEV_RX_OFFLOAD_SECURITY;
> > +	} else
> > +		return -1;
> 
> I was going to suggest rte_eth_dev_rx_offload_name(), strcasecmp() and loop.

+1

> It is possible to iterate on all U64 bits or add a way to get all
> offload bits from rte_ethdev.
> However it still requires list of offloads in the help below. So, it is
> not easy to get rig of
> offloads list completely. Just an idea. Up to you.
> 
> <snip>
> 
> > +cmdline_parse_inst_t cmd_config_per_port_rx_offload = {
> > +	.f = cmd_config_per_port_rx_offload_parsed,
> > +	.data = NULL,
> > +	.help_str = "rx_offload enable|disable per_port vlan_strip|ipv4_cksum|"
> > +		    "udp_cksum|tcp_cksum|tcp_lro|qinq_strip|outer_ipv4_cksum|"
> > +		    "macsec_strip|header_split|vlan_filter|vlan_extend|"
> > +		    "jumbo_frame|crc_strip|scatter|timestamp|security "
> > +		    "<port_id>",
> >
> 
> <snip>