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 793A4A0C4C for ; Fri, 15 Oct 2021 10:32:45 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 65841411B6; Fri, 15 Oct 2021 10:32:45 +0200 (CEST) Received: from mail-wm1-f48.google.com (mail-wm1-f48.google.com [209.85.128.48]) by mails.dpdk.org (Postfix) with ESMTP id 4774C40692 for ; Fri, 15 Oct 2021 10:32:43 +0200 (CEST) Received: by mail-wm1-f48.google.com with SMTP id g2so1827230wme.4 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=pLphTEBM2+Tq4Ok4yi3UkDWFXWLkXNgyI5eDPJkVPCsE015JEY1rGOvpZ97dVKS4pe Zy8c6bY2QhcHnx47Y2g0C3Gg785Je6PleUoUbu/f5Rf7eby5lrG5FF0pfetONOiKKRW2 BELmxdijJ3m8INE3m35IUTqyVe3oyMsTt9EcvYv8OjKqukFzabqEJrPQ7yIbhBHU8dGY t9yPKinxGfaAQKNTDeS/9IAeCnrmGXpF7+WnjCO1P6XG4wuVRyxrsSymvG0DceJqG1p6 brFaUeYuW/spVIFnzMl8QxmmXPgoexuK5jHTb0aZGq5h34sHIh1SB0YGaBKECooiF2ge sU/g== X-Gm-Message-State: AOAM5326Tvp3xmEiEK9EKDKwToMD8vS3purWLjRJt+Q88nwxgQtW6obD FBUtkFCI6wx2P/DqV7s9IVqaaw== 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-stable] [PATCH v2] net/virtio: handle Tx checksums correctly for tunnel packets X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" 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