[v2,3/3] app/testpmd: add debug command Tx metadata set

  As described in [1],[2] this series adds option to set metadata value as
match pattern when creating a new flow rule.

This patch introduces code for debug porpuse only.
The new debug command takes a 32 bit value and stores it per port.
testpmd will add to any Tx packet sent from this port the metadata
value, and set ol_flags accordingly.

[1] "ethdev: support metadata as flow rule criteria"
[2] "app/testpmd: support metadata as flow rule criteria"

Signed-off-by: Dekel Peled <dekelp@mellanox.com>
---
V2:
* Fix some checkpatch coding style issues (wrongly sent).
---

 app/test-pmd/cmdline.c                      | 46 +++++++++++++++++++++++++++++
 app/test-pmd/config.c                       |  6 ++++
 app/test-pmd/testpmd.c                      |  1 +
 app/test-pmd/testpmd.h                      |  4 +++
 app/test-pmd/txonly.c                       |  9 ++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++++
 6 files changed, 73 insertions(+)
  

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dekel Peled
> Sent: Sunday, September 16, 2018 10:38 PM
> To: wenzhuo.lu@intel.com; jingjing.wu@intel.com; bernard.iremonger@intel.com; dev@dpdk.org;
> olivier.matz@6wind.com; Adrien Mazarguil <adrien.mazarguil@6wind.com>; Thomas Monjalon
> <thomas@monjalon.net>; ferruh.yigit@intel.com; arybchenko@solarflare.com
> Cc: Shahaf Shuler <shahafs@mellanox.com>; Ori Kam <orika@mellanox.com>
> Subject: [dpdk-dev] [PATCH v2 3/3] app/testpmd: add debug command Tx metadata set
> 
> As described in [1],[2] this series adds option to set metadata value as match pattern when creating
> a new flow rule.
> 
> This patch introduces code for debug porpuse only.
> The new debug command takes a 32 bit value and stores it per port.
> testpmd will add to any Tx packet sent from this port the metadata value, and set ol_flags
> accordingly.
> 
> [1] "ethdev: support metadata as flow rule criteria"
> [2] "app/testpmd: support metadata as flow rule criteria"
> 
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> ---
> V2:
> * Fix some checkpatch coding style issues (wrongly sent).
> ---
> 
>  app/test-pmd/cmdline.c                      | 46 +++++++++++++++++++++++++++++
>  app/test-pmd/config.c                       |  6 ++++
>  app/test-pmd/testpmd.c                      |  1 +
>  app/test-pmd/testpmd.h                      |  4 +++
>  app/test-pmd/txonly.c                       |  9 ++++++
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  7 +++++
>  6 files changed, 73 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index 4559d59..83ed2b2 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -17604,6 +17604,51 @@ struct cmd_config_per_queue_tx_offload_result {
>  	}
>  };
> 
> +/* *** ENABLE METADATA INSERTION IN TX PACKETS SENT TO PMD *** */
> +struct cmd_tx_metadata_set_result {
> +	cmdline_fixed_string_t tx_metadata;
> +	cmdline_fixed_string_t set;
> +	portid_t port_id;
> +	uint32_t metadata;
> +};
> +
> +static void
> +cmd_tx_metadata_set_parsed(void *parsed_result,
> +		       __attribute__((unused)) struct cmdline *cl,
> +		       __attribute__((unused)) void *data) {
> +	struct cmd_tx_metadata_set_result *res = parsed_result;
> +
> +	tx_metadata_set(res->port_id, res->metadata); }
> +
> +cmdline_parse_token_string_t cmd_tx_metadata_set_tx_metadata =
> +	TOKEN_STRING_INITIALIZER(struct cmd_tx_metadata_set_result,
> +				tx_metadata, "tx_metadata");
> +cmdline_parse_token_string_t cmd_tx_metadata_set_set =
> +	TOKEN_STRING_INITIALIZER(struct cmd_tx_metadata_set_result,
> +				 set, "set");
> +cmdline_parse_token_num_t cmd_tx_metadata_set_portid =
> +	TOKEN_NUM_INITIALIZER(struct cmd_tx_metadata_set_result,
> +			      port_id, UINT16);
> +cmdline_parse_token_num_t cmd_tx_metadata_set_metadata =
> +	TOKEN_NUM_INITIALIZER(struct cmd_tx_metadata_set_result,
> +			      metadata, UINT32);
> +
> +cmdline_parse_inst_t cmd_tx_metadata_set = {
> +	.f = cmd_tx_metadata_set_parsed,
> +	.data = NULL,
> +	.help_str = "tx_metadata set <port_id> <metadata>: "
> +		    "Enable metadata insertion in packets sent to PMD",
> +	.tokens = {
> +		(void *)&cmd_tx_metadata_set_tx_metadata,
> +		(void *)&cmd_tx_metadata_set_set,
> +		(void *)&cmd_tx_metadata_set_portid,
> +		(void *)&cmd_tx_metadata_set_metadata,
> +		NULL,
> +	},
> +};
> +
>  /* ******************************************************************************** */
> 
>  /* list of instructions */
> @@ -17869,6 +17914,7 @@ struct cmd_config_per_queue_tx_offload_result {
>  	(cmdline_parse_inst_t *)&cmd_operate_bpf_ld_parse,
>  	(cmdline_parse_inst_t *)&cmd_operate_bpf_unld_parse,  #endif
> +	(cmdline_parse_inst_t *)&cmd_tx_metadata_set,
>  	NULL,
>  };
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index 70b2cce..b87c691 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -3828,3 +3828,9 @@ struct igb_ring_desc_16_bytes {
> 
>  	printf("\n\n");
>  }
> +
> +void
> +tx_metadata_set(portid_t port_id, uint32_t metadata) {
> +	ports[port_id].metadata = metadata;
> +}
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index 969eb7c..cd6823d 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -776,6 +776,7 @@ static void eth_dev_event_callback(char *device_name,
>  		/* set flag to initialize port/queue */
>  		port->need_reconfig = 1;
>  		port->need_reconfig_queues = 1;
> +		port->metadata = 0;
>  	}
> 
>  	/*
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index a1f6614..54c7e95 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -183,6 +183,8 @@ struct rte_port {
>  #ifdef SOFTNIC
>  	struct softnic_port     softport;  /**< softnic params */
>  #endif
> +	/* metadata value to add in tx packets (debug only) */
> +	uint32_t                metadata;
>  };
> 
>  /**
> @@ -743,6 +745,8 @@ enum print_warning {  queueid_t get_allowed_max_nb_txq(portid_t *pid);  int
> check_nb_txq(queueid_t txq);
> 
> +void tx_metadata_set(portid_t port_id, uint32_t metadata);
> +
>  /*
>   * Work-around of a compilation error with ICC on invocations of the
>   * rte_be_to_cpu_16() function.
> diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c index 1f08b6e..b5a4b35 100644
> --- a/app/test-pmd/txonly.c
> +++ b/app/test-pmd/txonly.c
> @@ -253,6 +253,15 @@
>  		pkt->l2_len = sizeof(struct ether_hdr);
>  		pkt->l3_len = sizeof(struct ipv4_hdr);
>  		pkts_burst[nb_pkt] = pkt;
> +
> +		/*
> +		 * If user configured metadata value add it to packet
> +		 * and set ol_flags accordingly
> +		 */
> +		if (ports[fs->tx_port].metadata) {
> +			pkt->hash.fdir.hi = ports[fs->tx_port].metadata;
> +			pkt->ol_flags |= PKT_TX_METADATA;
> +		}
>  	}
>  	nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_pkt);
>  	/*
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 7d86692..3688e1c 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -851,6 +851,13 @@ Disable hardware insertion of a VLAN header in packets sent on a port::
> 
>     testpmd> tx_vlan reset (port_id)
> 
> +tx_metadata set
> +~~~~~~~~~~~~~~~
> +
> +Set metadata value to insert in packets sent to PMD::
> +
> +   testpmd> tx_metadata set (port_id) (value)
> +
>  csum set
>  ~~~~~~~~
> 
> --
> 1.8.3.1

Acked-by: Xueming Li <xuemingl@mellanox.com>
  

On 9/16/2018 3:37 PM, Dekel Peled wrote:
> As described in [1],[2] this series adds option to set metadata value as
> match pattern when creating a new flow rule.
> 
> This patch introduces code for debug porpuse only.
> The new debug command takes a 32 bit value and stores it per port.
> testpmd will add to any Tx packet sent from this port the metadata
> value, and set ol_flags accordingly.
> 
> [1] "ethdev: support metadata as flow rule criteria"
> [2] "app/testpmd: support metadata as flow rule criteria"
> 
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>

<...>

> @@ -851,6 +851,13 @@ Disable hardware insertion of a VLAN header in packets sent on a port::
>  
>     testpmd> tx_vlan reset (port_id)
>  
> +tx_metadata set
> +~~~~~~~~~~~~~~~
> +
> +Set metadata value to insert in packets sent to PMD::
> +
> +   testpmd> tx_metadata set (port_id) (value)

My usual testpmd grunting:

Everybody is adding a high level command to testpmd, with own syntax and in the
scope of a specific feature, that nobody else knows about, it makes testpmd
confusing/hard to use.
If this is a command to set a port feature, why not extend existing,
"port config <port_id> ..."

Like "port config <port_id> tx_metadata <value>" ?


And second, I think it is good to have a set & show as pairs, this function sets
the "tx_metadata", is there a way to display existing "tx_metadata"? If not,
what about: "show port <port_id> tx_metadata"?
  

Thanks, PSB.

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Friday, October 5, 2018 4:20 PM
> To: Dekel Peled <dekelp@mellanox.com>; wenzhuo.lu@intel.com;
> jingjing.wu@intel.com; bernard.iremonger@intel.com; dev@dpdk.org;
> olivier.matz@6wind.com; Adrien Mazarguil <adrien.mazarguil@6wind.com>;
> Thomas Monjalon <thomas@monjalon.net>; arybchenko@solarflare.com
> Cc: Shahaf Shuler <shahafs@mellanox.com>; Ori Kam
> <orika@mellanox.com>
> Subject: Re: [PATCH v2 3/3] app/testpmd: add debug command Tx metadata
> set
> 
> On 9/16/2018 3:37 PM, Dekel Peled wrote:
> > As described in [1],[2] this series adds option to set metadata value
> > as match pattern when creating a new flow rule.
> >
> > This patch introduces code for debug porpuse only.
> > The new debug command takes a 32 bit value and stores it per port.
> > testpmd will add to any Tx packet sent from this port the metadata
> > value, and set ol_flags accordingly.
> >
> > [1] "ethdev: support metadata as flow rule criteria"
> > [2] "app/testpmd: support metadata as flow rule criteria"
> >
> > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> 
> <...>
> 
> > @@ -851,6 +851,13 @@ Disable hardware insertion of a VLAN header in
> packets sent on a port::
> >
> >     testpmd> tx_vlan reset (port_id)
> >
> > +tx_metadata set
> > +~~~~~~~~~~~~~~~
> > +
> > +Set metadata value to insert in packets sent to PMD::
> > +
> > +   testpmd> tx_metadata set (port_id) (value)
> 
> My usual testpmd grunting:
> 
> Everybody is adding a high level command to testpmd, with own syntax and
> in the scope of a specific feature, that nobody else knows about, it makes
> testpmd confusing/hard to use.
> If this is a command to set a port feature, why not extend existing, "port
> config <port_id> ..."
> 
> Like "port config <port_id> tx_metadata <value>" ?
> 
> 
> And second, I think it is good to have a set & show as pairs, this function sets
> the "tx_metadata", is there a way to display existing "tx_metadata"? If not,
> what about: "show port <port_id> tx_metadata"?

OK, will implement as suggested.