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 7C7FAA0A02; Tue, 27 Apr 2021 19:09:33 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0A2F5410D8; Tue, 27 Apr 2021 19:09:33 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 412E94003E for ; Tue, 27 Apr 2021 19:09:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1619543370; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=SdosuQU3r3lyJABjCnAYZ83Ro8Rdn7UvooyyPq6DrV4=; b=KX6T91yU4j8J+IgnB2QMLUOdI9vTmSW57BY0z7BiVmXYALvMo4bi5EUtXosysrocCR9xxz FQUeKQdEIdAtN+68byGQzOtJjiquBZ5RTun8gA+to1sGHi3JZ0x41R6W7nQjcK+dT7CJjB yhfSXml/I5YweGW1YPT0TfSQdBsknfc= Received: from mail-ua1-f71.google.com (mail-ua1-f71.google.com [209.85.222.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-424-DrfkHHfROuWxiqoPf2ueFg-1; Tue, 27 Apr 2021 13:09:25 -0400 X-MC-Unique: DrfkHHfROuWxiqoPf2ueFg-1 Received: by mail-ua1-f71.google.com with SMTP id t3-20020a9f35830000b02901e721b80f49so5781196uad.23 for ; Tue, 27 Apr 2021 10:09:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=SdosuQU3r3lyJABjCnAYZ83Ro8Rdn7UvooyyPq6DrV4=; b=GObi9bTnm+n511k+ggG6dU/Wchrioi1Fnio4VoA0IVNBInrSCneye1jHfn+uCfo3c+ +YXI9guViXU26Tad8/bdVpDD8JICc93zqIxyeu5MO8GxMwcGFjofpZ0OjAgUqdq210SG Erup3SmrCWkQfILaep/CVhKj7fxA6QYxYoPwDcKXI9O2S2BPg7QED5uOSIFgVz2vQReE wdCh7h3M5zfxwBSjlhufvDxjUTwV0DuFxWG3kQIX5CpJHZzkRhNzyGYmO1vrJA19LLkS GKtk1UwMVwqNClCr2xLYNCUrrM0pLrnP4QVs4CBq6fixdb9b+u1jFtOzFkajnILbZq8W xksw== X-Gm-Message-State: AOAM530fJCW1jAGAnE1USI3uRVvay2sL5PDqzZu+2Stbf0EG+uJ4m+Q4 KwDckIlVULu+GCMIx1hLJcbuePc3W6oZuOiSgsgBY3VegaCwW788PIxs2OpePdp9rNHwaL19sYB gU1b1D7wKMMTvtGzs0RE= X-Received: by 2002:ab0:23cf:: with SMTP id c15mr7742166uan.126.1619543364612; Tue, 27 Apr 2021 10:09:24 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwC74hDKBYwA1tHJz9ItLG2HtoE4Xf3B7zVzPZX31AbGPcKCRaFypdlZPTuICflAIZP6zl5PlV2jYT6FoP9R2w= X-Received: by 2002:ab0:23cf:: with SMTP id c15mr7742135uan.126.1619543364348; Tue, 27 Apr 2021 10:09:24 -0700 (PDT) MIME-Version: 1.0 References: <20210401095243.18211-1-david.marchand@redhat.com> <20210401095243.18211-6-david.marchand@redhat.com> <15cf74f2-4ec2-a641-9bdb-8d93a135a216@redhat.com> In-Reply-To: <15cf74f2-4ec2-a641-9bdb-8d93a135a216@redhat.com> From: David Marchand Date: Tue, 27 Apr 2021 19:09:13 +0200 Message-ID: To: Maxime Coquelin Cc: Flavio Leitner , dev , Olivier Matz , Ilya Maximets , Chenbo Xia , Jijiang Liu , Yuanhan Liu Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" 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" On Tue, Apr 13, 2021 at 5:27 PM Maxime Coquelin wrote: > On 4/8/21 8:38 PM, Flavio Leitner wrote: > > 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") > > > > There is change that before ECN was ignored and now it is invalid. > > I think that's the right way to go, but not sure if virtio blocks > > the negotiation of that feature. > > No, I just tested and the feature gets negotiated. I suppose you tested with testpmd, because I can see ECN is disabled by default with OVS. > > Disabling it in Vhost lib should be avoided to avoid breaking > live-migration. > > It might be safer to revert back to older behavior for it, i.e. just > ignore the bit. I don't think it is ever set, because otherwise we would > have had lots of reports since the Vhost log would be flooded with: - The VIRTIO_NET_HDR_GSO_ECN bit is supposed to be coupled with TSO bits. Copying a bit more of this code: switch (hdr->gso_type & ~VIRTIO_NET_HDR_GSO_ECN) { ... default: > > VHOST_LOG_DATA(WARNING, > "unsupported gso type %u.\n", hdr->gso_type); The absence of log does not mean the guest is not sending packets with VIRTIO_NET_HDR_GSO_ECN set. Otoh, getting this log instead indicates a bug in the virtio driver (as we discussed offlist). - It is not clear to me how deployed the ECN feature is. I think the Linux kernel won't try to start a TCP connection unless explicitly configuring it on a socket (but I am a bit lost). By default, VIRTIO_NET_F_HOST_ECN is announced as supported by vhost-user. So in theory, a guest virtio netdevice with NETIF_F_TSO_ECN can transmit packet (with SKB_GSO_TCP_ECN translated to VIRTIO_NET_HDR_GSO_ECN in virtio_net_hdr_from_skb) to a vhost-user backend. - Treating ECN with GSO requires special handling: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b0da8537037f337103348f239ad901477e907aa8 I can see some change in the i40e kernel driver at least. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=059dab69652da3525d320d77ac5422ec708ced14 The ixgbe kernel driver is not flagged with NETIF_F_TSO_ECN. We don't have such a distinction in DPDK: neither a per mbuf flag to mark packets, nor a device offloading flag/capability. And the rte_gso library probably does not handle correctly CWR. About the i40e driver, I can't find the same configuration than the kernel driver. - Now, about the next step... The "good" (I suppose you might disagree here) news, is that this feature is disabled in OVS: https://github.com/openvswitch/ovs/blob/master/lib/netdev-dpdk.c#L5162 About handling TSO + ECN, this is a generic problem with the DPDK API and we have been living for a long time. I understand passing such packets to hw that does not handle this correctly breaks the ECN feature not work properly. But "normal" TSO works. I agree, we can let such packets be received by vhost like it was done before my patch. Investigating the other side (GUEST_ECN + the virtio pmd) could be worth later, as I think GSO+ECN packets are dropped in the current code. -- David Marchand