From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id DBD59A00C3; Sat, 16 May 2020 07:58:19 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A9FD91DA18; Sat, 16 May 2020 07:58:18 +0200 (CEST) Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) by dpdk.org (Postfix) with ESMTP id 760071DA15 for ; Sat, 16 May 2020 07:58:17 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1589608696; 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=MXaWLsriGoOHfHzP2dWFuPBbXFTz3dnGwjYGgdHV6RY=; b=HevCMMRDZ8pL0iurN2ZSu3R736H1X4QyqjTdlT4035Dd6OfYckDc2TNRizj4zbW3Q9FLPt RSWOYjj5S1POleZn8zTOOq0qR0BXhv3RRYMu3But2tyZUYsCl9oxD/CL2RXxBU4fGnYFwA HpRTHlTnsjIFsNPEzSybRuSeR8D2jQM= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-13-k_gQXKQTM9ujksq2a0PP4A-1; Sat, 16 May 2020 01:58:07 -0400 X-MC-Unique: k_gQXKQTM9ujksq2a0PP4A-1 Received: by mail-wr1-f69.google.com with SMTP id z16so2230779wrq.21 for ; Fri, 15 May 2020 22:58:07 -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=MXaWLsriGoOHfHzP2dWFuPBbXFTz3dnGwjYGgdHV6RY=; b=f348Yyi3BU++EEClRuYHXE3fifP7fp/l1e7Gh2/gioF/RlZe3X2aOYduNFSs76Zlj4 EzRwd0ZZi+PC9c0FH4CsL/Xj8Uh9HMlfgRZ6i5RzB34Ugv5zXQ7OGhgkeRdoHjbb9b8X dt8tDoKydgNg0qHVBRkAFT/evQCrC+Un2j50dCXZsGg2kYPpSwX68yRYKaUN1xpA+q5D HOvIaoKLgpOKDuyHj91L23tsQlKNdTC/BpixvsSw+4p0HHQsh7hV0rLXFYb6x9ya2Z9X a7JMOzNxMhvFQ9jcZSAf87b8UaMALcOp2hoOV2IevH3/ljFphLCIyY2vpTTXAGtEwgBL TKww== X-Gm-Message-State: AOAM533eq6s3PQSvDVGBan3N4/KgeEv/7TfjETn7WsMCrdDXL8tKM5fn AQyAotmyVXGHo3HpPRH9acW+N5A51oOEURpvwPG9EcKeKsFDF4qJEy3YIaz7Bh9VRW+r8e3aLIR fCDoGj1HglFnGLiWBAaU= X-Received: by 2002:adf:f40a:: with SMTP id g10mr8522053wro.117.1589608686470; Fri, 15 May 2020 22:58:06 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzDwIQXR5v27fDERipE9vyyac0Hbw0IdNMyNMsUgRRnbwnnEDxUq+ttOgI4ParliJVdeLFZk7JZYwBQeBWbkkY= X-Received: by 2002:adf:f40a:: with SMTP id g10mr8522037wro.117.1589608686172; Fri, 15 May 2020 22:58:06 -0700 (PDT) MIME-Version: 1.0 References: <20200514115647.23604-1-gmuthukr@redhat.com> <5e2fd0dc-4810-1586-47ee-dc036effca5a@redhat.com> In-Reply-To: From: Gowrishankar Muthukrishnan Date: Sat, 16 May 2020 11:27:55 +0530 Message-ID: To: Maxime Coquelin Cc: dev@dpdk.org, Flavio Leitner , Tiwei Bie , Zhihong Wang X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] net/virtio: include host features supported in guest X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" Posted additional testing info with and without this patch in : https://patchwork.ozlabs.org/project/openvswitch/patch/0c4c167ad644d3dda51b992e51ec1c27b8492457.1589605823.git.gmuthukr@redhat.com/ Thanks, Gowrishankar On Fri, May 15, 2020 at 10:05 PM Gowrishankar Muthukrishnan < gmuthukr@redhat.com> wrote: > > > On Thu, May 14, 2020 at 5:48 PM Maxime Coquelin < > maxime.coquelin@redhat.com> wrote: > >> Hi, >> >> On 5/14/20 1:56 PM, Gowrishankar Muthukrishnan wrote: >> > Virtio pmd driver can not benefit from tso and csum offload >> > as they are not included in negotiation check with host. Add >> > them in virtio dev init and let negotiation decide the fate. >> > >> > Signed-off-by: Gowrishankar Muthukrishnan >> >> >> I think you don't need any patch to achieve that. >> Disabling TSO and csum offload by default is done on-purpose, as it is >> dependent on the application that drivers the Virtio PMD to support it. >> >> To enable offloads with Virtio PMD, your application has to enable by >> setting the proper offloads flags in device's ethdev config. Doing that, >> a re-negotiation will happen with the proper Virtio features added: >> >> Even before application (testpmd for dpdk pmd) or ethtool (kernel > driver) to enable it, the feature should be agreed both by guest and host > as commented further below. > > static int >> virtio_dev_configure(struct rte_eth_dev *dev) >> { >> const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode; >> const struct rte_eth_txmode *txmode = &dev->data->dev_conf.txmode; >> ... >> uint64_t rx_offloads = rxmode->offloads; >> uint64_t tx_offloads = txmode->offloads; >> ... >> >> if (rx_offloads & (DEV_RX_OFFLOAD_UDP_CKSUM | >> DEV_RX_OFFLOAD_TCP_CKSUM)) >> req_features |= (1ULL << VIRTIO_NET_F_GUEST_CSUM); >> >> if (rx_offloads & DEV_RX_OFFLOAD_TCP_LRO) >> req_features |= >> (1ULL << VIRTIO_NET_F_GUEST_TSO4) | >> (1ULL << VIRTIO_NET_F_GUEST_TSO6); >> >> if (tx_offloads & (DEV_TX_OFFLOAD_UDP_CKSUM | >> DEV_TX_OFFLOAD_TCP_CKSUM)) >> req_features |= (1ULL << VIRTIO_NET_F_CSUM); >> >> if (tx_offloads & DEV_TX_OFFLOAD_TCP_TSO) >> req_features |= >> (1ULL << VIRTIO_NET_F_HOST_TSO4) | >> (1ULL << VIRTIO_NET_F_HOST_TSO6); >> >> /* if request features changed, reinit the device */ >> if (req_features != hw->req_guest_features) { >> ret = virtio_init_device(dev, req_features); >> > > In negotiation (virtio_negotiate_features), negotiating elements have to > be in both guest (virtio hw->guest_features) as well as host(vtpci > hw->get_features). > When we do not have offload flags in hw->guest_features but host keeps > ready, host will not be able to enable offload for that socket instance to > guest, following set_features request from guest. > > hw->guest_features = vtpci_negotiate_features(hw, host_features << the > list with offload that host is offering); > vtpci_negotiate_features: > features = host_features & hw->guest_features << which is default list > of features w/o offload; > VTPCI_OPS(hw)->set_features(hw, features); > > Again, as far as the host is considered, it again depends on > whether vhostuser or vhostuserclient socket is connecting guest, but should > not be a problem for guest. > > vhostuser socket: > when backend tso is enabled (userspace tso), virtio bits are not removed > in negotiation with the guest which is not an issue for enabling offload > flags in front end negotiating set. > otherwise, these bits are removed, so vnic/testpmd would not be able to > set. > > vhostuserclient socket: > supported features (including offload bits) are enabled when backend > tso is enabled through dpdk vhost driver and hence vnic/testpmd will see > the same in get features call. > > Am I missing something ?. > > Thanks, > Gowrishankar > > } >> ... >> >> >> >> > -- >> > This patch has been tested with TSO tests in OVS-DPDK: >> > >> > https://patchwork.ozlabs.org/project/openvswitch/list/?series=176886 >> > >> > ## ------------------------------- ## >> > ## openvswitch 2.13.90 test suite. ## >> > ## ------------------------------- ## >> > >> > OVS-DPDK unit tests >> > >> > 1: OVS-DPDK - EAL init ok >> > 2: OVS-DPDK - add standard DPDK port ok >> > 3: OVS-DPDK - add vhost-user-client port ok >> > 4: OVS-DPDK - ping vhost-user ports ok >> > 5: OVS-DPDK - ping vhost-user-client ports ok >> > 6: OVS-DPDK - validate tso negotiation ok >> > >> > ## ------------- ## >> > ## Test results. ## >> > ## ------------- ## >> > >> > All 6 tests were successful. >> > >> > --- >> > drivers/net/virtio/virtio_ethdev.c | 4 ++-- >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/net/virtio/virtio_ethdev.c >> b/drivers/net/virtio/virtio_ethdev.c >> > index 044eb10..91f6f16 100644 >> > --- a/drivers/net/virtio/virtio_ethdev.c >> > +++ b/drivers/net/virtio/virtio_ethdev.c >> > @@ -1914,7 +1914,7 @@ static int virtio_dev_xstats_get_names(struct >> rte_eth_dev *dev, >> > } >> > >> > /* reset device and negotiate default features */ >> > - ret = virtio_init_device(eth_dev, >> VIRTIO_PMD_DEFAULT_GUEST_FEATURES); >> > + ret = virtio_init_device(eth_dev, >> VIRTIO_PMD_SUPPORTED_GUEST_FEATURES); >> > if (ret < 0) >> > goto err_virtio_init; >> > >> > @@ -2064,7 +2064,7 @@ static int eth_virtio_pci_remove(struct >> rte_pci_device *pci_dev) >> > int ret; >> > >> > PMD_INIT_LOG(DEBUG, "configure"); >> > - req_features = VIRTIO_PMD_DEFAULT_GUEST_FEATURES; >> > + req_features = VIRTIO_PMD_SUPPORTED_GUEST_FEATURES; >> > >> > if (rxmode->mq_mode != ETH_MQ_RX_NONE) { >> > PMD_DRV_LOG(ERR, >> > >> >> > > -- > Gowrishankar M > -- Gowrishankar M