DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/2] net/mlx5: add PMD dynf
@ 2020-01-13  9:29 Ori Kam
  2020-01-13  9:29 ` [dpdk-dev] [PATCH 1/2] app/testpmd: add dynamic flag support Ori Kam
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Ori Kam @ 2020-01-13  9:29 UTC (permalink / raw)
  Cc: dev, orika, ferruh.yigit, viacheslavo, matan

This patch-set uses the dynf feature to give the mlx5 pmd
hint if inline is needed.

The first patch,adds a generic way to regiter dynf and setting it in
case of Tx packet.

Ori Kam (2):
  app/testpmd: add dynamic flag support
  net/mlx5: add fine grain dynamic flag support

 app/test-pmd/cmdline.c                      | 88 +++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h                      | 16 ++++++
 app/test-pmd/util.c                         | 63 +++++++++++++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 10 ++++
 drivers/net/mlx5/mlx5.c                     | 15 +++++
 drivers/net/mlx5/mlx5_rxtx.c                |  2 +
 drivers/net/mlx5/mlx5_rxtx.h                |  3 +
 drivers/net/mlx5/mlx5_trigger.c             |  8 +++
 drivers/net/mlx5/rte_pmd_mlx5.h             | 32 +++++++++++
 drivers/net/mlx5/rte_pmd_mlx5_version.map   |  7 +++
 10 files changed, 244 insertions(+)
 create mode 100644 drivers/net/mlx5/rte_pmd_mlx5.h

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [dpdk-dev] [PATCH 1/2] app/testpmd: add dynamic flag support
  2020-01-13  9:29 [dpdk-dev] [PATCH 0/2] net/mlx5: add PMD dynf Ori Kam
@ 2020-01-13  9:29 ` Ori Kam
  2020-01-15 13:31   ` Ferruh Yigit
  2020-01-13  9:29 ` [dpdk-dev] [PATCH 2/2] net/mlx5: add fine grain " Ori Kam
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Ori Kam @ 2020-01-13  9:29 UTC (permalink / raw)
  To: Wenzhuo Lu, Jingjing Wu, Bernard Iremonger, John McNamara,
	Marko Kovacevic
  Cc: dev, orika, ferruh.yigit, viacheslavo, matan

DPDK now supports registration of dynamic flags (dynf) to the mbuf.
dynf can be given any name, and can be used with a supporting PMD or
supporting application.

Due to the generic concept of the dynf, it is impossible and meaningless,
to define register set/get function for each flag.
This commit introduce a generic way to register and set/clear such flags.

The basic syntax:
port config <port id> dynf <name> <set|clear>

The first step the new flag is registered. Regardless if the action is
set or clear.
There is no way to unregister the flag, after registring it.

The second step, if the action is set then we set the requested flag.
If this is the first flag that is enabled we also register a call back
for the Tx. In this call back we set the flag.
If the action is clear the requested flag is cleared, and if there
are no more flags that are set, the call back is removed.

The reason that the set is only applied in Tx is that in case of Rx
it is assumed that the value comes from the PMD.

If log is enabled the name of the flag, and value will be printed
in the packet info.
In order for the log to work correcly the registration of the flag
must be done before setting verbose.

Signed-off-by: Ori Kam <orika@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 app/test-pmd/cmdline.c                      | 88 +++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h                      | 16 ++++++
 app/test-pmd/util.c                         | 63 +++++++++++++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 10 ++++
 4 files changed, 177 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 2d74df8..46c9291 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -40,6 +40,7 @@
 #include <rte_devargs.h>
 #include <rte_flow.h>
 #include <rte_gro.h>
+#include <rte_mbuf_dyn.h>
 
 #include <cmdline_rdline.h>
 #include <cmdline_parse.h>
@@ -70,6 +71,8 @@
 #include "cmdline_tm.h"
 #include "bpf_cmd.h"
 
+char dynf_names[64][RTE_MBUF_DYN_NAMESIZE];
+
 static struct cmdline *testpmd_cl;
 
 static void cmd_reconfig_device_queue(portid_t id, uint8_t dev, uint8_t queue);
@@ -901,6 +904,11 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"port config (port_id) tx_metadata (value)\n"
 			"    Set Tx metadata value per port. Testpmd will add this value"
 			" to any Tx packet sent from this port\n\n"
+
+			"port config (port_id) dynf (name) set|clear\n"
+			"    Register a dynf and Set/clear this flag on Tx. "
+			"Testpmd will set this value to any Tx packet "
+			"sent from this port\n\n"
 		);
 	}
 
@@ -18837,6 +18845,85 @@ struct cmd_config_tx_metadata_specific_result {
 	},
 };
 
