From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 688812BA1 for ; Fri, 17 Mar 2017 06:50:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=intel; t=1489729837; x=1521265837; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=CEbiEu6okzcrp9c7zifJ6qfy03fhnW2NJRi0aUcjazI=; b=IZ3HZvgmN99ti61rNDwbFLGBYSTGFuRhNujFfIabubCqwEjl6G0vR5yS t8YisNAw8KwQNuuOh4BA8Qw+0zr01g==; Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Mar 2017 22:50:36 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,175,1486454400"; d="scan'208";a="68267376" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by orsmga004.jf.intel.com with ESMTP; 16 Mar 2017 22:50:35 -0700 Date: Fri, 17 Mar 2017 13:48:49 +0800 From: Yuanhan Liu To: Maxime Coquelin Cc: dev@dpdk.org, Harris James R , Liu Changpeng Message-ID: <20170317054849.GD18844@yliu-dev.sh.intel.com> References: <1488534682-3494-1-git-send-email-yuanhan.liu@linux.intel.com> <1488534682-3494-4-git-send-email-yuanhan.liu@linux.intel.com> <20170316074311.GR18844@yliu-dev.sh.intel.com> <54aee9bd-ae85-9565-0bcd-c30346c9fa38@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54aee9bd-ae85-9565-0bcd-c30346c9fa38@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH 03/17] vhost: use new APIs to handle features 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: Fri, 17 Mar 2017 05:50:37 -0000 On Thu, Mar 16, 2017 at 10:39:00AM +0100, Maxime Coquelin wrote: > > > On 03/16/2017 08:43 AM, Yuanhan Liu wrote: > >On Tue, Mar 14, 2017 at 11:43:44AM +0100, Maxime Coquelin wrote: > >>>diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c > >>>index 8433a54..f7227bf 100644 > >>>--- a/lib/librte_vhost/vhost_user.c > >>>+++ b/lib/librte_vhost/vhost_user.c > >>>@@ -143,9 +143,9 @@ > >>> * The features that we support are requested. > >>> */ > >>>static uint64_t > >>>-vhost_user_get_features(void) > >>>+vhost_user_get_features(struct virtio_net *dev) > >>>{ > >>>- return VHOST_FEATURES; > >>>+ return rte_vhost_driver_get_features(dev->ifname); > >>>} > >>> > >>>/* > >>>@@ -154,7 +154,7 @@ > >>>static int > >>>vhost_user_set_features(struct virtio_net *dev, uint64_t features) > >>>{ > >>>- if (features & ~VHOST_FEATURES) > >>>+ if (features & ~rte_vhost_driver_get_features(dev->ifname)) > >> > >>rte_vhost_driver_get_features() returns -1 if the socket is not found. > >>It would result in accepting any feature trying to be set. > > > >If we have gone here, I think rte_vhost_driver_get_features() should not > >return -1. The only exception is user unregistered such socket during > >the negotiation? > > Maybe this could never happen. > I just noticed that rte_vhost_driver_get_features() can fail, and in > that case, we wouldn't see it and the behavior could make hard the > cause to be hard to spot. > > As this is not in the hot code path, my view is that checking its return and > print an error message does not hurt. > > Or maybe we could directly do the error prints into > rte_vhost_driver_*_features() functions if the socket name is not found? Yes, I think we could do that. Thanks! --yliu