DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v1] net/i40e: argument in RSS action should have queue
@ 2020-11-10 18:04 Kumar Amber
  2020-11-11 10:10 ` [dpdk-dev] [PATCH v2] net/i40e: fix argument in RSS action Kumar Amber
  0 siblings, 1 reply; 8+ messages in thread
From: Kumar Amber @ 2020-11-10 18:04 UTC (permalink / raw)
  To: dev

The driver must check for the queue number
in the RSS action list and if not should
return with a proper error message to user.

Signed-off-by: Kumar Amber <kumar.amber@intel.com>
---
 drivers/net/i40e/i40e_flow.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
index 5bec0c7a84..975340cb1a 100644
--- a/drivers/net/i40e/i40e_flow.c
+++ b/drivers/net/i40e/i40e_flow.c
@@ -4917,6 +4917,17 @@ i40e_flow_parse_rss_action(struct rte_eth_dev *dev,
 	NEXT_ITEM_OF_ACTION(act, actions, index);
 	rss = act->conf;
 
+	/**
+	 * Check if Queue number is specified
+	 * in argument else throw an error.
+	 */
+	if (rss->queue == NULL) {
+		rte_flow_error_set(error, EINVAL,
+			RTE_FLOW_ERROR_TYPE_ACTION,
+			act, "Queue numbers not given.");
+		return -rte_errno;
+	}
+
 	/**
 	 * RSS only supports forwarding,
 	 * check if the first not void action is RSS.
-- 
2.17.1


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

* [dpdk-dev] [PATCH v2] net/i40e: fix argument in RSS action
  2020-11-10 18:04 [dpdk-dev] [PATCH v1] net/i40e: argument in RSS action should have queue Kumar Amber
@ 2020-11-11 10:10 ` Kumar Amber
  2020-11-12 10:41   ` [dpdk-dev] [PATCH v3] " Kumar Amber
                     ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Kumar Amber @ 2020-11-11 10:10 UTC (permalink / raw)
  To: dev; +Cc: wei.zhao1

The driver must check for the queue number
in the RSS action list and if not should
return with a proper error message to user.

Bugzilla ID: 573
Fixes: 9486d60b94b5 ("net/i40e: fix flow RSS queue index check")
Cc: wei.zhao1@intel.com

Signed-off-by: Kumar Amber <kumar.amber@intel.com>
---
 drivers/net/i40e/i40e_flow.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
index 5bec0c7a84..975340cb1a 100644
--- a/drivers/net/i40e/i40e_flow.c
+++ b/drivers/net/i40e/i40e_flow.c
@@ -4917,6 +4917,17 @@ i40e_flow_parse_rss_action(struct rte_eth_dev *dev,
 	NEXT_ITEM_OF_ACTION(act, actions, index);
 	rss = act->conf;
 
+	/**
+	 * Check if Queue number is specified
+	 * in argument else throw an error.
+	 */
+	if (rss->queue == NULL) {
+		rte_flow_error_set(error, EINVAL,
+			RTE_FLOW_ERROR_TYPE_ACTION,
+			act, "Queue numbers not given.");
+		return -rte_errno;
+	}
+
 	/**
 	 * RSS only supports forwarding,
 	 * check if the first not void action is RSS.
-- 
2.17.1


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

* [dpdk-dev] [PATCH v3] net/i40e: fix argument in RSS action
  2020-11-11 10:10 ` [dpdk-dev] [PATCH v2] net/i40e: fix argument in RSS action Kumar Amber
