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 35691A0579; Thu, 8 Apr 2021 10:28:59 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1AE99141119; Thu, 8 Apr 2021 10:28:59 +0200 (CEST) Received: from mail-wr1-f49.google.com (mail-wr1-f49.google.com [209.85.221.49]) by mails.dpdk.org (Postfix) with ESMTP id B2AAE40698 for ; Thu, 8 Apr 2021 10:28:57 +0200 (CEST) Received: by mail-wr1-f49.google.com with SMTP id q26so1143564wrz.9 for ; Thu, 08 Apr 2021 01:28:57 -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:user-agent; bh=fsuHXNbyW8jUyyLYFNpIDLo6Ud9lqE/wKFiqOnyN+hc=; b=VRD+t6iCneBuNgvha/m68seah4AzOSQlj5a/mC/ol+MVqn0XOukZpkwaIlcz5mkNJ5 mFsUhxDTSndi+xCF6gr+syqV37LC7E29V4VThVOkpBFt8fiUsYqXYb93oE3Dwk5YvCtJ En6oQZwcC5sgdg6Vu1+B4PE2VZUHGrMRuk7o5+yM221YdOODXCskYVGR1kQQJGEFUsZS nKNNab3amXepBEsDGXeaxdU2HrIifh/MJV939+50sYhYQQNnN75NjbooA4CsKbi6pLg1 Qh/7T5JIBEaOUzliuEUGOXViPkryQwevD1CQ26DUZj/0QDR5PoJDtf3T9oJvCiT1GKaC DyGA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=fsuHXNbyW8jUyyLYFNpIDLo6Ud9lqE/wKFiqOnyN+hc=; b=PBHyT17AMUormhFLhoIf4XH+5eZKPczMzJ+ZZHh//L2/h7i1p8Jn1GoIYg0H+CAh03 s2vPYlxA/gc+BN4mp4/Q100jqQRYwKUZP44QBdoCrJGn2N1mQizd1qx3H7hv3D1Rf8+1 CA9gKNRBEscDYc5PyzTGhHnqRUvpWW6Fr7HG32bw001lJhULYcejUvDp7geGzNwC9wkY WjvTScch60x9zUJDg0RnVWy5dJjI7zwpR320SMPoTVBZ0BSYQSUMiks/KBmKB4BF+sEa W5aMAs1DOEc7B4466jvq2HP01z24ilZJhOnc/KnrAXOW9cOfpxrjnxrwCgcA6UH2WQ6x KgMA== X-Gm-Message-State: AOAM5305a0boryI5qmIttBH0RvWjWuyWilY7A7uuQOgDJYrEMKhqNsyF BaCeuN2326q8zBjSkTAC51PT1w== X-Google-Smtp-Source: ABdhPJzlOK6QDFrU+ZXo6PZCA8gpFCgL+Amv1zZ2wnfNMUFKg21Wkqlf/bgkhIX7ps3uOZrJdfgQyw== X-Received: by 2002:adf:cd0a:: with SMTP id w10mr9575165wrm.39.1617870537498; Thu, 08 Apr 2021 01:28:57 -0700 (PDT) Received: from 6wind.com ([2a01:e0a:5ac:6460:c065:401d:87eb:9b25]) by smtp.gmail.com with ESMTPSA id y17sm11408683wmo.42.2021.04.08.01.28.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Apr 2021 01:28:57 -0700 (PDT) Date: Thu, 8 Apr 2021 10:28:56 +0200 From: Olivier Matz To: David Marchand Cc: dev@dpdk.org, maxime.coquelin@redhat.com, fbl@sysclose.org, i.maximets@ovn.org, Chenbo Xia , Jijiang Liu , Yuanhan Liu Message-ID: <20210408082856.GS1650@platinum> References: <20210401095243.18211-1-david.marchand@redhat.com> <20210401095243.18211-6-david.marchand@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210401095243.18211-6-david.marchand@redhat.com> User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [PATCH 5/5] 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" Hi David, On Thu, Apr 01, 2021 at 11:52:43AM +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. > > The vhost example needs a reworking as it was built with the assumption > that mbuf TSO configuration is set up by the vhost library. > This is not done in this patch for now so TSO activation is forcibly > refused. > > Fixes: 859b480d5afd ("vhost: add guest offload setting") > > Signed-off-by: David Marchand > --- Reviewed-by: Olivier Matz LGTM, just one little comment below. <...> > + m->ol_flags |= PKT_RX_IP_CKSUM_UNKNOWN; > + > + ptype = rte_net_get_ptype(m, &hdr_lens, RTE_PTYPE_ALL_MASK); > + m->packet_type = ptype; > + if ((ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_TCP || > + (ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_UDP || > + (ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_SCTP) > + l4_supported = 1; > + > + if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) { > + hdrlen = hdr_lens.l2_len + hdr_lens.l3_len + hdr_lens.l4_len; > + if (hdr->csum_start <= hdrlen && l4_supported) { > + m->ol_flags |= PKT_RX_L4_CKSUM_NONE; > + } else { > + /* Unknown proto or tunnel, do sw cksum. We can assume > + * the cksum field is in the first segment since the > + * buffers we provided to the host are large enough. > + * In case of SCTP, this will be wrong since it's a CRC > + * but there's nothing we can do. > + */ > + uint16_t csum = 0, off; > + > + if (rte_raw_cksum_mbuf(m, hdr->csum_start, > + rte_pktmbuf_pkt_len(m) - hdr->csum_start, > + &csum) < 0) > + return -EINVAL; > + if (likely(csum != 0xffff)) > + csum = ~csum; I was trying to remember the reason for this last test (which is also present in net/virtio). If this is a UDP checksum (on top of an unrecognized tunnel), it's indeed needed to do that, because we don't want to set the checksum to 0 in the packet (which means "no checksum" for UDPv4, or is fordidden for UDPv6). If this is something else than UDP, it shouldn't hurt to have a 0xffff in the packet instead of 0. Maybe it deserves a comment here, like: /* avoid 0 checksum for UDP, shouldn't hurt for other protocols */ What do you think?