From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f42.google.com (mail-pa0-f42.google.com [209.85.220.42]) by dpdk.org (Postfix) with ESMTP id 909654A63 for ; Mon, 5 Sep 2016 23:08:14 +0200 (CEST) Received: by mail-pa0-f42.google.com with SMTP id id6so5819651pad.3 for ; Mon, 05 Sep 2016 14:08:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=aRHG4DbFayubPwLR/HkeOI1r+3Y/O4dLT4jGa+Dj/Iw=; b=Yu8RvwNTQAi48ZutQlz+1AtU18RYoHe1OIZFqHnnaOb7DZKANuQhMfhREs8F4X5FpD FUdc8US0h/N0rDmXULKrJgtcQnIPwcIWLhEjeQBusanNwtoC5yIooe/oFV+89uUM3UI9 +5ZkEVaIowd3wSiAhA3PMtjR0lLY0fsOxGGhPREPd/YQ/f1urs4p7ftxAMQ1A+gLq9Ch CqiHov35XLezlMv4BH9rGiUgmV7In/ElReyA+uoAhkvc774qGNTnbXW1ZGKhTO57ID9q S34udRXZAWLHVsEbuwYkVPYM/3Q5HJFYeMa2TlSF9y/38yWj0GvAKlsspghfr8Q3Ykas Jw5Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=aRHG4DbFayubPwLR/HkeOI1r+3Y/O4dLT4jGa+Dj/Iw=; b=FjSxEkqqC2cGazGle0zBJGpPY/En7F5J6uiW8euJFsGwyvnsJ6mp65CVJGyyOnljD7 SjOjvmQw880tQfMNIgoyqrknc4vBLZhPM9jcf1Npok/QQSPJGzKZrV7ff28CjStQ37y6 teLyvZOoodHf+BAo67B+UnV2Ba7OwifLDwAKA9ZuGFwApo5qbzC9DAunie/jREaVx0Vo fWAnTk4g9wLYyr4umxBj6aQP+Htv9E4lrxIvtrzyq5R9Yit4ByLXJ4bVYTf0tcM+nRe5 M4rm0wt7e6GC19O4sLKKCWDKTkw0f1UfzwA9pUTnI88r6DAFtmVXJMwLhdCh5prKztYF +4cg== X-Gm-Message-State: AE9vXwN+i51cFXcg9rm+Qes0uOnbrxYHCH/G1IQFVb5Ww0FnXDYJDihdpb88kXHH1sSYhg== X-Received: by 10.66.197.131 with SMTP id iu3mr66322773pac.97.1473109693711; Mon, 05 Sep 2016 14:08:13 -0700 (PDT) Received: from xeon-e3 (static-50-53-69-251.bvtn.or.frontiernet.net. [50.53.69.251]) by smtp.gmail.com with ESMTPSA id m5sm35543297paw.40.2016.09.05.14.08.13 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 05 Sep 2016 14:08:13 -0700 (PDT) Date: Mon, 5 Sep 2016 14:08:24 -0700 From: Stephen Hemminger To: Maxime Coquelin Cc: "Pierre Pfister (ppfister)" , Yuanhan Liu , "dev@dpdk.org" Message-ID: <20160905140824.1f0c80a2@xeon-e3> In-Reply-To: <1dfca8cf-e5ef-1040-8a40-d617ed03e5c0@redhat.com> References: <516F65D3-4706-4CC5-916B-6ECD29CBE177@cisco.com> <20160905022028.GF30752@yliu-dev.sh.intel.com> <6EFF45F1-172E-4470-B4D7-AED90474DE50@cisco.com> <1dfca8cf-e5ef-1040-8a40-d617ed03e5c0@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v2] virtio: enable indirect descriptors feature X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Sep 2016 21:08:14 -0000 On Mon, 5 Sep 2016 16:24:13 +0200 Maxime Coquelin wrote: > Thanks Pierre for sending the fix. >=20 > Minor comments below: >=20 > On 09/05/2016 08:52 AM, Pierre Pfister (ppfister) wrote: > > Indirect descriptors support was disabled by commit 4a92b67151be11, > > presumably by accident as it was correctly supported. > > > > This patch simply adds VIRTIO_RING_F_INDIRECT_DESC back to > > the supported features bit mask, hence enabling the use of > > indirect descriptors when the feature is negociated with the > > device. > > =20 >=20 > You should add the below line: > Fixes: 4a92b671 ("virtio: clarify feature bit handling") >=20 > Also, maybe we should consider add stable@dpdk.org in cc:, > because the regression was introduced before v16.07 final tag. > But the problem is that all the final validation has been done > without this feature enabled, and it impact quite a few lines of > code in Virtio PMD. >=20 > Other than that, you can add: > Reviewed-by: Maxime Coquelin The patch is correct, but it doesn't fix a regression. The original virtio DPDK did not support INDIRECT descriptors at all. The original code in virtio_negotiate features was the inverse of what it i= s now. Read carefully, the values in mask were the bits that were rejected during guest negotiation at the time. static void virtio_negotiate_features(struct virtio_hw *hw) { - uint32_t host_features, mask; - - /* checksum offload not implemented */ - mask =3D VIRTIO_NET_F_CSUM | VIRTIO_NET_F_GUEST_CSUM; - - /* TSO and LRO are only available when their corresponding - * checksum offload feature is also negotiated. - */ - mask |=3D VIRTIO_NET_F_HOST_TSO4 | VIRTIO_NET_F_HOST_TSO6 | VIRTIO_NET_F_= HOST_ECN; - mask |=3D VIRTIO_NET_F_GUEST_TSO4 | VIRTIO_NET_F_GUEST_TSO6 | VIRTIO_NET_= F_GUEST_ECN; - mask |=3D VTNET_LRO_FEATURES; - - /* not negotiating INDIRECT descriptor table support */ - mask |=3D VIRTIO_RING_F_INDIRECT_DESC; + uint32_t host_features; =20 /* Prepare guest_features: feature that driver wants to support */ - hw->guest_features =3D VTNET_FEATURES & ~mask; + hw->guest_features =3D VIRTIO_PMD_GUEST_FEATURES; PMD_INIT_LOG(DEBUG, "guest_features before negotiate =3D %x", hw->guest_features); =20 Therefore INDIRECT descriptors were always disabled! Don't blame any commi= t. Use of indirect descriptors by DPDK did not happen until a later change. commit 6dc5de3a6aefba3946fe04368d93994db3f7a5fd Author: Stephen Hemminger Date: Fri Mar 4 10:19:19 2016 -0800 virtio: use indirect ring elements =20 The virtio ring in QEMU/KVM is usually limited to 256 entries and the normal way that virtio driver was queuing mbufs required nsegs + 1 ring elements. By using the indirect ring element feature if available, each packet will take only one ring slot even for multi-segment packets. =20 Signed-off-by: Stephen Hemminger Acked-by: Yuanhan Liu Acked-by: Huawei Xie