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

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

[Dekel Peled]

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

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

[dpdk-dev] [PATCH 2/2] ethdev: fix metadata documentation

[Dekel Peled]

Previous patch introduced the Tx metadata feature, with unnecessary
restrictions on data entry.

This fix updates the documentation, removing the data entry
restrictions on metadata item.

Fixes: aa0b9484eb5f ("ethdev: support metadata as flow rule criteria")
Cc: dekelp@mellanox.com

Signed-off-by: Dekel Peled <dekelp@mellanox.com>
---
 doc/guides/prog_guide/rte_flow.rst | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 6fb0535..d6683e4 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1196,21 +1196,21 @@ Item: ``META``
 
 Matches an application specific 32 bit metadata item.
 
-- Default ``mask`` matches the specified metadata value.
+- Default ``mask`` matches any 32 bit value.
 
 .. _table_rte_flow_item_meta:
 
 .. table:: META
 
-   +----------+----------+-----------------------+
-   | Field    | Subfield | Value                 |
-   +==========+==========+=======================+
-   | ``spec`` | ``data`` | 32 bit metadata value |
-   +----------+----------------------------------+
-   | ``last`` | ``data`` | ignored               |
-   +----------+----------+-----------------------+
-   | ``mask`` | ``data`` | ignored               |
-   +----------+----------+-----------------------+
+   +----------+----------+---------------------------------------+
+   | Field    | Subfield | Value                                 |
+   +==========+==========+=======================================+
+   | ``spec`` | ``data`` | 32 bit metadata value                 |
+   +----------+--------------------------------------------------+
+   | ``last`` | ``data`` | upper range value                     |
+   +----------+----------+---------------------------------------+
+   | ``mask`` | ``data`` | bit-mask applies to "spec" and "last" |
+   +----------+----------+---------------------------------------+
 
 Actions
 ~~~~~~~
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 2/2] ethdev: fix metadata documentation

[Ori Kam]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dekel Peled
> Sent: Tuesday, October 23, 2018 4:29 PM
> 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 2/2] ethdev: fix metadata documentation
> 
> Previous patch introduced the Tx metadata feature, with unnecessary
> restrictions on data entry.
> 
> This fix updates the documentation, removing the data entry
> restrictions on metadata item.
> 
> Fixes: aa0b9484eb5f ("ethdev: support metadata as flow rule criteria")
> Cc: dekelp@mellanox.com
> 
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> ---
>  doc/guides/prog_guide/rte_flow.rst | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index 6fb0535..d6683e4 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1196,21 +1196,21 @@ Item: ``META``
> 
>  Matches an application specific 32 bit metadata item.
> 
> -- Default ``mask`` matches the specified metadata value.
> +- Default ``mask`` matches any 32 bit value.
> 
Are you sure from the impelmetion it looks like default mask is all ones,
This means that the first comment is the correct one.

>  .. _table_rte_flow_item_meta:
> 
>  .. table:: META
> 
> -   +----------+----------+-----------------------+
> -   | Field    | Subfield | Value                 |
> -   +==========+==========+=======================+
> -   | ``spec`` | ``data`` | 32 bit metadata value |
> -   +----------+----------------------------------+
> -   | ``last`` | ``data`` | ignored               |
> -   +----------+----------+-----------------------+
> -   | ``mask`` | ``data`` | ignored               |
> -   +----------+----------+-----------------------+
> +   +----------+----------+---------------------------------------+
> +   | Field    | Subfield | Value                                 |
> +
> +==========+==========+=======================================+
> +   | ``spec`` | ``data`` | 32 bit metadata value                 |
> +   +----------+--------------------------------------------------+
> +   | ``last`` | ``data`` | upper range value                     |
> +   +----------+----------+---------------------------------------+
> +   | ``mask`` | ``data`` | bit-mask applies to "spec" and "last" |
> +   +----------+----------+---------------------------------------+
> 
>  Actions
>  ~~~~~~~
> --
> 1.8.3.1

Re: [dpdk-dev] [PATCH 2/2] ethdev: fix metadata documentation

[Dekel Peled]

> -----Original Message-----
> From: Ori Kam
> Sent: Tuesday, October 23, 2018 9:06 PM
> To: Dekel Peled <dekelp@mellanox.com>; 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; Dekel Peled
> <dekelp@mellanox.com>
> Subject: RE: [dpdk-dev] [PATCH 2/2] ethdev: fix metadata documentation
> 
> 
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Dekel Peled
> > Sent: Tuesday, October 23, 2018 4:29 PM
> > 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 2/2] ethdev: fix metadata documentation
> >
> > Previous patch introduced the Tx metadata feature, with unnecessary
> > restrictions on data entry.
> >
> > This fix updates the documentation, removing the data entry
> > restrictions on metadata item.
> >
> > Fixes: aa0b9484eb5f ("ethdev: support metadata as flow rule criteria")
> > Cc: dekelp@mellanox.com
> >
> > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> > ---
> >  doc/guides/prog_guide/rte_flow.rst | 20 ++++++++++----------
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> > b/doc/guides/prog_guide/rte_flow.rst
> > index 6fb0535..d6683e4 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -1196,21 +1196,21 @@ Item: ``META``
> >
> >  Matches an application specific 32 bit metadata item.
> >
> > -- Default ``mask`` matches the specified metadata value.
> > +- Default ``mask`` matches any 32 bit value.
> >
> Are you sure from the impelmetion it looks like default mask is all ones, This
> means that the first comment is the correct one.
> 

Right, will change it.