@ 2020-11-12 10:41   ` Kumar Amber
  2020-11-12 10:42   ` [dpdk-dev] [PATCH v4] " Kumar Amber
  2020-12-17  3:20   ` [dpdk-dev] [PATCH v2] " Xing, Beilei
  2 siblings, 0 replies; 8+ messages in thread
From: Kumar Amber @ 2020-11-12 10:41 UTC (permalink / raw)
  To: dev; +Cc: wei.zhao1

The driver must check for the queue number
in the RSS action list and if not should
return with a proper error message to user.

Bugzilla ID: 573
Fixes: 9486d60b94b5 ("net/i40e: fix flow RSS queue index check")
Cc: wei.zhao1@intel.com

Signed-off-by: Kumar Amber <kumar.amber@intel.com>
---
 drivers/net/i40e/i40e_flow.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
index 5bec0c7a84..975340cb1a 100644
--- a/drivers/net/i40e/i40e_flow.c
+++ b/drivers/net/i40e/i40e_flow.c
@@ -4917,6 +4917,17 @@ i40e_flow_parse_rss_action(struct rte_eth_dev *dev,
 	NEXT_ITEM_OF_ACTION(act, actions, index);
 	rss = act->conf;
 
+	/**
+	 * Check if Queue number is specified
+	 * in argument else throw an error.
+	 */
+	if (rss->queue == NULL) {
+		rte_flow_error_set(error, EINVAL,
+			RTE_FLOW_ERROR_TYPE_ACTION,
+			act, "Queue numbers not given.");
+		return -rte_errno;
+	}
+
 	/**
 	 * RSS only supports forwarding,
 	 * check if the first not void action is RSS.
-- 
2.17.1


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

* [dpdk-dev] [PATCH v4] net/i40e: fix argument in RSS action
  2020-11-11 10:10 ` [dpdk-dev] [PATCH v2] net/i40e: fix argument in RSS action Kumar Amber
  2020-11-12 10:41   ` [dpdk-dev] [PATCH v3] " Kumar Amber
@ 2020-11-12 10:42   ` Kumar Amber
  2020-12-24  1:00     ` Zhang, Qi Z
  2020-12-17  3:20   ` [dpdk-dev] [PATCH v2] " Xing, Beilei
  2 siblings, 1 reply; 8+ messages in thread
From: Kumar Amber @ 2020-11-12 10:42 UTC (permalink / raw)
  To: dev; +Cc: wei.zhao1

The driver must check for the queue number
in the RSS action list and if not should
return with a proper error message to user.

Bugzilla ID: 573
Fixes: 9486d60b94b5 ("net/i40e: fix flow RSS queue index check")
Cc: wei.zhao1@intel.com

Signed-off-by: Kumar Amber <kumar.amber@intel.com>
---
 drivers/net/i40e/i40e_flow.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
