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 893191075 for ; Thu, 16 Mar 2017 10:18:26 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7EB2480F98; Thu, 16 Mar 2017 09:18:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 7EB2480F98 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=maxime.coquelin@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 7EB2480F98 Received: from [10.36.116.177] (ovpn-116-177.ams2.redhat.com [10.36.116.177]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C549A6031D; Thu, 16 Mar 2017 09:18:24 +0000 (UTC) To: Yuanhan Liu References: <1488534682-3494-1-git-send-email-yuanhan.liu@linux.intel.com> <1488534682-3494-2-git-send-email-yuanhan.liu@linux.intel.com> <337dbdcd-0913-dd1f-5cba-15844a7380b1@redhat.com> <20170316070807.GO18844@yliu-dev.sh.intel.com> Cc: dev@dpdk.org, Harris James R , Liu Changpeng From: Maxime Coquelin Message-ID: Date: Thu, 16 Mar 2017 10:18:21 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <20170316070807.GO18844@yliu-dev.sh.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Thu, 16 Mar 2017 09:18:26 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH 01/17] vhost: introduce driver features related APIs 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: , X-List-Received-Date: Thu, 16 Mar 2017 09:18:26 -0000 On 03/16/2017 08:08 AM, Yuanhan Liu wrote: > On Tue, Mar 14, 2017 at 10:53:23AM +0100, Maxime Coquelin wrote: >> >> >> On 03/14/2017 10:46 AM, Maxime Coquelin wrote: >>> >>> >>> On 03/03/2017 10:51 AM, Yuanhan Liu wrote: >>>> Introduce few APIs to set/get/enable/disable driver features. >>>> >>>> Signed-off-by: Yuanhan Liu >>>> --- >>>> lib/librte_vhost/rte_vhost_version.map | 10 ++++ >>>> lib/librte_vhost/rte_virtio_net.h | 9 ++++ >>>> lib/librte_vhost/socket.c | 90 >>>> ++++++++++++++++++++++++++++++++++ >>>> 3 files changed, 109 insertions(+) >>> >>> Nice! >>> Reviewed-by: Maxime Coquelin >>> >>> I wonder whether we could protect from setting/enabling/disabling >>> features once negotiation is done? > > Those APIs won't be able to change the negotiated features. They are > just some interfaces before the vhost-user connection is established. Right. I meant it could return an error is the connection is already established. Else, the caller might think the feature has been successfully enabled/disabled, whereas it is not the case. But this is maybe over-engineering to handle this case. > Ideally, we could/should get rid of the enabling/disabling functions: > let the vhost-user driver to handle the features directly (say, for > vhost-user pmd, we could use vdev options to disable/enable few specific > features). Once that is done, use the rte_vhost_driver_set_features() > set the features once. Ok, but what about vhost lib users and TSO (I'm thinking of OVS). > The reason I introduced the enable/disable_features() is to keep the > compatability with the builtin vhost-user net driver (virtio_net.c). > If there is a plan to move it into vhost-pmd, they won't be needed. > And I don't think that will happen soon. I agree with you that would be ideal, but unlikely to happen soon. >> Oh, I forgot one comment on this patch. >> Since these new functions are part to the API, shouldn't them be >> documented? > > Yes, indeed, it's also noted in my cover letter. Oops, I missed that! Thanks, Maxime > --yliu >