From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id F135E5681 for ; Mon, 5 Sep 2016 16:24:17 +0200 (CEST) Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A69493D1D7; Mon, 5 Sep 2016 14:24:16 +0000 (UTC) Received: from [10.36.6.145] (vpn1-6-145.ams2.redhat.com [10.36.6.145]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u85EOEYC005475 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 5 Sep 2016 10:24:15 -0400 To: "Pierre Pfister (ppfister)" , Yuanhan Liu References: <516F65D3-4706-4CC5-916B-6ECD29CBE177@cisco.com> <20160905022028.GF30752@yliu-dev.sh.intel.com> <6EFF45F1-172E-4470-B4D7-AED90474DE50@cisco.com> Cc: "dev@dpdk.org" From: Maxime Coquelin Message-ID: <1dfca8cf-e5ef-1040-8a40-d617ed03e5c0@redhat.com> Date: Mon, 5 Sep 2016 16:24:13 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <6EFF45F1-172E-4470-B4D7-AED90474DE50@cisco.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Mon, 05 Sep 2016 14:24:16 +0000 (UTC) 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 14:24:18 -0000 Thanks Pierre for sending the fix. Minor comments below: 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. > You should add the below line: Fixes: 4a92b671 ("virtio: clarify feature bit handling") 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. Other than that, you can add: Reviewed-by: Maxime Coquelin > Signed-off-by: Pierre Pfister > --- > drivers/net/virtio/virtio_ethdev.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h > index 2ecec6e..31c91a5 100644 > --- a/drivers/net/virtio/virtio_ethdev.h > +++ b/drivers/net/virtio/virtio_ethdev.h > @@ -63,6 +63,7 @@ > 1u << VIRTIO_NET_F_CTRL_RX | \ > 1u << VIRTIO_NET_F_CTRL_VLAN | \ > 1u << VIRTIO_NET_F_MRG_RXBUF | \ > + 1u << VIRTIO_RING_F_INDIRECT_DESC | \ > 1ULL << VIRTIO_F_VERSION_1) > > /* > -- > 2.7.4 (Apple Git-66) > One off-topic question, do you measure some improvement in perfs using the feature? If yes, could you describe the use-case, and the figures? I ask because I have implemented TX indirect descriptor in vhost lib (see http://dpdk.org/dev/patchwork/patch/14797/), and failed to see some use-case benefiting of it. Regards, Maxime