+/* *** set dynf *** */
+struct cmd_config_tx_dynf_specific_result {
+	cmdline_fixed_string_t port;
+	cmdline_fixed_string_t keyword;
+	uint16_t port_id;
+	cmdline_fixed_string_t item;
+	cmdline_fixed_string_t name;
+	cmdline_fixed_string_t value;
+};
+
+static void
+cmd_config_dynf_specific_parsed(void *parsed_result,
+				__attribute__((unused)) struct cmdline *cl,
+				__attribute__((unused)) void *data)
+{
+	struct cmd_config_tx_dynf_specific_result *res = parsed_result;
+	struct rte_mbuf_dynflag desc_flag;
+	int flag;
+	uint64_t old_port_flags;
+
+	if (port_id_is_invalid(res->port_id, ENABLED_WARN))
+		return;
+	flag = rte_mbuf_dynflag_lookup(res->name, NULL);
+	if (flag <= 0) {
+		strcpy(desc_flag.name, res->name);
+		desc_flag.flags = 0;
+		flag = rte_mbuf_dynflag_register(&desc_flag);
+		if (flag < 0) {
+			printf("Can't register flag");
+			return;
+		}
+		strcpy(dynf_names[flag], res->name);
+	}
+	old_port_flags = ports[res->port_id].dynf;
+	if (!strcmp(res->value, "set")) {
+		ports[res->port_id].dynf |= 1UL << flag;
+		if (old_port_flags == 0)
+			add_tx_dynf_callback(res->port_id);
+	} else {
+		ports[res->port_id].dynf &= ~(1UL << flag);
+		if (ports[res->port_id].dynf == 0)
+			remove_tx_dynf_callback(res->port_id);
+	}
+}
+
+cmdline_parse_token_string_t cmd_config_tx_dynf_specific_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_tx_dynf_specific_result,
+			keyword, "port");
+cmdline_parse_token_string_t cmd_config_tx_dynf_specific_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_tx_dynf_specific_result,
+			keyword, "config");
+cmdline_parse_token_num_t cmd_config_tx_dynf_specific_port_id =
+	TOKEN_NUM_INITIALIZER(struct cmd_config_tx_dynf_specific_result,
+			port_id, UINT16);
+cmdline_parse_token_string_t cmd_config_tx_dynf_specific_item =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_tx_dynf_specific_result,
+			item, "dynf");
+cmdline_parse_token_string_t cmd_config_tx_dynf_specific_name =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_tx_dynf_specific_result,
+			name, NULL);
+cmdline_parse_token_string_t cmd_config_tx_dynf_specific_value =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_tx_dynf_specific_result,
+			value, "set#clear");
+
+cmdline_parse_inst_t cmd_config_tx_dynf_specific = {
+	.f = cmd_config_dynf_specific_parsed,
+	.data = NULL,
+	.help_str = "port config <port id> dynf <name> set|clear",
+	.tokens = {
+		(void *)&cmd_config_tx_dynf_specific_port,
+		(void *)&cmd_config_tx_dynf_specific_keyword,
+		(void *)&cmd_config_tx_dynf_specific_port_id,
+		(void *)&cmd_config_tx_dynf_specific_item,
+		(void *)&cmd_config_tx_dynf_specific_name,
+		(void *)&cmd_config_tx_dynf_specific_value,
+		NULL,
+	},
+};
+
 /* *** display tx_metadata per port configuration *** */
 struct cmd_show_tx_metadata_result {
 	cmdline_fixed_string_t cmd_show;
@@ -19520,6 +19607,7 @@ struct cmd_showport_macs_result {
 	(cmdline_parse_inst_t *)&cmd_set_raw,
 	(cmdline_parse_inst_t *)&cmd_show_set_raw,
 	(cmdline_parse_inst_t *)&cmd_show_set_raw_all,
+	(cmdline_parse_inst_t *)&cmd_config_tx_dynf_specific,
 	NULL,
 };
 
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 857a11f..49f3e43 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -104,6 +104,13 @@ struct rss_type_info {
 extern const struct rss_type_info rss_type_table[];
 
 /**
+ * Dynf name array.
+ *
+ * Array that holds the name for each dynf.
+ */
+extern char dynf_names[64][RTE_MBUF_DYN_NAMESIZE];
+
+/**
  * The data structure associated with a forwarding stream between a receive
  * port/queue and a transmit port/queue.
  */
@@ -193,6 +200,9 @@ struct rte_port {
 	/**< metadata value to insert in Tx packets. */
 	uint32_t		tx_metadata;
 	const struct rte_eth_rxtx_callback *tx_set_md_cb[RTE_MAX_QUEUES_PER_PORT+1];
+	/**< dynamic flags. */
+	uint64_t		dynf;
+	const struct rte_eth_rxtx_callback *tx_set_dynf_cb[RTE_MAX_QUEUES_PER_PORT+1];
 };
 
 /**
@@ -884,6 +894,12 @@ uint16_t tx_pkt_set_md(uint16_t port_id, __rte_unused uint16_t queue,
 void add_tx_md_callback(portid_t portid);
 void remove_tx_md_callback(portid_t portid);
 
+uint16_t tx_pkt_set_dynf(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_dynf_callback(portid_t portid);
+void remove_tx_dynf_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/util.c b/app/test-pmd/util.c
index b514be5..5af3d07 100644
--- a/app/test-pmd/util.c
+++ b/app/test-pmd/util.c
@@ -39,6 +39,7 @@
 	uint16_t udp_port;
 	uint32_t vx_vni;
 	const char *reason;
+	int dynf_index;
 
 	if (!nb_pkts)
 		return;
@@ -88,6 +89,12 @@
 		if (is_rx && (ol_flags & PKT_RX_DYNF_METADATA))
 			printf(" - Rx metadata: 0x%x",
 			       *RTE_FLOW_DYNF_METADATA(mb));
+		for (dynf_index = 0; dynf_index < 64; dynf_index++) {
+			if (dynf_names[dynf_index][0] != '\0')
+				printf(" - dynf %s: %d",
+				       dynf_names[dynf_index],
+				       !!(ol_flags & (1UL << dynf_index)));
+		}
 		if (mb->packet_type) {
 			rte_get_ptype_name(mb->packet_type, buf, sizeof(buf));
 			printf(" - hw ptype: %s", buf);
@@ -241,6 +248,62 @@
 		}
 }
 
+uint16_t
+tx_pkt_set_dynf(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;
+
+	if (ports[port_id].dynf)
+		for (i = 0; i < nb_pkts; i++)
+			pkts[i]->ol_flags |= ports[port_id].dynf;
+	return nb_pkts;
+}
+
+void
+add_tx_dynf_callback(portid_t portid)
+{
+	struct rte_eth_dev_info dev_info;
+	uint16_t queue;
+	int ret;
+
+	if (port_id_is_invalid(portid, ENABLED_WARN))
+		return;
+
+	ret = eth_dev_info_get_print_err(portid, &dev_info);
+	if (ret != 0)
+		return;
+
+	for (queue = 0; queue < dev_info.nb_tx_queues; queue++)
+		if (!ports[portid].tx_set_dynf_cb[queue])
+			ports[portid].tx_set_dynf_cb[queue] =
+				rte_eth_add_tx_callback(portid, queue,
+							tx_pkt_set_dynf, NULL);
+}
+
+void
+remove_tx_dynf_callback(portid_t portid)
+{
+	struct rte_eth_dev_info dev_info;
+	uint16_t queue;
+	int ret;
+
+	if (port_id_is_invalid(portid, ENABLED_WARN))
+		return;
+
+	ret = eth_dev_info_get_print_err(portid, &dev_info);
+	if (ret != 0)
+		return;
+
+	for (queue = 0; queue < dev_info.nb_tx_queues; queue++)
+		if (ports[portid].tx_set_dynf_cb[queue]) {
+			rte_eth_remove_tx_callback(portid, queue,
+				ports[portid].tx_set_dynf_cb[queue]);
+			ports[portid].tx_set_dynf_cb[queue] = NULL;
+		}
+}
+
 int
 eth_dev_info_get_print_err(uint16_t port_id,
 					struct rte_eth_dev_info *dev_info)
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 10aabf5..94327e1 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2317,6 +2317,16 @@ testpmd will add this value to any Tx packet sent from this port::
 
    testpmd> port config (port_id) tx_metadata (value)
 
+port config dynf
+~~~~~~~~~~~~~~~~
+
+Set/clear dynamic flag per port.
+testpmd will register this flag in the mbuf (same registration
+for both Tx and Rx). Then set/clear this flag for each Tx
+packet sent from this port. The set bit only works for Tx packet::
+
+   testpmd> port config (port_id) dynf (name) (set|clear)
+
 port config mtu
 ~~~~~~~~~~~~~~~
 
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [dpdk-dev] [PATCH 2/2] net/mlx5: add fine grain dynamic flag support
  2020-01-13  9:29 [dpdk-dev] [PATCH 0/2] net/mlx5: add PMD dynf Ori Kam
  2020-01-13  9:29 ` [dpdk-dev] [PATCH 1/2] app/testpmd: add dynamic flag support Ori Kam
@ 2020-01-13  9:29 ` Ori Kam
  2020-01-15 14:01   ` Ferruh Yigit
  2020-01-16 12:53 ` [dpdk-dev] [PATCH v2] app/testpmd: add " Ori Kam
  2020-01-29 12:21 ` [dpdk-dev] [PATCH v2 0/2] mlx5/net: hint PMD not to inline packet Viacheslav Ovsiienko
  3 siblings, 1 reply; 15+ messages in thread
From: Ori Kam @ 2020-01-13  9:29 UTC (permalink / raw)
  To: Matan Azrad, Shahaf Shuler, Viacheslav Ovsiienko; +Cc: dev, orika, ferruh.yigit

The inline feature is designed to save PCI bandwidth by copying some
of the data to the wqe. This feature if enabled works for all packets.

In some cases when using external memory, the PCI bandwidth is not
relevant since the memory can be accessed by other means.

This commit introduce the ability to control the inline with mbuf
granularity.

In order to use this feature the application should register the field
name, and restart the port.

Signed-off-by: Ori Kam <orika@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5.c                   | 15 +++++++++++++++
 drivers/net/mlx5/mlx5_rxtx.c              |  2 ++
 drivers/net/mlx5/mlx5_rxtx.h              |  3 +++
 drivers/net/mlx5/mlx5_trigger.c           |  8 ++++++++
 drivers/net/mlx5/rte_pmd_mlx5.h           | 32 +++++++++++++++++++++++++++++++
 drivers/net/mlx5/rte_pmd_mlx5_version.map |  7 +++++++
 6 files changed, 67 insertions(+)
 create mode 100644 drivers/net/mlx5/rte_pmd_mlx5.h

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 50960c9..27dbe27 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -46,6 +46,7 @@
 #include "mlx5_glue.h"
 #include "mlx5_mr.h"
 #include "mlx5_flow.h"
+#include "rte_pmd_mlx5.h"
 
 /* Device parameter to enable RX completion queue compression. */
 #define MLX5_RXQ_CQE_COMP_EN "rxq_cqe_comp_en"
@@ -1988,6 +1989,20 @@ struct mlx5_flow_id_pool *
 	return ret;
 }
 
+int
+rte_pmd_mlx5_get_dyn_flag_names(char *names[], uint16_t n)
+{
+	static const char *const dynf_names[] = {
+		RTE_PMD_MLX5_FINE_GRANULARITY_INLINE,
+	};
+	int num = RTE_MIN(n, RTE_DIM(dynf_names));
+	int i;
+
+	for (i = 0; i < num; i++)
+		strcpy(names[i], dynf_names[i]);
+	return num;
+}
+
 /**
  * Check sibling device configurations.
  *
diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index 67cafd1..aa6aa22 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -126,6 +126,8 @@ enum mlx5_txcmp_code {
 uint8_t mlx5_cksum_table[1 << 10] __rte_cache_aligned;
 uint8_t mlx5_swp_types_table[1 << 10] __rte_cache_aligned;
 
+uint64_t rte_net_mlx5_dynf_inline_mask;
+
 /**
  * Build a table to translate Rx completion flags to packet type.
  *
diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index e362b4a..7c38c57 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -42,6 +42,9 @@
 /* Support tunnel matching. */
 #define MLX5_FLOW_TUNNEL 9
 
+/* Mbuf dynamic flag offset for inline. */
+extern uint64_t rte_net_mlx5_dynf_inline_mask;
+
 struct mlx5_rxq_stats {
 #ifdef MLX5_PMD_SOFT_COUNTERS
 	uint64_t ipackets; /**< Total of successfully received packets. */
diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index ab6937a..ab253b2 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -13,6 +13,7 @@
 #include "mlx5.h"
 #include "mlx5_rxtx.h"
 #include "mlx5_utils.h"
+#include "rte_pmd_mlx5.h"
 
 /**
  * Stop traffic on Tx queues.
@@ -270,8 +271,15 @@
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
 	int ret;
+	int fine_inline;
 
 	DRV_LOG(DEBUG, "port %u starting device", dev->data->port_id);
+	fine_inline = rte_mbuf_dynflag_lookup
+		(RTE_PMD_MLX5_FINE_GRANULARITY_INLINE, NULL);
+	if (fine_inline > 0)
+		rte_net_mlx5_dynf_inline_mask = 1UL << fine_inline;
+	else
+		rte_net_mlx5_dynf_inline_mask = 0;
 	ret = mlx5_dev_configure_rss_reta(dev);
 	if (ret) {
 		DRV_LOG(ERR, "port %u reta config failed: %s",
diff --git a/drivers/net/mlx5/rte_pmd_mlx5.h b/drivers/net/mlx5/rte_pmd_mlx5.h
new file mode 100644
index 0000000..12e18ca
--- /dev/null
+++ b/drivers/net/mlx5/rte_pmd_mlx5.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+
+#ifndef RTE_PMD_PRIVATE_MLX5_H_
+#define RTE_PMD_PRIVATE_MLX5_H_
+
+/**
+ * @file
+ * MLX5 public header.
+ *
+ * This interface provides the ability to support private PMD
+ * dynamic flags.
+ */
+
+#define RTE_PMD_MLX5_FINE_GRANULARITY_INLINE "mlx5_fine_granularity_inline"
+
+/**
+ * Returns the dynamic flags name, that are supported.
+ *
+ * @param[out] names
+ *   Array that is used to return the supported dynamic flags names.
+ * @param[in] n
+ *   The number of elements in the names array.
+ *
+ * @return
+ *   The number of dynamic flags that were copied.
+ */
+__rte_experimental
+int rte_pmd_mlx5_get_dyn_flag_names(char *names[], uint16_t n);
+
+#endif
diff --git a/drivers/net/mlx5/rte_pmd_mlx5_version.map b/drivers/net/mlx5/rte_pmd_mlx5_version.map
index f9f17e4..c8b1031 100644
--- a/drivers/net/mlx5/rte_pmd_mlx5_version.map
+++ b/drivers/net/mlx5/rte_pmd_mlx5_version.map
@@ -1,3 +1,10 @@
 DPDK_20.0 {
 	local: *;
 };
+
+EXPERIMENTAL {
+        global:
+
+        # added in 20.02
+	rte_pmd_mlx5_get_dyn_flag_names;
+};
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] app/testpmd: add dynamic flag support
  2020-01-13  9:29 ` [dpdk-dev] [PATCH 1/2] app/testpmd: add dynamic flag support Ori Kam
@ 2020-01-15 13:31   ` Ferruh Yigit
  2020-01-16  8:07     ` Ori Kam
  0 siblings, 1 reply; 15+ messages in thread
From: Ferruh Yigit @ 2020-01-15 13:31 UTC (permalink / raw)
  To: Ori Kam, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger,
	John McNamara, Marko Kovacevic
  Cc: dev, viacheslavo, matan

On 1/13/2020 9:29 AM, Ori Kam wrote:
> DPDK now supports registration of dynamic flags (dynf) to the mbuf.
> dynf can be given any name, and can be used with a supporting PMD or
> supporting application.
> 
> Due to the generic concept of the dynf, it is impossible and meaningless,
> to define register set/get function for each flag.
> This commit introduce a generic way to register and set/clear such flags.
> 
> The basic syntax:
> port config <port id> dynf <name> <set|clear>

+1 to command

> 
> The first step the new flag is registered. Regardless if the action is
> set or clear.
> There is no way to unregister the flag, after registring it.
> 
> The second step, if the action is set then we set the requested flag.
> If this is the first flag that is enabled we also register a call back
> for the Tx. In this call back we set the flag.
> If the action is clear the requested flag is cleared, and if there
> are no more flags that are set, the call back is removed.
> 
> The reason that the set is only applied in Tx is that in case of Rx
> it is assumed that the value comes from the PMD.
> 
> If log is enabled the name of the flag, and value will be printed
> in the packet info.
> In order for the log to work correcly the registration of the flag
> must be done before setting verbose.
> 
> Signed-off-by: Ori Kam <orika@mellanox.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

<...>

> +++ b/app/test-pmd/cmdline.c
> @@ -40,6 +40,7 @@
>  #include <rte_devargs.h>
>  #include <rte_flow.h>
>  #include <rte_gro.h>
> +#include <rte_mbuf_dyn.h>
>  
>  #include <cmdline_rdline.h>
>  #include <cmdline_parse.h>
> @@ -70,6 +71,8 @@
>  #include "cmdline_tm.h"
>  #include "bpf_cmd.h"
>  
> +char dynf_names[64][RTE_MBUF_DYN_NAMESIZE];
> +

I don't think 'cmdline.c' is good place for this global variable, can you please
move it to 'testpmd.c' among other global variables and can you please add some
comment as others do in that same file.

<...>

> +static void
> +cmd_config_dynf_specific_parsed(void *parsed_result,
> +				__attribute__((unused)) struct cmdline *cl,
> +				__attribute__((unused)) void *data)
> +{
> +	struct cmd_config_tx_dynf_specific_result *res = parsed_result;
> +	struct rte_mbuf_dynflag desc_flag;
> +	int flag;
> +	uint64_t old_port_flags;
> +
> +	if (port_id_is_invalid(res->port_id, ENABLED_WARN))
> +		return;
> +	flag = rte_mbuf_dynflag_lookup(res->name, NULL);
> +	if (flag <= 0) {
> +		strcpy(desc_flag.name, res->name);
> +		desc_flag.flags = 0;
> +		flag = rte_mbuf_dynflag_register(&desc_flag);
> +		if (flag < 0) {
> +			printf("Can't register flag");

"\n" is missing, which prevents the io buffer to be flushed and the log
displayed (at least for a long time).

<...>

> @@ -193,6 +200,9 @@ struct rte_port {
>  	/**< metadata value to insert in Tx packets. */
>  	uint32_t		tx_metadata;
>  	const struct rte_eth_rxtx_callback *tx_set_md_cb[RTE_MAX_QUEUES_PER_PORT+1];
> +	/**< dynamic flags. */
> +	uint64_t		dynf;

Everywhe in this patch, variables/descriptions referred as 'dynf' or "dynamic
flags", I think it would be better to prefix 'mbuf' to it, in case in the fure
we throw more dynamic stuff, just "dynamic flags" missing context. Yes, it will
make variable names longer but I think it will be more clear.

Not sure on the testpmd command though, no strong optinion but there I think
context is clear enough to continue with 'dynf' ("port config <port id> dynf
<name> set|clear").

<...>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] net/mlx5: add fine grain dynamic flag support
  2020-01-13  9:29 ` [dpdk-dev] [PATCH 2/2] net/mlx5: add fine grain " Ori Kam
@ 2020-01-15 14:01   ` Ferruh Yigit
  2020-01-16 12:05     ` Ori Kam
  0 siblings, 1 reply; 15+ messages in thread
From: Ferruh Yigit @ 2020-01-15 14:01 UTC (permalink / raw)
  To: Ori Kam, Matan Azrad, Shahaf Shuler, Viacheslav Ovsiienko; +Cc: dev

On 1/13/2020 9:29 AM, Ori Kam wrote:
> The inline feature is designed to save PCI bandwidth by copying some
> of the data to the wqe. This feature if enabled works for all packets.
> 
> In some cases when using external memory, the PCI bandwidth is not
> relevant since the memory can be accessed by other means.
> 
> This commit introduce the ability to control the inline with mbuf
> granularity.
> 
> In order to use this feature the application should register the field
> name, and restart the port.
> 
> Signed-off-by: Ori Kam <orika@mellanox.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5.c                   | 15 +++++++++++++++
>  drivers/net/mlx5/mlx5_rxtx.c              |  2 ++
>  drivers/net/mlx5/mlx5_rxtx.h              |  3 +++
>  drivers/net/mlx5/mlx5_trigger.c           |  8 ++++++++
>  drivers/net/mlx5/rte_pmd_mlx5.h           | 32 +++++++++++++++++++++++++++++++
>  drivers/net/mlx5/rte_pmd_mlx5_version.map |  7 +++++++
>  6 files changed, 67 insertions(+)
>  create mode 100644 drivers/net/mlx5/rte_pmd_mlx5.h
> 
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index 50960c9..27dbe27 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -46,6 +46,7 @@
>  #include "mlx5_glue.h"
>  #include "mlx5_mr.h"
>  #include "mlx5_flow.h"
> +#include "rte_pmd_mlx5.h"
>  
>  /* Device parameter to enable RX completion queue compression. */
>  #define MLX5_RXQ_CQE_COMP_EN "rxq_cqe_comp_en"
> @@ -1988,6 +1989,20 @@ struct mlx5_flow_id_pool *
>  	return ret;
>  }
>  
> +int
> +rte_pmd_mlx5_get_dyn_flag_names(char *names[], uint16_t n)
> +{

Now this is a public API, it should validate the user input.

> +	static const char *const dynf_names[] = {
> +		RTE_PMD_MLX5_FINE_GRANULARITY_INLINE,
> +	};
> +	int num = RTE_MIN(n, RTE_DIM(dynf_names));
> +	int i;
> +
> +	for (i = 0; i < num; i++)
> +		strcpy(names[i], dynf_names[i]);
> +	return num;
> +}
> +
>  /**
>   * Check sibling device configurations.
>   *
> diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
> index 67cafd1..aa6aa22 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.c
> +++ b/drivers/net/mlx5/mlx5_rxtx.c
> @@ -126,6 +126,8 @@ enum mlx5_txcmp_code {
>  uint8_t mlx5_cksum_table[1 << 10] __rte_cache_aligned;
>  uint8_t mlx5_swp_types_table[1 << 10] __rte_cache_aligned;
>  
> +uint64_t rte_net_mlx5_dynf_inline_mask;
> +
>  /**
>   * Build a table to translate Rx completion flags to packet type.
>   *
> diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
> index e362b4a..7c38c57 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.h
> +++ b/drivers/net/mlx5/mlx5_rxtx.h
> @@ -42,6 +42,9 @@
>  /* Support tunnel matching. */
>  #define MLX5_FLOW_TUNNEL 9
>  
> +/* Mbuf dynamic flag offset for inline. */
> +extern uint64_t rte_net_mlx5_dynf_inline_mask;
> +
>  struct mlx5_rxq_stats {
>  #ifdef MLX5_PMD_SOFT_COUNTERS
>  	uint64_t ipackets; /**< Total of successfully received packets. */
> diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
> index ab6937a..ab253b2 100644
> --- a/drivers/net/mlx5/mlx5_trigger.c
> +++ b/drivers/net/mlx5/mlx5_trigger.c
> @@ -13,6 +13,7 @@
>  #include "mlx5.h"
>  #include "mlx5_rxtx.h"
>  #include "mlx5_utils.h"
> +#include "rte_pmd_mlx5.h"
>  
>  /**
>   * Stop traffic on Tx queues.
> @@ -270,8 +271,15 @@
>  {
>  	struct mlx5_priv *priv = dev->data->dev_private;
>  	int ret;
> +	int fine_inline;
>  
>  	DRV_LOG(DEBUG, "port %u starting device", dev->data->port_id);
> +	fine_inline = rte_mbuf_dynflag_lookup
> +		(RTE_PMD_MLX5_FINE_GRANULARITY_INLINE, NULL);
> +	if (fine_inline > 0)
> +		rte_net_mlx5_dynf_inline_mask = 1UL << fine_inline;
> +	else
> +		rte_net_mlx5_dynf_inline_mask = 0;
>  	ret = mlx5_dev_configure_rss_reta(dev);
>  	if (ret) {
>  		DRV_LOG(ERR, "port %u reta config failed: %s",
> diff --git a/drivers/net/mlx5/rte_pmd_mlx5.h b/drivers/net/mlx5/rte_pmd_mlx5.h
> new file mode 100644
> index 0000000..12e18ca
> --- /dev/null
> +++ b/drivers/net/mlx5/rte_pmd_mlx5.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright 2020 Mellanox Technologies, Ltd
> + */
> +
> +#ifndef RTE_PMD_PRIVATE_MLX5_H_
> +#define RTE_PMD_PRIVATE_MLX5_H_
> +
> +/**
> + * @file
> + * MLX5 public header.
> + *
> + * This interface provides the ability to support private PMD
> + * dynamic flags.
> + */
> +
> +#define RTE_PMD_MLX5_FINE_GRANULARITY_INLINE "mlx5_fine_granularity_inline"
> +
> +/**
> + * Returns the dynamic flags name, that are supported.
> + *
> + * @param[out] names
> + *   Array that is used to return the supported dynamic flags names.
> + * @param[in] n
> + *   The number of elements in the names array.
> + *
> + * @return
> + *   The number of dynamic flags that were copied.
> + */
> +__rte_experimental
> +int rte_pmd_mlx5_get_dyn_flag_names(char *names[], uint16_t n);

Can you please add this header to the API documentation index,
doc/api/doxy-api-index.md, so it will be part of API document.

> +
> +#endif
> diff --git a/drivers/net/mlx5/rte_pmd_mlx5_version.map b/drivers/net/mlx5/rte_pmd_mlx5_version.map
> index f9f17e4..c8b1031 100644
> --- a/drivers/net/mlx5/rte_pmd_mlx5_version.map
> +++ b/drivers/net/mlx5/rte_pmd_mlx5_version.map
> @@ -1,3 +1,10 @@
>  DPDK_20.0 {
>  	local: *;
>  };
> +
> +EXPERIMENTAL {
> +        global:
> +
> +        # added in 20.02
> +	rte_pmd_mlx5_get_dyn_flag_names;
> +};
> 

Isn't the datapath implementation missing? Where this new mbuf dynamic flag set
or checked?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] app/testpmd: add dynamic flag support
  2020-01-15 13:31   ` Ferruh Yigit
@ 2020-01-16  8:07     ` Ori Kam
  0 siblings, 0 replies; 15+ messages in thread
