RE: [PATCH] examples/l3fwd: relax the RSS/Offload requirement

RE: [PATCH] examples/l3fwd: relax the RSS/Offload requirement

[Trevor Tao]

Hi Thomas, dev group:

After I sent out this patch, I can't find it at the patch site:
https://patches.dpdk.org/project/dpdk/list/?param=2&page=1
I would like to ask if there's anything I should do first here.

Thanks,

Best Regards,

Zijin Tao(Trevor Tao, 陶孜谨)
ARM Electronic Technology (Shanghai) Co., Ltd
安谋电子科技（上海）有限公司
Building 11, Shanghai Busininess ParkⅢ ,
No.1016 Tianlin Rd, Minhang District, Shanghai, 200233 China
上海市闵行区田林路1016号科技绿洲三期2号楼10楼，200233
Cell:      +86-153 7109 6192

-----Original Message-----
From: Trevor Tao <trevor.tao@arm.com>
Sent: Thursday, May 11, 2023 4:06 PM
To: thomas@monjalon.net
Cc: dev@dpdk.org; nd <nd@arm.com>; Trevor Tao <Trevor.Tao@arm.com>; stable@dpdk.org
Subject: [PATCH] examples/l3fwd: relax the RSS/Offload requirement

Now the port Rx mq_mode had been set to RTE_ETH_MQ_RX_RSS, and offload mode set to RTE_ETH_RX_OFFLOAD_CHECKSUM by default, but some hardware 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 may only partly support RTE_ETH_RX_OFFLOAD_UDP_CKSUM/ RTE_ETH_RX_OFFLOAD_TCP_CKSUM, but not RTE_ETH_RX_OFFLOAD_IPV4_CKSUM, and the error msg here:

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

and:
Ethdev port_id=0 requested Rx offloads 0xe doesn't match Rx offloads capabilities 0x201d in rte_eth_dev_configure()

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.
A warning msg would be provided to user in case it happens here.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Trevor Tao <trevor.tao@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
---
 .mailmap              |  1 +
 examples/l3fwd/main.c | 18 +++++++++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/.mailmap b/.mailmap
index 0859104404..cf4a59fb41 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1380,6 +1380,7 @@ Tom Rix <trix@redhat.com>  Tone Zhang <tone.zhang@arm.com>  Tonghao Zhang <xiangxia.m.yue@gmail.com> <nic@opencloud.tech>  Tony Nguyen <anthony.l.nguyen@intel.com>
+Trevor Tao <trevor.tao@arm.com>
 Tsotne Chakhvadze <tsotne.chakhvadze@intel.com>  Tudor Brindus <me@tbrindus.ca>  Tudor Cornea <tudor.cornea@gmail.com> <tudor.cornea@keysight.com> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c index a4f061537e..2cdb6b7ae6 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -1233,8 +1233,12 @@ l3fwd_poll_resource_setup(void)
                local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
                        dev_info.flow_type_rss_offloads;

-               if (dev_info.max_rx_queues == 1)
+               /* relax the rx rss requirement */
+               if (dev_info.max_rx_queues == 1 || !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) { @@ -1245,6 +1249,18 @@ l3fwd_poll_resource_setup(void)
                                local_port_conf.rx_adv_conf.rss_conf.rss_hf);
                }

+               /* relax the rx offload requirement */
+               if ((local_port_conf.rxmode.offloads & dev_info.rx_offload_capa) !=
+                       local_port_conf.rxmode.offloads) {
+                       printf("Port %u requested Rx offloads 0x%"PRIx64" doesn't"
+                               " match Rx offloads capabilities 0x%"PRIx64"\n",
+                               portid, local_port_conf.rxmode.offloads,
+                               dev_info.rx_offload_capa);
+                       local_port_conf.rxmode.offloads &= dev_info.rx_offload_capa;
+                       printf("warning: modified the rx offload to 0x%"PRIx64" based on device"
+                               " capability\n", local_port_conf.rxmode.offloads);
+               }
+
                ret = rte_eth_dev_configure(portid, nb_rx_queue,
                                        (uint16_t)n_tx_queue, &local_port_conf);
                if (ret < 0)
--
2.17.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH] examples/l3fwd: relax the RSS/Offload requirement

[Stephen Hemminger]

On Thu, 11 May 2023 09:33:35 +0000
Trevor Tao <Trevor.Tao@arm.com> wrote:

> +               /* relax the rx rss requirement */
> +               if (dev_info.max_rx_queues == 1 || !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;
> +               }

There is no point in doing RSS if only a single queue.
Therefore remove the !local_port.conf portion of the expression.

And since this is normal, no printf is needed.

RE: [PATCH] examples/l3fwd: relax the RSS/Offload requirement

[Trevor Tao]

Hi Stephen:

Yes,  I think there is no point in doing RSS if only a single queue, but for the 2nd part:
"|| !local_port_conf.rx_adv_conf.rss_conf.rss_hf"
It's an "or" relationship with the dev_info.max_rx_queues==1, and in my case, the device(virtio) information showed that
the max_rx_queues is not 1 but the rss_hf is 0:
......
Initializing port 0 ... Creating queues: nb_rxq=1 nb_txq=1...
Trevor: dev_info.max_rx_queues=16, local_port_conf.rx_adv_conf.rss_conf.rss_hf = 0x0
......

So in this case, the mq_mode should also be set to RTE_ETH_MQ_RX_NONE to disable the RSS.

Thanks,

Best Regards,

Zijin Tao(Trevor Tao, 陶孜谨)
ARM Electronic Technology (Shanghai) Co., Ltd
安谋电子科技（上海）有限公司
Building 11, Shanghai Busininess ParkⅢ ,
No.1016 Tianlin Rd, Minhang District, Shanghai, 200233 China
上海市闵行区田林路1016号科技绿洲三期2号楼10楼，200233
Cell:      +86-153 7109 6192

-----Original Message-----
From: Stephen Hemminger <stephen@networkplumber.org>
Sent: Thursday, May 11, 2023 11:39 PM
To: Trevor Tao <Trevor.Tao@arm.com>
Cc: thomas@monjalon.net; dev@dpdk.org
Subject: Re: [PATCH] examples/l3fwd: relax the RSS/Offload requirement

On Thu, 11 May 2023 09:33:35 +0000
Trevor Tao <Trevor.Tao@arm.com> wrote:

> +               /* relax the rx rss requirement */
> +               if (dev_info.max_rx_queues == 1 || !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;
> +               }

There is no point in doing RSS if only a single queue.
Therefore remove the !local_port.conf portion of the expression.

And since this is normal, no printf is needed.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.