index 5bec0c7a84..397ed0ae77 100644
--- a/drivers/net/i40e/i40e_flow.c
+++ b/drivers/net/i40e/i40e_flow.c
@@ -4917,6 +4917,18 @@ i40e_flow_parse_rss_action(struct rte_eth_dev *dev,
 	NEXT_ITEM_OF_ACTION(act, actions, index);
 	rss = act->conf;
 
+	/**
+	 * Check if Queue number is specified
+	 * in argument else throw an error.
+	 */
+	if (!rss->queue || !rss->queue_num) {
+		rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ACTION,
+				act,
+			   "no valid queues");
+		return -rte_errno;
+	}
+
 	/**
 	 * RSS only supports forwarding,
 	 * check if the first not void action is RSS.
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH v2] net/i40e: fix argument in RSS action
  2020-11-11 10:10 ` [dpdk-dev] [PATCH v2] net/i40e: fix argument in RSS action Kumar Amber
  2020-11-12 10:41   ` [dpdk-dev] [PATCH v3] " Kumar Amber
  2020-11-12 10:42   ` [dpdk-dev] [PATCH v4] " Kumar Amber
@ 2020-12-17  3:20   ` Xing, Beilei
  2020-12-17  9:06     ` Chen, BoX C
  2 siblings, 1 reply; 8+ messages in thread
From: Xing, Beilei @ 2020-12-17  3:20 UTC (permalink / raw)
  To: Amber, Kumar, dev, Chen, BoX C; +Cc: Zhao1, Wei

Hi, 

According to Bo's test, this fix patch will cause other cases fail, so NACK.
@Bo, could you please detail which cases will FAIL, thanks.

BR,
Beilei

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Kumar Amber
> Sent: Wednesday, November 11, 2020 6:11 PM
> To: dev@dpdk.org
> Cc: Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [dpdk-dev] [PATCH v2] net/i40e: fix argument in RSS action
> 
> The driver must check for the queue number in the RSS action list and if not
> should return with a proper error message to user.
> 
> Bugzilla ID: 573
> Fixes: 9486d60b94b5 ("net/i40e: fix flow RSS queue index check")
> Cc: wei.zhao1@intel.com
> 
> Signed-off-by: Kumar Amber <kumar.amber@intel.com>
> ---
>  drivers/net/i40e/i40e_flow.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c index
> 5bec0c7a84..975340cb1a 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
> @@ -4917,6 +4917,17 @@ i40e_flow_parse_rss_action(struct rte_eth_dev
> *dev,
>  	NEXT_ITEM_OF_ACTION(act, actions, index);
>  	rss = act->conf;
> 
> +	/**
> +	 * Check if Queue number is specified
> +	 * in argument else throw an error.
> +	 */
> +	if (rss->queue == NULL) {
> +		rte_flow_error_set(error, EINVAL,
> +			RTE_FLOW_ERROR_TYPE_ACTION,
> +			act, "Queue numbers not given.");
> +		return -rte_errno;
> +	}
> +
>  	/**
>  	 * RSS only supports forwarding,
>  	 * check if the first not void action is RSS.
> --
> 2.17.1


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

* Re: [dpdk-dev] [PATCH v2] net/i40e: fix argument in RSS action
  2020-12-17  3:20   ` [dpdk-dev] [PATCH v2] " Xing, Beilei
@ 2020-12-17  9:06     ` Chen, BoX C
  0 siblings, 0 replies; 8+ messages in thread
From: Chen, BoX C @ 2020-12-17  9:06 UTC (permalink / raw)
  To: Xing, Beilei, Amber, Kumar, dev; +Cc: Zhao1, Wei

Hi, Beilei

If you merge this patch, some common rules will fail to set on fvl, such as:
flow create 0 ingress pattern eth / ipv4 / udp / end actions rss types ipv4-udp end queues end / end

Regards,
Chen Bo

> -----Original Message-----
> From: Xing, Beilei <beilei.xing@intel.com>
> Sent: December 17, 2020 11:20
> To: Amber, Kumar <kumar.amber@intel.com>; dev@dpdk.org; Chen, BoX C
> <box.c.chen@intel.com>
> Cc: Zhao1, Wei <wei.zhao1@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix argument in RSS action
> 
> Hi,
> 
> According to Bo's test, this fix patch will cause other cases fail, so NACK.
> @Bo, could you please detail which cases will FAIL, thanks.
> 
> BR,
> Beilei
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Kumar Amber
> > Sent: Wednesday, November 11, 2020 6:11 PM
> > To: dev@dpdk.org
> > Cc: Zhao1, Wei <wei.zhao1@intel.com>
> > Subject: [dpdk-dev] [PATCH v2] net/i40e: fix argument in RSS action
> >
> > The driver must check for the queue number in the RSS action list and
> > if not should return with a proper error message to user.
> >
> > Bugzilla ID: 573
> > Fixes: 9486d60b94b5 ("net/i40e: fix flow RSS queue index check")
> > Cc: wei.zhao1@intel.com
> >
> > Signed-off-by: Kumar Amber <kumar.amber@intel.com>
> > ---
> >  drivers/net/i40e/i40e_flow.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/net/i40e/i40e_flow.c
> > b/drivers/net/i40e/i40e_flow.c index 5bec0c7a84..975340cb1a 100644
> > --- a/drivers/net/i40e/i40e_flow.c
> > +++ b/drivers/net/i40e/i40e_flow.c
> > @@ -4917,6 +4917,17 @@ i40e_flow_parse_rss_action(struct rte_eth_dev
> > *dev,
> >  	NEXT_ITEM_OF_ACTION(act, actions, index);
> >  	rss = act->conf;
> >
> > +	/**
> > +	 * Check if Queue number is specified
> > +	 * in argument else throw an error.
> > +	 */
> > +	if (rss->queue == NULL) {
> > +		rte_flow_error_set(error, EINVAL,
> > +			RTE_FLOW_ERROR_TYPE_ACTION,
> > +			act, "Queue numbers not given.");
> > +		return -rte_errno;
> > +	}
> > +
> >  	/**
> >  	 * RSS only supports forwarding,
> >  	 * check if the first not void action is RSS.
> > --
> > 2.17.1


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

* Re: [dpdk-dev] [PATCH v4] net/i40e: fix argument in RSS action
  2020-11-12 10:42   ` [dpdk-dev] [PATCH v4] " Kumar Amber
@ 2020-12-24  1:00     ` Zhang, Qi Z
  2021-01-04 10:04       ` Amber, Kumar
  0 siblings, 1 reply; 8+ messages in thread
From: Zhang, Qi Z @ 2020-12-24  1:00 UTC (permalink / raw)
  To: Amber, Kumar, dev; +Cc: Zhao1, Wei



> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Kumar Amber
> Sent: Thursday, November 12, 2020 6:43 PM
> To: dev@dpdk.org
> Cc: Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [dpdk-dev] [PATCH v4] net/i40e: fix argument in RSS action
> 
> The driver must check for the queue number in the RSS action list and if not
> should return with a proper error message to user.
> 
> Bugzilla ID: 573
> Fixes: 9486d60b94b5 ("net/i40e: fix flow RSS queue index check")
> Cc: wei.zhao1@intel.com
> 
> Signed-off-by: Kumar Amber <kumar.amber@intel.com>
> ---
>  drivers/net/i40e/i40e_flow.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c index
> 5bec0c7a84..397ed0ae77 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
> @@ -4917,6 +4917,18 @@ i40e_flow_parse_rss_action(struct rte_eth_dev
> *dev,
>  	NEXT_ITEM_OF_ACTION(act, actions, index);
>  	rss = act->conf;
> 
> +	/**
> +	 * Check if Queue number is specified
> +	 * in argument else throw an error.
> +	 */
> +	if (!rss->queue || !rss->queue_num) {
> +		rte_flow_error_set(error, EINVAL,
> +				RTE_FLOW_ERROR_TYPE_ACTION,
> +				act,
> +			   "no valid queues");
> +		return -rte_errno;
> +	}

