[dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and FDIR

[dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and FDIR

[Beilei Xing]

This patch adds bitmask support for RSS, FDIR
and FDIR flexible payload.

Signed-off-by: Beilei Xing <beilei.xing@intel.com>
---
 app/test-pmd/cmdline.c                      | 199 +++++++++++++++++++++++++---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |   8 +-
 2 files changed, 187 insertions(+), 20 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 589121d..a227554 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -902,7 +902,12 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Update a flow type to pctype mapping item on a port\n\n"
 
 			"port config (port_id) pctype (pctype_id) hash_inset|"
-			"fdir_inset|fdir_flx_inset get|set|clear field\n"
+			"fdir_inset|fdir_flx_inset set field\n"
+			" (field_idx) mask (mask_val)\n"
+			"    Set RSS|FDIR|FDIR_FLX input set for some pctype\n\n"
+
+			"port config (port_id) pctype (pctype_id) hash_inset|"
+			"fdir_inset|fdir_flx_inset get|clear field\n"
 			" (field_idx)\n"
 			"    Configure RSS|FDIR|FDIR_FLX input set for some pctype\n\n"
 
@@ -15707,9 +15712,11 @@ struct cmd_cfg_input_set_result {
 	cmdline_fixed_string_t pctype;
 	uint8_t pctype_id;
 	cmdline_fixed_string_t inset_type;
-	cmdline_fixed_string_t opt;
+	cmdline_fixed_string_t set;
 	cmdline_fixed_string_t field;
 	uint8_t field_idx;
+	cmdline_fixed_string_t mask;
+	uint16_t mask_val;
 };
 
 static void
@@ -15722,6 +15729,7 @@ cmd_cfg_input_set_parsed(
 	struct cmd_cfg_input_set_result *res = parsed_result;
 	enum rte_pmd_i40e_inset_type inset_type = INSET_NONE;
 	struct rte_pmd_i40e_inset inset;
+	uint8_t i;
 #endif
 	int ret = -ENOTSUP;
 
@@ -15744,25 +15752,26 @@ cmd_cfg_input_set_parsed(
 		return;
 	}
 
-	if (!strcmp(res->opt, "get")) {
-		ret = rte_pmd_i40e_inset_field_get(inset.inset,
+	ret = rte_pmd_i40e_inset_field_set(&inset.inset,
 						   res->field_idx);
-		if (ret)
-			printf("Field index %d is enabled.\n", res->field_idx);
-		else
-			printf("Field index %d is disabled.\n", res->field_idx);
-		return;
-	} else if (!strcmp(res->opt, "set"))
-		ret = rte_pmd_i40e_inset_field_set(&inset.inset,
-						   res->field_idx);
-	else if (!strcmp(res->opt, "clear"))
-		ret = rte_pmd_i40e_inset_field_clear(&inset.inset,
-						     res->field_idx);
 	if (ret) {
 		printf("Failed to configure input set field.\n");
 		return;
 	}
 
+	for (i = 0; i < 2; i++) {
+		if (!inset.mask[i].mask ||
+		    inset.mask[i].field_idx == res->field_idx) {
+			inset.mask[i].field_idx = res->field_idx;
+			inset.mask[i].mask = res->mask_val;
+			break;
+		}
+	}
+	if (i == 2) {
+		printf("exceed maximal number of bitmasks.\n");
+		return;
+	}
+
 	ret = rte_pmd_i40e_inset_set(res->port_id, res->pctype_id,
 				     &inset, inset_type);
 	if (ret) {
@@ -15794,21 +15803,28 @@ cmdline_parse_token_string_t cmd_cfg_input_set_inset_type =
 	TOKEN_STRING_INITIALIZER(struct cmd_cfg_input_set_result,
 				 inset_type,
 				 "hash_inset#fdir_inset#fdir_flx_inset");
-cmdline_parse_token_string_t cmd_cfg_input_set_opt =
+cmdline_parse_token_string_t cmd_cfg_input_set_set =
 	TOKEN_STRING_INITIALIZER(struct cmd_cfg_input_set_result,
-				 opt, "get#set#clear");
+				 set, "set");
 cmdline_parse_token_string_t cmd_cfg_input_set_field =
 	TOKEN_STRING_INITIALIZER(struct cmd_cfg_input_set_result,
 				 field, "field");
 cmdline_parse_token_num_t cmd_cfg_input_set_field_idx =
 	TOKEN_NUM_INITIALIZER(struct cmd_cfg_input_set_result,
 			      field_idx, UINT8);
+cmdline_parse_token_string_t cmd_cfg_input_set_mask =
+	TOKEN_STRING_INITIALIZER(struct cmd_cfg_input_set_result,
+				 mask, "mask");
+cmdline_parse_token_num_t cmd_cfg_input_set_mask_val =
+	TOKEN_NUM_INITIALIZER(struct cmd_cfg_input_set_result,
+			      mask_val, UINT16);
 
 cmdline_parse_inst_t cmd_cfg_input_set = {
 	.f = cmd_cfg_input_set_parsed,
 	.data = NULL,
 	.help_str = "port config <port_id> pctype <pctype_id> hash_inset|"
-		    "fdir_inset|fdir_flx_inset get|set|clear field <field_idx>",
+		    "fdir_inset|fdir_flx_inset set field <field_idx> "
+		    "mask <mask_val>",
 	.tokens = {
 		(void *)&cmd_cfg_input_set_port,
 		(void *)&cmd_cfg_input_set_cfg,
@@ -15816,9 +15832,153 @@ cmdline_parse_inst_t cmd_cfg_input_set = {
 		(void *)&cmd_cfg_input_set_pctype,
 		(void *)&cmd_cfg_input_set_pctype_id,
 		(void *)&cmd_cfg_input_set_inset_type,
-		(void *)&cmd_cfg_input_set_opt,
+		(void *)&cmd_cfg_input_set_set,
 		(void *)&cmd_cfg_input_set_field,
 		(void *)&cmd_cfg_input_set_field_idx,
+		(void *)&cmd_cfg_input_set_mask,
+		(void *)&cmd_cfg_input_set_mask_val,
+		NULL,
+	},
+};
+
+/* Get field info or clear some field */
+struct cmd_get_clr_input_set_result {
+	cmdline_fixed_string_t port;
+	cmdline_fixed_string_t cfg;
+	portid_t port_id;
+	cmdline_fixed_string_t pctype;
+	uint8_t pctype_id;
+	cmdline_fixed_string_t inset_type;
+	cmdline_fixed_string_t opt;
+	cmdline_fixed_string_t field;
+	uint8_t field_idx;
+};
+
+static void
+cmd_get_clr_input_set_parsed(
+	void *parsed_result,
+	__attribute__((unused)) struct cmdline *cl,
+	__attribute__((unused)) void *data)
+{
+	struct cmd_get_clr_input_set_result *res = parsed_result;
+#ifdef RTE_LIBRTE_I40E_PMD
+	enum rte_pmd_i40e_inset_type inset_type = INSET_NONE;
+	struct rte_pmd_i40e_inset inset;
+	uint8_t i;
+#endif
+	int ret = -ENOTSUP;
+
+	if (res->port_id > nb_ports) {
+		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
+		return;
+	}
+
+	if (!all_ports_stopped()) {
+		printf("Please stop all ports first\n");
+		return;
+	}
+
+#ifdef RTE_LIBRTE_I40E_PMD
+	if (!strcmp(res->inset_type, "hash_inset"))
+		inset_type = INSET_HASH;
+	else if (!strcmp(res->inset_type, "fdir_inset"))
+		inset_type = INSET_FDIR;
+	else if (!strcmp(res->inset_type, "fdir_flx_inset"))
+		inset_type = INSET_FDIR_FLX;
+	ret = rte_pmd_i40e_inset_get(res->port_id, res->pctype_id,
+				     &inset, inset_type);
+	if (ret) {
+		printf("Failed to get input set.\n");
+		return;
+	}
+
+	if (!strcmp(res->opt, "get")) {
+		ret = rte_pmd_i40e_inset_field_get(inset.inset,
+						   res->field_idx);
+		if (ret) {
+			printf("Field index %d is enabled.\n", res->field_idx);
+			for (i = 0; i < 2; i++) {
+				if (inset.mask[i].field_idx == res->field_idx) {
+					printf("Mask is: 0x%x\n",
+					       inset.mask[i].mask);
+					break;
+				}
+			}
+		} else {
+			printf("Field index %d is disabled.\n", res->field_idx);
+		}
+		return;
+	} else if (!strcmp(res->opt, "clear")) {
+		ret = rte_pmd_i40e_inset_field_clear(&inset.inset,
+						     res->field_idx);
+		if (ret) {
+			printf("Failed to clear input set field.\n");
+			return;
+		}
+		for (i = 0; i < 2; i++) {
+			if (inset.mask[i].field_idx == res->field_idx) {
+				inset.mask[i].mask = 0;
+				break;
+			}
+		}
+	}
+
+	ret = rte_pmd_i40e_inset_set(res->port_id, res->pctype_id,
+				     &inset, inset_type);
+	if (ret) {
+		printf("Failed to configure input set.\n");
+		return;
+	}
+#endif
+
+	if (ret == -ENOTSUP)
+		printf("Function not supported\n");
+}
+
+cmdline_parse_token_string_t cmd_get_clr_input_set_port =
+	TOKEN_STRING_INITIALIZER(struct cmd_get_clr_input_set_result,
+				 port, "port");
+cmdline_parse_token_string_t cmd_get_clr_input_set_cfg =
+	TOKEN_STRING_INITIALIZER(struct cmd_get_clr_input_set_result,
+				 cfg, "config");
+cmdline_parse_token_num_t cmd_get_clr_input_set_port_id =
+	TOKEN_NUM_INITIALIZER(struct cmd_get_clr_input_set_result,
+			      port_id, UINT16);
+cmdline_parse_token_string_t cmd_get_clr_input_set_pctype =
+	TOKEN_STRING_INITIALIZER(struct cmd_get_clr_input_set_result,
+				 pctype, "pctype");
+cmdline_parse_token_num_t cmd_get_clr_input_set_pctype_id =
+	TOKEN_NUM_INITIALIZER(struct cmd_get_clr_input_set_result,
+			      pctype_id, UINT8);
+cmdline_parse_token_string_t cmd_get_clr_input_set_inset_type =
+	TOKEN_STRING_INITIALIZER(struct cmd_get_clr_input_set_result,
+				 inset_type,
+				 "hash_inset#fdir_inset#fdir_flx_inset");
+cmdline_parse_token_string_t cmd_get_clr_input_set_opt =
+	TOKEN_STRING_INITIALIZER(struct cmd_get_clr_input_set_result,
+				 opt, "get#clear");
+cmdline_parse_token_string_t cmd_get_clr_input_set_field =
+	TOKEN_STRING_INITIALIZER(struct cmd_get_clr_input_set_result,
+				 field, "field");
+cmdline_parse_token_num_t cmd_get_clr_input_set_field_idx =
+	TOKEN_NUM_INITIALIZER(struct cmd_get_clr_input_set_result,
+			      field_idx, UINT8);
+
+cmdline_parse_inst_t cmd_get_clr_input_set = {
+	.f = cmd_get_clr_input_set_parsed,
+	.data = NULL,
+	.help_str = "port config <port_id> pctype <pctype_id> hash_inset|"
+		    "fdir_inset|fdir_flx_inset get|clear field <field_idx>",
+	.tokens = {
+		(void *)&cmd_get_clr_input_set_port,
+		(void *)&cmd_get_clr_input_set_cfg,
+		(void *)&cmd_get_clr_input_set_port_id,
+		(void *)&cmd_get_clr_input_set_pctype,
+		(void *)&cmd_get_clr_input_set_pctype_id,
+		(void *)&cmd_get_clr_input_set_inset_type,
+		(void *)&cmd_get_clr_input_set_opt,
+		(void *)&cmd_get_clr_input_set_field,
+		(void *)&cmd_get_clr_input_set_field_idx,
 		NULL,
 	},
 };
@@ -17819,6 +17979,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_ddp_get_list,
 	(cmdline_parse_inst_t *)&cmd_ddp_get_info,
 	(cmdline_parse_inst_t *)&cmd_cfg_input_set,
+	(cmdline_parse_inst_t *)&cmd_get_clr_input_set,
 	(cmdline_parse_inst_t *)&cmd_clear_input_set,
 	(cmdline_parse_inst_t *)&cmd_show_vf_stats,
 	(cmdline_parse_inst_t *)&cmd_clear_vf_stats,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index dde205a..670f81c 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -1989,7 +1989,13 @@ port config input set
 Config RSS/FDIR/FDIR flexible payload input set for some pctype::
    testpmd> port config (port_id) pctype (pctype_id) \
             (hash_inset|fdir_inset|fdir_flx_inset) \
-	    (get|set|clear) field (field_idx)
+	    set field (field_idx) mask (mask_val)
+
+Get some field info or clear some field of RSS/FDIR/FDIR flexible
+payload input set for some pctype::
+   testpmd> port config (port_id) pctype (pctype_id) \
+               (hash_inset|fdir_inset|fdir_flx_inset) \
+	       (get|clear) field (field_idx)
 
 Clear RSS/FDIR/FDIR flexible payload input set for some pctype::
    testpmd> port config (port_id) pctype (pctype_id) \
-- 
2.5.5

Re: [dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and FDIR

[Thomas Monjalon]

06/08/2018 07:45, Beilei Xing:
> This patch adds bitmask support for RSS, FDIR
> and FDIR flexible payload.
> 
> Signed-off-by: Beilei Xing <beilei.xing@intel.com>

Flow director API is deprecated for almost 2 years:
	http://git.dpdk.org/dpdk/commit/?id=7fdcde6
We asked several times to stop using it in i40e.
It has not been listened.
We have even accepted a workaround in flow_filtering example for i40e: 
	http://git.dpdk.org/dpdk/commit/?id=9a93446
We need to stop and move all to a common rte_flow API.

As a consequence, I will be against this patch.

Re: [dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and FDIR

[Xing, Beilei]

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, August 6, 2018 5:31 PM
> To: Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and
> FDIR
> 
> 06/08/2018 07:45, Beilei Xing:
> > This patch adds bitmask support for RSS, FDIR and FDIR flexible
> > payload.
> >
> > Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> 
> Flow director API is deprecated for almost 2 years:
> 	http://git.dpdk.org/dpdk/commit/?id=7fdcde6
> We asked several times to stop using it in i40e.
> It has not been listened.
> We have even accepted a workaround in flow_filtering example for i40e:
> 	http://git.dpdk.org/dpdk/commit/?id=9a93446
> We need to stop and move all to a common rte_flow API.
> 
> As a consequence, I will be against this patch.
> 

It's not for deprecated flow director API, it's mainly for i40e private API rte_pmd_i40e_inset_set.
This patch is an extend of:
	https://patches.dpdk.org/patch/33031/

Thanks,
Beilei

Re: [dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and FDIR

[Jerin Jacob]

-----Original Message-----
> Date: Mon, 6 Aug 2018 13:45:35 +0800
> From: Beilei Xing <beilei.xing@intel.com>
> To: wenzhuo.lu@intel.com, jingjing.wu@intel.com
> CC: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and FDIR
> X-Mailer: git-send-email 2.5.5
> 
> 
> This patch adds bitmask support for RSS, FDIR
> and FDIR flexible payload.
> 
> Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> ---
>  app/test-pmd/cmdline.c                      | 199 +++++++++++++++++++++++++---
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |   8 +-
>  2 files changed, 187 insertions(+), 20 deletions(-)
> 
> +#ifdef RTE_LIBRTE_I40E_PMD


How about moving all driver specific tests to driver/net/<driver>/?
I think, it wont scale if everyone starts adding their own
driver specific tests to testpmd.

How about creating 
#A generic string parser in testpmd?  something like,
port config (port_id) <driver name> ....
# and do driver_name to port_id lookup
# and introduce "test" or "self_test" ops in ethdev
# and call "test" op on the selected port_id


> +       if (!strcmp(res->inset_type, "hash_inset"))
> +               inset_type = INSET_HASH;
> +       else if (!strcmp(res->inset_type, "fdir_inset"))
> +               inset_type = INSET_FDIR;
> +       else if (!strcmp(res->inset_type, "fdir_flx_inset"))
> +               inset_type = INSET_FDIR_FLX;
> +       ret = rte_pmd_i40e_inset_get(res->port_id, res->pctype_id,
> +                                    &inset, inset_type);
> +       if (ret) {
> +               printf("Failed to get input set.\n");
> +               return;
> +       }
> +
> +       if (!strcmp(res->opt, "get")) {
> +               ret = rte_pmd_i40e_inset_field_get(inset.inset,
> +                                                  res->field_idx);
> +               if (ret) {
> +                       printf("Field index %d is enabled.\n", res->field_idx);
> +                       for (i = 0; i < 2; i++) {
> +                               if (inset.mask[i].field_idx == res->field_idx) {
> +                                       printf("Mask is: 0x%x\n",
> +                                              inset.mask[i].mask);
> +                                       break;
> +                               }
> +                       }
> +               } else {
> +                       printf("Field index %d is disabled.\n", res->field_idx);
> +               }
> +               return;
> +       } else if (!strcmp(res->opt, "clear")) {
> +               ret = rte_pmd_i40e_inset_field_clear(&inset.inset,
> +                                                    res->field_idx);
> +               if (ret) {
> +                       printf("Failed to clear input set field.\n");
> +                       return;
> +               }
> +               for (i = 0; i < 2; i++) {
> +                       if (inset.mask[i].field_idx == res->field_idx) {
> +                               inset.mask[i].mask = 0;
> +                               break;
> +                       }
> +               }
> +       }
> +
> +       ret = rte_pmd_i40e_inset_set(res->port_id, res->pctype_id,
> +                                    &inset, inset_type);
> +       if (ret) {
> +               printf("Failed to configure input set.\n");
> +               return;
> +       }
> +#endif
> +
> +       if (ret == -ENOTSUP)
> +               printf("Function not supported\n");
> +}

Re: [dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and FDIR

[Xing, Beilei]

Hi Jacob,

> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Friday, August 10, 2018 11:45 AM
> To: Xing, Beilei <beilei.xing@intel.com>
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and
> FDIR
> 
> -----Original Message-----
> > Date: Mon, 6 Aug 2018 13:45:35 +0800
> > From: Beilei Xing <beilei.xing@intel.com>
> > To: wenzhuo.lu@intel.com, jingjing.wu@intel.com
> > CC: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and
> > FDIR
> > X-Mailer: git-send-email 2.5.5
> >
> >
> > This patch adds bitmask support for RSS, FDIR and FDIR flexible
> > payload.
> >
> > Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> > ---
> >  app/test-pmd/cmdline.c                      | 199 +++++++++++++++++++++++++---
> >  doc/guides/testpmd_app_ug/testpmd_funcs.rst |   8 +-
> >  2 files changed, 187 insertions(+), 20 deletions(-)
> >
> > +#ifdef RTE_LIBRTE_I40E_PMD
> 
> 
> How about moving all driver specific tests to driver/net/<driver>/?
> I think, it wont scale if everyone starts adding their own driver specific tests
> to testpmd.
> 
> How about creating
> #A generic string parser in testpmd?  something like, port config (port_id)
> <driver name> ....
> # and do driver_name to port_id lookup
> # and introduce "test" or "self_test" ops in ethdev # and call "test" op on the
> selected port_id
> 

Thanks for the comment, but if so, then will remove and redesign all existed commands. I think it can be discussed in another thread.

Thanks,
Beilei

> 
> > +       if (!strcmp(res->inset_type, "hash_inset"))
> > +               inset_type = INSET_HASH;
> > +       else if (!strcmp(res->inset_type, "fdir_inset"))
> > +               inset_type = INSET_FDIR;
> > +       else if (!strcmp(res->inset_type, "fdir_flx_inset"))
> > +               inset_type = INSET_FDIR_FLX;
> > +       ret = rte_pmd_i40e_inset_get(res->port_id, res->pctype_id,
> > +                                    &inset, inset_type);
> > +       if (ret) {
> > +               printf("Failed to get input set.\n");
> > +               return;
> > +       }
> > +
> > +       if (!strcmp(res->opt, "get")) {
> > +               ret = rte_pmd_i40e_inset_field_get(inset.inset,
> > +                                                  res->field_idx);
> > +               if (ret) {
> > +                       printf("Field index %d is enabled.\n", res->field_idx);
> > +                       for (i = 0; i < 2; i++) {
> > +                               if (inset.mask[i].field_idx == res->field_idx) {
> > +                                       printf("Mask is: 0x%x\n",
> > +                                              inset.mask[i].mask);
> > +                                       break;
> > +                               }
> > +                       }
> > +               } else {
> > +                       printf("Field index %d is disabled.\n", res->field_idx);
> > +               }
> > +               return;
> > +       } else if (!strcmp(res->opt, "clear")) {
> > +               ret = rte_pmd_i40e_inset_field_clear(&inset.inset,
> > +                                                    res->field_idx);
> > +               if (ret) {
> > +                       printf("Failed to clear input set field.\n");
> > +                       return;
> > +               }
> > +               for (i = 0; i < 2; i++) {
> > +                       if (inset.mask[i].field_idx == res->field_idx) {
> > +                               inset.mask[i].mask = 0;
> > +                               break;
> > +                       }
> > +               }
> > +       }
> > +
> > +       ret = rte_pmd_i40e_inset_set(res->port_id, res->pctype_id,
> > +                                    &inset, inset_type);
> > +       if (ret) {
> > +               printf("Failed to configure input set.\n");
> > +               return;
> > +       }
> > +#endif
> > +
> > +       if (ret == -ENOTSUP)
> > +               printf("Function not supported\n");
> > +}

Re: [dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and FDIR

[Thomas Monjalon]

10/08/2018 04:31, Xing, Beilei:
> Hi Thomas,
> 
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 06/08/2018 07:45, Beilei Xing:
> > > This patch adds bitmask support for RSS, FDIR and FDIR flexible
> > > payload.
> > >
> > > Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> > 
> > Flow director API is deprecated for almost 2 years:
> > 	http://git.dpdk.org/dpdk/commit/?id=7fdcde6
> > We asked several times to stop using it in i40e.
> > It has not been listened.
> > We have even accepted a workaround in flow_filtering example for i40e:
> > 	http://git.dpdk.org/dpdk/commit/?id=9a93446
> > We need to stop and move all to a common rte_flow API.
> > 
> > As a consequence, I will be against this patch.
> > 
> 
> It's not for deprecated flow director API, it's mainly for i40e private API rte_pmd_i40e_inset_set.
> This patch is an extend of:
> 	https://patches.dpdk.org/patch/33031/

It is a private API for flow director.
I feel you play with me, it's worst.

Re: [dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and FDIR

[Jerin Jacob]

-----Original Message-----
> Date: Fri, 10 Aug 2018 07:37:21 +0000
> From: "Xing, Beilei" <beilei.xing@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: "Lu, Wenzhuo" <wenzhuo.lu@intel.com>, "Wu, Jingjing"
>  <jingjing.wu@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and
>  FDIR
> 
> 
> Hi Jacob,

Hi Xing,

> 
> > -----Original Message-----
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Friday, August 10, 2018 11:45 AM
> > To: Xing, Beilei <beilei.xing@intel.com>
> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> > <jingjing.wu@intel.com>; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and
> > FDIR
> >
> > -----Original Message-----
> > > Date: Mon, 6 Aug 2018 13:45:35 +0800
> > > From: Beilei Xing <beilei.xing@intel.com>
> > > To: wenzhuo.lu@intel.com, jingjing.wu@intel.com
> > > CC: dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and
> > > FDIR
> > > X-Mailer: git-send-email 2.5.5
> > >
> > >
> > > This patch adds bitmask support for RSS, FDIR and FDIR flexible
> > > payload.
> > >
> > > Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> > > ---
> > >  app/test-pmd/cmdline.c                      | 199 +++++++++++++++++++++++++---
> > >  doc/guides/testpmd_app_ug/testpmd_funcs.rst |   8 +-
> > >  2 files changed, 187 insertions(+), 20 deletions(-)
> > >
> > > +#ifdef RTE_LIBRTE_I40E_PMD
> >
> >
> > How about moving all driver specific tests to driver/net/<driver>/?
> > I think, it wont scale if everyone starts adding their own driver specific tests
> > to testpmd.
> >
> > How about creating
> > #A generic string parser in testpmd?  something like, port config (port_id)
> > <driver name> ....
> > # and do driver_name to port_id lookup
> > # and introduce "test" or "self_test" ops in ethdev # and call "test" op on the
> > selected port_id
> >
> 
> Thanks for the comment, but if so, then will remove and redesign all existed commands. I think it can be discussed in another thread.

The new commands can fall in generic scheme. Existing ones can moved
after deprecation notice procedure etc.

Otherwise hook under rte_flow API, so we don't need the private API.

Re: [dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and FDIR

[Xing, Beilei]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, August 10, 2018 4:58 PM
> To: Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: support bitmask for RSS and
> FDIR
> 
> 10/08/2018 04:31, Xing, Beilei:
> > Hi Thomas,
> >
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > 06/08/2018 07:45, Beilei Xing:
> > > > This patch adds bitmask support for RSS, FDIR and FDIR flexible
> > > > payload.
> > > >
> > > > Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> > >
> > > Flow director API is deprecated for almost 2 years:
> > > 	http://git.dpdk.org/dpdk/commit/?id=7fdcde6
> > > We asked several times to stop using it in i40e.
> > > It has not been listened.
> > > We have even accepted a workaround in flow_filtering example for i40e:
> > > 	http://git.dpdk.org/dpdk/commit/?id=9a93446
> > > We need to stop and move all to a common rte_flow API.
> > >
> > > As a consequence, I will be against this patch.
> > >
> >
> > It's not for deprecated flow director API, it's mainly for i40e private API
> rte_pmd_i40e_inset_set.
> > This patch is an extend of:
> > 	https://patches.dpdk.org/patch/33031/
> 
> It is a private API for flow director.
> I feel you play with me, it's worst.
> 

This API is designed when i40e supports DDP (Dynamic Device Personalization), which can support any protocol customer wants. Supported protocols in rte_flow are limited, if HW supports a new protocol, we should also change rte_flow and driver correspondingly each time, it's not dynamic then. So the API is designed, although it works for FDIR, but it's not designed for FDIR especially but for input set, sorry the title annoyed you, it's my fault. 
And you are right the private API is not a good way after all, I will abandon the patch first and investigate if there's other way. Thanks for the comments.