[v3,1/2] examples/l3fwd: relax RSS requirement with option

Message ID 20231112071611.101503-2-taozj888@163.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series example/l3fwd: relax l3fwd rx RSS/Offload if needed |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Trevor Tao Nov. 12, 2023, 7:16 a.m. UTC
  Now the port Rx mq_mode had been set to RTE_ETH_MQ_RX_RSS
by default, but some hw and/or virtual interface does not
support the RSS and offload mode presupposed, e.g., some
virtio interfaces in the cloud don't support
RSS and the error msg may like:

virtio_dev_configure(): RSS support requested but not supported by
the device
Port0 dev_configure = -95

So to enable the l3fwd running in that environment, the Rx mode requirement
can be relaxed to reflect the hardware feature reality here, and the l3fwd
can run smoothly then.

An option named "relax-rx-mode" is added to enable the relax action
here, and it's disabled by default.

Signed-off-by: Trevor Tao <taozj888@163.com>
---
 examples/l3fwd/main.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
  

Comments

Stephen Hemminger Nov. 12, 2023, 11:33 p.m. UTC | #1
On Sun, 12 Nov 2023 07:16:10 +0000
Trevor Tao <taozj888@163.com> wrote:

> Now the port Rx mq_mode had been set to RTE_ETH_MQ_RX_RSS
> by default, but some hw and/or virtual interface does not
> support the RSS and offload mode presupposed, e.g., some
> virtio interfaces in the cloud don't support
> RSS and the error msg may like:
> 
> virtio_dev_configure(): RSS support requested but not supported by
> the device
> Port0 dev_configure = -95
> 
> So to enable the l3fwd running in that environment, the Rx mode requirement
> can be relaxed to reflect the hardware feature reality here, and the l3fwd
> can run smoothly then.
> 
> An option named "relax-rx-mode" is added to enable the relax action
> here, and it's disabled by default.
> 
> Signed-off-by: Trevor Tao <taozj888@163.com>

Doesn't need to be an option. The application can just look at the offload
flags and quickly determine that if multiple queues are being used than
RSS is required. If device doesn't support RSS, then fallback to single
queue with a warning.
  
Trevor Tao Nov. 13, 2023, 11:47 a.m. UTC | #2
At 2023-11-13 07:33:49, "Stephen Hemminger" <stephen@networkplumber.org> wrote:
>On Sun, 12 Nov 2023 07:16:10 +0000
>Trevor Tao <taozj888@163.com> wrote:
>
>> Now the port Rx mq_mode had been set to RTE_ETH_MQ_RX_RSS
>> by default, but some hw and/or virtual interface does not
>> support the RSS and offload mode presupposed, e.g., some
>> virtio interfaces in the cloud don't support
>> RSS and the error msg may like:
>> 
>> virtio_dev_configure(): RSS support requested but not supported by
>> the device
>> Port0 dev_configure = -95
>> 
>> So to enable the l3fwd running in that environment, the Rx mode requirement
>> can be relaxed to reflect the hardware feature reality here, and the l3fwd
>> can run smoothly then.
>> 
>> An option named "relax-rx-mode" is added to enable the relax action
>> here, and it's disabled by default.
>> 
>> Signed-off-by: Trevor Tao <taozj888@163.com>
>
>Doesn't need to be an option. The application can just look at the offload
>flags and quickly determine that if multiple queues are being used than
>RSS is required. If device doesn't support RSS, then fallback to single

>queue with a warning.


Hi Stephen:


I think the option here was suggested/required by Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>.
The discussion thread is listed below:


">>>>Should we probably instead have a new commnad-line option to explicitly 
>>>>disable RSS?
>>> 
>>>>Something like: '--no-rss' or so?
>>> Trevor: the RSS capability for a certain port was got by the rte_eth_dev_info_get() automatically, and we think the user should not care about its status beforehand, but if it's missing, a warning notification for the degrade here would be proposed to make it run smoothly.
>>
>>Personally, I still think it would be better the user will
>>have an ability to disable it explicitly.
>>Same as l3fwd does now with 'parse-ptype'.