From: Ori Kam @ 2020-01-16  8:07 UTC (permalink / raw)
  To: Ferruh Yigit, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger,
	John McNamara, Marko Kovacevic
  Cc: dev, Slava Ovsiienko, Matan Azrad

Hi Ferruh,

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, January 15, 2020 3:32 PM
> To: Ori Kam <orika@mellanox.com>; Wenzhuo Lu <wenzhuo.lu@intel.com>;
> Jingjing Wu <jingjing.wu@intel.com>; Bernard Iremonger
> <bernard.iremonger@intel.com>; John McNamara
> <john.mcnamara@intel.com>; Marko Kovacevic
> <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org; Slava Ovsiienko <viacheslavo@mellanox.com>; Matan
> Azrad <matan@mellanox.com>
> Subject: Re: [PATCH 1/2] app/testpmd: add dynamic flag support
> 
> On 1/13/2020 9:29 AM, Ori Kam wrote:
> > DPDK now supports registration of dynamic flags (dynf) to the mbuf.
> > dynf can be given any name, and can be used with a supporting PMD or
> > supporting application.
> >
> > Due to the generic concept of the dynf, it is impossible and meaningless,
> > to define register set/get function for each flag.
> > This commit introduce a generic way to register and set/clear such flags.
> >
> > The basic syntax:
> > port config <port id> dynf <name> <set|clear>
> 
> +1 to command
> 
> >
> > The first step the new flag is registered. Regardless if the action is
> > set or clear.
> > There is no way to unregister the flag, after registring it.
> >
> > The second step, if the action is set then we set the requested flag.
> > If this is the first flag that is enabled we also register a call back
> > for the Tx. In this call back we set the flag.
> > If the action is clear the requested flag is cleared, and if there
> > are no more flags that are set, the call back is removed.
> >
> > The reason that the set is only applied in Tx is that in case of Rx
> > it is assumed that the value comes from the PMD.
> >
> > If log is enabled the name of the flag, and value will be printed
> > in the packet info.
> > In order for the log to work correcly the registration of the flag
> > must be done before setting verbose.
> >
> > Signed-off-by: Ori Kam <orika@mellanox.com>
> > Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> 
> <...>
> 
> > +++ b/app/test-pmd/cmdline.c
> > @@ -40,6 +40,7 @@
> >  #include <rte_devargs.h>
> >  #include <rte_flow.h>
> >  #include <rte_gro.h>
> > +#include <rte_mbuf_dyn.h>
> >
> >  #include <cmdline_rdline.h>
> >  #include <cmdline_parse.h>
> > @@ -70,6 +71,8 @@
> >  #include "cmdline_tm.h"
> >  #include "bpf_cmd.h"
> >
> > +char dynf_names[64][RTE_MBUF_DYN_NAMESIZE];
> > +
> 
> I don't think 'cmdline.c' is good place for this global variable, can you please
> move it to 'testpmd.c' among other global variables and can you please add
> some
> comment as others do in that same file.
> 

Sure will move.