> >  .. _table_rte_flow_item_meta:
> >
> >  .. table:: META
> >
> > -   +----------+----------+-----------------------+
> > -   | Field    | Subfield | Value                 |
> > -   +==========+==========+=======================+
> > -   | ``spec`` | ``data`` | 32 bit metadata value |
> > -   +----------+----------------------------------+
> > -   | ``last`` | ``data`` | ignored               |
> > -   +----------+----------+-----------------------+
> > -   | ``mask`` | ``data`` | ignored               |
> > -   +----------+----------+-----------------------+
> > +   +----------+----------+---------------------------------------+
> > +   | Field    | Subfield | Value                                 |
> > +
> >
> +==========+==========+===================================
> ====+
> > +   | ``spec`` | ``data`` | 32 bit metadata value                 |
> > +   +----------+--------------------------------------------------+
> > +   | ``last`` | ``data`` | upper range value                     |
> > +   +----------+----------+---------------------------------------+
> > +   | ``mask`` | ``data`` | bit-mask applies to "spec" and "last" |
> > +   +----------+----------+---------------------------------------+
> >
> >  Actions
> >  ~~~~~~~
> > --
> > 1.8.3.1

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

[Dekel Peled]

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

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

[Ori Kam]

> -----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

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

[Ferruh Yigit]

On 10/24/2018 11:49 AM, Ori Kam wrote:
> 
> 
>> -----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>
<...>
> Acked-by: Ori Kam <orika@mellanox.com>

Series applied to dpdk-next-net/master, thanks.


Thomas,
Since the patches has been fixed are in next-net, the Fixes tags will be wrong
when you pull, can you please fix them while merging?

Thanks.

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

[Thomas Monjalon]

25/10/2018 12:45, Ferruh Yigit:
> On 10/24/2018 11:49 AM, Ori Kam wrote:
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Dekel Peled
> >>
> >> 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>
> <...>
> > Acked-by: Ori Kam <orika@mellanox.com>
> 
> Series applied to dpdk-next-net/master, thanks.
> 
> 
> Thomas,
> Since the patches has been fixed are in next-net, the Fixes tags will be wrong
> when you pull, can you please fix them while merging?

OK, no problem

[dpdk-dev] [PATCH v2 2/2] ethdev: fix metadata documentation

[Dekel Peled]

Previous patch introduced the Tx metadata feature, with unnecessary
restrictions on data entry.

This fix updates the documentation, removing the data entry
restrictions on metadata item.

Fixes: aa0b9484eb5f ("ethdev: support metadata as flow rule criteria")
Cc: dekelp@mellanox.com

---
v2:
Restore the correct 'mask' description.
---

Signed-off-by: Dekel Peled <dekelp@mellanox.com>
---
 doc/guides/prog_guide/rte_flow.rst | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 6fb0535..5c5dd90 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1202,15 +1202,15 @@ Matches an application specific 32 bit metadata item.
 
 .. table:: META
 
-   +----------+----------+-----------------------+
-   | Field    | Subfield | Value                 |
-   +==========+==========+=======================+
-   | ``spec`` | ``data`` | 32 bit metadata value |
-   +----------+----------------------------------+
-   | ``last`` | ``data`` | ignored               |
-   +----------+----------+-----------------------+
-   | ``mask`` | ``data`` | ignored               |
-   +----------+----------+-----------------------+
+   +----------+----------+---------------------------------------+
+   | Field    | Subfield | Value                                 |
+   +==========+==========+=======================================+
+   | ``spec`` | ``data`` | 32 bit metadata value                 |
+   +----------+--------------------------------------------------+
+   | ``last`` | ``data`` | upper range value                     |
+   +----------+----------+---------------------------------------+
+   | ``mask`` | ``data`` | bit-mask applies to "spec" and "last" |
+   +----------+----------+---------------------------------------+
 
 Actions
 ~~~~~~~
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 2/2] ethdev: fix metadata documentation

[Ori Kam]

> -----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 2/2] ethdev: fix metadata documentation
> 
> Previous patch introduced the Tx metadata feature, with unnecessary
> restrictions on data entry.
> 
> This fix updates the documentation, removing the data entry
> restrictions on metadata item.
> 
> Fixes: aa0b9484eb5f ("ethdev: support metadata as flow rule criteria")
> Cc: dekelp@mellanox.com
> 
> ---
> v2:
> Restore the correct 'mask' description.
> ---
> 
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> ---
>  doc/guides/prog_guide/rte_flow.rst | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index 6fb0535..5c5dd90 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1202,15 +1202,15 @@ Matches an application specific 32 bit metadata
> item.
> 
>  .. table:: META
> 
> -   +----------+----------+-----------------------+
> -   | Field    | Subfield | Value                 |
> -   +==========+==========+=======================+
> -   | ``spec`` | ``data`` | 32 bit metadata value |
> -   +----------+----------------------------------+
> -   | ``last`` | ``data`` | ignored               |
> -   +----------+----------+-----------------------+
> -   | ``mask`` | ``data`` | ignored               |
> -   +----------+----------+-----------------------+
> +   +----------+----------+---------------------------------------+
> +   | Field    | Subfield | Value                                 |
> +
> +==========+==========+=======================================+
> +   | ``spec`` | ``data`` | 32 bit metadata value                 |
> +   +----------+--------------------------------------------------+
> +   | ``last`` | ``data`` | upper range value                     |
> +   +----------+----------+---------------------------------------+
> +   | ``mask`` | ``data`` | bit-mask applies to "spec" and "last" |
> +   +----------+----------+---------------------------------------+
> 
>  Actions
>  ~~~~~~~
> --
> 1.8.3.1

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