From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B604B425C5; Sun, 17 Sep 2023 20:04:23 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4134440272; Sun, 17 Sep 2023 20:04:23 +0200 (CEST) Received: from forward500b.mail.yandex.net (forward500b.mail.yandex.net [178.154.239.144]) by mails.dpdk.org (Postfix) with ESMTP id 947E64025E; Sun, 17 Sep 2023 20:04:22 +0200 (CEST) Received: from mail-nwsmtp-smtp-production-main-54.iva.yp-c.yandex.net (mail-nwsmtp-smtp-production-main-54.iva.yp-c.yandex.net [IPv6:2a02:6b8:c0c:1e27:0:640:d1a0:0]) by forward500b.mail.yandex.net (Yandex) with ESMTP id E18025E7DD; Sun, 17 Sep 2023 21:04:21 +0300 (MSK) Received: by mail-nwsmtp-smtp-production-main-54.iva.yp-c.yandex.net (smtp/Yandex) with ESMTPSA id J4okdQtDda60-5gEYqAN0; Sun, 17 Sep 2023 21:04:21 +0300 X-Yandex-Fwd: 1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1694973861; bh=z3qpN9geg++ld+TvILnNYX8z9J2xmp3IYm0TMnc1Aps=; h=From:In-Reply-To:Cc:Date:References:To:Subject:Message-ID; b=Um4J4kcFMqYFtFXjo0j94G1tSIzNWcXbuJa1422SRhBVh+lQnIkwIw1qW7t01M9Sd ZX7e8+bJjJ8qOPRTcKfn216kqc1ELZswLRVvDNswiKXPAySQmyE6HwceM/FscQ+ee9 mTk8UG42hnG0nIb2jZ6ujcAX5OBCrLh0F9V3DEvg= Authentication-Results: mail-nwsmtp-smtp-production-main-54.iva.yp-c.yandex.net; dkim=pass header.i=@yandex.ru Message-ID: Date: Sun, 17 Sep 2023 19:04:19 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.0 Subject: Re: [PATCH v1] examples/l3fwd: relax the RSS/Offload requirement Content-Language: en-US, ru-RU To: Trevor Tao , dev@dpdk.org Cc: thomas@monjalon.net, stable@dpdk.org References: <20230903040111.126695-1-taozj888@163.com> From: Konstantin Ananyev In-Reply-To: <20230903040111.126695-1-taozj888@163.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 03/09/2023 05:01, Trevor Tao пишет: > 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 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. > > On the other side, enabling the software cksum check in case the > hw support missing. > > Fixes: af75078fece3 ("first public release") > Cc: stable@dpdk.org I don't think there was abug here. We are talking about changing current requirements for the app. So not sure it is a real fix and that such change can be propagated to stable releases. > > Signed-off-by: Trevor Tao > --- > examples/l3fwd/l3fwd.h | 12 +++++++++++- > examples/l3fwd/main.c | 21 +++++++++++++++++++-- > 2 files changed, 30 insertions(+), 3 deletions(-) > > diff --git a/examples/l3fwd/l3fwd.h b/examples/l3fwd/l3fwd.h > index b55855c932..cc10643c4b 100644 > --- a/examples/l3fwd/l3fwd.h > +++ b/examples/l3fwd/l3fwd.h > @@ -115,6 +115,8 @@ extern struct acl_algorithms acl_alg[]; > > extern uint32_t max_pkt_len; > > +extern struct rte_eth_conf port_conf; > + > /* Send burst of packets on an output interface */ > static inline int > send_burst(struct lcore_conf *qconf, uint16_t n, uint16_t port) > @@ -170,7 +172,15 @@ is_valid_ipv4_pkt(struct rte_ipv4_hdr *pkt, uint32_t link_len) > return -1; > > /* 2. The IP checksum must be correct. */ > - /* this is checked in H/W */ > + /* if this is not checked in H/W, check it. */ > + if ((port_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_IPV4_CKSUM) == 0) { Might be better to check particular mbuf flag: if ((mbuf->ol_flags & RTE_MBUF_F_RX_IP_CKSUM_MASK) == TE_MBUF_F_RX_IP_CKSUM_UNKNOWN) {...} > + uint16_t actual_cksum, expected_cksum; > + actual_cksum = pkt->hdr_checksum; > + pkt->hdr_checksum = 0; > + expected_cksum = rte_ipv4_cksum(pkt); > + if (actual_cksum != expected_cksum) > + return -2; > + } > > /* > * 3. The IP version number must be 4. If the version number is not 4 > diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c > index 6063eb1399..37aec64718 100644 > --- a/examples/l3fwd/main.c > +++ b/examples/l3fwd/main.c > @@ -117,7 +117,7 @@ static struct lcore_params * lcore_params = lcore_params_array_default; > static uint16_t nb_lcore_params = sizeof(lcore_params_array_default) / > sizeof(lcore_params_array_default[0]); > > -static struct rte_eth_conf port_conf = { > +struct rte_eth_conf port_conf = { > .rxmode = { > .mq_mode = RTE_ETH_MQ_RX_RSS, > .offloads = RTE_ETH_RX_OFFLOAD_CHECKSUM, > @@ -1257,8 +1257,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; Should we probably instead have a new commnad-line option to explicitly disable RSS? Something like: '--no-rss' or so? > + } > > if (local_port_conf.rx_adv_conf.rss_conf.rss_hf != > port_conf.rx_adv_conf.rss_conf.rss_hf) { > @@ -1269,6 +1273,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; Why to remove offloads in port_conf? There could be multiple ports, and on others desired HW offloads might be supported. > + 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)