> <...>
> 
> > +static void
> > +cmd_config_dynf_specific_parsed(void *parsed_result,
> > +				__attribute__((unused)) struct cmdline *cl,
> > +				__attribute__((unused)) void *data)
> > +{
> > +	struct cmd_config_tx_dynf_specific_result *res = parsed_result;
> > +	struct rte_mbuf_dynflag desc_flag;
> > +	int flag;
> > +	uint64_t old_port_flags;
> > +
> > +	if (port_id_is_invalid(res->port_id, ENABLED_WARN))
> > +		return;
> > +	flag = rte_mbuf_dynflag_lookup(res->name, NULL);
> > +	if (flag <= 0) {
> > +		strcpy(desc_flag.name, res->name);
> > +		desc_flag.flags = 0;
> > +		flag = rte_mbuf_dynflag_register(&desc_flag);
> > +		if (flag < 0) {
> > +			printf("Can't register flag");
> 
> "\n" is missing, which prevents the io buffer to be flushed and the log
> displayed (at least for a long time).
> 

Will add missing \n

> <...>
> 
> > @@ -193,6 +200,9 @@ struct rte_port {
> >  	/**< metadata value to insert in Tx packets. */
> >  	uint32_t		tx_metadata;
> >  	const struct rte_eth_rxtx_callback
> *tx_set_md_cb[RTE_MAX_QUEUES_PER_PORT+1];
> > +	/**< dynamic flags. */
> > +	uint64_t		dynf;
> 
> Everywhe in this patch, variables/descriptions referred as 'dynf' or "dynamic
> flags", I think it would be better to prefix 'mbuf' to it, in case in the fure
> we throw more dynamic stuff, just "dynamic flags" missing context. Yes, it
> will
> make variable names longer but I think it will be more clear.
>

O.K will add mbuf_
 
> Not sure on the testpmd command though, no strong optinion but there I
> think
> context is clear enough to continue with 'dynf' ("port config <port id> dynf
> <name> set|clear").
> 
I will leave it as it is now.

> <...>

Best,
Ori

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] net/mlx5: add fine grain dynamic flag support
  2020-01-15 14:01   ` Ferruh Yigit
@ 2020-01-16 12:05     ` Ori Kam
  2020-01-16 12:24       ` Ferruh Yigit
  0 siblings, 1 reply; 15+ messages in thread
From: Ori Kam @ 2020-01-16 12:05 UTC (permalink / raw)
  To: Ferruh Yigit, Matan Azrad, Shahaf Shuler, Slava Ovsiienko; +Cc: dev

Hi Ferruh,

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, January 15, 2020 4:02 PM
> To: Ori Kam <orika@mellanox.com>; Matan Azrad <matan@mellanox.com>;
> Shahaf Shuler <shahafs@mellanox.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH 2/2] net/mlx5: add fine grain dynamic flag support
> 
> On 1/13/2020 9:29 AM, Ori Kam wrote:
> > The inline feature is designed to save PCI bandwidth by copying some
> > of the data to the wqe. This feature if enabled works for all packets.
> >
> > In some cases when using external memory, the PCI bandwidth is not
> > relevant since the memory can be accessed by other means.
> >
> > This commit introduce the ability to control the inline with mbuf
> > granularity.
> >
> > In order to use this feature the application should register the field
> > name, and restart the port.
> >
> > Signed-off-by: Ori Kam <orika@mellanox.com>
> > Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > ---
> >  drivers/net/mlx5/mlx5.c                   | 15 +++++++++++++++
> >  drivers/net/mlx5/mlx5_rxtx.c              |  2 ++
> >  drivers/net/mlx5/mlx5_rxtx.h              |  3 +++
> >  drivers/net/mlx5/mlx5_trigger.c           |  8 ++++++++
> >  drivers/net/mlx5/rte_pmd_mlx5.h           | 32
> +++++++++++++++++++++++++++++++
> >  drivers/net/mlx5/rte_pmd_mlx5_version.map |  7 +++++++
> >  6 files changed, 67 insertions(+)
> >  create mode 100644 drivers/net/mlx5/rte_pmd_mlx5.h
> >
> > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> > index 50960c9..27dbe27 100644
> > --- a/drivers/net/mlx5/mlx5.c
> > +++ b/drivers/net/mlx5/mlx5.c
> > @@ -46,6 +46,7 @@
> >  #include "mlx5_glue.h"
> >  #include "mlx5_mr.h"
> >  #include "mlx5_flow.h"
> > +#include "rte_pmd_mlx5.h"
> >
> >  /* Device parameter to enable RX completion queue compression. */
> >  #define MLX5_RXQ_CQE_COMP_EN "rxq_cqe_comp_en"
> > @@ -1988,6 +1989,20 @@ struct mlx5_flow_id_pool *
> >  	return ret;
> >  }
> >
> > +int
> > +rte_pmd_mlx5_get_dyn_flag_names(char *names[], uint16_t n)
> > +{
> 
> Now this is a public API, it should validate the user input.
> 
Will add validation to make sure names != NULL,

> > +	static const char *const dynf_names[] = {
> > +		RTE_PMD_MLX5_FINE_GRANULARITY_INLINE,
> > +	};
> > +	int num = RTE_MIN(n, RTE_DIM(dynf_names));
> > +	int i;
> > +
> > +	for (i = 0; i < num; i++)
> > +		strcpy(names[i], dynf_names[i]);
> > +	return num;
> > +}
> > +
> >  /**
> >   * Check sibling device configurations.
> >   *
> > diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
> > index 67cafd1..aa6aa22 100644
> > --- a/drivers/net/mlx5/mlx5_rxtx.c
> > +++ b/drivers/net/mlx5/mlx5_rxtx.c
> > @@ -126,6 +126,8 @@ enum mlx5_txcmp_code {
> >  uint8_t mlx5_cksum_table[1 << 10] __rte_cache_aligned;
> >  uint8_t mlx5_swp_types_table[1 << 10] __rte_cache_aligned;
> >
> > +uint64_t rte_net_mlx5_dynf_inline_mask;
> > +
> >  /**
> >   * Build a table to translate Rx completion flags to packet type.
> >   *
> > diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
> > index e362b4a..7c38c57 100644
> > --- a/drivers/net/mlx5/mlx5_rxtx.h
> > +++ b/drivers/net/mlx5/mlx5_rxtx.h
> > @@ -42,6 +42,9 @@
> >  /* Support tunnel matching. */
> >  #define MLX5_FLOW_TUNNEL 9
> >
> > +/* Mbuf dynamic flag offset for inline. */
> > +extern uint64_t rte_net_mlx5_dynf_inline_mask;
> > +
> >  struct mlx5_rxq_stats {
> >  #ifdef MLX5_PMD_SOFT_COUNTERS
> >  	uint64_t ipackets; /**< Total of successfully received packets. */
> > diff --git a/drivers/net/mlx5/mlx5_trigger.c
> b/drivers/net/mlx5/mlx5_trigger.c
> > index ab6937a..ab253b2 100644
> > --- a/drivers/net/mlx5/mlx5_trigger.c
> > +++ b/drivers/net/mlx5/mlx5_trigger.c
> > @@ -13,6 +13,7 @@
> >  #include "mlx5.h"
> >  #include "mlx5_rxtx.h"
> >  #include "mlx5_utils.h"
> > +#include "rte_pmd_mlx5.h"
> >
> >  /**
> >   * Stop traffic on Tx queues.
> > @@ -270,8 +271,15 @@
> >  {
> >  	struct mlx5_priv *priv = dev->data->dev_private;
> >  	int ret;
> > +	int fine_inline;
> >
> >  	DRV_LOG(DEBUG, "port %u starting device", dev->data->port_id);
> > +	fine_inline = rte_mbuf_dynflag_lookup
> > +		(RTE_PMD_MLX5_FINE_GRANULARITY_INLINE, NULL);
> > +	if (fine_inline > 0)
> > +		rte_net_mlx5_dynf_inline_mask = 1UL << fine_inline;
> > +	else
> > +		rte_net_mlx5_dynf_inline_mask = 0;
> >  	ret = mlx5_dev_configure_rss_reta(dev);
> >  	if (ret) {
> >  		DRV_LOG(ERR, "port %u reta config failed: %s",
> > diff --git a/drivers/net/mlx5/rte_pmd_mlx5.h
> b/drivers/net/mlx5/rte_pmd_mlx5.h
> > new file mode 100644
> > index 0000000..12e18ca
> > --- /dev/null
> > +++ b/drivers/net/mlx5/rte_pmd_mlx5.h
> > @@ -0,0 +1,32 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright 2020 Mellanox Technologies, Ltd
> > + */
> > +
> > +#ifndef RTE_PMD_PRIVATE_MLX5_H_
> > +#define RTE_PMD_PRIVATE_MLX5_H_
> > +
> > +/**
> > + * @file
> > + * MLX5 public header.
> > + *
> > + * This interface provides the ability to support private PMD
> > + * dynamic flags.
> > + */
> > +
> > +#define RTE_PMD_MLX5_FINE_GRANULARITY_INLINE
> "mlx5_fine_granularity_inline"
> > +
> > +/**
> > + * Returns the dynamic flags name, that are supported.
> > + *
> > + * @param[out] names
> > + *   Array that is used to return the supported dynamic flags names.
> > + * @param[in] n
> > + *   The number of elements in the names array.
> > + *
> > + * @return
> > + *   The number of dynamic flags that were copied.
> > + */
> > +__rte_experimental
> > +int rte_pmd_mlx5_get_dyn_flag_names(char *names[], uint16_t n);
> 
> Can you please add this header to the API documentation index,
> doc/api/doxy-api-index.md, so it will be part of API document.
> 

Will add.

> > +
> > +#endif
> > diff --git a/drivers/net/mlx5/rte_pmd_mlx5_version.map
> b/drivers/net/mlx5/rte_pmd_mlx5_version.map
> > index f9f17e4..c8b1031 100644
> > --- a/drivers/net/mlx5/rte_pmd_mlx5_version.map
> > +++ b/drivers/net/mlx5/rte_pmd_mlx5_version.map
> > @@ -1,3 +1,10 @@
> >  DPDK_20.0 {
> >  	local: *;
> >  };
> > +
> > +EXPERIMENTAL {
> > +        global:
> > +
> > +        # added in 20.02
> > +	rte_pmd_mlx5_get_dyn_flag_names;
> > +};
> >
> 
> Isn't the datapath implementation missing? Where this new mbuf dynamic
> flag set
> or checked?

The data path implementation will be done in different patch.
The flags is set for example using the testpmd new API or by the application.
The reason that I added this patch is to show usage for the testpmd patch, I can remove this patch and 
re send it when sending the datapath patch.
What do you think?



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] net/mlx5: add fine grain dynamic flag support
  2020-01-16 12:05     ` Ori Kam
@ 2020-01-16 12:24       ` Ferruh Yigit
  2020-01-16 12:37         ` Ori Kam
  0 siblings, 1 reply; 15+ messages in thread
From: Ferruh Yigit @ 2020-01-16 12:24 UTC (permalink / raw)
  To: Ori Kam, Matan Azrad, Shahaf Shuler, Slava Ovsiienko; +Cc: dev

On 1/16/2020 12:05 PM, Ori Kam wrote:
> Hi Ferruh,
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Wednesday, January 15, 2020 4:02 PM
>> To: Ori Kam <orika@mellanox.com>; Matan Azrad <matan@mellanox.com>;
>> Shahaf Shuler <shahafs@mellanox.com>; Slava Ovsiienko
>> <viacheslavo@mellanox.com>
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH 2/2] net/mlx5: add fine grain dynamic flag support
>>
>> On 1/13/2020 9:29 AM, Ori Kam wrote:
>>> The inline feature is designed to save PCI bandwidth by copying some
>>> of the data to the wqe. This feature if enabled works for all packets.
>>>
>>> In some cases when using external memory, the PCI bandwidth is not
>>> relevant since the memory can be accessed by other means.
>>>
>>> This commit introduce the ability to control the inline with mbuf
>>> granularity.
>>>
>>> In order to use this feature the application should register the field
>>> name, and restart the port.
>>>
>>> Signed-off-by: Ori Kam <orika@mellanox.com>
>>> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
>>> ---
>>>  drivers/net/mlx5/mlx5.c                   | 15 +++++++++++++++
>>>  drivers/net/mlx5/mlx5_rxtx.c              |  2 ++
>>>  drivers/net/mlx5/mlx5_rxtx.h              |  3 +++
>>>  drivers/net/mlx5/mlx5_trigger.c           |  8 ++++++++
>>>  drivers/net/mlx5/rte_pmd_mlx5.h           | 32
>> +++++++++++++++++++++++++++++++
>>>  drivers/net/mlx5/rte_pmd_mlx5_version.map |  7 +++++++
>>>  6 files changed, 67 insertions(+)
>>>  create mode 100644 drivers/net/mlx5/rte_pmd_mlx5.h
>>>
>>> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
>>> index 50960c9..27dbe27 100644
>>> --- a/drivers/net/mlx5/mlx5.c
>>> +++ b/drivers/net/mlx5/mlx5.c
>>> @@ -46,6 +46,7 @@
>>>  #include "mlx5_glue.h"
>>>  #include "mlx5_mr.h"
>>>  #include "mlx5_flow.h"
>>> +#include "rte_pmd_mlx5.h"
>>>
>>>  /* Device parameter to enable RX completion queue compression. */
>>>  #define MLX5_RXQ_CQE_COMP_EN "rxq_cqe_comp_en"
>>> @@ -1988,6 +1989,20 @@ struct mlx5_flow_id_pool *
>>>  	return ret;
>>>  }
>>>
>>> +int
>>> +rte_pmd_mlx5_get_dyn_flag_names(char *names[], uint16_t n)
>>> +{
>>
>> Now this is a public API, it should validate the user input.
>>
> Will add validation to make sure names != NULL,
> 
>>> +	static const char *const dynf_names[] = {
>>> +		RTE_PMD_MLX5_FINE_GRANULARITY_INLINE,
>>> +	};
>>> +	int num = RTE_MIN(n, RTE_DIM(dynf_names));
>>> +	int i;
>>> +
>>> +	for (i = 0; i < num; i++)
>>> +		strcpy(names[i], dynf_names[i]);
>>> +	return num;
>>> +}
>>> +
>>>  /**
>>>   * Check sibling device configurations.
>>>   *
>>> diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
>>> index 67cafd1..aa6aa22 100644
>>> --- a/drivers/net/mlx5/mlx5_rxtx.c
>>> +++ b/drivers/net/mlx5/mlx5_rxtx.c
>>> @@ -126,6 +126,8 @@ enum mlx5_txcmp_code {
>>>  uint8_t mlx5_cksum_table[1 << 10] __rte_cache_aligned;
>>>  uint8_t mlx5_swp_types_table[1 << 10] __rte_cache_aligned;
>>>
>>> +uint64_t rte_net_mlx5_dynf_inline_mask;
>>> +
>>>  /**
>>>   * Build a table to translate Rx completion flags to packet type.
>>>   *
>>> diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
>>> index e362b4a..7c38c57 100644
>>> --- a/drivers/net/mlx5/mlx5_rxtx.h
>>> +++ b/drivers/net/mlx5/mlx5_rxtx.h
>>> @@ -42,6 +42,9 @@
>>>  /* Support tunnel matching. */
>>>  #define MLX5_FLOW_TUNNEL 9
>>>
>>> +/* Mbuf dynamic flag offset for inline. */
>>> +extern uint64_t rte_net_mlx5_dynf_inline_mask;
>>> +
>>>  struct mlx5_rxq_stats {
>>>  #ifdef MLX5_PMD_SOFT_COUNTERS
>>>  	uint64_t ipackets; /**< Total of successfully received packets. */
>>> diff --git a/drivers/net/mlx5/mlx5_trigger.c
>> b/drivers/net/mlx5/mlx5_trigger.c
>>> index ab6937a..ab253b2 100644
>>> --- a/drivers/net/mlx5/mlx5_trigger.c
>>> +++ b/drivers/net/mlx5/mlx5_trigger.c
>>> @@ -13,6 +13,7 @@
>>>  #include "mlx5.h"
>>>  #include "mlx5_rxtx.h"
>>>  #include "mlx5_utils.h"
>>> +#include "rte_pmd_mlx5.h"
>>>
>>>  /**
>>>   * Stop traffic on Tx queues.
>>> @@ -270,8 +271,15 @@
>>>  {
>>>  	struct mlx5_priv *priv = dev->data->dev_private;
>>>  	int ret;
>>> +	int fine_inline;
>>>
>>>  	DRV_LOG(DEBUG, "port %u starting device", dev->data->port_id);
>>> +	fine_inline = rte_mbuf_dynflag_lookup
>>> +		(RTE_PMD_MLX5_FINE_GRANULARITY_INLINE, NULL);
>>> +	if (fine_inline > 0)
>>> +		rte_net_mlx5_dynf_inline_mask = 1UL << fine_inline;
>>> +	else
>>> +		rte_net_mlx5_dynf_inline_mask = 0;
>>>  	ret = mlx5_dev_configure_rss_reta(dev);
>>>  	if (ret) {
>>>  		DRV_LOG(ERR, "port %u reta config failed: %s",
>>> diff --git a/drivers/net/mlx5/rte_pmd_mlx5.h
>> b/drivers/net/mlx5/rte_pmd_mlx5.h
>>> new file mode 100644
>>> index 0000000..12e18ca
>>> --- /dev/null
>>> +++ b/drivers/net/mlx5/rte_pmd_mlx5.h
>>> @@ -0,0 +1,32 @@
>>> +/* SPDX-License-Identifier: BSD-3-Clause
>>> + * Copyright 2020 Mellanox Technologies, Ltd
>>> + */
>>> +
>>> +#ifndef RTE_PMD_PRIVATE_MLX5_H_
>>> +#define RTE_PMD_PRIVATE_MLX5_H_
>>> +
>>> +/**
>>> + * @file
>>> + * MLX5 public header.
>>> + *
>>> + * This interface provides the ability to support private PMD
>>> + * dynamic flags.
>>> + */
>>> +
>>> +#define RTE_PMD_MLX5_FINE_GRANULARITY_INLINE
>> "mlx5_fine_granularity_inline"
>>> +
>>> +/**
>>> + * Returns the dynamic flags name, that are supported.
>>> + *
>>> + * @param[out] names
>>> + *   Array that is used to return the supported dynamic flags names.
>>> + * @param[in] n
>>> + *   The number of elements in the names array.
>>> + *
>>> + * @return
>>> + *   The number of dynamic flags that were copied.
>>> + */
>>> +__rte_experimental
>>> +int rte_pmd_mlx5_get_dyn_flag_names(char *names[], uint16_t n);
>>
>> Can you please add this header to the API documentation index,
>> doc/api/doxy-api-index.md, so it will be part of API document.
>>
> 
> Will add.
> 
>>> +
>>> +#endif
>>> diff --git a/drivers/net/mlx5/rte_pmd_mlx5_version.map
>> b/drivers/net/mlx5/rte_pmd_mlx5_version.map
>>> index f9f17e4..c8b1031 100644
>>> --- a/drivers/net/mlx5/rte_pmd_mlx5_version.map
>>> +++ b/drivers/net/mlx5/rte_pmd_mlx5_version.map
>>> @@ -1,3 +1,10 @@
>>>  DPDK_20.0 {
>>>  	local: *;
>>>  };
>>> +
>>> +EXPERIMENTAL {
>>> +        global:
>>> +
>>> +        # added in 20.02
>>> +	rte_pmd_mlx5_get_dyn_flag_names;
>>> +};
>>>
>>
>> Isn't the datapath implementation missing? Where this new mbuf dynamic
>> flag set
>> or checked?
> 
> The data path implementation will be done in different patch.
> The flags is set for example using the testpmd new API or by the application.
> The reason that I added this patch is to show usage for the testpmd patch, I can remove this patch and 
> re send it when sending the datapath patch.
> What do you think?
> 

I think testpmd patch is clear enough on its own and better to send PMD patch
separately when it is complete.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] net/mlx5: add fine grain dynamic flag support
  2020-01-16 12:24       ` Ferruh Yigit
@ 2020-01-16 12:37         ` Ori Kam
  0 siblings, 0 replies; 15+ messages in thread
From: Ori Kam @ 2020-01-16 12:37 UTC (permalink / raw)
  To: Ferruh Yigit, Matan Azrad, Shahaf Shuler, Slava Ovsiienko; +Cc: dev



> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, January 16, 2020 2:24 PM
> To: Ori Kam <orika@mellanox.com>; Matan Azrad <matan@mellanox.com>;
> Shahaf Shuler <shahafs@mellanox.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/2] net/mlx5: add fine grain dynamic flag
> support
> 
> On 1/16/2020 12:05 PM, Ori Kam wrote:
> > Hi Ferruh,
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> Sent: Wednesday, January 15, 2020 4:02 PM
> >> To: Ori Kam <orika@mellanox.com>; Matan Azrad
> <matan@mellanox.com>;
> >> Shahaf Shuler <shahafs@mellanox.com>; Slava Ovsiienko
> >> <viacheslavo@mellanox.com>
> >> Cc: dev@dpdk.org
> >> Subject: Re: [PATCH 2/2] net/mlx5: add fine grain dynamic flag support
> >>
> >> On 1/13/2020 9:29 AM, Ori Kam wrote:
> >>> The inline feature is designed to save PCI bandwidth by copying some
> >>> of the data to the wqe. This feature if enabled works for all packets.
> >>>
> >>> In some cases when using external memory, the PCI bandwidth is not
> >>> relevant since the memory can be accessed by other means.
> >>>
> >>> This commit introduce the ability to control the inline with mbuf
> >>> granularity.
> >>>
> >>> In order to use this feature the application should register the field
> >>> name, and restart the port.
> >>>
> >>> Signed-off-by: Ori Kam <orika@mellanox.com>
> >>> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> >>> ---
> >>>  drivers/net/mlx5/mlx5.c                   | 15 +++++++++++++++
> >>>  drivers/net/mlx5/mlx5_rxtx.c              |  2 ++
> >>>  drivers/net/mlx5/mlx5_rxtx.h              |  3 +++
> >>>  drivers/net/mlx5/mlx5_trigger.c           |  8 ++++++++
> >>>  drivers/net/mlx5/rte_pmd_mlx5.h           | 32
> >> +++++++++++++++++++++++++++++++
> >>>  drivers/net/mlx5/rte_pmd_mlx5_version.map |  7 +++++++
> >>>  6 files changed, 67 insertions(+)
> >>>  create mode 100644 drivers/net/mlx5/rte_pmd_mlx5.h
> >>>
> >>> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> >>> index 50960c9..27dbe27 100644
> >>> --- a/drivers/net/mlx5/mlx5.c
> >>> +++ b/drivers/net/mlx5/mlx5.c
> >>> @@ -46,6 +46,7 @@
> >>>  #include "mlx5_glue.h"
> >>>  #include "mlx5_mr.h"
> >>>  #include "mlx5_flow.h"
> >>> +#include "rte_pmd_mlx5.h"
> >>>
> >>>  /* Device parameter to enable RX completion queue compression. */
> >>>  #define MLX5_RXQ_CQE_COMP_EN "rxq_cqe_comp_en"
> >>> @@ -1988,6 +1989,20 @@ struct mlx5_flow_id_pool *
> >>>  	return ret;
> >>>  }
> >>>
> >>> +int
> >>> +rte_pmd_mlx5_get_dyn_flag_names(char *names[], uint16_t n)
> >>> +{
> >>
> >> Now this is a public API, it should validate the user input.
> >>
> > Will add validation to make sure names != NULL,
> >
> >>> +	static const char *const dynf_names[] = {
> >>> +		RTE_PMD_MLX5_FINE_GRANULARITY_INLINE,
> >>> +	};
> >>> +	int num = RTE_MIN(n, RTE_DIM(dynf_names));
> >>> +	int i;
> >>> +
> >>> +	for (i = 0; i < num; i++)
> >>> +		strcpy(names[i], dynf_names[i]);
> >>> +	return num;
> >>> +}
> >>> +
> >>>  /**
> >>>   * Check sibling device configurations.
> >>>   *
> >>> diff --git a/drivers/net/mlx5/mlx5_rxtx.c
> b/drivers/net/mlx5/mlx5_rxtx.c
> >>> index 67cafd1..aa6aa22 100644
> >>> --- a/drivers/net/mlx5/mlx5_rxtx.c
> >>> +++ b/drivers/net/mlx5/mlx5_rxtx.c
> >>> @@ -126,6 +126,8 @@ enum mlx5_txcmp_code {
> >>>  uint8_t mlx5_cksum_table[1 << 10] __rte_cache_aligned;
> >>>  uint8_t mlx5_swp_types_table[1 << 10] __rte_cache_aligned;
> >>>
> >>> +uint64_t rte_net_mlx5_dynf_inline_mask;
> >>> +
> >>>  /**
> >>>   * Build a table to translate Rx completion flags to packet type.
> >>>   *
> >>> diff --git a/drivers/net/mlx5/mlx5_rxtx.h
> b/drivers/net/mlx5/mlx5_rxtx.h
> >>> index e362b4a..7c38c57 100644
> >>> --- a/drivers/net/mlx5/mlx5_rxtx.h
> >>> +++ b/drivers/net/mlx5/mlx5_rxtx.h
> >>> @@ -42,6 +42,9 @@
> >>>  /* Support tunnel matching. */
> >>>  #define MLX5_FLOW_TUNNEL 9
> >>>
> >>> +/* Mbuf dynamic flag offset for inline. */
> >>> +extern uint64_t rte_net_mlx5_dynf_inline_mask;
> >>> +
> >>>  struct mlx5_rxq_stats {
> >>>  #ifdef MLX5_PMD_SOFT_COUNTERS
> >>>  	uint64_t ipackets; /**< Total of successfully received packets. */
> >>> diff --git a/drivers/net/mlx5/mlx5_trigger.c
> >> b/drivers/net/mlx5/mlx5_trigger.c
> >>> index ab6937a..ab253b2 100644
> >>> --- a/drivers/net/mlx5/mlx5_trigger.c
> >>> +++ b/drivers/net/mlx5/mlx5_trigger.c
> >>> @@ -13,6 +13,7 @@
> >>>  #include "mlx5.h"
> >>>  #include "mlx5_rxtx.h"
> >>>  #include "mlx5_utils.h"
> >>> +#include "rte_pmd_mlx5.h"
> >>>
> >>>  /**
> >>>   * Stop traffic on Tx queues.
> >>> @@ -270,8 +271,15 @@
> >>>  {
> >>>  	struct mlx5_priv *priv = dev->data->dev_private;
> >>>  	int ret;
> >>> +	int fine_inline;
> >>>
> >>>  	DRV_LOG(DEBUG, "port %u starting device", dev->data->port_id);
> >>> +	fine_inline = rte_mbuf_dynflag_lookup
> >>> +		(RTE_PMD_MLX5_FINE_GRANULARITY_INLINE, NULL);
> >>> +	if (fine_inline > 0)
> >>> +		rte_net_mlx5_dynf_inline_mask = 1UL << fine_inline;
> >>> +	else
> >>> +		rte_net_mlx5_dynf_inline_mask = 0;
> >>>  	ret = mlx5_dev_configure_rss_reta(dev);
> >>>  	if (ret) {
> >>>  		DRV_LOG(ERR, "port %u reta config failed: %s",
> >>> diff --git a/drivers/net/mlx5/rte_pmd_mlx5.h
> >> b/drivers/net/mlx5/rte_pmd_mlx5.h
> >>> new file mode 100644
> >>> index 0000000..12e18ca
> >>> --- /dev/null
> >>> +++ b/drivers/net/mlx5/rte_pmd_mlx5.h
> >>> @@ -0,0 +1,32 @@
> >>> +/* SPDX-License-Identifier: BSD-3-Clause
> >>> + * Copyright 2020 Mellanox Technologies, Ltd
> >>> + */
> >>> +
> >>> +#ifndef RTE_PMD_PRIVATE_MLX5_H_
> >>> +#define RTE_PMD_PRIVATE_MLX5_H_
> >>> +
> >>> +/**
> >>> + * @file
> >>> + * MLX5 public header.
> >>> + *
> >>> + * This interface provides the ability to support private PMD
> >>> + * dynamic flags.
> >>> + */
> >>> +
> >>> +#define RTE_PMD_MLX5_FINE_GRANULARITY_INLINE
> >> "mlx5_fine_granularity_inline"
> >>> +
> >>> +/**
> >>> + * Returns the dynamic flags name, that are supported.
> >>> + *
> >>> + * @param[out] names
> >>> + *   Array that is used to return the supported dynamic flags names.
> >>> + * @param[in] n
> >>> + *   The number of elements in the names array.
> >>> + *
> >>> + * @return
> >>> + *   The number of dynamic flags that were copied.
> >>> + */
> >>> +__rte_experimental
> >>> +int rte_pmd_mlx5_get_dyn_flag_names(char *names[], uint16_t n);
> >>
> >> Can you please add this header to the API documentation index,
> >> doc/api/doxy-api-index.md, so it will be part of API document.
> >>
> >
> > Will add.
> >
> >>> +
> >>> +#endif
> >>> diff --git a/drivers/net/mlx5/rte_pmd_mlx5_version.map
> >> b/drivers/net/mlx5/rte_pmd_mlx5_version.map
> >>> index f9f17e4..c8b1031 100644
> >>> --- a/drivers/net/mlx5/rte_pmd_mlx5_version.map
> >>> +++ b/drivers/net/mlx5/rte_pmd_mlx5_version.map
> >>> @@ -1,3 +1,10 @@
> >>>  DPDK_20.0 {
> >>>  	local: *;
> >>>  };
> >>> +
> >>> +EXPERIMENTAL {
> >>> +        global:
> >>> +
> >>> +        # added in 20.02
> >>> +	rte_pmd_mlx5_get_dyn_flag_names;
> >>> +};
> >>>
> >>
> >> Isn't the datapath implementation missing? Where this new mbuf
> dynamic
> >> flag set
> >> or checked?
> >
> > The data path implementation will be done in different patch.
> > The flags is set for example using the testpmd new API or by the
> application.
> > The reason that I added this patch is to show usage for the testpmd patch, I
> can remove this patch and
> > re send it when sending the datapath patch.
> > What do you think?
> >
> 
> I think testpmd patch is clear enough on its own and better to send PMD
> patch
> separately when it is complete.

Will remove this patch.

Thanks,
Ori

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [dpdk-dev] [PATCH v2] app/testpmd: add dynamic flag support
  2020-01-13  9:29 [dpdk-dev] [PATCH 0/2] net/mlx5: add PMD dynf Ori Kam
  2020-01-13  9:29 ` [dpdk-dev] [PATCH 1/2] app/testpmd: add dynamic flag support Ori Kam
  2020-01-13  9:29 ` [dpdk-dev] [PATCH 2/2] net/mlx5: add fine grain " Ori Kam
@ 2020-01-16 12:53 ` Ori Kam
  2020-01-16 19:59   ` Ferruh Yigit
  2020-01-29 12:21 ` [dpdk-dev] [PATCH v2 0/2] mlx5/net: hint PMD not to inline packet Viacheslav Ovsiienko
  3 siblings, 1 reply; 15+ messages in thread
From: Ori Kam @ 2020-01-16 12:53 UTC (permalink / raw)
  To: Wenzhuo Lu, Jingjing Wu, Bernard Iremonger, John McNamara,
	Marko Kovacevic
  Cc: dev, orika, ferruh.yigit, viacheslavo, matan

DPDK now supports registration of dynamic flags (dynf) to the mbuf.
dynf can be given any name, and can be used with a supporting PMD or
supporting application.

Due to the generic concept of the dynf, it is impossible and meaningless,
to define register set/get function for each flag.
This commit introduce a generic way to register and set/clear such flags.

The basic syntax:
port config <port id> dynf <name> <set|clear>

The first step the new flag is registered. Regardless if the action is
set or clear.
There is no way to unregister the flag, after registring it.

The second step, if the action is set then we set the requested flag.
If this is the first flag that is enabled we also register a call back
for the Tx. In this call back we set the flag.
If the action is clear the requested flag is cleared, and if there
are no more flags that are set, the call back is removed.

The reason that the set is only applied in Tx is that in case of Rx
it is assumed that the value comes from the PMD.

If log is enabled the name of the flag, and value will be printed
in the packet info.
In order for the log to work correcly the registration of the flag
must be done before setting verbose.

Signed-off-by: Ori Kam <orika@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
V2:
* Modify according to ML comments.
---

 app/test-pmd/cmdline.c                      | 86 +++++++++++++++++++++++++++++
 app/test-pmd/testpmd.c                      |  3 +
 app/test-pmd/testpmd.h                      | 16 ++++++
 app/test-pmd/util.c                         | 63 +++++++++++++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 10 ++++
 5 files changed, 178 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 2d74df8..dab22bc 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -40,6 +40,7 @@
 #include <rte_devargs.h>
 #include <rte_flow.h>
 #include <rte_gro.h>
+#include <rte_mbuf_dyn.h>
 
 #include <cmdline_rdline.h>
 #include <cmdline_parse.h>
@@ -901,6 +902,11 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"port config (port_id) tx_metadata (value)\n"
 			"    Set Tx metadata value per port. Testpmd will add this value"
 			" to any Tx packet sent from this port\n\n"
+
+			"port config (port_id) dynf (name) set|clear\n"
+			"    Register a dynf and Set/clear this flag on Tx. "
+			"Testpmd will set this value to any Tx packet "
+			"sent from this port\n\n"
 		);
 	}
 
@@ -18837,6 +18843,85 @@ struct cmd_config_tx_metadata_specific_result {
 	},
 };
 
+/* *** set dynf *** */
+struct cmd_config_tx_dynf_specific_result {
+	cmdline_fixed_string_t port;
+	cmdline_fixed_string_t keyword;
+	uint16_t port_id;
+	cmdline_fixed_string_t item;
+	cmdline_fixed_string_t name;
+	cmdline_fixed_string_t value;
+};
+
+static void
+cmd_config_dynf_specific_parsed(void *parsed_result,
+				__attribute__((unused)) struct cmdline *cl,
+				__attribute__((unused)) void *data)
+{
+	struct cmd_config_tx_dynf_specific_result *res = parsed_result;
+	struct rte_mbuf_dynflag desc_flag;
+	int flag;
+	uint64_t old_port_flags;
+
+	if (port_id_is_invalid(res->port_id, ENABLED_WARN))
+		return;
+	flag = rte_mbuf_dynflag_lookup(res->name, NULL);
+	if (flag <= 0) {
+		strcpy(desc_flag.name, res->name);
+		desc_flag.flags = 0;
+		flag = rte_mbuf_dynflag_register(&desc_flag);
+		if (flag < 0) {
+			printf("Can't register flag\n");
+			return;
+		}
+		strcpy(dynf_names[flag], res->name);
+	}
+	old_port_flags = ports[res->port_id].mbuf_dynf;
+	if (!strcmp(res->value, "set")) {
+		ports[res->port_id].mbuf_dynf |= 1UL << flag;
+		if (old_port_flags == 0)
+			add_tx_dynf_callback(res->port_id);
+	} else {
+		ports[res->port_id].mbuf_dynf &= ~(1UL << flag);
+		if (ports[res->port_id].mbuf_dynf == 0)
+			remove_tx_dynf_callback(res->port_id);
+	}
+}
+
+cmdline_parse_token_string_t cmd_config_tx_dynf_specific_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_tx_dynf_specific_result,
+			keyword, "port");
+cmdline_parse_token_string_t cmd_config_tx_dynf_specific_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_tx_dynf_specific_result,
+			keyword, "config");
+cmdline_parse_token_num_t cmd_config_tx_dynf_specific_port_id =
+	TOKEN_NUM_INITIALIZER(struct cmd_config_tx_dynf_specific_result,
+			port_id, UINT16);
+cmdline_parse_token_string_t cmd_config_tx_dynf_specific_item =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_tx_dynf_specific_result,
+			item, "dynf");
+cmdline_parse_token_string_t cmd_config_tx_dynf_specific_name =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_tx_dynf_specific_result,
+			name, NULL);
+cmdline_parse_token_string_t cmd_config_tx_dynf_specific_value =
+	TOKEN_STRING_INITIALIZER(struct cmd_config_tx_dynf_specific_result,
+			value, "set#clear");
+
+cmdline_parse_inst_t cmd_config_tx_dynf_specific = {
+	.f = cmd_config_dynf_specific_parsed,
+	.data = NULL,
+	.help_str = "port config <port id> dynf <name> set|clear",
+	.tokens = {
+		(void *)&cmd_config_tx_dynf_specific_port,
+		(void *)&cmd_config_tx_dynf_specific_keyword,
+		(void *)&cmd_config_tx_dynf_specific_port_id,
+		(void *)&cmd_config_tx_dynf_specific_item,
+		(void *)&cmd_config_tx_dynf_specific_name,
+		(void *)&cmd_config_tx_dynf_specific_value,
+		NULL,
+	},
+};
+
 /* *** display tx_metadata per port configuration *** */
 struct cmd_show_tx_metadata_result {
 	cmdline_fixed_string_t cmd_show;
@@ -19520,6 +19605,7 @@ struct cmd_showport_macs_result {
 	(cmdline_parse_inst_t *)&cmd_set_raw,
 	(cmdline_parse_inst_t *)&cmd_show_set_raw,
 	(cmdline_parse_inst_t *)&cmd_show_set_raw_all,
+	(cmdline_parse_inst_t *)&cmd_config_tx_dynf_specific,
 	NULL,
 };
 
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index b374682..eaf2388 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -502,6 +502,9 @@ static void dev_event_callback(const char *device_name,
 struct gso_status gso_ports[RTE_MAX_ETHPORTS];
 uint16_t gso_max_segment_size = RTE_ETHER_MAX_LEN - RTE_ETHER_CRC_LEN;
 
+/* Holds the registered mbuf dynamic flags names. */
+char dynf_names[64][RTE_MBUF_DYN_NAMESIZE];
+
 /*
  * Helper function to check if socket is already discovered.
  * If yes, return positive value. If not, return zero.
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 857a11f..c407bd2 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -104,6 +104,13 @@ struct rss_type_info {
 extern const struct rss_type_info rss_type_table[];
 
 /**
+ * Dynf name array.
+ *
+ * Array that holds the name for each dynf.
+ */
+extern char dynf_names[64][RTE_MBUF_DYN_NAMESIZE];
+
+/**
  * The data structure associated with a forwarding stream between a receive
  * port/queue and a transmit port/queue.
  */
@@ -193,6 +200,9 @@ struct rte_port {
 	/**< metadata value to insert in Tx packets. */
 	uint32_t		tx_metadata;
 	const struct rte_eth_rxtx_callback *tx_set_md_cb[RTE_MAX_QUEUES_PER_PORT+1];
+	/**< dynamic flags. */
+	uint64_t		mbuf_dynf;
+	const struct rte_eth_rxtx_callback *tx_set_dynf_cb[RTE_MAX_QUEUES_PER_PORT+1];
 };
 
 /**
@@ -884,6 +894,12 @@ uint16_t tx_pkt_set_md(uint16_t port_id, __rte_unused uint16_t queue,
 void add_tx_md_callback(portid_t portid);
 void remove_tx_md_callback(portid_t portid);
 
+uint16_t tx_pkt_set_dynf(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_dynf_callback(portid_t portid);
+void remove_tx_dynf_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/util.c b/app/test-pmd/util.c
index b514be5..418d74e 100644
--- a/app/test-pmd/util.c
+++ b/app/test-pmd/util.c
@@ -39,6 +39,7 @@
 	uint16_t udp_port;
 	uint32_t vx_vni;
 	const char *reason;
+	int dynf_index;
 
 	if (!nb_pkts)
 		return;
@@ -88,6 +89,12 @@
 		if (is_rx && (ol_flags & PKT_RX_DYNF_METADATA))
 			printf(" - Rx metadata: 0x%x",
 			       *RTE_FLOW_DYNF_METADATA(mb));
+		for (dynf_index = 0; dynf_index < 64; dynf_index++) {
+			if (dynf_names[dynf_index][0] != '\0')
+				printf(" - dynf %s: %d",
+				       dynf_names[dynf_index],
+				       !!(ol_flags & (1UL << dynf_index)));
+		}
 		if (mb->packet_type) {
 			rte_get_ptype_name(mb->packet_type, buf, sizeof(buf));
 			printf(" - hw ptype: %s", buf);
@@ -241,6 +248,62 @@
 		}
 }
 
+uint16_t
+tx_pkt_set_dynf(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;
+
+	if (ports[port_id].mbuf_dynf)
+		for (i = 0; i < nb_pkts; i++)
+			pkts[i]->ol_flags |= ports[port_id].mbuf_dynf;
+	return nb_pkts;
+}
+
+void
+add_tx_dynf_callback(portid_t portid)
+{
+	struct rte_eth_dev_info dev_info;
+	uint16_t queue;
+	int ret;
+
+	if (port_id_is_invalid(portid, ENABLED_WARN))
+		return;
+
+	ret = eth_dev_info_get_print_err(portid, &dev_info);
+	if (ret != 0)
+		return;
+
+	for (queue = 0; queue < dev_info.nb_tx_queues; queue++)
+		if (!ports[portid].tx_set_dynf_cb[queue])
+			ports[portid].tx_set_dynf_cb[queue] =
+				rte_eth_add_tx_callback(portid, queue,
+							tx_pkt_set_dynf, NULL);
+}
+
+void
+remove_tx_dynf_callback(portid_t portid)
+{
+	struct rte_eth_dev_info dev_info;
+	uint16_t queue;
+	int ret;
+
+	if (port_id_is_invalid(portid, ENABLED_WARN))
+		return;
+
+	ret = eth_dev_info_get_print_err(portid, &dev_info);
+	if (ret != 0)
+		return;
+
+	for (queue = 0; queue < dev_info.nb_tx_queues; queue++)
+		if (ports[portid].tx_set_dynf_cb[queue]) {
+			rte_eth_remove_tx_callback(portid, queue,
+				ports[portid].tx_set_dynf_cb[queue]);
+			ports[portid].tx_set_dynf_cb[queue] = NULL;
+		}
+}
+
 int
 eth_dev_info_get_print_err(uint16_t port_id,
 					struct rte_eth_dev_info *dev_info)
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 10aabf5..94327e1 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2317,6 +2317,16 @@ testpmd will add this value to any Tx packet sent from this port::
 
    testpmd> port config (port_id) tx_metadata (value)
 
+port config dynf
+~~~~~~~~~~~~~~~~
+
+Set/clear dynamic flag per port.
+testpmd will register this flag in the mbuf (same registration
+for both Tx and Rx). Then set/clear this flag for each Tx
+packet sent from this port. The set bit only works for Tx packet::
+
+   testpmd> port config (port_id) dynf (name) (set|clear)
+
 port config mtu
 ~~~~~~~~~~~~~~~
 
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH v2] app/testpmd: add dynamic flag support
  2020-01-16 12:53 ` [dpdk-dev] [PATCH v2] app/testpmd: add " Ori Kam
@ 2020-01-16 19:59   ` Ferruh Yigit
  0 siblings, 0 replies; 15+ messages in thread
From: Ferruh Yigit @ 2020-01-16 19:59 UTC (permalink / raw)
  To: Ori Kam, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger,
	John McNamara, Marko Kovacevic
  Cc: dev, viacheslavo, matan

On 1/16/2020 12:53 PM, Ori Kam wrote:
> DPDK now supports registration of dynamic flags (dynf) to the mbuf.
> dynf can be given any name, and can be used with a supporting PMD or
> supporting application.
> 
> Due to the generic concept of the dynf, it is impossible and meaningless,
> to define register set/get function for each flag.
> This commit introduce a generic way to register and set/clear such flags.
> 
> The basic syntax:
> port config <port id> dynf <name> <set|clear>
> 
> The first step the new flag is registered. Regardless if the action is
> set or clear.
> There is no way to unregister the flag, after registring it.
> 
> The second step, if the action is set then we set the requested flag.
> If this is the first flag that is enabled we also register a call back
> for the Tx. In this call back we set the flag.
> If the action is clear the requested flag is cleared, and if there
> are no more flags that are set, the call back is removed.
> 
> The reason that the set is only applied in Tx is that in case of Rx
> it is assumed that the value comes from the PMD.
> 
> If log is enabled the name of the flag, and value will be printed
> in the packet info.
> In order for the log to work correcly the registration of the flag
> must be done before setting verbose.
> 
> Signed-off-by: Ori Kam <orika@mellanox.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [dpdk-dev] [PATCH v2 0/2] mlx5/net: hint PMD not to inline packet
  2020-01-13  9:29 [dpdk-dev] [PATCH 0/2] net/mlx5: add PMD dynf Ori Kam
                   ` (2 preceding siblings ...)
  2020-01-16 12:53 ` [dpdk-dev] [PATCH v2] app/testpmd: add " Ori Kam
@ 2020-01-29 12:21 ` Viacheslav Ovsiienko
  2020-01-29 12:21   ` [dpdk-dev] [PATCH v2 1/2] net/mlx5: add fine grain dynamic flag support Viacheslav Ovsiienko
                     ` (2 more replies)
  3 siblings, 3 replies; 15+ messages in thread
From: Viacheslav Ovsiienko @ 2020-01-29 12:21 UTC (permalink / raw)
  To: dev; +Cc: matan, rasland, orika, shahafs, thomas, olivier.matz, ferruh.yigit

Some PMDs inline the mbuf data buffer directly to device transmit descriptor.
This is in order to save the overhead of the PCI headers imposed when the
device DMA reads the data by buffer pointer. For some devices it is essential
in order to provide the full bandwidth.

However, there are cases where such inlining is in-efficient. For example, when
the data buffer resides on other device memory (like GPU or storage device).
Attempt to inline such buffer will result in high PCI overhead for reading
and copying the data from the remote device to the host memory.

To support a mixed traffic pattern (some buffers from local host memory, some
buffers from other devices) with high bandwidth, a hint flag is introduced in
the mbuf.

Application will hint the PMD whether or not it should try to inline the
given mbuf data buffer. PMD should do the best effort to act upon this request.

The hint flag RTE_NET_MLX5_DYNFLAG_NO_INLINE_NAME is supposed to be dynamic,
registered by application with rte_mbuf_dynflag_register(). This flag is
purely vendor specific and declared in PMD specific header rte_pmd_mlx5.h,
which is intended to be used by specific application.

To query the supported specific flags in runtime the private routine is
introduced:

int rte_pmd_mlx5_get_dyn_flag_names(
        uint16_t port,
	char *names[],
        uint16_t n)

It returns the array of currently (over present hardware and configuration)
supported specific flags.

The "not inline hint" feature operating flow is the following one:
- application start
- probe the devices, ports are created
- query the port capabilities
- if port supporting the feature is found
  - register dynamic flag RTE_NET_MLX5_DYNFLAG_NO_INLINE_NAME
- application starts the ports
- on dev_start() PMD checks whether the feature flag is registered and
  enables the feature support in datapath
- application might set this flag in ol_flags field of mbuf in the packets
  being sent and PMD will handle ones appropriately.

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

---
RFC: https://patches.dpdk.org/patch/61348/

This patchset combines the parts of the following:

v1/testpmd: http://patches.dpdk.org/cover/64541/
v1/mlx5: http://patches.dpdk.org/patch/64622/

---
Ori Kam (1):
  net/mlx5: add fine grain dynamic flag support

Viacheslav Ovsiienko (1):
  net/mlx5: update Tx datapath to support no inline hint

 drivers/net/mlx5/mlx5.c                   |  20 ++++++
 drivers/net/mlx5/mlx5_rxtx.c              | 106 +++++++++++++++++++++++++-----
 drivers/net/mlx5/mlx5_rxtx.h              |   3 +
 drivers/net/mlx5/mlx5_trigger.c           |   8 +++
 drivers/net/mlx5/rte_pmd_mlx5.h           |  35 ++++++++++
 drivers/net/mlx5/rte_pmd_mlx5_version.map |   7 ++
 6 files changed, 163 insertions(+), 16 deletions(-)
 create mode 100644 drivers/net/mlx5/rte_pmd_mlx5.h

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [dpdk-dev] [PATCH v2 1/2] net/mlx5: add fine grain dynamic flag support
  2020-01-29 12:21 ` [dpdk-dev] [PATCH v2 0/2] mlx5/net: hint PMD not to inline packet Viacheslav Ovsiienko
@ 2020-01-29 12:21   ` Viacheslav Ovsiienko
  2020-01-29 12:21   ` [dpdk-dev] [PATCH v2 2/2] net/mlx5: update Tx datapath to support no inline hint Viacheslav Ovsiienko
  2020-01-30 13:52   ` [dpdk-dev] [PATCH v2 0/2] mlx5/net: hint PMD not to inline packet Raslan Darawsheh
  2 siblings, 0 replies; 15+ messages in thread
From: Viacheslav Ovsiienko @ 2020-01-29 12:21 UTC (permalink / raw)
  To: dev; +Cc: matan, rasland, orika, shahafs, thomas, olivier.matz, ferruh.yigit

From: Ori Kam <orika@mellanox.com>

The inline feature is designed to save PCI bandwidth by copying some
of the data to the wqe. This feature if enabled works for all packets.

In some cases when using external memory, the PCI bandwidth is not
relevant since the memory can be accessed by other means.

This commit introduce the ability to control the inline with mbuf
granularity.

In order to use this feature the application should register the field
name, and restart the port.

Signed-off-by: Ori Kam <orika@mellanox.com>
Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/mlx5/mlx5.c                   | 20 ++++++++++++++++++
 drivers/net/mlx5/mlx5_rxtx.c              |  2 ++
 drivers/net/mlx5/mlx5_rxtx.h              |  3 +++
 drivers/net/mlx5/mlx5_trigger.c           |  8 +++++++
 drivers/net/mlx5/rte_pmd_mlx5.h           | 35 +++++++++++++++++++++++++++++++
 drivers/net/mlx5/rte_pmd_mlx5_version.map |  7 +++++++
 6 files changed, 75 insertions(+)
 create mode 100644 drivers/net/mlx5/rte_pmd_mlx5.h

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 439b7b8..3abe686 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -48,6 +48,7 @@
 #include "mlx5_autoconf.h"
 #include "mlx5_mr.h"
 #include "mlx5_flow.h"
+#include "rte_pmd_mlx5.h"
 
 /* Device parameter to enable RX completion queue compression. */
 #define MLX5_RXQ_CQE_COMP_EN "rxq_cqe_comp_en"
@@ -1996,6 +1997,25 @@ struct mlx5_flow_id_pool *
 	return ret;
 }
 