I'm not sure if this is the right solution, the case we may have is: apply a RSS for a specific pattern for all enabled queues, so an empty queue configure that implicit for all enabled queues could still be acceptable. Can you share what kind of expected failure you are looking for?

> +
>  	/**
>  	 * RSS only supports forwarding,
>  	 * check if the first not void action is RSS.
> --
> 2.17.1


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

* Re: [dpdk-dev] [PATCH v4] net/i40e: fix argument in RSS action
  2020-12-24  1:00     ` Zhang, Qi Z
@ 2021-01-04 10:04       ` Amber, Kumar
  0 siblings, 0 replies; 8+ messages in thread
From: Amber, Kumar @ 2021-01-04 10:04 UTC (permalink / raw)
  To: Zhang, Qi Z, dev; +Cc: Zhao1, Wei

Hi Zhang ,

With the current understanding we agree we don't actually need to fix .
Will abandon the patch and close the issue .

Regards
Amber 



-----Original Message-----
From: Zhang, Qi Z <qi.z.zhang@intel.com> 
Sent: Thursday, December 24, 2020 6:30 AM
To: Amber, Kumar <kumar.amber@intel.com>; dev@dpdk.org
Cc: Zhao1, Wei <wei.zhao1@intel.com>
Subject: RE: [dpdk-dev] [PATCH v4] net/i40e: fix argument in RSS action



> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Kumar Amber
> Sent: Thursday, November 12, 2020 6:43 PM
> To: dev@dpdk.org
> Cc: Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [dpdk-dev] [PATCH v4] net/i40e: fix argument in RSS action
> 
> The driver must check for the queue number in the RSS action list and 
> if not should return with a proper error message to user.
> 
> Bugzilla ID: 573
> Fixes: 9486d60b94b5 ("net/i40e: fix flow RSS queue index check")
> Cc: wei.zhao1@intel.com
> 
> Signed-off-by: Kumar Amber <kumar.amber@intel.com>
> ---
>  drivers/net/i40e/i40e_flow.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_flow.c 
> b/drivers/net/i40e/i40e_flow.c index
> 5bec0c7a84..397ed0ae77 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
> @@ -4917,6 +4917,18 @@ i40e_flow_parse_rss_action(struct rte_eth_dev 
> *dev,  NEXT_ITEM_OF_ACTION(act, actions, index);  rss = act->conf;
> 
> +/**
> + * Check if Queue number is specified
> + * in argument else throw an error.
> + */
> +if (!rss->queue || !rss->queue_num) { rte_flow_error_set(error, 
> +EINVAL, RTE_FLOW_ERROR_TYPE_ACTION, act,
> +   "no valid queues");
> +return -rte_errno;
> +}

I'm not sure if this is the right solution, the case we may have is: apply a RSS for a specific pattern for all enabled queues, so an empty queue configure that implicit for all enabled queues could still be acceptable. Can you share what kind of expected failure you are looking for?

> +
>  /**
>   * RSS only supports forwarding,
>   * check if the first not void action is RSS.
> --
> 2.17.1



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

end of thread, other threads:[~2021-01-04 10:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-10 18:04 [dpdk-dev] [PATCH v1] net/i40e: argument in RSS action should have queue Kumar Amber
2020-11-11 10:10 ` [dpdk-dev] [PATCH v2] net/i40e: fix argument in RSS action Kumar Amber
2020-11-12 10:41   ` [dpdk-dev] [PATCH v3] " Kumar Amber
2020-11-12 10:42   ` [dpdk-dev] [PATCH v4] " Kumar Amber
2020-12-24  1:00     ` Zhang, Qi Z
2021-01-04 10:04       ` Amber, Kumar
2020-12-17  3:20   ` [dpdk-dev] [PATCH v2] " Xing, Beilei
2020-12-17  9:06     ` Chen, BoX C

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git