net/i40e: argument in RSS action should have queue

PATCH Superseded Kumar Amber 4 participants 8 messages Nov 10, 2020 – Jan 4, 2021 Network drivers

Share & tools

Kumar Amber

author

2020-11-10 18:09 UTC #0 lore raw

PATCH 1/1 net/i40e: argument in RSS action should have queue

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 <[email protected]>

---
 drivers/net/i40e/i40e_flow.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

drivers/net/i40e/i40e_flow.c +11 -0

index 5bec0c7a84..975340cb1a 100644
@@ -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.

Signature · 1 line

2.17.1

Kumar Amber

author

2020-11-11 10:16 UTC #1 lore raw

PATCH v2 1/1 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 [email protected]

Signed-off-by Kumar Amber <[email protected]>

---
 drivers/net/i40e/i40e_flow.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

drivers/net/i40e/i40e_flow.c +11 -0

index 5bec0c7a84..975340cb1a 100644
@@ -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.

Signature · 1 line

2.17.1

Kumar Amber

author

2020-11-12 10:46 UTC #2 lore raw

PATCH v3 1/1 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 [email protected]

Signed-off-by Kumar Amber <[email protected]>

---
 drivers/net/i40e/i40e_flow.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

drivers/net/i40e/i40e_flow.c +11 -0

index 5bec0c7a84..975340cb1a 100644
@@ -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.

Signature · 1 line

2.17.1

Kumar Amber

author

2020-11-12 10:48 UTC #3 lore raw

PATCH v4 1/1 net/i40e: fix argument in RSS action

In reply to Kumar Amber

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 [email protected]

Signed-off-by Kumar Amber <[email protected]>

---
 drivers/net/i40e/i40e_flow.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

drivers/net/i40e/i40e_flow.c +12 -0

index 5bec0c7a84..397ed0ae77 100644
@@ -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.

Signature · 1 line

2.17.1

Zhang, Qi Z

2020-12-24 01:00 UTC #4 lore raw

Re: [dpdk-dev] [PATCH v4] net/i40e: fix argument in RSS action

Quoted 31 lines

-----Original Message-----
From: dev <[email protected]> On Behalf Of Kumar Amber
Sent: Thursday, November 12, 2020 6:43 PM
To: [email protected]
Cc: Zhao1, Wei <[email protected]>
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 [email protected]
Signed-off-by Kumar Amber <[email protected]>
---
 drivers/net/i40e/i40e_flow.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
drivers/net/i40e/i40e_flow.c index renamed from drivers/net/i40e/i40e_flow.c +0 -0
5bec0c7a84..397ed0ae77 100644
drivers/net/i40e/i40e_flow.c +1 -0
@@ -4917,6 +4917,18 @@ i40e_flow_parse_rss_action(struct rte_eth_dev
 *dev,
 	NEXT_ITEM_OF_ACTION(act, actions, index);
 	rss = act->conf;
 
+	/**

drivers/net/i40e/i40e_flow.c i40e_flow_parse_rss_action(struct rte_eth_dev · line 4931

Quoted 10 lines

drivers/net/i40e/i40e_flow.c +10 -0

+	 * 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?

Quoted 6 lines

drivers/net/i40e/i40e_flow.c +1 -0
+
 	/**
 	 * RSS only supports forwarding,
* check if the first not void action is RSS.
--
2.17.1

Amber, Kumar

author

2021-01-04 10:04 UTC #5 lore raw

Re: [dpdk-dev] [PATCH v4] net/i40e: fix argument in RSS action

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 <[email protected]>
Sent: Thursday, December 24, 2020 6:30 AM
To: Amber, Kumar <[email protected]>; [email protected]
Cc: Zhao1, Wei <[email protected]>
Subject: RE: [dpdk-dev] [PATCH v4] net/i40e: fix argument in RSS action

Quoted 26 lines

-----Original Message-----
From: dev <[email protected]> On Behalf Of Kumar Amber
Sent: Thursday, November 12, 2020 6:43 PM
To: [email protected]
Cc: Zhao1, Wei <[email protected]>
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 [email protected]
Signed-off-by Kumar Amber <[email protected]>
---
 drivers/net/i40e/i40e_flow.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
(quoted diff) +0 -0
b/drivers/net/i40e/i40e_flow.c index
5bec0c7a84..397ed0ae77 100644
drivers/net/i40e/i40e_flow.c +0 -0
@@ -4917,6 +4917,18 @@ i40e_flow_parse_rss_action(struct rte_eth_dev 
 *dev,  NEXT_ITEM_OF_ACTION(act, actions, index);  rss = act->conf;

drivers/net/i40e/i40e_flow.c i40e_flow_parse_rss_action(struct rte_eth_dev · line 4927

Quoted 10 lines

drivers/net/i40e/i40e_flow.c +9 -0

 
+/**
+ * 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;
+}

Quoted 6 lines

drivers/net/i40e/i40e_flow.c +1 -1

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

2.17.1

Xing, Beilei

2020-12-17 03:20 UTC #6 lore raw

Re: [dpdk-dev] [PATCH v2] net/i40e: fix argument in RSS action

In reply to Kumar Amber

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

Quoted 44 lines

-----Original Message-----
From: dev <[email protected]> On Behalf Of Kumar Amber
Sent: Wednesday, November 11, 2020 6:11 PM
To: [email protected]
Cc: Zhao1, Wei <[email protected]>
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 [email protected]
Signed-off-by Kumar Amber <[email protected]>
---
 drivers/net/i40e/i40e_flow.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
drivers/net/i40e/i40e_flow.c index renamed from drivers/net/i40e/i40e_flow.c +0 -0
5bec0c7a84..975340cb1a 100644
drivers/net/i40e/i40e_flow.c +11 -0
@@ -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

Chen, BoX C

2020-12-17 09:06 UTC #7 lore raw

Re: [dpdk-dev] [PATCH v2] net/i40e: fix argument in RSS action

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

Quoted 60 lines

-----Original Message-----
From: Xing, Beilei <[email protected]>
Sent: December 17, 2020 11:20
To: Amber, Kumar <[email protected]>; [email protected]; Chen, BoX C <[email protected]>
Cc: Zhao1, Wei <[email protected]>
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
Quoted 44 lines
-----Original Message-----
From: dev <[email protected]> On Behalf Of Kumar Amber
Sent: Wednesday, November 11, 2020 6:11 PM
To: [email protected]
Cc: Zhao1, Wei <[email protected]>
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 [email protected]
Signed-off-by Kumar Amber <[email protected]>
---
 drivers/net/i40e/i40e_flow.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
(quoted diff) +0 -0
b/drivers/net/i40e/i40e_flow.c index 5bec0c7a84..975340cb1a 100644
drivers/net/i40e/i40e_flow.c +11 -0
@@ -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