>>
"


Thanks,


Trevor Tao
  
Morten Brørup Nov. 13, 2023, 12:40 p.m. UTC | #3
From: taozj888 [mailto:taozj888@163.com] 
Sent: Monday, 13 November 2023 12.47



At 2023-11-13 07:33:49, "Stephen Hemminger" <stephen@networkplumber.org> wrote:

>On Sun, 12 Nov 2023 07:16:10 +0000
>Trevor Tao <taozj888@163.com> wrote:
> 
>> Now the port Rx mq_mode had been set to RTE_ETH_MQ_RX_RSS
>> by default, but some hw and/or virtual interface does not
>> support the RSS and offload mode presupposed, e.g., some
>> virtio interfaces in the cloud don't support
>> RSS and the error msg may like:
>> 
>> virtio_dev_configure(): RSS support requested but not supported by
>> the device
>> Port0 dev_configure = -95
>> 
>> So to enable the l3fwd running in that environment, the Rx mode requirement
>> can be relaxed to reflect the hardware feature reality here, and the l3fwd
>> can run smoothly then.
>> 
>> An option named "relax-rx-mode" is added to enable the relax action
>> here, and it's disabled by default.
>> 
>> Signed-off-by: Trevor Tao <taozj888@163.com>
> 
>Doesn't need to be an option. The application can just look at the offload
>flags and quickly determine that if multiple queues are being used than
>RSS is required. If device doesn't support RSS, then fallback to single
>queue with a warning.
 
Hi Stephen:
 
I think the option here was suggested/required by Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>.
The discussion thread is listed below:
 
">>>>Should we probably instead have a new commnad-line option to explicitly 
>>>>disable RSS?
>>> 
>>>>Something like: '--no-rss' or so?
>>> Trevor: the RSS capability for a certain port was got by the rte_eth_dev_info_get() automatically, and we think the user should not care about its status beforehand, but if it's missing, a warning notification for the degrade here would be proposed to make it run smoothly.
>> 
>>Personally, I still think it would be better the user will
>>have an ability to disable it explicitly.
>>Same as l3fwd does now with 'parse-ptype'.
>> 
"
 
[Morten]: The "relax-rx-mode" does not explicitly disable RSS, it enables the behavior Stephen suggested: If RSS is not supported by the device, fallback to single queue with a warning. We don't need an option for this; it should be the default behavior.
[Morten]: I think Konstantin was asking for an option "no-rss" (or "disable-rss") to force use of single queue (disable RSS), regardless if the device supports RSS or not.
 
Thanks,
 
Trevor Tao
  
Konstantin Ananyev Nov. 13, 2023, 1:19 p.m. UTC | #4
>> Now the port Rx mq_mode had been set to RTE_ETH_MQ_RX_RSS
>> by default, but some hw and/or virtual interface does not
>> support the RSS and offload mode presupposed, e.g., some
>> virtio interfaces in the cloud don't support
>> RSS and the error msg may like:
>> 
>> virtio_dev_configure(): RSS support requested but not supported by
>> the device
>> Port0 dev_configure = -95
>> 
>> So to enable the l3fwd running in that environment, the Rx mode requirement
>> can be relaxed to reflect the hardware feature reality here, and the l3fwd
>> can run smoothly then.
>> 
>> An option named "relax-rx-mode" is added to enable the relax action
>> here, and it's disabled by default.
>> 
>> Signed-off-by: Trevor Tao <mailto:taozj888@163.com>
>
>Doesn't need to be an option. The application can just look at the offload
>flags and quickly determine that if multiple queues are being used than
>RSS is required. If device doesn't support RSS, then fallback to single
>queue with a warning.
>
>Hi Stephen:
>
>I think the option here was suggested/required by Konstantin Ananyev <mailto:konstantin.v.ananyev@yandex.ru>.
>The discussion thread is listed below:
>
">>>>Should we probably instead have a new commnad-line option to explicitly 
>>>>disable RSS?
>>> 
>>>>Something like: '--no-rss' or so?
>>> Trevor: the RSS capability for a certain port was got by the rte_eth_dev_info_get() automatically, and we think the user should not care about its status beforehand, but if it's missing, a warning notification for the degrade here would be proposed to make it run smoothly.
>>
>>Personally, I still think it would be better the user will
>>have an ability to disable it explicitly.
>>Same as l3fwd does now with 'parse-ptype'.
>>
>
>[Morten]: The "relax-rx-mode" does not explicitly disable RSS, it enables the behavior Stephen suggested: If RSS is not supported by the device, fallback to single queue with a warning. We don't need an option >for this; it should be the default behavior.
>[Morten]: I think Konstantin was asking for an option "no-rss" (or "disable-rss") to force use of single queue (disable RSS), regardless if the device supports RSS or not.

