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

[Trevor Tao]

HI Konstantin:

I do understand your requirement on the SW support for the IPV4 cksum verification, and I think it really can be added here later some time when missing HW support.
Anyway, there is a "warning:" message had been sent out to notify the user there is a lack of HW capability support for packets, and it would enable this missing case can run smoothly for some urgent users which really exist with our experiences.
On the other side, maybe another hint of "warning: no HW check for IPv4 checksum" or something alike could be helpful to users before the SW support added. 

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: Konstantin Ananyev <konstantin.ananyev at huawei.com> 
Sent: Friday, July 28, 2023 4:03 PM
To: Trevor Tao <Trevor.Tao at arm.com>; thomas at monjalon.net
Cc: dev at dpdk.org; nd <nd at arm.com>; stable at dpdk.org; Ruifeng Wang <Ruifeng.Wang at arm.com>; Feifei Wang <Feifei.Wang2 at arm.com>
Subject: RE: [PATCH v1] 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:

Well, these HW offloads are there for the good reason - l3fwd app relies on these HW features to provide functionality requested.
It relies on RTE_ETH_RX_OFFLOAD_IPV4_CKSUM to avoid checks of ip cksum in SW:
static inline int
is_valid_ipv4_pkt(struct rte_ipv4_hdr *pkt, uint32_t link_len) {
        /* From http://www.rfc-editor.org/rfc/rfc1812.txt section 5.2.2 */
        /*
         * 1. The packet length reported by the Link Layer must be large
         * enough to hold the minimum length legal IP datagram (20 bytes).
         */
        if (link_len < sizeof(struct rte_ipv4_hdr))
                return -1;

        /* 2. The IP checksum must be correct. */
        /* this is checked in H/W */ 
        ....

By having RSS enabled it ensures that packets from the same 'flow' will be processed and send out in order. Probably not a strict requirement for l3fwd itself, but definitely nice to have feature that majority of DPDK customers are interested in.
I do understand your desire to lower HW requirements for l3fwd, but then probably shouldn't be just blind disable, but instead add SW support for them when essential HW feature is missing. 

Konstantin
 
> 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 does not 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 at dpdk.org
> 
> Signed-off-by: Trevor Tao <trevor.tao at arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang at arm.com>
> Reviewed-by: Feifei Wang <feifei.wang2 at arm.com>
> ---
>  .mailmap              |  1 +
>  examples/l3fwd/main.c | 19 ++++++++++++++++++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/.mailmap b/.mailmap
> index 8e3940a253..602d8cbc6b 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -1403,6 +1403,7 @@ Tom Rix <trix at redhat.com>  Tone Zhang 
> <tone.zhang at arm.com>  Tonghao Zhang <xiangxia.m.yue at gmail.com> 
> <nic at opencloud.tech>  Tony Nguyen <anthony.l.nguyen at intel.com>
> +Trevor Tao <trevor.tao at arm.com>
>  Tsotne Chakhvadze <tsotne.chakhvadze at intel.com>  Tudor Brindus 
> <me at tbrindus.ca>  Tudor Cornea <tudor.cornea at gmail.com> 
> <tudor.cornea at keysight.com> diff --git a/examples/l3fwd/main.c 
> b/examples/l3fwd/main.c index a4f061537e..cec87d95d1 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,19 @@ 
> 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" does not"
> +				" 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;
> +			port_conf.rxmode.offloads = local_port_conf.rxmode.offloads;
> +			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.41.0
>