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 08B83A0547; Thu, 29 Apr 2021 20:39:38 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E34AC410DF; Thu, 29 Apr 2021 20:39:37 +0200 (CEST) Received: from sysclose.org (smtp.sysclose.org [69.164.214.230]) by mails.dpdk.org (Postfix) with ESMTP id 8BE1D410DD for ; Thu, 29 Apr 2021 20:39:36 +0200 (CEST) Received: from localhost (unknown [45.71.104.233]) by sysclose.org (Postfix) with ESMTPSA id 0D5FC2C10; Thu, 29 Apr 2021 18:39:38 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 sysclose.org 0D5FC2C10 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sysclose.org; s=201903; t=1619721579; bh=UtzZVDxSHvntJ1fJaLt4srsKW/olLJUcZf4eIgwd2Vo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=n9btFSt0f9/2mu+A9EPsCRaGhW+w3BH3VqyICmLmZXYxL/8lxfkZBRnoWlI0MI22Z /U/u7US8ansZle/WdJatO5KSm+yj+G2/4rE7lxXliMOSp9nBK9yXFhvzaSl8wVoqrb XDdfOWvXVyCKpluoLq9mXyWD+PfqG9+seFZCq7wmMlsSL6dmRKCJHs/t2+YKDH4pRy q2kLWcqaGWDXKc4jUGuCOshS8uQT8FxFcuD/3fHaewDrq7JWCxx7Jq/voNJ1tQ21op BGotmgoVmSwyohy7abEkv7YCwEdkB6LiCF9edWTZ1SDSTjzIjiYevPN6JIxAG3dWDk Xk5tffBam4f1A== Date: Thu, 29 Apr 2021 15:39:31 -0300 From: Flavio Leitner To: David Marchand Cc: dev@dpdk.org, maxime.coquelin@redhat.com, olivier.matz@6wind.com, i.maximets@ovn.org, Chenbo Xia , Jijiang Liu , Yuanhan Liu Message-ID: References: <20210401095243.18211-1-david.marchand@redhat.com> <20210429080438.15032-1-david.marchand@redhat.com> <20210429080438.15032-5-david.marchand@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210429080438.15032-5-david.marchand@redhat.com> Subject: Re: [dpdk-dev] [PATCH v2 4/4] vhost: fix offload flags in Rx path 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 Sender: "dev" On Thu, Apr 29, 2021 at 10:04:38AM +0200, David Marchand wrote: > The vhost library current configures Tx offloading (PKT_TX_*) on any > packet received from a guest virtio device which asks for some offloading. > > This is problematic, as Tx offloading is something that the application > must ask for: the application needs to configure devices > to support every used offloads (ip, tcp checksumming, tso..), and the > various l2/l3/l4 lengths must be set following any processing that > happened in the application itself. > > On the other hand, the received packets are not marked wrt current > packet l3/l4 checksumming info. > > Copy virtio rx processing to fix those offload flags but accepting > VIRTIO_NET_HDR_GSO_ECN and VIRTIO_NET_HDR_GSO_UDP too. > > The vhost example has been updated accordingly: TSO is applied to any > packet marked LRO. > > Fixes: 859b480d5afd ("vhost: add guest offload setting") > > Signed-off-by: David Marchand > --- > Changes since v1: > - updated vhost example, > - restored VIRTIO_NET_HDR_GSO_ECN and VIRTIO_NET_HDR_GSO_UDP support, > - restored log on buggy offload request, > > --- > examples/vhost/main.c | 42 +++++++------ > lib/vhost/virtio_net.c | 139 +++++++++++++++++------------------------ > 2 files changed, 78 insertions(+), 103 deletions(-) > [...] > - if (l4_hdr && hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { > + /* GSO request, save required information in mbuf */ > + if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) { > + /* Check unsupported modes */ > + if (hdr->gso_size == 0) > + return -EINVAL; > + > switch (hdr->gso_type & ~VIRTIO_NET_HDR_GSO_ECN) { > case VIRTIO_NET_HDR_GSO_TCPV4: > case VIRTIO_NET_HDR_GSO_TCPV6: > - tcp_hdr = l4_hdr; > - m->ol_flags |= PKT_TX_TCP_SEG; > - m->tso_segsz = hdr->gso_size; > - m->l4_len = (tcp_hdr->data_off & 0xf0) >> 2; > - break; > case VIRTIO_NET_HDR_GSO_UDP: > - m->ol_flags |= PKT_TX_UDP_SEG; > + m->ol_flags |= PKT_RX_LRO | PKT_RX_L4_CKSUM_NONE; My understanding of the virtio 1.1 spec is that GSO can be used independently of CSUM. There is nothing preventing to send a fully checksummed TSO packet. Anyways, that's unusual and not the goal of this patch. Acked-by: Flavio Leitner fbl > + /* Update mss lengths in mbuf */ > m->tso_segsz = hdr->gso_size; > - m->l4_len = sizeof(struct rte_udp_hdr); > break; > default: > VHOST_LOG_DATA(WARNING, > "unsupported gso type %u.\n", hdr->gso_type); > - break; > + return -EINVAL; > } > } > + > + return 0; > } > > static __rte_noinline void > @@ -2084,8 +2054,11 @@ copy_desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq, > prev->data_len = mbuf_offset; > m->pkt_len += mbuf_offset; > > - if (hdr) > - vhost_dequeue_offload(hdr, m); > + if (hdr && vhost_dequeue_offload(hdr, m) < 0) { > + VHOST_LOG_DATA(ERR, "Packet with invalid offloads.\n"); > + error = -1; > + goto out; > + } > > out: > > -- > 2.23.0 > -- fbl