Re: [dpdk-dev] [PATCH v2 1/2] app/testpmd: fix metadata API and Tx insertion

[Ori Kam <orika@mellanox.com>]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dekel Peled
> Sent: Wednesday, October 24, 2018 9:22 AM
> To: wenzhuo.lu@intel.com; jingjing.wu@intel.com;
> bernard.iremonger@intel.com; 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>; dev@dpdk.org; Ori Kam
> <orika@mellanox.com>; Dekel Peled <dekelp@mellanox.com>
> Subject: [dpdk-dev] [PATCH v2 1/2] app/testpmd: fix metadata API and Tx
> insertion
> 
> Previous patch introduces the Tx metadata feature, with unnecessary
> restrictions on data entry.
> It also used the metadata in txonly fwd engine only.
> 
> This fix removes the data entry restrictions on metadata item.
> It also implements callback function to add the metadata in every
> Tx packet, sent by any fwd engine.
> 
> Fixes: 32eef22f0b79 ("app/testpmd: support metadata as flow rule item")
> Cc: dekelp@mellanox.com
> 
> ---
> v2:
> No change in this patch.
> Modified other patch in series.
> ---
> 
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> ---
>  app/test-pmd/cmdline.c      |  5 +++++
>  app/test-pmd/cmdline_flow.c |  7 +------
>  app/test-pmd/testpmd.h      |  7 +++++++
>  app/test-pmd/txonly.c       |  9 --------
>  app/test-pmd/util.c         | 51
> +++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 64 insertions(+), 15 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index fc74b95..27264eb 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -17718,6 +17718,11 @@ struct cmd_config_tx_metadata_specific_result {
>  	if (port_id_is_invalid(res->port_id, ENABLED_WARN))
>  		return;
>  	ports[res->port_id].tx_metadata = rte_cpu_to_be_32(res->value);
> +	/* Add/remove callback to insert valid metadata in every Tx packet. */
> +	if (ports[res->port_id].tx_metadata)
> +		add_tx_md_callback(res->port_id);
> +	else
> +		remove_tx_md_callback(res->port_id);
>  }
> 
>  cmdline_parse_token_string_t cmd_config_tx_metadata_specific_port =
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 7932b54..dfb7c147 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -548,11 +548,6 @@ struct parse_action_priv {
>  	ZERO,
>  };
> 
> -static const enum index item_param_is[] = {
> -	ITEM_PARAM_IS,
> -	ZERO,
> -};
> -
>  static const enum index next_item[] = {
>  	ITEM_END,
>  	ITEM_VOID,
> @@ -2094,7 +2089,7 @@ static int comp_vc_action_rss_queue(struct context
> *, const struct token *,
>  	[ITEM_META_DATA] = {
>  		.name = "data",
>  		.help = "metadata value",
> -		.next = NEXT(item_meta, NEXT_ENTRY(UNSIGNED),
> item_param_is),
> +		.next = NEXT(item_meta, NEXT_ENTRY(UNSIGNED),
> item_param),
>  		.args = ARGS(ARGS_ENTRY_MASK_HTON(struct
> rte_flow_item_meta,
>  						  data, "\xff\xff\xff\xff")),
>  	},
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index c07548e..102f2f4 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -198,6 +198,7 @@ struct rte_port {
>  #endif
>  	/**< metadata value to insert in Tx packets. */
>  	rte_be32_t		tx_metadata;
> +	const struct rte_eth_rxtx_callback *tx_set_md_cb[MAX_QUEUE_ID+1];
>  };
> 
>  /**
> @@ -781,6 +782,12 @@ uint16_t dump_tx_pkts(uint16_t port_id, uint16_t
> queue, struct rte_mbuf *pkts[],
>  void remove_tx_dump_callbacks(portid_t portid);
>  void configure_rxtx_dump_callbacks(uint16_t verbose);
> 
> +uint16_t tx_pkt_set_md(uint16_t port_id, __rte_unused uint16_t queue,
> +		       struct rte_mbuf *pkts[], uint16_t nb_pkts,
> +		       __rte_unused void *user_param);
> +void add_tx_md_callback(portid_t portid);
> +void remove_tx_md_callback(portid_t portid);
> +
>  /*
>   * 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 fae84ca..1f08b6e 100644
> --- a/app/test-pmd/txonly.c
> +++ b/app/test-pmd/txonly.c
> @@ -253,15 +253,6 @@
>  		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].tx_metadata) {
> -			pkt->tx_metadata = ports[fs->tx_port].tx_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/app/test-pmd/util.c b/app/test-pmd/util.c
> index f4125df..687bfa4 100644
> --- a/app/test-pmd/util.c
> +++ b/app/test-pmd/util.c
> @@ -166,3 +166,54 @@
>  	dump_pkt_burst(port_id, queue, pkts, nb_pkts, 0);
>  	return nb_pkts;
>  }
> +
> +uint16_t
> +tx_pkt_set_md(uint16_t port_id, __rte_unused uint16_t queue,
> +	      struct rte_mbuf *pkts[], uint16_t nb_pkts,
> +	      __rte_unused void *user_param)
> +{
> +	uint16_t i = 0;
> +
> +	/*
> +	 * Add metadata value to every Tx packet,
> +	 * and set ol_flags accordingly.
> +	 */
> +	for (i = 0; i < nb_pkts; i++) {
> +		pkts[i]->tx_metadata = ports[port_id].tx_metadata;
> +		pkts[i]->ol_flags |= PKT_TX_METADATA;
> +	}
> +	return nb_pkts;
> +}
> +
> +void
> +add_tx_md_callback(portid_t portid)
> +{
> +	struct rte_eth_dev_info dev_info;
> +	uint16_t queue;
> +
> +	if (port_id_is_invalid(portid, ENABLED_WARN))
> +		return;
> +	rte_eth_dev_info_get(portid, &dev_info);
> +	for (queue = 0; queue < dev_info.nb_tx_queues; queue++)
> +		if (!ports[portid].tx_set_md_cb[queue])
> +			ports[portid].tx_set_md_cb[queue] =
> +				rte_eth_add_tx_callback(portid, queue,
> +							tx_pkt_set_md, NULL);
> +}
> +
> +void
> +remove_tx_md_callback(portid_t portid)
> +{
> +	struct rte_eth_dev_info dev_info;
> +	uint16_t queue;
> +
> +	if (port_id_is_invalid(portid, ENABLED_WARN))
> +		return;
> +	rte_eth_dev_info_get(portid, &dev_info);
> +	for (queue = 0; queue < dev_info.nb_tx_queues; queue++)
> +		if (ports[portid].tx_set_md_cb[queue]) {
> +			rte_eth_remove_tx_callback(portid, queue,
> +				ports[portid].tx_set_md_cb[queue]);
> +			ports[portid].tx_set_md_cb[queue] = NULL;
> +		}
> +}
> --
> 1.8.3.1

Acked-by: Ori Kam <orika@mellanox.com>
Thanks,
Ori