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 EA95FA0C4B; Fri, 15 Oct 2021 10:32:44 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7742140692; Fri, 15 Oct 2021 10:32:44 +0200 (CEST) Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) by mails.dpdk.org (Postfix) with ESMTP id 3D7BC40041 for ; Fri, 15 Oct 2021 10:32:43 +0200 (CEST) Received: by mail-wm1-f41.google.com with SMTP id l38-20020a05600c1d2600b0030d80c3667aso1757403wms.5 for ; Fri, 15 Oct 2021 01:32:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=ZgrsGX6mNw2cscEbX9c9IByTZIOWEfIIclOu7E+7B8Q=; b=IfckA8Sa/6BmcZ/TnguXxb/d7u3pPt2hAhfmc1mMmb/e1kaQ3htNCZE3y+U7XWEoHp 8hbFsmW0PzWyc/itZEZa7ddugd0Ps+b2HpEudQIwL5KLsbsmKMS1439gS4c5F4aFZjYK 71PfY5f3OW0swHJWLVFeVTzxg2cCkjFholcFHHF1BDTh2gdcXdp7ibhz5TO0SlS8/RZY n87BFSbFUbSZeUv9uzPSgSBHEikh99kiPsBH3DusNNXUas2Y8tDW6SHd3L3mTTIRwbRT Kw7vTHcoOngMoHRFBb7Llt+zwmcDPo+WR3oJUz7N4kaZ2koeRcu+2hKIC5iN7+bJMiwl 5miw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=ZgrsGX6mNw2cscEbX9c9IByTZIOWEfIIclOu7E+7B8Q=; b=4ILTLC45pcoP+i0x++S4sSwdMl5sG9wvmllR5tZ6RuanxlRFkDRJxHvY9pAqqyOTor Kric2hL2lyi8P3drrcFRIBsZRBTiQDj3ocyaVoYzYroxSW2vUv1s6dHmZrQAKGEWIWn4 p7uINqf5Ca5o3YhGKwlar1uWtvxQBEvXfvF3zEtGyXYjTnyJKx74w9tTUcfNeuQDOV9U Z0V3PRBeROk2L45pXH+Pfyq1Sq9pT4+TgIs4wibq7iSGM1R/mYO/KXL02KhZkLg4j3o7 Fkw5xUPBHe1P4BYVOfbgkICtHhVnY7L+VV0mS6VMiT8o4KZPR83rrGr4DcZSbbYJKTp1 leLA== X-Gm-Message-State: AOAM530uNjFsnEHOpOO5H42skIvsT/Dbh5IH4Mel3beSP00RvVqHascw X3A5C1rTXNu5oEQrw6HfIWZtCQ== X-Google-Smtp-Source: ABdhPJzbfZwtvm2kGKUYtVdNXOJp5t3TTvWYYj8rBeAovJNv6xvizgF0oigA0n1eCwwFw0FOAhZSbw== X-Received: by 2002:a1c:f00a:: with SMTP id a10mr11131213wmb.112.1634286762946; Fri, 15 Oct 2021 01:32:42 -0700 (PDT) Received: from 6wind.com ([2a01:e0a:5ac:6460:c065:401d:87eb:9b25]) by smtp.gmail.com with ESMTPSA id j13sm4294352wro.97.2021.10.15.01.32.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 15 Oct 2021 01:32:42 -0700 (PDT) Date: Fri, 15 Oct 2021 10:32:41 +0200 From: Olivier Matz To: "Xia, Chenbo" Cc: Ivan Malov , "dev@dpdk.org" , Maxime Coquelin , "stable@dpdk.org" , Andrew Rybchenko , Yuanhan Liu Message-ID: References: <20210830142655.18373-1-ivan.malov@oktetlabs.ru> <20210916184955.2755-1-ivan.malov@oktetlabs.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH v2] net/virtio: handle Tx checksums correctly for tunnel packets 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, Oct 14, 2021 at 07:12:29AM +0000, Xia, Chenbo wrote: > > -----Original Message----- > > From: Ivan Malov > > Sent: Friday, September 17, 2021 2:50 AM > > To: dev@dpdk.org > > Cc: Maxime Coquelin ; stable@dpdk.org; Andrew > > Rybchenko ; Xia, Chenbo ; > > Yuanhan Liu ; Olivier Matz > > > > Subject: [PATCH v2] net/virtio: handle Tx checksums correctly for tunnel > > packets > > > > Tx prepare method calls rte_net_intel_cksum_prepare(), which > > handles tunnel packets correctly, but Tx burst path does not > > take tunnel presence into account when computing the offsets. > > > > Fixes: 58169a9c8153 ("net/virtio: support Tx checksum offload") > > Cc: stable@dpdk.org > > > > Signed-off-by: Ivan Malov > > Reviewed-by: Andrew Rybchenko > > --- > > drivers/net/virtio/virtqueue.h | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h > > index 03957b2bd0..b83ff32efb 100644 > > --- a/drivers/net/virtio/virtqueue.h > > +++ b/drivers/net/virtio/virtqueue.h > > @@ -620,19 +620,21 @@ static inline void > > virtqueue_xmit_offload(struct virtio_net_hdr *hdr, struct rte_mbuf *cookie) > > { > > uint64_t csum_l4 = cookie->ol_flags & PKT_TX_L4_MASK; > > + uint16_t o_l23_len = (cookie->ol_flags & PKT_TX_TUNNEL_MASK) ? > > + cookie->outer_l2_len + cookie->outer_l3_len : 0; > > > > if (cookie->ol_flags & PKT_TX_TCP_SEG) > > csum_l4 |= PKT_TX_TCP_CKSUM; > > > > switch (csum_l4) { > > case PKT_TX_UDP_CKSUM: > > - hdr->csum_start = cookie->l2_len + cookie->l3_len; > > + hdr->csum_start = o_l23_len + cookie->l2_len + cookie->l3_len; > > hdr->csum_offset = offsetof(struct rte_udp_hdr, dgram_cksum); > > hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM; > > break; > > > > case PKT_TX_TCP_CKSUM: > > - hdr->csum_start = cookie->l2_len + cookie->l3_len; > > + hdr->csum_start = o_l23_len + cookie->l2_len + cookie->l3_len; > > hdr->csum_offset = offsetof(struct rte_tcp_hdr, cksum); > > hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM; > > break; > > @@ -650,7 +652,8 @@ virtqueue_xmit_offload(struct virtio_net_hdr *hdr, struct > > rte_mbuf *cookie) > > VIRTIO_NET_HDR_GSO_TCPV6 : > > VIRTIO_NET_HDR_GSO_TCPV4; > > hdr->gso_size = cookie->tso_segsz; > > - hdr->hdr_len = cookie->l2_len + cookie->l3_len + cookie->l4_len; > > + hdr->hdr_len = o_l23_len + cookie->l2_len + cookie->l3_len + > > + cookie->l4_len; > > } else { > > ASSIGN_UNLESS_EQUAL(hdr->gso_type, 0); > > ASSIGN_UNLESS_EQUAL(hdr->gso_size, 0); > > -- > > 2.20.1 > > Reviewed-by: Chenbo Xia > I have one comment to mention that from application perspective, it has to take care that the driver does not support outer tunnel offload (this matches the advertised capabilities). For instance, in case of a vxlan tunnel, if the outer checksum needs to be calculated, it has to be done by the application. In short, the application can ask to offload the inner part if no offload is required on the outer part. Also, since grep "PKT_TX_TUNNEL" in driver/net/ixgbe gives nothing, it seems the ixgbe driver does not support the same offload request than described in this patch: (m->ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_XXXXX m->outer_l2_len = outer l2 length m->outer_l3_len = outer l3 length m->l2_len = outer l4 length + tunnel len + inner l2 len m->l3_len = inner l3 len m->l4_len = inner l4 len An alternative for doing the same (that would work with ixgbe and current virtio) is to give: (m->ol_flags & PKT_TX_TUNNEL_MASK) == 0 m->l2_len = outer lengths + tunnel len + inner l2 len m->l3_len = inner l3 len m->l4_len = inner l4 len I think a capability may be missing to differentiate which drivers support which mode. Or, all drivers could be fixed to support both modes (and this would make this patch valid). Thanks, Olivier