diff mbox series

[v4] net/i40e: fix argument in RSS action

Message ID 20201112104245.91324-1-kumar.amber@intel.com (mailing list archive)
State Rejected, archived
Delegated to: Qi Zhang
Headers
Series [v4] net/i40e: fix argument in RSS action |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Amber, Kumar Nov. 12, 2020, 10:42 a.m. UTC 
  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(+)

Comments

Qi Zhang Dec. 24, 2020, 1 a.m. UTC | #1 
> -----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
Amber, Kumar Jan. 4, 2021, 10:04 a.m. UTC | #2 
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
diff mbox series

Patch

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.