+int
+rte_pmd_mlx5_get_dyn_flag_names(char *names[], unsigned int n)
+{
+	static const char *const dynf_names[] = {
+		RTE_PMD_MLX5_FINE_GRANULARITY_INLINE,
+		RTE_MBUF_DYNFLAG_METADATA_NAME
+	};
+	unsigned int i;
+
+	if (n < RTE_DIM(dynf_names))
+		return -ENOMEM;
+	for (i = 0; i < RTE_DIM(dynf_names); i++) {
+		if (names[i] == NULL)
+			return -EINVAL;
+		strcpy(names[i], dynf_names[i]);
+	}
+	return RTE_DIM(dynf_names);
+}
+
 /**
  * Check sibling device configurations.
  *
diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index b14ae31..b9ecc30 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -129,6 +129,8 @@ enum mlx5_txcmp_code {
 uint8_t mlx5_cksum_table[1 << 10] __rte_cache_aligned;
 uint8_t mlx5_swp_types_table[1 << 10] __rte_cache_aligned;
 
+uint64_t rte_net_mlx5_dynf_inline_mask;
+
 /**
  * Build a table to translate Rx completion flags to packet type.
  *
diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index c2cd23b..f9b611a 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -44,6 +44,9 @@
 /* Support tunnel matching. */
 #define MLX5_FLOW_TUNNEL 10
 
