[v2,5/5] app/testpmd: support query RSS config in flow query

Message ID 20200615021858.13985-6-chenxux.di@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series re-implement legacy filter functions by private API |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK

Commit Message

Chenxu Di June 15, 2020, 2:18 a.m. UTC
  This patch support RSS action in flow query.
It can display the RSS configuration of the specified rule.

Signed-off-by: Chenxu Di <chenxux.di@intel.com>
---
 app/test-pmd/config.c | 55 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)
  

Comments

Guo, Jia June 30, 2020, 6:37 a.m. UTC | #1
hi, chenxu

On 6/15/2020 10:18 AM, Chenxu Di wrote:
> This patch support RSS action in flow query.
> It can display the RSS configuration of the specified rule.


Could you add some example command here for better know the usage and 
the details.


> Signed-off-by: Chenxu Di <chenxux.di@intel.com>
> ---
>   app/test-pmd/config.c | 55 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 55 insertions(+)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index f519246c7..7e3cccf9a 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1378,6 +1378,56 @@ port_flow_complain(struct rte_flow_error *error)
>   	return -err;
>   }
>   
> +static void
> +rss_config_display(struct rte_flow_action_rss *rss_conf)
> +{
> +	uint8_t i;
> +
> +	if (rss_conf == NULL) {
> +		printf("Invalid rule\n");
> +		return;
> +	}
> +
> +	printf("RSS:\n"
> +	       " queues:");
> +	if (rss_conf->queue_num == 0)
> +		printf(" none");
> +	for (i = 0; i < rss_conf->queue_num; i++)
> +		printf(" %d", rss_conf->queue[i]);
> +
> +	printf("\n function:");
> +	switch (rss_conf->func) {
> +	case RTE_ETH_HASH_FUNCTION_DEFAULT:
> +		printf(" default\n");
> +		break;
> +	case RTE_ETH_HASH_FUNCTION_TOEPLITZ:
> +		printf(" toeplitz\n");
> +		break;
> +	case RTE_ETH_HASH_FUNCTION_SIMPLE_XOR:
> +		printf(" simple_xor\n");
> +		break;
> +	case RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ:
> +		printf(" symmetric_toeplitz\n");
> +		break;
> +	default:
> +		printf(" Unknown function\n");
> +		return;
> +	}
> +
> +	printf(" types:\n");
> +	if (rss_conf->types == 0) {
> +		printf("  none\n");
> +		return;
> +	}
> +	for (i = 0; rss_type_table[i].str; i++) {
> +		if ((rss_conf->types &
> +		    rss_type_table[i].rss_type) ==
> +		    rss_type_table[i].rss_type &&
> +		    rss_type_table[i].rss_type != 0)
> +			printf("  %s\n", rss_type_table[i].str);
> +	}
> +}
> +
>   /** Validate flow rule. */
>   int
>   port_flow_validate(portid_t port_id,
> @@ -1564,6 +1614,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
>   	const char *name;
>   	union {
>   		struct rte_flow_query_count count;
> +		struct rte_flow_action_rss rss_conf;
>   	} query;
>   	int ret;
>   
> @@ -1585,6 +1636,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
>   		return port_flow_complain(&error);
>   	switch (action->type) {
>   	case RTE_FLOW_ACTION_TYPE_COUNT:
> +	case RTE_FLOW_ACTION_TYPE_RSS:
>   		break;
>   	default:
>   		printf("Cannot query action type %d (%s)\n",
> @@ -1609,6 +1661,9 @@ port_flow_query(portid_t port_id, uint32_t rule,
>   		       query.count.hits,
>   		       query.count.bytes);
>   		break;
> +	case RTE_FLOW_ACTION_TYPE_RSS:
> +		rss_config_display(&query.rss_conf);
> +		break;
>   	default:
>   		printf("Cannot display result for action type %d (%s)\n",
>   		       action->type, name);
  
Qiming Yang June 30, 2020, 10:40 a.m. UTC | #2
> -----Original Message-----
> From: Di, ChenxuX <chenxux.di@intel.com>
> Sent: Monday, June 15, 2020 10:19
> To: dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Di, ChenxuX <chenxux.di@intel.com>
> Subject: [PATCH v2 5/5] app/testpmd: support query RSS config in flow query
> 
> This patch support RSS action in flow query.
> It can display the RSS configuration of the specified rule.
> 
> Signed-off-by: Chenxu Di <chenxux.di@intel.com>
> ---
>  app/test-pmd/config.c | 55
> +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> f519246c7..7e3cccf9a 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1378,6 +1378,56 @@ port_flow_complain(struct rte_flow_error *error)
>  	return -err;
>  }
> 
> +static void
> +rss_config_display(struct rte_flow_action_rss *rss_conf) {
> +	uint8_t i;
> +
> +	if (rss_conf == NULL) {
> +		printf("Invalid rule\n");
> +		return;
> +	}
> +
> +	printf("RSS:\n"
> +	       " queues:");

Two lines needed? And why don't you add the space in the end of this printf?

> +	if (rss_conf->queue_num == 0)

If(!rss_conf->queue_num)

> +		printf(" none");
> +	for (i = 0; i < rss_conf->queue_num; i++)
> +		printf(" %d", rss_conf->queue[i]);
> +
> +	printf("\n function:");
> +	switch (rss_conf->func) {
> +	case RTE_ETH_HASH_FUNCTION_DEFAULT:
> +		printf(" default\n");

No needed space

> +		break;
> +	case RTE_ETH_HASH_FUNCTION_TOEPLITZ:
> +		printf(" toeplitz\n");

Same

> +		break;
> +	case RTE_ETH_HASH_FUNCTION_SIMPLE_XOR:
> +		printf(" simple_xor\n");
...

> +		break;
> +	case RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ:
> +		printf(" symmetric_toeplitz\n");
..

> +		break;
> +	default:
> +		printf(" Unknown function\n");
...

> +		return;
> +	}
> +
> +	printf(" types:\n");

Same...

> +	if (rss_conf->types == 0) {
> +		printf("  none\n");

Same...

> +		return;
> +	}
> +	for (i = 0; rss_type_table[i].str; i++) {
> +		if ((rss_conf->types &
> +		    rss_type_table[i].rss_type) ==
> +		    rss_type_table[i].rss_type &&
> +		    rss_type_table[i].rss_type != 0)
> +			printf("  %s\n", rss_type_table[i].str);

All the same..

> +	}
> +}
> +
>  /** Validate flow rule. */
>  int
>  port_flow_validate(portid_t port_id,
> @@ -1564,6 +1614,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
>  	const char *name;
>  	union {
>  		struct rte_flow_query_count count;
> +		struct rte_flow_action_rss rss_conf;
>  	} query;
>  	int ret;
> 
> @@ -1585,6 +1636,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
>  		return port_flow_complain(&error);
>  	switch (action->type) {
>  	case RTE_FLOW_ACTION_TYPE_COUNT:
> +	case RTE_FLOW_ACTION_TYPE_RSS:
>  		break;
>  	default:
>  		printf("Cannot query action type %d (%s)\n", @@ -1609,6
> +1661,9 @@ port_flow_query(portid_t port_id, uint32_t rule,
>  		       query.count.hits,
>  		       query.count.bytes);
>  		break;
> +	case RTE_FLOW_ACTION_TYPE_RSS:
> +		rss_config_display(&query.rss_conf);
> +		break;
>  	default:
>  		printf("Cannot display result for action type %d (%s)\n",
>  		       action->type, name);
> --
> 2.17.1
  
Chenxu Di July 1, 2020, 1:25 a.m. UTC | #3
Hi,

For the current code about flow query <portid> <ruleid> count 
It is printf("%s:\n"
	" hits_set: %u\n"
	" bytes_set: %u\n"
	" hits: %" PRIu64 "\n"
	" bytes: %" PRIu64 "\n",
	name,
	query.count.hits_set,
	query.count.bytes_set,
	query.count.hits,
	query.count.bytes);
and it will display the info like:
COUNT:
 hits_set: xxx
 bytes_set : xxx
 hits: xxx
 bytes: xxx

so for the code style and info style like it, I add some space in the log to make sure the log like
RSS:
 Queues: none|0|1|...
 Function: default| Toeplitz| simple_xor| symmetric_toeplitz| Unknown function
 Types:
  none
  Ipv4-tcp
  Ipv4-udp
  
For the reason the types may be many types, and if the line in testpmd log is too long.
It may cause error, so the types info is displayed per line and with one more space before.

> -----Original Message-----
> From: Yang, Qiming
> Sent: Tuesday, June 30, 2020 6:41 PM
> To: Di, ChenxuX <chenxux.di@intel.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>
> Subject: RE: [PATCH v2 5/5] app/testpmd: support query RSS config in flow query
> 
> 
> 
> > -----Original Message-----
> > From: Di, ChenxuX <chenxux.di@intel.com>
> > Sent: Monday, June 15, 2020 10:19
> > To: dev@dpdk.org
> > Cc: Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming
> > <qiming.yang@intel.com>; Di, ChenxuX <chenxux.di@intel.com>
> > Subject: [PATCH v2 5/5] app/testpmd: support query RSS config in flow
> > query
> >
> > This patch support RSS action in flow query.
> > It can display the RSS configuration of the specified rule.
> >
> > Signed-off-by: Chenxu Di <chenxux.di@intel.com>
> > ---
> >  app/test-pmd/config.c | 55
> > +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 55 insertions(+)
> >
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> > f519246c7..7e3cccf9a 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -1378,6 +1378,56 @@ port_flow_complain(struct rte_flow_error
> > *error)  return -err;  }
> >
> > +static void
> > +rss_config_display(struct rte_flow_action_rss *rss_conf) { uint8_t i;
> > +
> > +if (rss_conf == NULL) {
> > +printf("Invalid rule\n");
> > +return;
> > +}
> > +
> > +printf("RSS:\n"
> > +       " queues:");
> 
> Two lines needed? And why don't you add the space in the end of this printf?
> 
> > +if (rss_conf->queue_num == 0)
> 
> If(!rss_conf->queue_num)
> 
> > +printf(" none");
> > +for (i = 0; i < rss_conf->queue_num; i++) printf(" %d",
> > +rss_conf->queue[i]);
> > +
> > +printf("\n function:");
> > +switch (rss_conf->func) {
> > +case RTE_ETH_HASH_FUNCTION_DEFAULT:
> > +printf(" default\n");
> 
> No needed space
> 
> > +break;
> > +case RTE_ETH_HASH_FUNCTION_TOEPLITZ:
> > +printf(" toeplitz\n");
> 
> Same
> 
> > +break;
> > +case RTE_ETH_HASH_FUNCTION_SIMPLE_XOR:
> > +printf(" simple_xor\n");
> ...
> 
> > +break;
> > +case RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ:
> > +printf(" symmetric_toeplitz\n");
> ..
> 
> > +break;
> > +default:
> > +printf(" Unknown function\n");
> ...
> 
> > +return;
> > +}
> > +
> > +printf(" types:\n");
> 
> Same...
> 
> > +if (rss_conf->types == 0) {
> > +printf("  none\n");
> 
> Same...
> 
> > +return;
> > +}
> > +for (i = 0; rss_type_table[i].str; i++) { if ((rss_conf->types &
> > +    rss_type_table[i].rss_type) ==
> > +    rss_type_table[i].rss_type &&
> > +    rss_type_table[i].rss_type != 0)
> > +printf("  %s\n", rss_type_table[i].str);
> 
> All the same..
> 
> > +}
> > +}
> > +
> >  /** Validate flow rule. */
> >  int
> >  port_flow_validate(portid_t port_id,
> > @@ -1564,6 +1614,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
> > const char *name;  union {  struct rte_flow_query_count count;
> > +struct rte_flow_action_rss rss_conf;
> >  } query;
> >  int ret;
> >
> > @@ -1585,6 +1636,7 @@ port_flow_query(portid_t port_id, uint32_t rule,
> > return port_flow_complain(&error);  switch (action->type) {  case
> > RTE_FLOW_ACTION_TYPE_COUNT:
> > +case RTE_FLOW_ACTION_TYPE_RSS:
> >  break;
> >  default:
> >  printf("Cannot query action type %d (%s)\n", @@ -1609,6
> > +1661,9 @@ port_flow_query(portid_t port_id, uint32_t rule,
> >         query.count.hits,
> >         query.count.bytes);
> >  break;
> > +case RTE_FLOW_ACTION_TYPE_RSS:
> > +rss_config_display(&query.rss_conf);
> > +break;
> >  default:
> >  printf("Cannot display result for action type %d (%s)\n",
> >         action->type, name);
> > --
> > 2.17.1
>
  

Patch

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index f519246c7..7e3cccf9a 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1378,6 +1378,56 @@  port_flow_complain(struct rte_flow_error *error)
 	return -err;
 }
 
+static void
+rss_config_display(struct rte_flow_action_rss *rss_conf)
+{
+	uint8_t i;
+
+	if (rss_conf == NULL) {
+		printf("Invalid rule\n");
+		return;
+	}
+
+	printf("RSS:\n"
+	       " queues:");
+	if (rss_conf->queue_num == 0)
+		printf(" none");
+	for (i = 0; i < rss_conf->queue_num; i++)
+		printf(" %d", rss_conf->queue[i]);
+
+	printf("\n function:");
+	switch (rss_conf->func) {
+	case RTE_ETH_HASH_FUNCTION_DEFAULT:
+		printf(" default\n");
+		break;
+	case RTE_ETH_HASH_FUNCTION_TOEPLITZ:
+		printf(" toeplitz\n");
+		break;
+	case RTE_ETH_HASH_FUNCTION_SIMPLE_XOR:
+		printf(" simple_xor\n");
+		break;
+	case RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ:
+		printf(" symmetric_toeplitz\n");
+		break;
+	default:
+		printf(" Unknown function\n");
+		return;
+	}
+
+	printf(" types:\n");
+	if (rss_conf->types == 0) {
+		printf("  none\n");
+		return;
+	}
+	for (i = 0; rss_type_table[i].str; i++) {
+		if ((rss_conf->types &
+		    rss_type_table[i].rss_type) ==
+		    rss_type_table[i].rss_type &&
+		    rss_type_table[i].rss_type != 0)
+			printf("  %s\n", rss_type_table[i].str);
+	}
+}
+
 /** Validate flow rule. */
 int
 port_flow_validate(portid_t port_id,
@@ -1564,6 +1614,7 @@  port_flow_query(portid_t port_id, uint32_t rule,
 	const char *name;
 	union {
 		struct rte_flow_query_count count;
+		struct rte_flow_action_rss rss_conf;
 	} query;
 	int ret;
 
@@ -1585,6 +1636,7 @@  port_flow_query(portid_t port_id, uint32_t rule,
 		return port_flow_complain(&error);
 	switch (action->type) {
 	case RTE_FLOW_ACTION_TYPE_COUNT:
+	case RTE_FLOW_ACTION_TYPE_RSS:
 		break;
 	default:
 		printf("Cannot query action type %d (%s)\n",
@@ -1609,6 +1661,9 @@  port_flow_query(portid_t port_id, uint32_t rule,
 		       query.count.hits,
 		       query.count.bytes);
 		break;
+	case RTE_FLOW_ACTION_TYPE_RSS:
+		rss_config_display(&query.rss_conf);
+		break;
 	default:
 		printf("Cannot display result for action type %d (%s)\n",
 		       action->type, name);