[dpdk-dev] [PATCH v3 0/8] Add support for VXLAN and NVGRE encap as a sample actions

[dpdk-dev] [PATCH v3 0/8] Add support for VXLAN and NVGRE encap as a sample actions

[Salem Sol]

This series adds support for VXLAN and NVGRE encap as a sample actions with the proper
documentation, this series depends on [1] for the documentation part.
    
[1] http://patches.dpdk.org/project/dpdk/patch/1615774238-51875-1-git-send-email-jiaweiw@nvidia.com/

V2: updated ("doc: update sample actions support in testpmd guide") fixing a build issue.
V3: added ("doc: update dpdk 21.05 release notes").

Jiawei Wang (1):
  app/testpmd: store VXLAN/NVGRE encap data globally

Salem Sol (7):
  net/mlx5: support VXLAN encap action in sample
  net/mlx5: support NVGRE encap action in sample
  app/testpmd: support VXLAN encap for sample action
  app/testpmd: support NVGRE encap for sample action
  doc: update sample actions support in testpmd guide
  doc: update sample actions support in mlx5 guide
  doc: update dpdk 21.05 release notes

 app/test-pmd/cmdline_flow.c                 | 90 ++++++++++++++-------
 doc/guides/nics/mlx5.rst                    |  4 +-
 doc/guides/rel_notes/release_21_05.rst      |  6 ++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 22 +++++
 drivers/net/mlx5/mlx5_flow_dv.c             | 13 +++
 5 files changed, 106 insertions(+), 29 deletions(-)

-- 
2.21.0

[dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE encap data globally

[Salem Sol]

From: Jiawei Wang <jiaweiw@nvidia.com>

With the current code the VXLAN/NVGRE parsing routine
stored the configuration of the header on stack, this
might lead to overwriting the data on the stack.

This patch stores the external data of vxlan and nvgre encap
into global data as a pre-step to supporting vxlan and nvgre
encap as a sample actions.

Signed-off-by: Jiawei Wang <jiaweiw@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 76 ++++++++++++++++++++++++-------------
 1 file changed, 49 insertions(+), 27 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 49d9f9c043..84676a2e45 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -5244,31 +5244,14 @@ parse_vc_action_rss_queue(struct context *ctx, const struct token *token,
 	return len;
 }
 
-/** Parse VXLAN encap action. */
+/** Setup VXLAN encap configuration. */
 static int
-parse_vc_action_vxlan_encap(struct context *ctx, const struct token *token,
-			    const char *str, unsigned int len,
-			    void *buf, unsigned int size)
+parse_setup_vxlan_encap_data
+		(struct action_vxlan_encap_data *action_vxlan_encap_data)
 {
-	struct buffer *out = buf;
-	struct rte_flow_action *action;
-	struct action_vxlan_encap_data *action_vxlan_encap_data;
-	int ret;
-
-	ret = parse_vc(ctx, token, str, len, buf, size);
-	if (ret < 0)
-		return ret;
-	/* Nothing else to do if there is no buffer. */
-	if (!out)
-		return ret;
-	if (!out->args.vc.actions_n)
+	if (!action_vxlan_encap_data)
 		return -1;
-	action = &out->args.vc.actions[out->args.vc.actions_n - 1];
-	/* Point to selected object. */
-	ctx->object = out->args.vc.data;
-	ctx->objmask = NULL;
 	/* Set up default configuration. */
-	action_vxlan_encap_data = ctx->object;
 	*action_vxlan_encap_data = (struct action_vxlan_encap_data){
 		.conf = (struct rte_flow_action_vxlan_encap){
 			.definition = action_vxlan_encap_data->items,
@@ -5372,19 +5355,18 @@ parse_vc_action_vxlan_encap(struct context *ctx, const struct token *token,
 	}
 	memcpy(action_vxlan_encap_data->item_vxlan.vni, vxlan_encap_conf.vni,
 	       RTE_DIM(vxlan_encap_conf.vni));
-	action->conf = &action_vxlan_encap_data->conf;
-	return ret;
+	return 0;
 }
 
-/** Parse NVGRE encap action. */
+/** Parse VXLAN encap action. */
 static int
-parse_vc_action_nvgre_encap(struct context *ctx, const struct token *token,
+parse_vc_action_vxlan_encap(struct context *ctx, const struct token *token,
 			    const char *str, unsigned int len,
 			    void *buf, unsigned int size)
 {
 	struct buffer *out = buf;
 	struct rte_flow_action *action;
-	struct action_nvgre_encap_data *action_nvgre_encap_data;
+	struct action_vxlan_encap_data *action_vxlan_encap_data;
 	int ret;
 
 	ret = parse_vc(ctx, token, str, len, buf, size);
@@ -5399,8 +5381,20 @@ parse_vc_action_nvgre_encap(struct context *ctx, const struct token *token,
 	/* Point to selected object. */
 	ctx->object = out->args.vc.data;
 	ctx->objmask = NULL;
+	action_vxlan_encap_data = ctx->object;
+	parse_setup_vxlan_encap_data(action_vxlan_encap_data);
+	action->conf = &action_vxlan_encap_data->conf;
+	return ret;
+}
+
+/** Setup NVGRE encap configuration. */
+static int
+parse_setup_nvgre_encap_data
+		(struct action_nvgre_encap_data *action_nvgre_encap_data)
+{
+	if (!action_nvgre_encap_data)
+		return -1;
 	/* Set up default configuration. */
-	action_nvgre_encap_data = ctx->object;
 	*action_nvgre_encap_data = (struct action_nvgre_encap_data){
 		.conf = (struct rte_flow_action_nvgre_encap){
 			.definition = action_nvgre_encap_data->items,
@@ -5463,6 +5457,34 @@ parse_vc_action_nvgre_encap(struct context *ctx, const struct token *token,
 			RTE_FLOW_ITEM_TYPE_VOID;
 	memcpy(action_nvgre_encap_data->item_nvgre.tni, nvgre_encap_conf.tni,
 	       RTE_DIM(nvgre_encap_conf.tni));
+	return 0;
+}
+
+/** Parse NVGRE encap action. */
+static int
+parse_vc_action_nvgre_encap(struct context *ctx, const struct token *token,
+			    const char *str, unsigned int len,
+			    void *buf, unsigned int size)
+{
+	struct buffer *out = buf;
+	struct rte_flow_action *action;
+	struct action_nvgre_encap_data *action_nvgre_encap_data;
+	int ret;
+
+	ret = parse_vc(ctx, token, str, len, buf, size);
+	if (ret < 0)
+		return ret;
+	/* Nothing else to do if there is no buffer. */
+	if (!out)
+		return ret;
+	if (!out->args.vc.actions_n)
+		return -1;
+	action = &out->args.vc.actions[out->args.vc.actions_n - 1];
+	/* Point to selected object. */
+	ctx->object = out->args.vc.data;
+	ctx->objmask = NULL;
+	action_nvgre_encap_data = ctx->object;
+	parse_setup_nvgre_encap_data(action_nvgre_encap_data);
 	action->conf = &action_nvgre_encap_data->conf;
 	return ret;
 }
-- 
2.21.0

[dpdk-dev] [PATCH v3 2/8] net/mlx5: support VXLAN encap action in sample

[Salem Sol]

Add support for VXLAN encap as a sample action
and validate it.

Signed-off-by: Salem Sol <salems@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 1a74d5ac2b..4b2db47e39 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -5242,6 +5242,16 @@ flow_dv_validate_action_sample(uint64_t *action_flags,
 				return ret;
 			++actions_n;
 			break;
+		case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP:
+			ret = flow_dv_validate_action_l2_encap(dev,
+							       sub_action_flags,
+							       act, attr,
+							       error);
+			if (ret < 0)
+				return ret;
+			sub_action_flags |= MLX5_FLOW_ACTION_ENCAP;
+			++actions_n;
+			break;
 		default:
 			return rte_flow_error_set(error, ENOTSUP,
 						  RTE_FLOW_ERROR_TYPE_ACTION,
@@ -10407,6 +10417,7 @@ flow_dv_translate_action_sample(struct rte_eth_dev *dev,
 			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
 			break;
 		}
+		case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP:
 		case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
 			/* Save the encap resource before sample */
 			pre_rix = dev_flow->handle->dvh.rix_encap_decap;
-- 
2.21.0

[dpdk-dev] [PATCH v3 3/8] net/mlx5: support NVGRE encap action in sample

[Salem Sol]

Add support for NVGRE encap as a sample action
and validate it.

Signed-off-by: Salem Sol <salems@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 4b2db47e39..590abdc822 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -5243,6 +5243,7 @@ flow_dv_validate_action_sample(uint64_t *action_flags,
 			++actions_n;
 			break;
 		case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP:
+		case RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP:
 			ret = flow_dv_validate_action_l2_encap(dev,
 							       sub_action_flags,
 							       act, attr,
@@ -10418,6 +10419,7 @@ flow_dv_translate_action_sample(struct rte_eth_dev *dev,
 			break;
 		}
 		case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP:
+		case RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP:
 		case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
 			/* Save the encap resource before sample */
 			pre_rix = dev_flow->handle->dvh.rix_encap_decap;
-- 
2.21.0

[dpdk-dev] [PATCH v3 4/8] app/testpmd: support VXLAN encap for sample action

[Salem Sol]

Add support for rte_flow_action_vxlan_encap as a sample action.

The example of test-pmd command:

1.  set vxlan ip-version ... vni ... udp-src ...
    set raw_encap 1 eth src.../ ipv4.../...
    set sample_actions 2 vxlan_encap / port_id id 0 / end
    flow create 0 ... pattern eth / end actions
       sample ratio 1 index 2 / raw_encap index 1 / port_id id 0...

    The flow will result in all the matched egress packets will be
    encapsulated and sent to wire, and also mirrored the packets
    using VXLAN encapsulation data and sent to wire.

Signed-off-by: Salem Sol <salems@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 84676a2e45..61dfaab8fd 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -582,6 +582,7 @@ struct rte_flow_action_queue sample_queue[RAW_SAMPLE_CONFS_MAX_NUM];
 struct rte_flow_action_count sample_count[RAW_SAMPLE_CONFS_MAX_NUM];
 struct rte_flow_action_port_id sample_port_id[RAW_SAMPLE_CONFS_MAX_NUM];
 struct rte_flow_action_raw_encap sample_encap[RAW_SAMPLE_CONFS_MAX_NUM];
+struct action_vxlan_encap_data sample_vxlan_encap[RAW_SAMPLE_CONFS_MAX_NUM];
 struct action_rss_data sample_rss_data[RAW_SAMPLE_CONFS_MAX_NUM];
 struct rte_flow_action_vf sample_vf[RAW_SAMPLE_CONFS_MAX_NUM];
 
@@ -1615,6 +1616,7 @@ static const enum index next_action_sample[] = {
 	ACTION_COUNT,
 	ACTION_PORT_ID,
 	ACTION_RAW_ENCAP,
+	ACTION_VXLAN_ENCAP,
 	ACTION_NEXT,
 	ZERO,
 };
@@ -7949,6 +7951,11 @@ cmd_set_raw_parsed_sample(const struct buffer *in)
 					(const void *)action->conf, size);
 			action->conf = &sample_vf[idx];
 			break;
+		case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP:
+			size = sizeof(struct rte_flow_action_vxlan_encap);
+			parse_setup_vxlan_encap_data(&sample_vxlan_encap[idx]);
+			action->conf = &sample_vxlan_encap[idx].conf;
+			break;
 		default:
 			printf("Error - Not supported action\n");
 			return;
-- 
2.21.0

[dpdk-dev] [PATCH v3 5/8] app/testpmd: support NVGRE encap for sample action

[Salem Sol]

Add support for rte_flow_action_nvge_encap as a sample action.

The example of test-pmd command:

1.  set nvgre ip-version ... tni ... ip-src ... ip-dst ...
    set raw_encap 1 eth src... / ipv4... /...
    set sample_actions 2 nvgre / port_id id 0 / end
    flow create 0 ... pattern eth / end actions
       sample ratio 1 index 2 / raw_encap index 1 / port_id id 0...

    The flow will result in all the matched egress packets will be
    encapsulated and sent to wire, and also mirrored the packets
    using NVGRE encapsulation data and sent to wire.

Signed-off-by: Salem Sol <salems@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 61dfaab8fd..0e33410005 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -583,6 +583,7 @@ struct rte_flow_action_count sample_count[RAW_SAMPLE_CONFS_MAX_NUM];
 struct rte_flow_action_port_id sample_port_id[RAW_SAMPLE_CONFS_MAX_NUM];
 struct rte_flow_action_raw_encap sample_encap[RAW_SAMPLE_CONFS_MAX_NUM];
 struct action_vxlan_encap_data sample_vxlan_encap[RAW_SAMPLE_CONFS_MAX_NUM];
+struct action_nvgre_encap_data sample_nvgre_encap[RAW_SAMPLE_CONFS_MAX_NUM];
 struct action_rss_data sample_rss_data[RAW_SAMPLE_CONFS_MAX_NUM];
 struct rte_flow_action_vf sample_vf[RAW_SAMPLE_CONFS_MAX_NUM];
 
@@ -1617,6 +1618,7 @@ static const enum index next_action_sample[] = {
 	ACTION_PORT_ID,
 	ACTION_RAW_ENCAP,
 	ACTION_VXLAN_ENCAP,
+	ACTION_NVGRE_ENCAP,
 	ACTION_NEXT,
 	ZERO,
 };
@@ -7956,6 +7958,11 @@ cmd_set_raw_parsed_sample(const struct buffer *in)
 			parse_setup_vxlan_encap_data(&sample_vxlan_encap[idx]);
 			action->conf = &sample_vxlan_encap[idx].conf;
 			break;
+		case RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP:
+			size = sizeof(struct rte_flow_action_nvgre_encap);
+			parse_setup_nvgre_encap_data(&sample_nvgre_encap[idx]);
+			action->conf = &sample_nvgre_encap[idx];
+			break;
 		default:
 			printf("Error - Not supported action\n");
 			return;
-- 
2.21.0

[dpdk-dev] [PATCH v3 6/8] doc: update sample actions support in testpmd guide

[Salem Sol]

Update documentation for sample action usage in testpmd utilizing
rte_flow_action_vxlan_encap and rte_flow_action_nvgre_encap and
show the command line example.

Signed-off-by: Salem Sol <salems@nvidia.com>
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 22 +++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 3a31cc6237..392e3a31cf 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -4901,6 +4901,28 @@ and also mirrored the packets with encapsulation header and sent to port id 0.
  testpmd> flow create 0 ingress transfer pattern eth / end actions
         sample ratio 1 index 0  / port_id id 2 / end
 
+E-Switch Mirroring rule, the matched ingress packets are sent to port id 2,
+and also mirrored the packets with VXLAN encapsulation header and sent to port id 0.
+
+::
+
+ testpmd> set vxlan ip-version ipv4 vni 4 udp-src 4 udp-dst 4 ip-src 127.0.0.1
+        ip-dst 128.0.0.1 eth-src 11:11:11:11:11:11 eth-dst 22:22:22:22:22:22
+ testpmd> set sample_actions 0 vxlan_encap / port_id id 0 / end
+ testpmd> flow create 0 ingress transfer pattern eth / end actions
+        sample ratio 1 index 0  / port_id id 2 / end
+
+E-Switch Mirroring rule, the matched ingress packets are sent to port id 2,
+and also mirrored the packets with NVGRE encapsulation header and sent to port id 0.
+
+::
+
+ testpmd> set nvgre ip-version ipv4 tni 4 ip-src 127.0.0.1 ip-dst 128.0.0.1
+        eth-src 11:11:11:11:11:11 eth-dst 22:22:22:22:22:22
+ testpmd> set sample_actions 0 nvgre_encap / port_id id 0 / end
+ testpmd> flow create 0 ingress transfer pattern eth / end actions
+        sample ratio 1 index 0  / port_id id 2 / end
+
 BPF Functions
 --------------
 
-- 
2.21.0

[dpdk-dev] [PATCH v3 7/8] doc: update sample actions support in mlx5 guide

[Salem Sol]

Updates the documentation with the added support for sample actions VXLAN
and NVGRE encap in E-Switch steering flow.

Signed-off-by: Salem Sol <salems@nvidia.com>
---
 doc/guides/nics/mlx5.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 96fce36e3c..378b7202d9 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -376,8 +376,8 @@ Limitations
     encapsulation actions.
   - For NIC Rx flow, supports ``MARK``, ``COUNT``, ``QUEUE``, ``RSS`` in the
     sample actions list.
-  - For E-Switch mirroring flow, supports ``RAW ENCAP``, ``Port ID`` in the
-    sample actions list.
+  - For E-Switch mirroring flow, supports ``RAW ENCAP``, ``Port ID``,
+    ``VXLAN ENCAP``, ``NVGRE ENCAP`` in the sample actions list.
 
 - Modify Field flow:
 
-- 
2.21.0

[dpdk-dev] [PATCH v3 8/8] doc: update dpdk 21.05 release notes

[Salem Sol]

Update release notes for dpdk 21.05 to include the new
Mellanox driver's support for VXLAN and NVGRE encap as sample
actions.

Signed-off-by: Salem Sol <salems@nvidia.com>
---
 doc/guides/rel_notes/release_21_05.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
index 23f7f0bff9..312b3cbe61 100644
--- a/doc/guides/rel_notes/release_21_05.rst
+++ b/doc/guides/rel_notes/release_21_05.rst
@@ -65,6 +65,12 @@ New Features
 
   * Added support for txgbevf PMD.
 
+* **Updated Mellanox mlx5 driver.**
+
+  Updated the Mellanox mlx5 driver with new features and improvements, including:
+
+  * Added support for VXLAN and NVGRE encap as a sample actions.
+
 * **Updated testpmd.**
 
   * Added command to display Rx queue used descriptor count.
-- 
2.21.0

Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE encap data globally

[Slava Ovsiienko]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Salem Sol
> Sent: Wednesday, March 17, 2021 11:26
> To: dev@dpdk.org
> Cc: Jiawei(Jonny) Wang <jiaweiw@nvidia.com>; Ori Kam
> <orika@nvidia.com>; Xiaoyun Li <xiaoyun.li@intel.com>
> Subject: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE
> encap data globally
> 
> From: Jiawei Wang <jiaweiw@nvidia.com>
> 
> With the current code the VXLAN/NVGRE parsing routine stored the
> configuration of the header on stack, this might lead to overwriting the data
> on the stack.
> 
> This patch stores the external data of vxlan and nvgre encap into global data
> as a pre-step to supporting vxlan and nvgre encap as a sample actions.
> 
> Signed-off-by: Jiawei Wang <jiaweiw@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

Re: [dpdk-dev] [PATCH v3 2/8] net/mlx5: support VXLAN encap action in sample

[Slava Ovsiienko]

> -----Original Message-----
> From: Salem Sol <salems@nvidia.com>
> Sent: Wednesday, March 17, 2021 11:26
> To: dev@dpdk.org
> Cc: Salem Sol <salems@nvidia.com>; Matan Azrad <matan@nvidia.com>;
> Shahaf Shuler <shahafs@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>
> Subject: [PATCH v3 2/8] net/mlx5: support VXLAN encap action in sample
> 
> Add support for VXLAN encap as a sample action and validate it.
> 
> Signed-off-by: Salem Sol <salems@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

Re: [dpdk-dev] [PATCH v3 3/8] net/mlx5: support NVGRE encap action in sample

[Slava Ovsiienko]

> -----Original Message-----
> From: Salem Sol <salems@nvidia.com>
> Sent: Wednesday, March 17, 2021 11:26
> To: dev@dpdk.org
> Cc: Salem Sol <salems@nvidia.com>; Matan Azrad <matan@nvidia.com>;
> Shahaf Shuler <shahafs@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>
> Subject: [PATCH v3 3/8] net/mlx5: support NVGRE encap action in sample
> 
> Add support for NVGRE encap as a sample action and validate it.
> 
> Signed-off-by: Salem Sol <salems@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

Re: [dpdk-dev] [PATCH v3 4/8] app/testpmd: support VXLAN encap for sample action

[Slava Ovsiienko]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Salem Sol
> Sent: Wednesday, March 17, 2021 11:26
> To: dev@dpdk.org
> Cc: Salem Sol <salems@nvidia.com>; Ori Kam <orika@nvidia.com>; Xiaoyun Li
> <xiaoyun.li@intel.com>
> Subject: [dpdk-dev] [PATCH v3 4/8] app/testpmd: support VXLAN encap for
> sample action
> 
> Add support for rte_flow_action_vxlan_encap as a sample action.
> 
> The example of test-pmd command:
> 
> 1.  set vxlan ip-version ... vni ... udp-src ...
>     set raw_encap 1 eth src.../ ipv4.../...
>     set sample_actions 2 vxlan_encap / port_id id 0 / end
>     flow create 0 ... pattern eth / end actions
>        sample ratio 1 index 2 / raw_encap index 1 / port_id id 0...
> 
>     The flow will result in all the matched egress packets will be
>     encapsulated and sent to wire, and also mirrored the packets
>     using VXLAN encapsulation data and sent to wire.
> 
> Signed-off-by: Salem Sol <salems@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

Re: [dpdk-dev] [PATCH v3 5/8] app/testpmd: support NVGRE encap for sample action

[Slava Ovsiienko]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Salem Sol
> Sent: Wednesday, March 17, 2021 11:26
> To: dev@dpdk.org
> Cc: Salem Sol <salems@nvidia.com>; Ori Kam <orika@nvidia.com>; Xiaoyun Li
> <xiaoyun.li@intel.com>
> Subject: [dpdk-dev] [PATCH v3 5/8] app/testpmd: support NVGRE encap for
> sample action
> 
> Add support for rte_flow_action_nvge_encap as a sample action.
> 
> The example of test-pmd command:
> 
> 1.  set nvgre ip-version ... tni ... ip-src ... ip-dst ...
>     set raw_encap 1 eth src... / ipv4... /...
>     set sample_actions 2 nvgre / port_id id 0 / end
>     flow create 0 ... pattern eth / end actions
>        sample ratio 1 index 2 / raw_encap index 1 / port_id id 0...
> 
>     The flow will result in all the matched egress packets will be
>     encapsulated and sent to wire, and also mirrored the packets
>     using NVGRE encapsulation data and sent to wire.
> 
> Signed-off-by: Salem Sol <salems@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

Re: [dpdk-dev] [PATCH v3 6/8] doc: update sample actions support in testpmd guide

[Slava Ovsiienko]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Salem Sol
> Sent: Wednesday, March 17, 2021 11:26
> To: dev@dpdk.org
> Cc: Salem Sol <salems@nvidia.com>; Xiaoyun Li <xiaoyun.li@intel.com>
> Subject: [dpdk-dev] [PATCH v3 6/8] doc: update sample actions support in
> testpmd guide
> 
> Update documentation for sample action usage in testpmd utilizing
> rte_flow_action_vxlan_encap and rte_flow_action_nvgre_encap and show
> the command line example.
> 
> Signed-off-by: Salem Sol <salems@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

Re: [dpdk-dev] [PATCH v3 7/8] doc: update sample actions support in mlx5 guide

[Slava Ovsiienko]

> -----Original Message-----
> From: Salem Sol <salems@nvidia.com>
> Sent: Wednesday, March 17, 2021 11:26
> To: dev@dpdk.org
> Cc: Salem Sol <salems@nvidia.com>; Matan Azrad <matan@nvidia.com>;
> Shahaf Shuler <shahafs@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>
> Subject: [PATCH v3 7/8] doc: update sample actions support in mlx5 guide
> 
> Updates the documentation with the added support for sample actions
> VXLAN and NVGRE encap in E-Switch steering flow.
> 
> Signed-off-by: Salem Sol <salems@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

Re: [dpdk-dev] [PATCH v3 8/8] doc: update dpdk 21.05 release notes

[Slava Ovsiienko]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Salem Sol
> Sent: Wednesday, March 17, 2021 11:26
> To: dev@dpdk.org
> Cc: Salem Sol <salems@nvidia.com>
> Subject: [dpdk-dev] [PATCH v3 8/8] doc: update dpdk 21.05 release notes
> 
> Update release notes for dpdk 21.05 to include the new Mellanox driver's
> support for VXLAN and NVGRE encap as sample actions.
> 
> Signed-off-by: Salem Sol <salems@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

Re: [dpdk-dev] [PATCH v3 6/8] doc: update sample actions support in testpmd guide

[Ferruh Yigit]

On 3/17/2021 9:26 AM, Salem Sol wrote:
> Update documentation for sample action usage in testpmd utilizing
> rte_flow_action_vxlan_encap and rte_flow_action_nvgre_encap and
> show the command line example.
> 

This patch has dependency to [1], right, can you please confirm it?

[1]: 
https://patches.dpdk.org/project/dpdk/patch/1617180669-225007-1-git-send-email-jiaweiw@nvidia.com/

> Signed-off-by: Salem Sol <salems@nvidia.com>
> ---
>   doc/guides/testpmd_app_ug/testpmd_funcs.rst | 22 +++++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 3a31cc6237..392e3a31cf 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -4901,6 +4901,28 @@ and also mirrored the packets with encapsulation header and sent to port id 0.
>    testpmd> flow create 0 ingress transfer pattern eth / end actions
>           sample ratio 1 index 0  / port_id id 2 / end
>   
> +E-Switch Mirroring rule, the matched ingress packets are sent to port id 2,
> +and also mirrored the packets with VXLAN encapsulation header and sent to port id 0.
> +

Similar comment on 'E-Switch', the mirroring is generic feature but 'E-Switch' 
is vendor specific, can you please update testpmd in a generic way?

> +::
> +
> + testpmd> set vxlan ip-version ipv4 vni 4 udp-src 4 udp-dst 4 ip-src 127.0.0.1
> +        ip-dst 128.0.0.1 eth-src 11:11:11:11:11:11 eth-dst 22:22:22:22:22:22
> + testpmd> set sample_actions 0 vxlan_encap / port_id id 0 / end
> + testpmd> flow create 0 ingress transfer pattern eth / end actions
> +        sample ratio 1 index 0  / port_id id 2 / end
> +
> +E-Switch Mirroring rule, the matched ingress packets are sent to port id 2,
> +and also mirrored the packets with NVGRE encapsulation header and sent to port id 0.
> +
> +::
> +
> + testpmd> set nvgre ip-version ipv4 tni 4 ip-src 127.0.0.1 ip-dst 128.0.0.1
> +        eth-src 11:11:11:11:11:11 eth-dst 22:22:22:22:22:22
> + testpmd> set sample_actions 0 nvgre_encap / port_id id 0 / end
> + testpmd> flow create 0 ingress transfer pattern eth / end actions
> +        sample ratio 1 index 0  / port_id id 2 / end
> +
>   BPF Functions
>   --------------
>   
> 

Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE encap data globally

[Ferruh Yigit]

On 3/17/2021 9:26 AM, Salem Sol wrote:
> From: Jiawei Wang <jiaweiw@nvidia.com>
> 
> With the current code the VXLAN/NVGRE parsing routine
> stored the configuration of the header on stack, this
> might lead to overwriting the data on the stack.
> 
> This patch stores the external data of vxlan and nvgre encap
> into global data as a pre-step to supporting vxlan and nvgre
> encap as a sample actions.
> 

I didn't get what was on stack and moved in to the global data, can you please 
elaborate.

For example for nvgre, 'action_nvgre_encap_data' is pointer in stack but the 
actual object is 'ctx->object', so it is not really in the stack.

Tough, OK to refactor and split the function as preparation to support the 
sample action.

> Signed-off-by: Jiawei Wang <jiaweiw@nvidia.com>

<...>

> -/** Parse VXLAN encap action. */
> +/** Setup VXLAN encap configuration. */
>   static int
> -parse_vc_action_vxlan_encap(struct context *ctx, const struct token *token,
> -			    const char *str, unsigned int len,
> -			    void *buf, unsigned int size)
> +parse_setup_vxlan_encap_data
> +		(struct action_vxlan_encap_data *action_vxlan_encap_data)

Can you please fix the syntax, I guess this is done to keep within in 80 column 
limit, but from readability perspective I think better to go over the 80 column 
a little instead of breaking the line like this.

<...>

> +/** Setup NVGRE encap configuration. */
> +static int
> +parse_setup_nvgre_encap_data
> +		(struct action_nvgre_encap_data *action_nvgre_encap_data)
> +{
> +	if (!action_nvgre_encap_data)
> +		return -1;

This is a static function, and the input of it is in our control, so not sure if 
we should verify the input here.
But if we need to, can you please check the return value of this function where 
it is called.

Re: [dpdk-dev] [PATCH v3 8/8] doc: update dpdk 21.05 release notes

[Ferruh Yigit]

On 3/17/2021 9:26 AM, Salem Sol wrote:
> Update release notes for dpdk 21.05 to include the new
> Mellanox driver's support for VXLAN and NVGRE encap as sample
> actions.
> 
> Signed-off-by: Salem Sol <salems@nvidia.com>
> ---
>   doc/guides/rel_notes/release_21_05.rst | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
> index 23f7f0bff9..312b3cbe61 100644
> --- a/doc/guides/rel_notes/release_21_05.rst
> +++ b/doc/guides/rel_notes/release_21_05.rst
> @@ -65,6 +65,12 @@ New Features
>   
>     * Added support for txgbevf PMD.
>   
> +* **Updated Mellanox mlx5 driver.**
> +
> +  Updated the Mellanox mlx5 driver with new features and improvements, including:
> +
> +  * Added support for VXLAN and NVGRE encap as a sample actions.
> +
>   * **Updated testpmd.**
>   
>     * Added command to display Rx queue used descriptor count.
> 

Can you please distribute the content of this patch to the patch that actually 
adds the feature, instead of having a separate release notes update patch?

Thanks.

Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE encap data globally

[Jiawei(Jonny) Wang]

Hello Ferruh,

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, March 31, 2021 8:08 PM
> To: Salem Sol <salems@nvidia.com>; dev@dpdk.org
> Cc: Jiawei(Jonny) Wang <jiaweiw@nvidia.com>; Ori Kam <orika@nvidia.com>;
> Xiaoyun Li <xiaoyun.li@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE
> encap data globally
> 
> On 3/17/2021 9:26 AM, Salem Sol wrote:
> > From: Jiawei Wang <jiaweiw@nvidia.com>
> >
> > With the current code the VXLAN/NVGRE parsing routine stored the
> > configuration of the header on stack, this might lead to overwriting
> > the data on the stack.
> >
> > This patch stores the external data of vxlan and nvgre encap into
> > global data as a pre-step to supporting vxlan and nvgre encap as a
> > sample actions.
> >
> 
> I didn't get what was on stack and moved in to the global data, can you
> please elaborate.
> 

This patch split the function and saved input data into input parameter:
So it mentioned here "pre-step" for next store the data of vxlan/nvgre into global.

The global var for sample action is defined in: 
(https://patches.dpdk.org/project/dpdk/patch/20210317092610.71000-5-salems@nvidia.com/)
struct action_vxlan_encap_data sample_vxlan_encap[RAW_SAMPLE_CONFS_MAX_NUM];

> For example for nvgre, 'action_nvgre_encap_data' is pointer in stack but the
> actual object is 'ctx->object', so it is not really in the stack.
> 

After call 'set sample 0 nvgre .. ', the data be stored into 'ctx->object', the 'ctx->object' will be reused
for the following CLI command, After that, while we try to use previous 'sample action' to fetch nvgre data,
these data may be lost.

For sample action, use global data can save the previous nvgre configurations data.

> Tough, OK to refactor and split the function as preparation to support the
> sample action.
> 
> > Signed-off-by: Jiawei Wang <jiaweiw@nvidia.com>
> 
> <...>
> 
> > -/** Parse VXLAN encap action. */
> > +/** Setup VXLAN encap configuration. */
> >   static int
> > -parse_vc_action_vxlan_encap(struct context *ctx, const struct token
> *token,
> > -			    const char *str, unsigned int len,
> > -			    void *buf, unsigned int size)
> > +parse_setup_vxlan_encap_data
> > +		(struct action_vxlan_encap_data *action_vxlan_encap_data)
> 
> Can you please fix the syntax, I guess this is done to keep within in 80 column
> limit, but from readability perspective I think better to go over the 80 column
> a little instead of breaking the line like this.
> 

Ok, will change into one line.

> <...>
> 
> > +/** Setup NVGRE encap configuration. */ static int
> > +parse_setup_nvgre_encap_data
> > +		(struct action_nvgre_encap_data *action_nvgre_encap_data)
> {
> > +	if (!action_nvgre_encap_data)
> > +		return -1;
> 
> This is a static function, and the input of it is in our control, so not sure if we
> should verify the input here.
> But if we need to, can you please check the return value of this function
> where it is called.

I agree with you that can remove this checking inside, since we make sure it's valid before call.

Thanks.

Re: [dpdk-dev] [PATCH v3 6/8] doc: update sample actions support in testpmd guide

[Salem Sol]

Hi Ferruh,

Indeed this patch is dependent on [1], it's also mentioned in the cover letter, I will rebase and post V4 addressing all the comments once [1] is accepted.

[1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatches.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F1617180669-225007-1-git-send-email-jiaweiw%40nvidia.com%2F&data=04%7C01%7Csalems%40nvidia.com%7C477a7e09192e4c62c0e508d8f43d58f2%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637527891628225504%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=BB7zufQaPsAeu1l3jZ0xKOxtx86%2F0rUrUuSKQuVGn%2FE%3D&reserved=0

Thanks,

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Wednesday, March 31, 2021 3:06 PM
To: Salem Sol <salems@nvidia.com>; dev@dpdk.org
Cc: Xiaoyun Li <xiaoyun.li@intel.com>; NBU-Contact-Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH v3 6/8] doc: update sample actions support in testpmd guide

External email: Use caution opening links or attachments


On 3/17/2021 9:26 AM, Salem Sol wrote:
> Update documentation for sample action usage in testpmd utilizing 
> rte_flow_action_vxlan_encap and rte_flow_action_nvgre_encap and show 
> the command line example.
>

This patch has dependency to [1], right, can you please confirm it?

[1]:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatches.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F1617180669-225007-1-git-send-email-jiaweiw%40nvidia.com%2F&amp;data=04%7C01%7Csalems%40nvidia.com%7C477a7e09192e4c62c0e508d8f43d58f2%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637527891628225504%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=BB7zufQaPsAeu1l3jZ0xKOxtx86%2F0rUrUuSKQuVGn%2FE%3D&amp;reserved=0

> Signed-off-by: Salem Sol <salems@nvidia.com>
> ---
>   doc/guides/testpmd_app_ug/testpmd_funcs.rst | 22 +++++++++++++++++++++
>   1 file changed, 22 insertions(+)
>
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst 
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 3a31cc6237..392e3a31cf 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -4901,6 +4901,28 @@ and also mirrored the packets with encapsulation header and sent to port id 0.
>    testpmd> flow create 0 ingress transfer pattern eth / end actions
>           sample ratio 1 index 0  / port_id id 2 / end
>
> +E-Switch Mirroring rule, the matched ingress packets are sent to port 
> +id 2, and also mirrored the packets with VXLAN encapsulation header and sent to port id 0.
> +

Similar comment on 'E-Switch', the mirroring is generic feature but 'E-Switch'
is vendor specific, can you please update testpmd in a generic way?

> +::
> +
> + testpmd> set vxlan ip-version ipv4 vni 4 udp-src 4 udp-dst 4 ip-src 
> + testpmd> 127.0.0.1
> +        ip-dst 128.0.0.1 eth-src 11:11:11:11:11:11 eth-dst 
> + 22:22:22:22:22:22
> + testpmd> set sample_actions 0 vxlan_encap / port_id id 0 / end flow 
> + testpmd> create 0 ingress transfer pattern eth / end actions
> +        sample ratio 1 index 0  / port_id id 2 / end
> +
> +E-Switch Mirroring rule, the matched ingress packets are sent to port 
> +id 2, and also mirrored the packets with NVGRE encapsulation header and sent to port id 0.
> +
> +::
> +
> + testpmd> set nvgre ip-version ipv4 tni 4 ip-src 127.0.0.1 ip-dst 
> + testpmd> 128.0.0.1
> +        eth-src 11:11:11:11:11:11 eth-dst 22:22:22:22:22:22
> + testpmd> set sample_actions 0 nvgre_encap / port_id id 0 / end flow 
> + testpmd> create 0 ingress transfer pattern eth / end actions
> +        sample ratio 1 index 0  / port_id id 2 / end
> +
>   BPF Functions
>   --------------
>
>

Re: [dpdk-dev] [PATCH v3 6/8] doc: update sample actions support in testpmd guide

[Ferruh Yigit]

On 4/1/2021 11:39 AM, Salem Sol wrote:
> Hi Ferruh,
> 
> Indeed this patch is dependent on [1], it's also mentioned in the cover letter, I will rebase and post V4 addressing all the comments once [1] is accepted.
> 

Cover letter one links to a mlx doc patch, that was the source of the confusion.

And the dependent tespmd patch has a new version, which looks good and I am 
planning to get it today, in next a few hours, fyi.

> [1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatches.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F1617180669-225007-1-git-send-email-jiaweiw%40nvidia.com%2F&data=04%7C01%7Csalems%40nvidia.com%7C477a7e09192e4c62c0e508d8f43d58f2%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637527891628225504%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=BB7zufQaPsAeu1l3jZ0xKOxtx86%2F0rUrUuSKQuVGn%2FE%3D&reserved=0
> 
> Thanks,
> 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, March 31, 2021 3:06 PM
> To: Salem Sol <salems@nvidia.com>; dev@dpdk.org
> Cc: Xiaoyun Li <xiaoyun.li@intel.com>; NBU-Contact-Thomas Monjalon <thomas@monjalon.net>
> Subject: Re: [dpdk-dev] [PATCH v3 6/8] doc: update sample actions support in testpmd guide
> 
> External email: Use caution opening links or attachments
> 
> 
> On 3/17/2021 9:26 AM, Salem Sol wrote:
>> Update documentation for sample action usage in testpmd utilizing
>> rte_flow_action_vxlan_encap and rte_flow_action_nvgre_encap and show
>> the command line example.
>>
> 
> This patch has dependency to [1], right, can you please confirm it?
> 
> [1]:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatches.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F1617180669-225007-1-git-send-email-jiaweiw%40nvidia.com%2F&amp;data=04%7C01%7Csalems%40nvidia.com%7C477a7e09192e4c62c0e508d8f43d58f2%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637527891628225504%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=BB7zufQaPsAeu1l3jZ0xKOxtx86%2F0rUrUuSKQuVGn%2FE%3D&amp;reserved=0
> 
>> Signed-off-by: Salem Sol <salems@nvidia.com>
>> ---
>>    doc/guides/testpmd_app_ug/testpmd_funcs.rst | 22 +++++++++++++++++++++
>>    1 file changed, 22 insertions(+)
>>
>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> index 3a31cc6237..392e3a31cf 100644
>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> @@ -4901,6 +4901,28 @@ and also mirrored the packets with encapsulation header and sent to port id 0.
>>     testpmd> flow create 0 ingress transfer pattern eth / end actions
>>            sample ratio 1 index 0  / port_id id 2 / end
>>
>> +E-Switch Mirroring rule, the matched ingress packets are sent to port
>> +id 2, and also mirrored the packets with VXLAN encapsulation header and sent to port id 0.
>> +
> 
> Similar comment on 'E-Switch', the mirroring is generic feature but 'E-Switch'
> is vendor specific, can you please update testpmd in a generic way?
> 
>> +::
>> +
>> + testpmd> set vxlan ip-version ipv4 vni 4 udp-src 4 udp-dst 4 ip-src
>> + testpmd> 127.0.0.1
>> +        ip-dst 128.0.0.1 eth-src 11:11:11:11:11:11 eth-dst
>> + 22:22:22:22:22:22
>> + testpmd> set sample_actions 0 vxlan_encap / port_id id 0 / end flow
>> + testpmd> create 0 ingress transfer pattern eth / end actions
>> +        sample ratio 1 index 0  / port_id id 2 / end
>> +
>> +E-Switch Mirroring rule, the matched ingress packets are sent to port
>> +id 2, and also mirrored the packets with NVGRE encapsulation header and sent to port id 0.
>> +
>> +::
>> +
>> + testpmd> set nvgre ip-version ipv4 tni 4 ip-src 127.0.0.1 ip-dst
>> + testpmd> 128.0.0.1
>> +        eth-src 11:11:11:11:11:11 eth-dst 22:22:22:22:22:22
>> + testpmd> set sample_actions 0 nvgre_encap / port_id id 0 / end flow
>> + testpmd> create 0 ingress transfer pattern eth / end actions
>> +        sample ratio 1 index 0  / port_id id 2 / end
>> +
>>    BPF Functions
>>    --------------
>>
>>
> 

Re: [dpdk-dev] [PATCH v3 6/8] doc: update sample actions support in testpmd guide

[Salem Sol]

Sorry for the confusion, it was meant to show dependency on the whole series  which had two commits [1] and [2].
I will keep an eye for when the patches are merged to post the v4, thanks for the update.

[1]: https://patches.dpdk.org/project/dpdk/patch/1615907899-399082-1-git-send-email-jiaweiw@nvidia.com/
[2]: https://patches.dpdk.org/project/dpdk/patch/1617244796-358287-1-git-send-email-jiaweiw@nvidia.com/

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Thursday, April 1, 2021 1:43 PM
To: Salem Sol <salems@nvidia.com>; dev@dpdk.org
Cc: Xiaoyun Li <xiaoyun.li@intel.com>; NBU-Contact-Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH v3 6/8] doc: update sample actions support in testpmd guide

External email: Use caution opening links or attachments


On 4/1/2021 11:39 AM, Salem Sol wrote:
> Hi Ferruh,
>
> Indeed this patch is dependent on [1], it's also mentioned in the cover letter, I will rebase and post V4 addressing all the comments once [1] is accepted.
>

Cover letter one links to a mlx doc patch, that was the source of the confusion.

And the dependent tespmd patch has a new version, which looks good and I am planning to get it today, in next a few hours, fyi.

> [1] 
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> hes.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F1617180669-225007-1-git-send-e
> mail-jiaweiw%40nvidia.com%2F&amp;data=04%7C01%7Csalems%40nvidia.com%7C
> 44180cd39e2f49fe373308d8f4faf834%7C43083d15727340c1b7db39efd9ccc17a%7C
> 0%7C0%7C637528706034278930%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDA
> iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=3obAy
> KaqOoxY7qpJCvyI3om%2FRwMaS2NaTr0S79PfimM%3D&amp;reserved=0
>
> Thanks,
>
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, March 31, 2021 3:06 PM
> To: Salem Sol <salems@nvidia.com>; dev@dpdk.org
> Cc: Xiaoyun Li <xiaoyun.li@intel.com>; NBU-Contact-Thomas Monjalon 
> <thomas@monjalon.net>
> Subject: Re: [dpdk-dev] [PATCH v3 6/8] doc: update sample actions 
> support in testpmd guide
>
> External email: Use caution opening links or attachments
>
>
> On 3/17/2021 9:26 AM, Salem Sol wrote:
>> Update documentation for sample action usage in testpmd utilizing 
>> rte_flow_action_vxlan_encap and rte_flow_action_nvgre_encap and show 
>> the command line example.
>>
>
> This patch has dependency to [1], right, can you please confirm it?
>
> [1]:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> hes.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F1617180669-225007-1-git-send-e
> mail-jiaweiw%40nvidia.com%2F&amp;data=04%7C01%7Csalems%40nvidia.com%7C
> 44180cd39e2f49fe373308d8f4faf834%7C43083d15727340c1b7db39efd9ccc17a%7C
> 0%7C0%7C637528706034288924%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDA
> iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ae25l
> Npwcz33WuxNorJZ9%2B%2BPxviVpV%2BAWNT6OhUER04%3D&amp;reserved=0
>
>> Signed-off-by: Salem Sol <salems@nvidia.com>
>> ---
>>    doc/guides/testpmd_app_ug/testpmd_funcs.rst | 22 +++++++++++++++++++++
>>    1 file changed, 22 insertions(+)
>>
>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> index 3a31cc6237..392e3a31cf 100644
>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> @@ -4901,6 +4901,28 @@ and also mirrored the packets with encapsulation header and sent to port id 0.
>>     testpmd> flow create 0 ingress transfer pattern eth / end actions
>>            sample ratio 1 index 0  / port_id id 2 / end
>>
>> +E-Switch Mirroring rule, the matched ingress packets are sent to 
>> +port id 2, and also mirrored the packets with VXLAN encapsulation header and sent to port id 0.
>> +
>
> Similar comment on 'E-Switch', the mirroring is generic feature but 'E-Switch'
> is vendor specific, can you please update testpmd in a generic way?
>
>> +::
>> +
>> + testpmd> set vxlan ip-version ipv4 vni 4 udp-src 4 udp-dst 4 ip-src
>> + testpmd> 127.0.0.1
>> +        ip-dst 128.0.0.1 eth-src 11:11:11:11:11:11 eth-dst
>> + 22:22:22:22:22:22
>> + testpmd> set sample_actions 0 vxlan_encap / port_id id 0 / end flow 
>> + testpmd> create 0 ingress transfer pattern eth / end actions
>> +        sample ratio 1 index 0  / port_id id 2 / end
>> +
>> +E-Switch Mirroring rule, the matched ingress packets are sent to 
>> +port id 2, and also mirrored the packets with NVGRE encapsulation header and sent to port id 0.
>> +
>> +::
>> +
>> + testpmd> set nvgre ip-version ipv4 tni 4 ip-src 127.0.0.1 ip-dst
>> + testpmd> 128.0.0.1
>> +        eth-src 11:11:11:11:11:11 eth-dst 22:22:22:22:22:22
>> + testpmd> set sample_actions 0 nvgre_encap / port_id id 0 / end flow 
>> + testpmd> create 0 ingress transfer pattern eth / end actions
>> +        sample ratio 1 index 0  / port_id id 2 / end
>> +
>>    BPF Functions
>>    --------------
>>
>>
>

Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE encap data globally

[Ferruh Yigit]

On 4/1/2021 5:13 AM, Jiawei(Jonny) Wang wrote:
> Hello Ferruh,
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Wednesday, March 31, 2021 8:08 PM
>> To: Salem Sol <salems@nvidia.com>; dev@dpdk.org
>> Cc: Jiawei(Jonny) Wang <jiaweiw@nvidia.com>; Ori Kam <orika@nvidia.com>;
>> Xiaoyun Li <xiaoyun.li@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE
>> encap data globally
>>
>> On 3/17/2021 9:26 AM, Salem Sol wrote:
>>> From: Jiawei Wang <jiaweiw@nvidia.com>
>>>
>>> With the current code the VXLAN/NVGRE parsing routine stored the
>>> configuration of the header on stack, this might lead to overwriting
>>> the data on the stack.
>>>
>>> This patch stores the external data of vxlan and nvgre encap into
>>> global data as a pre-step to supporting vxlan and nvgre encap as a
>>> sample actions.
>>>
>>
>> I didn't get what was on stack and moved in to the global data, can you
>> please elaborate.
>>
> 
> This patch split the function and saved input data into input parameter:
> So it mentioned here "pre-step" for next store the data of vxlan/nvgre into global.
> 
> The global var for sample action is defined in:
> (https://patches.dpdk.org/project/dpdk/patch/20210317092610.71000-5-salems@nvidia.com/)
> struct action_vxlan_encap_data sample_vxlan_encap[RAW_SAMPLE_CONFS_MAX_NUM];
> 

Commit log says:

"
This patch stores the external data of vxlan and nvgre encap
into global data as a pre-step to supporting vxlan and nvgre
encap as a sample actions.
"

It says this patch does storing into the global data, but as far as I can see 
from code and above description, this patch is just preparation for it.
I can see there is a new version which has same commit log, can you please 
update it in new version?

>> For example for nvgre, 'action_nvgre_encap_data' is pointer in stack but the
>> actual object is 'ctx->object', so it is not really in the stack.
>>
> 
> After call 'set sample 0 nvgre .. ', the data be stored into 'ctx->object', the 'ctx->object' will be reused
> for the following CLI command, After that, while we try to use previous 'sample action' to fetch nvgre data,
> these data may be lost.
> 
> For sample action, use global data can save the previous nvgre configurations data.
> 

Got it, the target is to use "set sample_actions ..." testpmd command to store 
vxlan/nvgre encap sample actions.
For record, can you please document what was the way to the same before this 
support, can you please document the old testpmd command.

>> Tough, OK to refactor and split the function as preparation to support the
>> sample action.
>>
>>> Signed-off-by: Jiawei Wang <jiaweiw@nvidia.com>
>>
>> <...>
>>
>>> -/** Parse VXLAN encap action. */
>>> +/** Setup VXLAN encap configuration. */
>>>    static int
>>> -parse_vc_action_vxlan_encap(struct context *ctx, const struct token
>> *token,
>>> -			    const char *str, unsigned int len,
>>> -			    void *buf, unsigned int size)
>>> +parse_setup_vxlan_encap_data
>>> +		(struct action_vxlan_encap_data *action_vxlan_encap_data)
>>
>> Can you please fix the syntax, I guess this is done to keep within in 80 column
>> limit, but from readability perspective I think better to go over the 80 column
>> a little instead of breaking the line like this.
>>
> 
> Ok, will change into one line.
> 
>> <...>
>>
>>> +/** Setup NVGRE encap configuration. */ static int
>>> +parse_setup_nvgre_encap_data
>>> +		(struct action_nvgre_encap_data *action_nvgre_encap_data)
>> {
>>> +	if (!action_nvgre_encap_data)
>>> +		return -1;
>>
>> This is a static function, and the input of it is in our control, so not sure if we
>> should verify the input here.
>> But if we need to, can you please check the return value of this function
>> where it is called.
> 
> I agree with you that can remove this checking inside, since we make sure it's valid before call.
> 
> Thanks.
> 

Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE encap data globally

[Salem Sol]

Hi Ferruh,

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Tuesday, April 6, 2021 5:44 PM
To: Jiawei(Jonny) Wang <jiaweiw@nvidia.com>; Salem Sol <salems@nvidia.com>; dev@dpdk.org
Cc: Ori Kam <orika@nvidia.com>; Xiaoyun Li <xiaoyun.li@intel.com>
Subject: Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE encap data globally

External email: Use caution opening links or attachments


On 4/1/2021 5:13 AM, Jiawei(Jonny) Wang wrote:
> Hello Ferruh,
>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Wednesday, March 31, 2021 8:08 PM
>> To: Salem Sol <salems@nvidia.com>; dev@dpdk.org
>> Cc: Jiawei(Jonny) Wang <jiaweiw@nvidia.com>; Ori Kam 
>> <orika@nvidia.com>; Xiaoyun Li <xiaoyun.li@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE 
>> encap data globally
>>
>> On 3/17/2021 9:26 AM, Salem Sol wrote:
>>> From: Jiawei Wang <jiaweiw@nvidia.com>
>>>
>>> With the current code the VXLAN/NVGRE parsing routine stored the 
>>> configuration of the header on stack, this might lead to overwriting 
>>> the data on the stack.
>>>
>>> This patch stores the external data of vxlan and nvgre encap into 
>>> global data as a pre-step to supporting vxlan and nvgre encap as a 
>>> sample actions.
>>>
>>
>> I didn't get what was on stack and moved in to the global data, can 
>> you please elaborate.
>>
>
> This patch split the function and saved input data into input parameter:
> So it mentioned here "pre-step" for next store the data of vxlan/nvgre into global.
>
> The global var for sample action is defined in:
> (https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> ches.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F20210317092610.71000-5-salems
> %40nvidia.com%2F&amp;data=04%7C01%7Csalems%40nvidia.com%7Cd9baadbc48fc
> 44caf4fc08d8f90a7553%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C6375
> 33170601193210%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
> uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=olwLyRnHnM7TyKDFI
> xVH3Dj6KhWtUzdXgAyGgON4M9M%3D&amp;reserved=0)
> struct action_vxlan_encap_data 
> sample_vxlan_encap[RAW_SAMPLE_CONFS_MAX_NUM];
>

Commit log says:

"
This patch stores the external data of vxlan and nvgre encap into global data as a pre-step to supporting vxlan and nvgre encap as a sample actions.
"

It says this patch does storing into the global data, but as far as I can see from code and above description, this patch is just preparation for it.
I can see there is a new version which has same commit log, can you please update it in new version?

I will update in the next series.

>> For example for nvgre, 'action_nvgre_encap_data' is pointer in stack 
>> but the actual object is 'ctx->object', so it is not really in the stack.
>>
>
> After call 'set sample 0 nvgre .. ', the data be stored into 
> 'ctx->object', the 'ctx->object' will be reused for the following CLI 
> command, After that, while we try to use previous 'sample action' to fetch nvgre data, these data may be lost.
>
> For sample action, use global data can save the previous nvgre configurations data.
>

Got it, the target is to use "set sample_actions ..." testpmd command to store vxlan/nvgre encap sample actions.
For record, can you please document what was the way to the same before this support, can you please document the old testpmd command.

Can you please elaborate regarding where did you want this documentation? 
Prior to this support it is already documented, in http://patches.dpdk.org/project/dpdk/patch/1617244796-358287-1-git-send-email-jiaweiw@nvidia.com/
With the "raw_encap"

>> Tough, OK to refactor and split the function as preparation to 
>> support the sample action.
>>
>>> Signed-off-by: Jiawei Wang <jiaweiw@nvidia.com>
>>
>> <...>
>>
>>> -/** Parse VXLAN encap action. */
>>> +/** Setup VXLAN encap configuration. */
>>>    static int
>>> -parse_vc_action_vxlan_encap(struct context *ctx, const struct token
>> *token,
>>> -                       const char *str, unsigned int len,
>>> -                       void *buf, unsigned int size)
>>> +parse_setup_vxlan_encap_data
>>> +           (struct action_vxlan_encap_data 
>>> +*action_vxlan_encap_data)
>>
>> Can you please fix the syntax, I guess this is done to keep within in 
>> 80 column limit, but from readability perspective I think better to 
>> go over the 80 column a little instead of breaking the line like this.
>>
>
> Ok, will change into one line.
>
>> <...>
>>
>>> +/** Setup NVGRE encap configuration. */ static int 
>>> +parse_setup_nvgre_encap_data
>>> +           (struct action_nvgre_encap_data 
>>> +*action_nvgre_encap_data)
>> {
>>> +   if (!action_nvgre_encap_data)
>>> +           return -1;
>>
>> This is a static function, and the input of it is in our control, so 
>> not sure if we should verify the input here.
>> But if we need to, can you please check the return value of this 
>> function where it is called.
>
> I agree with you that can remove this checking inside, since we make sure it's valid before call.
>
> Thanks.
>

Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE encap data globally

[Ferruh Yigit]

On 4/7/2021 9:19 AM, Salem Sol wrote:
> Hi Ferruh,
> 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, April 6, 2021 5:44 PM
> To: Jiawei(Jonny) Wang <jiaweiw@nvidia.com>; Salem Sol <salems@nvidia.com>; dev@dpdk.org
> Cc: Ori Kam <orika@nvidia.com>; Xiaoyun Li <xiaoyun.li@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE encap data globally
> 
> External email: Use caution opening links or attachments
> 
> 
> On 4/1/2021 5:13 AM, Jiawei(Jonny) Wang wrote:
>> Hello Ferruh,
>>
>>> -----Original Message-----
>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>> Sent: Wednesday, March 31, 2021 8:08 PM
>>> To: Salem Sol <salems@nvidia.com>; dev@dpdk.org
>>> Cc: Jiawei(Jonny) Wang <jiaweiw@nvidia.com>; Ori Kam
>>> <orika@nvidia.com>; Xiaoyun Li <xiaoyun.li@intel.com>
>>> Subject: Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE
>>> encap data globally
>>>
>>> On 3/17/2021 9:26 AM, Salem Sol wrote:
>>>> From: Jiawei Wang <jiaweiw@nvidia.com>
>>>>
>>>> With the current code the VXLAN/NVGRE parsing routine stored the
>>>> configuration of the header on stack, this might lead to overwriting
>>>> the data on the stack.
>>>>
>>>> This patch stores the external data of vxlan and nvgre encap into
>>>> global data as a pre-step to supporting vxlan and nvgre encap as a
>>>> sample actions.
>>>>
>>>
>>> I didn't get what was on stack and moved in to the global data, can
>>> you please elaborate.
>>>
>>
>> This patch split the function and saved input data into input parameter:
>> So it mentioned here "pre-step" for next store the data of vxlan/nvgre into global.
>>
>> The global var for sample action is defined in:
>> (https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
>> ches.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F20210317092610.71000-5-salems
>> %40nvidia.com%2F&amp;data=04%7C01%7Csalems%40nvidia.com%7Cd9baadbc48fc
>> 44caf4fc08d8f90a7553%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C6375
>> 33170601193210%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
>> uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=olwLyRnHnM7TyKDFI
>> xVH3Dj6KhWtUzdXgAyGgON4M9M%3D&amp;reserved=0)
>> struct action_vxlan_encap_data
>> sample_vxlan_encap[RAW_SAMPLE_CONFS_MAX_NUM];
>>
> 
> Commit log says:
> 
> "
> This patch stores the external data of vxlan and nvgre encap into global data as a pre-step to supporting vxlan and nvgre encap as a sample actions.
> "
> 
> It says this patch does storing into the global data, but as far as I can see from code and above description, this patch is just preparation for it.
> I can see there is a new version which has same commit log, can you please update it in new version?
> 
> I will update in the next series.
> 
>>> For example for nvgre, 'action_nvgre_encap_data' is pointer in stack
>>> but the actual object is 'ctx->object', so it is not really in the stack.
>>>
>>
>> After call 'set sample 0 nvgre .. ', the data be stored into
>> 'ctx->object', the 'ctx->object' will be reused for the following CLI
>> command, After that, while we try to use previous 'sample action' to fetch nvgre data, these data may be lost.
>>
>> For sample action, use global data can save the previous nvgre configurations data.
>>
> 
> Got it, the target is to use "set sample_actions ..." testpmd command to store vxlan/nvgre encap sample actions.
> For record, can you please document what was the way to the same before this support, can you please document the old testpmd command.
> 
> Can you please elaborate regarding where did you want this documentation?
> Prior to this support it is already documented, in http://patches.dpdk.org/project/dpdk/patch/1617244796-358287-1-git-send-email-jiaweiw@nvidia.com/
> With the "raw_encap"
> 

I was just thinking putting it in the commit log, for reference. To record how 
it was previously done.

>>> Tough, OK to refactor and split the function as preparation to
>>> support the sample action.
>>>
>>>> Signed-off-by: Jiawei Wang <jiaweiw@nvidia.com>
>>>
>>> <...>
>>>
>>>> -/** Parse VXLAN encap action. */
>>>> +/** Setup VXLAN encap configuration. */
>>>>     static int
>>>> -parse_vc_action_vxlan_encap(struct context *ctx, const struct token
>>> *token,
>>>> -                       const char *str, unsigned int len,
>>>> -                       void *buf, unsigned int size)
>>>> +parse_setup_vxlan_encap_data
>>>> +           (struct action_vxlan_encap_data
>>>> +*action_vxlan_encap_data)
>>>
>>> Can you please fix the syntax, I guess this is done to keep within in
>>> 80 column limit, but from readability perspective I think better to
>>> go over the 80 column a little instead of breaking the line like this.
>>>
>>
>> Ok, will change into one line.
>>
>>> <...>
>>>
>>>> +/** Setup NVGRE encap configuration. */ static int
>>>> +parse_setup_nvgre_encap_data
>>>> +           (struct action_nvgre_encap_data
>>>> +*action_nvgre_encap_data)
>>> {
>>>> +   if (!action_nvgre_encap_data)
>>>> +           return -1;
>>>
>>> This is a static function, and the input of it is in our control, so
>>> not sure if we should verify the input here.
>>> But if we need to, can you please check the return value of this
>>> function where it is called.
>>
>> I agree with you that can remove this checking inside, since we make sure it's valid before call.
>>
>> Thanks.
>>
> 

Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE encap data globally

[Salem Sol]

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Wednesday, April 7, 2021 11:24 AM
To: Salem Sol <salems@nvidia.com>; Jiawei(Jonny) Wang <jiaweiw@nvidia.com>; dev@dpdk.org
Cc: Ori Kam <orika@nvidia.com>; Xiaoyun Li <xiaoyun.li@intel.com>
Subject: Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE encap data globally

External email: Use caution opening links or attachments


On 4/7/2021 9:19 AM, Salem Sol wrote:
> Hi Ferruh,
>
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, April 6, 2021 5:44 PM
> To: Jiawei(Jonny) Wang <jiaweiw@nvidia.com>; Salem Sol 
> <salems@nvidia.com>; dev@dpdk.org
> Cc: Ori Kam <orika@nvidia.com>; Xiaoyun Li <xiaoyun.li@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE 
> encap data globally
>
> External email: Use caution opening links or attachments
>
>
> On 4/1/2021 5:13 AM, Jiawei(Jonny) Wang wrote:
>> Hello Ferruh,
>>
>>> -----Original Message-----
>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>> Sent: Wednesday, March 31, 2021 8:08 PM
>>> To: Salem Sol <salems@nvidia.com>; dev@dpdk.org
>>> Cc: Jiawei(Jonny) Wang <jiaweiw@nvidia.com>; Ori Kam 
>>> <orika@nvidia.com>; Xiaoyun Li <xiaoyun.li@intel.com>
>>> Subject: Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store 
>>> VXLAN/NVGRE encap data globally
>>>
>>> On 3/17/2021 9:26 AM, Salem Sol wrote:
>>>> From: Jiawei Wang <jiaweiw@nvidia.com>
>>>>
>>>> With the current code the VXLAN/NVGRE parsing routine stored the 
>>>> configuration of the header on stack, this might lead to 
>>>> overwriting the data on the stack.
>>>>
>>>> This patch stores the external data of vxlan and nvgre encap into 
>>>> global data as a pre-step to supporting vxlan and nvgre encap as a 
>>>> sample actions.
>>>>
>>>
>>> I didn't get what was on stack and moved in to the global data, can 
>>> you please elaborate.
>>>
>>
>> This patch split the function and saved input data into input parameter:
>> So it mentioned here "pre-step" for next store the data of vxlan/nvgre into global.
>>
>> The global var for sample action is defined in:
>> (https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
>> t 
>> ches.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F20210317092610.71000-5-salem
>> s 
>> %40nvidia.com%2F&amp;data=04%7C01%7Csalems%40nvidia.com%7Cd9baadbc48f
>> c
>> 44caf4fc08d8f90a7553%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637
>> 5 
>> 33170601193210%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2
>> l 
>> uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=olwLyRnHnM7TyKDF
>> I
>> xVH3Dj6KhWtUzdXgAyGgON4M9M%3D&amp;reserved=0)
>> struct action_vxlan_encap_data
>> sample_vxlan_encap[RAW_SAMPLE_CONFS_MAX_NUM];
>>
>
> Commit log says:
>
> "
> This patch stores the external data of vxlan and nvgre encap into global data as a pre-step to supporting vxlan and nvgre encap as a sample actions.
> "
>
> It says this patch does storing into the global data, but as far as I can see from code and above description, this patch is just preparation for it.
> I can see there is a new version which has same commit log, can you please update it in new version?
>
> I will update in the next series.
>
>>> For example for nvgre, 'action_nvgre_encap_data' is pointer in stack 
>>> but the actual object is 'ctx->object', so it is not really in the stack.
>>>
>>
>> After call 'set sample 0 nvgre .. ', the data be stored into 
>> 'ctx->object', the 'ctx->object' will be reused for the following CLI 
>> command, After that, while we try to use previous 'sample action' to fetch nvgre data, these data may be lost.
>>
>> For sample action, use global data can save the previous nvgre configurations data.
>>
>
> Got it, the target is to use "set sample_actions ..." testpmd command to store vxlan/nvgre encap sample actions.
> For record, can you please document what was the way to the same before this support, can you please document the old testpmd command.
>
> Can you please elaborate regarding where did you want this documentation?
> Prior to this support it is already documented, in 
> http://patches.dpdk.org/project/dpdk/patch/1617244796-358287-1-git-sen
> d-email-jiaweiw@nvidia.com/
> With the "raw_encap"
>

> I was just thinking putting it in the commit log, for reference. To record how it was previously done.

Does this seem acceptable? 
" app/testpmd: prepare storing VXLAN/NVGRE encap data globally
 
 With the current code the VXLAN/NVGRE parsing routine
 stored the configuration of the header on stack, this
 might lead to overwriting the data on the stack.
 
 Currently having VXLAN/NVGRE encap as sample actions
 is done using RAW_ENCAP, for example:
 1. set raw_encap 1 eth src.../ vxlan vni.../ ipv4.../ ...
    set sample_actions 0 raw_encap / port_id id 0 / end
    flow create 0 ... pattern eth / end actions
       sample ration 1 index 0 / jump group 1 / end
 
 The goal is to utilize the rte_flow_action_vxlan_encap
 and rte_flow_action_nvgre_encap for sample actions.
 
 This patch prepares storing the external data of vxlan and 
 nvgre encap into global data as a pre-step to supporting 
 vxlan and nvgre encap as a sample actions."

>>> Tough, OK to refactor and split the function as preparation to 
>>> support the sample action.
>>>
>>>> Signed-off-by: Jiawei Wang <jiaweiw@nvidia.com>
>>>
>>> <...>
>>>
>>>> -/** Parse VXLAN encap action. */
>>>> +/** Setup VXLAN encap configuration. */
>>>>     static int
>>>> -parse_vc_action_vxlan_encap(struct context *ctx, const struct 
>>>> token
>>> *token,
>>>> -                       const char *str, unsigned int len,
>>>> -                       void *buf, unsigned int size)
>>>> +parse_setup_vxlan_encap_data
>>>> +           (struct action_vxlan_encap_data
>>>> +*action_vxlan_encap_data)
>>>
>>> Can you please fix the syntax, I guess this is done to keep within 
>>> in
>>> 80 column limit, but from readability perspective I think better to 
>>> go over the 80 column a little instead of breaking the line like this.
>>>
>>
>> Ok, will change into one line.
>>
>>> <...>
>>>
>>>> +/** Setup NVGRE encap configuration. */ static int 
>>>> +parse_setup_nvgre_encap_data
>>>> +           (struct action_nvgre_encap_data
>>>> +*action_nvgre_encap_data)
>>> {
>>>> +   if (!action_nvgre_encap_data)
>>>> +           return -1;
>>>
>>> This is a static function, and the input of it is in our control, so 
>>> not sure if we should verify the input here.
>>> But if we need to, can you please check the return value of this 
>>> function where it is called.
>>
>> I agree with you that can remove this checking inside, since we make sure it's valid before call.
>>
>> Thanks.
>>
>

Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE encap data globally

[Ferruh Yigit]

On 4/7/2021 9:35 AM, Salem Sol wrote:
> 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, April 7, 2021 11:24 AM
> To: Salem Sol <salems@nvidia.com>; Jiawei(Jonny) Wang <jiaweiw@nvidia.com>; dev@dpdk.org
> Cc: Ori Kam <orika@nvidia.com>; Xiaoyun Li <xiaoyun.li@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE encap data globally
> 
> External email: Use caution opening links or attachments
> 
> 
> On 4/7/2021 9:19 AM, Salem Sol wrote:
>> Hi Ferruh,
>>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Tuesday, April 6, 2021 5:44 PM
>> To: Jiawei(Jonny) Wang <jiaweiw@nvidia.com>; Salem Sol
>> <salems@nvidia.com>; dev@dpdk.org
>> Cc: Ori Kam <orika@nvidia.com>; Xiaoyun Li <xiaoyun.li@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store VXLAN/NVGRE
>> encap data globally
>>
>> External email: Use caution opening links or attachments
>>
>>
>> On 4/1/2021 5:13 AM, Jiawei(Jonny) Wang wrote:
>>> Hello Ferruh,
>>>
>>>> -----Original Message-----
>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> Sent: Wednesday, March 31, 2021 8:08 PM
>>>> To: Salem Sol <salems@nvidia.com>; dev@dpdk.org
>>>> Cc: Jiawei(Jonny) Wang <jiaweiw@nvidia.com>; Ori Kam
>>>> <orika@nvidia.com>; Xiaoyun Li <xiaoyun.li@intel.com>
>>>> Subject: Re: [dpdk-dev] [PATCH v3 1/8] app/testpmd: store
>>>> VXLAN/NVGRE encap data globally
>>>>
>>>> On 3/17/2021 9:26 AM, Salem Sol wrote:
>>>>> From: Jiawei Wang <jiaweiw@nvidia.com>
>>>>>
>>>>> With the current code the VXLAN/NVGRE parsing routine stored the
>>>>> configuration of the header on stack, this might lead to
>>>>> overwriting the data on the stack.
>>>>>
>>>>> This patch stores the external data of vxlan and nvgre encap into
>>>>> global data as a pre-step to supporting vxlan and nvgre encap as a
>>>>> sample actions.
>>>>>
>>>>
>>>> I didn't get what was on stack and moved in to the global data, can
>>>> you please elaborate.
>>>>
>>>
>>> This patch split the function and saved input data into input parameter:
>>> So it mentioned here "pre-step" for next store the data of vxlan/nvgre into global.
>>>
>>> The global var for sample action is defined in:
>>> (https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa
>>> t
>>> ches.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F20210317092610.71000-5-salem
>>> s
>>> %40nvidia.com%2F&amp;data=04%7C01%7Csalems%40nvidia.com%7Cd9baadbc48f
>>> c
>>> 44caf4fc08d8f90a7553%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637
>>> 5
>>> 33170601193210%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2
>>> l
>>> uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=olwLyRnHnM7TyKDF
>>> I
>>> xVH3Dj6KhWtUzdXgAyGgON4M9M%3D&amp;reserved=0)
>>> struct action_vxlan_encap_data
>>> sample_vxlan_encap[RAW_SAMPLE_CONFS_MAX_NUM];
>>>
>>
>> Commit log says:
>>
>> "
>> This patch stores the external data of vxlan and nvgre encap into global data as a pre-step to supporting vxlan and nvgre encap as a sample actions.
>> "
>>
>> It says this patch does storing into the global data, but as far as I can see from code and above description, this patch is just preparation for it.
>> I can see there is a new version which has same commit log, can you please update it in new version?
>>
>> I will update in the next series.
>>
>>>> For example for nvgre, 'action_nvgre_encap_data' is pointer in stack
>>>> but the actual object is 'ctx->object', so it is not really in the stack.
>>>>
>>>
>>> After call 'set sample 0 nvgre .. ', the data be stored into
>>> 'ctx->object', the 'ctx->object' will be reused for the following CLI
>>> command, After that, while we try to use previous 'sample action' to fetch nvgre data, these data may be lost.
>>>
>>> For sample action, use global data can save the previous nvgre configurations data.
>>>
>>
>> Got it, the target is to use "set sample_actions ..." testpmd command to store vxlan/nvgre encap sample actions.
>> For record, can you please document what was the way to the same before this support, can you please document the old testpmd command.
>>
>> Can you please elaborate regarding where did you want this documentation?
>> Prior to this support it is already documented, in
>> http://patches.dpdk.org/project/dpdk/patch/1617244796-358287-1-git-sen
>> d-email-jiaweiw@nvidia.com/
>> With the "raw_encap"
>>
> 
>> I was just thinking putting it in the commit log, for reference. To record how it was previously done.
> 
> Does this seem acceptable?
> " app/testpmd: prepare storing VXLAN/NVGRE encap data globally
>   
>   With the current code the VXLAN/NVGRE parsing routine
>   stored the configuration of the header on stack, this
>   might lead to overwriting the data on the stack.
>   
>   Currently having VXLAN/NVGRE encap as sample actions
>   is done using RAW_ENCAP, for example:
>   1. set raw_encap 1 eth src.../ vxlan vni.../ ipv4.../ ...
>      set sample_actions 0 raw_encap / port_id id 0 / end
>      flow create 0 ... pattern eth / end actions
>         sample ration 1 index 0 / jump group 1 / end
>   
>   The goal is to utilize the rte_flow_action_vxlan_encap
>   and rte_flow_action_nvgre_encap for sample actions.
>   
>   This patch prepares storing the external data of vxlan and
>   nvgre encap into global data as a pre-step to supporting
>   vxlan and nvgre encap as a sample actions."
> 

Sounds good, thank you.

>>>> Tough, OK to refactor and split the function as preparation to
>>>> support the sample action.
>>>>
>>>>> Signed-off-by: Jiawei Wang <jiaweiw@nvidia.com>
>>>>
>>>> <...>
>>>>
>>>>> -/** Parse VXLAN encap action. */
>>>>> +/** Setup VXLAN encap configuration. */
>>>>>      static int
>>>>> -parse_vc_action_vxlan_encap(struct context *ctx, const struct
>>>>> token
>>>> *token,
>>>>> -                       const char *str, unsigned int len,
>>>>> -                       void *buf, unsigned int size)
>>>>> +parse_setup_vxlan_encap_data
>>>>> +           (struct action_vxlan_encap_data
>>>>> +*action_vxlan_encap_data)
>>>>
>>>> Can you please fix the syntax, I guess this is done to keep within
>>>> in
>>>> 80 column limit, but from readability perspective I think better to
>>>> go over the 80 column a little instead of breaking the line like this.
>>>>
>>>
>>> Ok, will change into one line.
>>>
>>>> <...>
>>>>
>>>>> +/** Setup NVGRE encap configuration. */ static int
>>>>> +parse_setup_nvgre_encap_data
>>>>> +           (struct action_nvgre_encap_data
>>>>> +*action_nvgre_encap_data)
>>>> {
>>>>> +   if (!action_nvgre_encap_data)
>>>>> +           return -1;
>>>>
>>>> This is a static function, and the input of it is in our control, so
>>>> not sure if we should verify the input here.
>>>> But if we need to, can you please check the return value of this
>>>> function where it is called.
>>>
>>> I agree with you that can remove this checking inside, since we make sure it's valid before call.
>>>
>>> Thanks.
>>>
>>
>