+/* Mbuf dynamic flag offset for inline. */
+extern uint64_t rte_net_mlx5_dynf_inline_mask;
+
 struct mlx5_rxq_stats {
 #ifdef MLX5_PMD_SOFT_COUNTERS
 	uint64_t ipackets; /**< Total of successfully received packets. */
diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index ab6937a..ab253b2 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -13,6 +13,7 @@
 #include "mlx5.h"
 #include "mlx5_rxtx.h"
 #include "mlx5_utils.h"
+#include "rte_pmd_mlx5.h"
 
 /**
  * Stop traffic on Tx queues.
@@ -270,8 +271,15 @@
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
 	int ret;
+	int fine_inline;
 
 	DRV_LOG(DEBUG, "port %u starting device", dev->data->port_id);
+	fine_inline = rte_mbuf_dynflag_lookup
+		(RTE_PMD_MLX5_FINE_GRANULARITY_INLINE, NULL);
+	if (fine_inline > 0)
+		rte_net_mlx5_dynf_inline_mask = 1UL << fine_inline;
+	else
+		rte_net_mlx5_dynf_inline_mask = 0;
 	ret = mlx5_dev_configure_rss_reta(dev);
 	if (ret) {
 		DRV_LOG(ERR, "port %u reta config failed: %s",
diff --git a/drivers/net/mlx5/rte_pmd_mlx5.h b/drivers/net/mlx5/rte_pmd_mlx5.h
new file mode 100644
index 0000000..8c69228
--- /dev/null
+++ b/drivers/net/mlx5/rte_pmd_mlx5.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2020 Mellanox Technologies, Ltd
+ */
+
+#ifndef RTE_PMD_PRIVATE_MLX5_H_
+#define RTE_PMD_PRIVATE_MLX5_H_
+
+/**
+ * @file
+ * MLX5 public header.
+ *
+ * This interface provides the ability to support private PMD
+ * dynamic flags.
+ */
+
+#define RTE_PMD_MLX5_FINE_GRANULARITY_INLINE "mlx5_fine_granularity_inline"
+
+/**
+ * Returns the dynamic flags name, that are supported.
+ *
+ * @param[out] names
+ *   Array that is used to return the supported dynamic flags names.
+ * @param[in] n
+ *   The number of elements in the names array.
+ *
+ * @return
+ *   The number of dynamic flags that were copied if not negative.
+ *   Otherwise:
+ *   - ENOMEM - not enough entries in the array
+ *   - EINVAL - invalid array entry
+ */
+__rte_experimental
+int rte_pmd_mlx5_get_dyn_flag_names(char *names[], unsigned int n);
+
+#endif
diff --git a/drivers/net/mlx5/rte_pmd_mlx5_version.map b/drivers/net/mlx5/rte_pmd_mlx5_version.map
index f9f17e4..c8b1031 100644
--- a/drivers/net/mlx5/rte_pmd_mlx5_version.map
+++ b/drivers/net/mlx5/rte_pmd_mlx5_version.map
@@ -1,3 +1,10 @@
 DPDK_20.0 {
 	local: *;
 };
+
+EXPERIMENTAL {
+        global:
+
+        # added in 20.02
+	rte_pmd_mlx5_get_dyn_flag_names;
+};
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [dpdk-dev] [PATCH v2 2/2] net/mlx5: update Tx datapath to support no inline hint
  2020-01-29 12:21 ` [dpdk-dev] [PATCH v2 0/2] mlx5/net: hint PMD not to inline packet Viacheslav Ovsiienko
  2020-01-29 12:21   ` [dpdk-dev] [PATCH v2 1/2] net/mlx5: add fine grain dynamic flag support Viacheslav Ovsiienko
@ 2020-01-29 12:21   ` Viacheslav Ovsiienko
  2020-01-30 13:52   ` [dpdk-dev] [PATCH v2 0/2] mlx5/net: hint PMD not to inline packet Raslan Darawsheh
  2 siblings, 0 replies; 15+ messages in thread
From: Viacheslav Ovsiienko @ 2020-01-29 12:21 UTC (permalink / raw)
  To: dev; +Cc: matan, rasland, orika, shahafs, thomas, olivier.matz, ferruh.yigit

This patch adds support for dynamic flag that hints transmit
datapath do not copy data to the descriptors. This flag is
useful when data are located in the memory of another (not NIC)
physical device and copying to the host memory is undesirable.

This hint flag is per mbuf for multi-segment packets.

This hint flag might be partially ignored if:

- hardware requires minimal data header to be inline into
  descriptor, it depends on the hardware type and its configuration.
  In this case PMD copies the minimal required number of bytes to
  the descriptor, ignoring the no inline hint flag, the rest of data
  is not copied.

- VLAN tag insertion offload is requested and hardware does not
  support this options. In this case the VLAN tag is inserted by
  software means and at least 18B are copied to descriptor.

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>

---
RFC: http://patches.dpdk.org/patch/61348/
v1: - http://patches.dpdk.org/patch/64622/
v2: - patch to control dynamic flag is moved to patchset
    - minor perfromance issues
---
 drivers/net/mlx5/mlx5_rxtx.c | 104 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 88 insertions(+), 16 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index b9ecc30..37a2084 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -130,6 +130,7 @@ enum mlx5_txcmp_code {
 uint8_t mlx5_swp_types_table[1 << 10] __rte_cache_aligned;
 
 uint64_t rte_net_mlx5_dynf_inline_mask;
+#define PKT_TX_DYNF_NOINLINE rte_net_mlx5_dynf_inline_mask
 
 /**
  * Build a table to translate Rx completion flags to packet type.
@@ -2530,21 +2531,30 @@ enum mlx5_txcmp_code {
  *   Pointer to burst routine local context.
  * @param len
  *   Length of data to be copied.
+ * @param must
+ *   Length of data to be copied ignoring no inline hint.
  * @param olx
  *   Configured Tx offloads mask. It is fully defined at
  *   compile time and may be used for optimization.
+ *
+ * @return
+ *   Number of actual copied data bytes. This is always greater than or
+ *   equal to must parameter and might be lesser than len in no inline
+ *   hint flag is encountered.
  */
-static __rte_always_inline void
+static __rte_always_inline unsigned int
 mlx5_tx_mseg_memcpy(uint8_t *pdst,
 		    struct mlx5_txq_local *restrict loc,
 		    unsigned int len,
+		    unsigned int must,
 		    unsigned int olx __rte_unused)
 {
 	struct rte_mbuf *mbuf;
-	unsigned int part, dlen;
+	unsigned int part, dlen, copy = 0;
 	uint8_t *psrc;
 
 	assert(len);
+	assert(must <= len);
 	do {
 		/* Allow zero length packets, must check first. */
 		dlen = rte_pktmbuf_data_len(loc->mbuf);
@@ -2557,6 +2567,25 @@ enum mlx5_txcmp_code {
 			assert(loc->mbuf_nseg > 1);
 			assert(loc->mbuf);
 			--loc->mbuf_nseg;
+			if (loc->mbuf->ol_flags & PKT_TX_DYNF_NOINLINE) {
+				unsigned int diff;
+
+				if (copy >= must) {
+					/*
+					 * We already copied the minimal
+					 * requested amount of data.
+					 */
+					return copy;
+				}
+				diff = must - copy;
+				if (diff <= rte_pktmbuf_data_len(loc->mbuf)) {
+					/*
+					 * Copy only the minimal required
+					 * part of the data buffer.
+					 */
+					len = diff;
+				}
+			}
 			continue;
 		}
 		dlen -= loc->mbuf_off;
@@ -2564,6 +2593,7 @@ enum mlx5_txcmp_code {
 					       loc->mbuf_off);
 		part = RTE_MIN(len, dlen);
 		rte_memcpy(pdst, psrc, part);
+		copy += part;
 		loc->mbuf_off += part;
 		len -= part;
 		if (!len) {
@@ -2577,7 +2607,7 @@ enum mlx5_txcmp_code {
 				assert(loc->mbuf_nseg >= 1);
 				--loc->mbuf_nseg;
 			}
-			return;
+			return copy;
 		}
 		pdst += part;
 	} while (true);
@@ -2622,7 +2652,7 @@ enum mlx5_txcmp_code {
 	struct mlx5_wqe_eseg *restrict es = &wqe->eseg;
 	uint32_t csum;
 	uint8_t *pdst;
-	unsigned int part;
+	unsigned int part, tlen = 0;
 
 	/*
 	 * Calculate and set check sum flags first, uint32_t field
@@ -2655,17 +2685,18 @@ enum mlx5_txcmp_code {
 				 2 * RTE_ETHER_ADDR_LEN),
 		      "invalid Ethernet Segment data size");
 	assert(inlen >= MLX5_ESEG_MIN_INLINE_SIZE);
-	es->inline_hdr_sz = rte_cpu_to_be_16(inlen);
 	pdst = (uint8_t *)&es->inline_data;
 	if (MLX5_TXOFF_CONFIG(VLAN) && vlan) {
 		/* Implement VLAN tag insertion as part inline data. */
-		mlx5_tx_mseg_memcpy(pdst, loc, 2 * RTE_ETHER_ADDR_LEN, olx);
+		mlx5_tx_mseg_memcpy(pdst, loc,
+				    2 * RTE_ETHER_ADDR_LEN,
+				    2 * RTE_ETHER_ADDR_LEN, olx);
 		pdst += 2 * RTE_ETHER_ADDR_LEN;
 		*(unaligned_uint32_t *)pdst = rte_cpu_to_be_32
 						((RTE_ETHER_TYPE_VLAN << 16) |
 						 loc->mbuf->vlan_tci);
 		pdst += sizeof(struct rte_vlan_hdr);
-		inlen -= 2 * RTE_ETHER_ADDR_LEN + sizeof(struct rte_vlan_hdr);
+		tlen += 2 * RTE_ETHER_ADDR_LEN + sizeof(struct rte_vlan_hdr);
 	}
 	assert(pdst < (uint8_t *)txq->wqes_end);
 	/*
@@ -2673,18 +2704,26 @@ enum mlx5_txcmp_code {
 	 * Here we should be aware of WQE ring buffer wraparound only.
 	 */
 	part = (uint8_t *)txq->wqes_end - pdst;
-	part = RTE_MIN(part, inlen);
+	part = RTE_MIN(part, inlen - tlen);
 	assert(part);
 	do {
-		mlx5_tx_mseg_memcpy(pdst, loc, part, olx);
-		inlen -= part;
-		if (likely(!inlen)) {
-			pdst += part;
+		unsigned int copy;
+
+		/*
+		 * Copying may be interrupted inside the routine
+		 * if run into no inline hint flag.
+		 */
+		copy = tlen >= txq->inlen_mode ? 0 : (txq->inlen_mode - tlen);
+		copy = mlx5_tx_mseg_memcpy(pdst, loc, part, copy, olx);
+		tlen += copy;
+		if (likely(inlen <= tlen) || copy < part) {
+			es->inline_hdr_sz = rte_cpu_to_be_16(tlen);
+			pdst += copy;
 			pdst = RTE_PTR_ALIGN(pdst, MLX5_WSEG_SIZE);
 			return (struct mlx5_wqe_dseg *)pdst;
 		}
 		pdst = (uint8_t *)txq->wqes;
-		part = inlen;
+		part = inlen - tlen;
 	} while (true);
 }
 
@@ -3283,7 +3322,8 @@ enum mlx5_txcmp_code {
 	if (inlen <= MLX5_ESEG_MIN_INLINE_SIZE)
 		return MLX5_TXCMP_CODE_ERROR;
 	assert(txq->inlen_send >= MLX5_ESEG_MIN_INLINE_SIZE);
-	if (inlen > txq->inlen_send) {
+	if (inlen > txq->inlen_send ||
+	    loc->mbuf->ol_flags & PKT_TX_DYNF_NOINLINE) {
 		struct rte_mbuf *mbuf;
 		unsigned int nxlen;
 		uintptr_t start;
@@ -3298,7 +3338,8 @@ enum mlx5_txcmp_code {
 			assert(txq->inlen_mode <= txq->inlen_send);
 			inlen = txq->inlen_mode;
 		} else {
-			if (!vlan || txq->vlan_en) {
+			if (loc->mbuf->ol_flags & PKT_TX_DYNF_NOINLINE ||
+			    !vlan || txq->vlan_en) {
 				/*
 				 * VLAN insertion will be done inside by HW.
 				 * It is not utmost effective - VLAN flag is
@@ -4109,7 +4150,8 @@ enum mlx5_txcmp_code {
 				return MLX5_TXCMP_CODE_ERROR;
 			}
 			/* Inline or not inline - that's the Question. */
-			if (dlen > txq->inlen_empw)
+			if (dlen > txq->inlen_empw ||
+			    loc->mbuf->ol_flags & PKT_TX_DYNF_NOINLINE)
 				goto pointer_empw;
 			/* Inline entire packet, optional VLAN insertion. */
 			tlen = sizeof(dseg->bcount) + dlen;
@@ -4305,6 +4347,33 @@ enum mlx5_txcmp_code {
 				/* Check against minimal length. */
 				if (inlen <= MLX5_ESEG_MIN_INLINE_SIZE)
 					return MLX5_TXCMP_CODE_ERROR;
+				if (loc->mbuf->ol_flags &
+				    PKT_TX_DYNF_NOINLINE) {
+					/*
+					 * The hint flag not to inline packet
+					 * data is set. Check whether we can
+					 * follow the hint.
+					 */
+					if ((!MLX5_TXOFF_CONFIG(EMPW) &&
+					      txq->inlen_mode) ||
+					    (MLX5_TXOFF_CONFIG(MPW) &&
+					     txq->inlen_mode)) {
+						/*
+						 * The hardware requires the
+						 * minimal inline data header.
+						 */
+						goto single_min_inline;
+					}
+					if (MLX5_TXOFF_CONFIG(VLAN) &&
+					    vlan && !txq->vlan_en) {
+						/*
+						 * We must insert VLAN tag
+						 * by software means.
+						 */
+						goto single_part_inline;
+					}
+					goto single_no_inline;
+				}
 				/*
 				 * Completely inlined packet data WQE:
 				 * - Control Segment, SEND opcode
@@ -4354,6 +4423,7 @@ enum mlx5_txcmp_code {
 				 * We should check the free space in
 				 * WQE ring buffer to inline partially.
 				 */
+single_min_inline:
 				assert(txq->inlen_send >= txq->inlen_mode);
 				assert(inlen > txq->inlen_mode);
 				assert(txq->inlen_mode >=
@@ -4421,6 +4491,7 @@ enum mlx5_txcmp_code {
 				 * We also get here if VLAN insertion is not
 				 * supported by HW, the inline is enabled.
 				 */
+single_part_inline:
 				wqe = txq->wqes + (txq->wqe_ci & txq->wqe_m);
 				loc->wqe_last = wqe;
 				mlx5_tx_cseg_init(txq, loc, wqe, 4,
@@ -4461,6 +4532,7 @@ enum mlx5_txcmp_code {
 			 * - Ethernet Segment, optional VLAN, no inline
 			 * - Data Segment, pointer type
 			 */
+single_no_inline:
 			wqe = txq->wqes + (txq->wqe_ci & txq->wqe_m);
 			loc->wqe_last = wqe;
 			mlx5_tx_cseg_init(txq, loc, wqe, 3,
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/2] mlx5/net: hint PMD not to inline packet
  2020-01-29 12:21 ` [dpdk-dev] [PATCH v2 0/2] mlx5/net: hint PMD not to inline packet Viacheslav Ovsiienko
  2020-01-29 12:21   ` [dpdk-dev] [PATCH v2 1/2] net/mlx5: add fine grain dynamic flag support Viacheslav Ovsiienko
  2020-01-29 12:21   ` [dpdk-dev] [PATCH v2 2/2] net/mlx5: update Tx datapath to support no inline hint Viacheslav Ovsiienko
@ 2020-01-30 13:52   ` Raslan Darawsheh
  2 siblings, 0 replies; 15+ messages in thread
From: Raslan Darawsheh @ 2020-01-30 13:52 UTC (permalink / raw)
  To: Slava Ovsiienko, dev
  Cc: Matan Azrad, Ori Kam, Shahaf Shuler, thomas, olivier.matz, ferruh.yigit

Hi,

> -----Original Message-----
> From: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> Sent: Wednesday, January 29, 2020 2:21 PM
> To: dev@dpdk.org
> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; Ori Kam <orika@mellanox.com>; Shahaf Shuler
> <shahafs@mellanox.com>; thomas@mellanox.net;
> olivier.matz@6wind.com; ferruh.yigit@intel.com
> Subject: [PATCH v2 0/2] mlx5/net: hint PMD not to inline packet
> 
> Some PMDs inline the mbuf data buffer directly to device transmit
> descriptor.
> This is in order to save the overhead of the PCI headers imposed when the
> device DMA reads the data by buffer pointer. For some devices it is essential
> in order to provide the full bandwidth.
> 
> However, there are cases where such inlining is in-efficient. For example,
> when
> the data buffer resides on other device memory (like GPU or storage
> device).
> Attempt to inline such buffer will result in high PCI overhead for reading
> and copying the data from the remote device to the host memory.
> 
> To support a mixed traffic pattern (some buffers from local host memory,
> some
> buffers from other devices) with high bandwidth, a hint flag is introduced in
> the mbuf.
> 
> Application will hint the PMD whether or not it should try to inline the
> given mbuf data buffer. PMD should do the best effort to act upon this
> request.
> 
> The hint flag RTE_NET_MLX5_DYNFLAG_NO_INLINE_NAME is supposed to
> be dynamic,
> registered by application with rte_mbuf_dynflag_register(). This flag is
> purely vendor specific and declared in PMD specific header rte_pmd_mlx5.h,
> which is intended to be used by specific application.
> 
> To query the supported specific flags in runtime the private routine is
> introduced:
> 
> int rte_pmd_mlx5_get_dyn_flag_names(
>         uint16_t port,
> 	char *names[],
>         uint16_t n)
> 
> It returns the array of currently (over present hardware and configuration)
> supported specific flags.
> 
> The "not inline hint" feature operating flow is the following one:
> - application start
> - probe the devices, ports are created
> - query the port capabilities
> - if port supporting the feature is found
>   - register dynamic flag RTE_NET_MLX5_DYNFLAG_NO_INLINE_NAME
> - application starts the ports
> - on dev_start() PMD checks whether the feature flag is registered and
>   enables the feature support in datapath
> - application might set this flag in ol_flags field of mbuf in the packets
>   being sent and PMD will handle ones appropriately.
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> 
> ---
> RFC:
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> es.dpdk.org%2Fpatch%2F61348%2F&amp;data=02%7C01%7Crasland%40mell
> anox.com%7C7b9dad01f6f24fc054df08d7a4b5c1aa%7Ca652971c7d2e4d9ba6a
> 4d149256f461b%7C0%7C0%7C637158972862376366&amp;sdata=GVQd0sNOS
> 8Bbi3z33j2USdZpx%2FPE8IzwcfTg4QBj%2BwI%3D&amp;reserved=0
> 
> This patchset combines the parts of the following:
> 
> v1/testpmd:
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
> es.dpdk.org%2Fcover%2F64541%2F&amp;data=02%7C01%7Crasland%40mell
> anox.com%7C7b9dad01f6f24fc054df08d7a4b5c1aa%7Ca652971c7d2e4d9ba6a
> 4d149256f461b%7C0%7C0%7C637158972862376366&amp;sdata=wpMH45Orli
> mz1y4Bd7Emb%2F%2Fz4hsu%2BLUMN8sortguMUE%3D&amp;reserved=0
> v1/mlx5:
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
> es.dpdk.org%2Fpatch%2F64622%2F&amp;data=02%7C01%7Crasland%40mell
> anox.com%7C7b9dad01f6f24fc054df08d7a4b5c1aa%7Ca652971c7d2e4d9ba6a
> 4d149256f461b%7C0%7C0%7C637158972862376366&amp;sdata=RAA3Qw104
> dV6rujRoxXIOm0gcAI0DY5DyAdMAwryeb8%3D&amp;reserved=0
> 
> ---
> Ori Kam (1):
>   net/mlx5: add fine grain dynamic flag support
> 
> Viacheslav Ovsiienko (1):
>   net/mlx5: update Tx datapath to support no inline hint
> 
>  drivers/net/mlx5/mlx5.c                   |  20 ++++++
>  drivers/net/mlx5/mlx5_rxtx.c              | 106 +++++++++++++++++++++++++--
> ---
>  drivers/net/mlx5/mlx5_rxtx.h              |   3 +
>  drivers/net/mlx5/mlx5_trigger.c           |   8 +++
>  drivers/net/mlx5/rte_pmd_mlx5.h           |  35 ++++++++++
>  drivers/net/mlx5/rte_pmd_mlx5_version.map |   7 ++
>  6 files changed, 163 insertions(+), 16 deletions(-)
>  create mode 100644 drivers/net/mlx5/rte_pmd_mlx5.h
> 
> --
> 1.8.3.1

Series applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-01-30 13:52 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-13  9:29 [dpdk-dev] [PATCH 0/2] net/mlx5: add PMD dynf Ori Kam
2020-01-13  9:29 ` [dpdk-dev] [PATCH 1/2] app/testpmd: add dynamic flag support Ori Kam
2020-01-15 13:31   ` Ferruh Yigit
2020-01-16  8:07     ` Ori Kam
2020-01-13  9:29 ` [dpdk-dev] [PATCH 2/2] net/mlx5: add fine grain " Ori Kam
2020-01-15 14:01   ` Ferruh Yigit
2020-01-16 12:05     ` Ori Kam
2020-01-16 12:24       ` Ferruh Yigit
2020-01-16 12:37         ` Ori Kam
2020-01-16 12:53 ` [dpdk-dev] [PATCH v2] app/testpmd: add " Ori Kam
2020-01-16 19:59   ` Ferruh Yigit
2020-01-29 12:21 ` [dpdk-dev] [PATCH v2 0/2] mlx5/net: hint PMD not to inline packet Viacheslav Ovsiienko
2020-01-29 12:21   ` [dpdk-dev] [PATCH v2 1/2] net/mlx5: add fine grain dynamic flag support Viacheslav Ovsiienko
2020-01-29 12:21   ` [dpdk-dev] [PATCH v2 2/2] net/mlx5: update Tx datapath to support no inline hint Viacheslav Ovsiienko
2020-01-30 13:52   ` [dpdk-dev] [PATCH v2 0/2] mlx5/net: hint PMD not to inline packet Raslan Darawsheh

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