Yes, that what was my initial suggestion: let people explicitly disable RSS if they want to.
BTW,  I think it wouldn't change number of HW queues in any way.
It would just disable distribution of packets among HW queues based on RSS.
Konstantin
  
Trevor Tao Nov. 13, 2023, 4:06 p.m. UTC | #5
Hi all:

At 2023-11-13 21:19:14, "Konstantin Ananyev" <konstantin.ananyev@huawei.com> wrote:
>
>
>>> Now the port Rx mq_mode had been set to RTE_ETH_MQ_RX_RSS
>>> by default, but some hw and/or virtual interface does not
>>> support the RSS and offload mode presupposed, e.g., some
>>> virtio interfaces in the cloud don't support
>>> RSS and the error msg may like:
>>> 
>>> virtio_dev_configure(): RSS support requested but not supported by
>>> the device
>>> Port0 dev_configure = -95
>>> 
>>> So to enable the l3fwd running in that environment, the Rx mode requirement
>>> can be relaxed to reflect the hardware feature reality here, and the l3fwd
>>> can run smoothly then.
>>> 
>>> An option named "relax-rx-mode" is added to enable the relax action
>>> here, and it's disabled by default.
>>> 
>>> Signed-off-by: Trevor Tao <mailto:taozj888@163.com>
>>
>>Doesn't need to be an option. The application can just look at the offload
>>flags and quickly determine that if multiple queues are being used than
>>RSS is required. If device doesn't support RSS, then fallback to single
>>queue with a warning.
>>
>>Hi Stephen:
>>
>>I think the option here was suggested/required by Konstantin Ananyev <mailto:konstantin.v.ananyev@yandex.ru>.
>>The discussion thread is listed below:
>>
>">>>>Should we probably instead have a new commnad-line option to explicitly 
>>>>>disable RSS?
>>>> 
>>>>>Something like: '--no-rss' or so?
>>>> Trevor: the RSS capability for a certain port was got by the rte_eth_dev_info_get() automatically, and we think the user should not care about its status beforehand, but if it's missing, a warning notification for the degrade here would be proposed to make it run smoothly.
>>>
>>>Personally, I still think it would be better the user will
>>>have an ability to disable it explicitly.
>>>Same as l3fwd does now with 'parse-ptype'.
>>>
>>
>>[Morten]: The "relax-rx-mode" does not explicitly disable RSS, it enables the behavior Stephen suggested: If RSS is not supported by the device, fallback to single queue with a warning. We don't need an option >for this; it should be the default behavior.
>>[Morten]: I think Konstantin was asking for an option "no-rss" (or "disable-rss") to force use of single queue (disable RSS), regardless if the device supports RSS or not.
>
>Yes, that what was my initial suggestion: let people explicitly disable RSS if they want to.
>BTW,  I think it wouldn't change number of HW queues in any way.
>It would just disable distribution of packets among HW queues based on RSS.
>Konstantin 

>


Thanks for your comments. I addressed your ideas with a v4 patch series.
1. A new option named "disable-rss" was added to enable the "diable-rss" case;
2. A new option  named "relax-rx-offload" was added to enable the relax rx offload case; and the SW IPv4 hdr cksum was added if needed;


Trevor Tao 


Trevor Tao
  

Patch

diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 6063eb1399..89ad546a5e 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -73,6 +73,7 @@  static enum L3FWD_LOOKUP_MODE lookup_mode;
 static int numa_on = 1; /**< NUMA is enabled by default. */
 static int parse_ptype; /**< Parse packet type using rx callback, and */
 			/**< disabled by default */
+static int relax_rx_mode; /**< Relax RX mode is disabled by default */
 static int per_port_pool; /**< Use separate buffer pools per port; disabled */
 			  /**< by default */
 
@@ -678,6 +679,7 @@  static const char short_options[] =
 #define CMD_LINE_OPT_MAX_PKT_LEN "max-pkt-len"
 #define CMD_LINE_OPT_HASH_ENTRY_NUM "hash-entry-num"
 #define CMD_LINE_OPT_PARSE_PTYPE "parse-ptype"
+#define CMD_LINE_OPT_RELAX_RX_MODE "relax-rx-mode"
 #define CMD_LINE_OPT_PER_PORT_POOL "per-port-pool"
 #define CMD_LINE_OPT_MODE "mode"
 #define CMD_LINE_OPT_EVENTQ_SYNC "eventq-sched"
@@ -705,6 +707,7 @@  enum {
 	CMD_LINE_OPT_MAX_PKT_LEN_NUM,
 	CMD_LINE_OPT_HASH_ENTRY_NUM_NUM,
 	CMD_LINE_OPT_PARSE_PTYPE_NUM,
+	CMD_LINE_OPT_RELAX_RX_MODE_NUM,
 	CMD_LINE_OPT_RULE_IPV4_NUM,
 	CMD_LINE_OPT_RULE_IPV6_NUM,
 	CMD_LINE_OPT_ALG_NUM,
@@ -728,6 +731,7 @@  static const struct option lgopts[] = {
 	{CMD_LINE_OPT_MAX_PKT_LEN, 1, 0, CMD_LINE_OPT_MAX_PKT_LEN_NUM},
 	{CMD_LINE_OPT_HASH_ENTRY_NUM, 1, 0, CMD_LINE_OPT_HASH_ENTRY_NUM_NUM},
 	{CMD_LINE_OPT_PARSE_PTYPE, 0, 0, CMD_LINE_OPT_PARSE_PTYPE_NUM},
+	{CMD_LINE_OPT_RELAX_RX_MODE, 0, 0, CMD_LINE_OPT_RELAX_RX_MODE_NUM},
 	{CMD_LINE_OPT_PER_PORT_POOL, 0, 0, CMD_LINE_OPT_PARSE_PER_PORT_POOL},
 	{CMD_LINE_OPT_MODE, 1, 0, CMD_LINE_OPT_MODE_NUM},
 	{CMD_LINE_OPT_EVENTQ_SYNC, 1, 0, CMD_LINE_OPT_EVENTQ_SYNC_NUM},
@@ -853,6 +857,11 @@  parse_args(int argc, char **argv)
 			parse_ptype = 1;
 			break;
 
+		case CMD_LINE_OPT_RELAX_RX_MODE_NUM:
+			printf("Relax rx mode is enabled\n");
+			relax_rx_mode = 1;
+			break;
+
 		case CMD_LINE_OPT_PARSE_PER_PORT_POOL:
 			printf("per port buffer pool is enabled\n");
 			per_port_pool = 1;
@@ -1260,6 +1269,13 @@  l3fwd_poll_resource_setup(void)
 		if (dev_info.max_rx_queues == 1)
 			local_port_conf.rxmode.mq_mode = RTE_ETH_MQ_RX_NONE;
 
+		/* relax the rx rss requirement */
+		if (relax_rx_mode && !local_port_conf.rx_adv_conf.rss_conf.rss_hf) {
+			printf("warning: modified the rx mq_mode to RTE_ETH_MQ_RX_NONE base on"
+					" device capability\n");
+				local_port_conf.rxmode.mq_mode = RTE_ETH_MQ_RX_NONE;
+		}
+
 		if (local_port_conf.rx_adv_conf.rss_conf.rss_hf !=
 				port_conf.rx_adv_conf.rss_conf.rss_hf) {
 			printf("Port %u modified RSS hash function based on hardware support,"