From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <maxime.coquelin@redhat.com>
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
by dpdk.org (Postfix) with ESMTP id E49A31396
for <dev@dpdk.org>; Thu, 16 Mar 2017 10:39:04 +0100 (CET)
Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com
[10.5.11.16])
(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
(No client certificate requested)
by mx1.redhat.com (Postfix) with ESMTPS id 01122437F4B;
Thu, 16 Mar 2017 09:39:05 +0000 (UTC)
DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 01122437F4B
Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com;
dmarc=none (p=none dis=none) header.from=redhat.com
Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com;
spf=pass smtp.mailfrom=maxime.coquelin@redhat.com
DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 01122437F4B
Received: from [10.36.117.151] (ovpn-117-151.ams2.redhat.com [10.36.117.151])
by smtp.corp.redhat.com (Postfix) with ESMTPS id 7CFB471C87;
Thu, 16 Mar 2017 09:39:03 +0000 (UTC)
To: Yuanhan Liu <yuanhan.liu@linux.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>
<b9b581b0-efd1-51a0-e3f7-4fec7460ea50@redhat.com>
<20170316074311.GR18844@yliu-dev.sh.intel.com>
Cc: dev@dpdk.org, Harris James R <james.r.harris@intel.com>,
Liu Changpeng <changpeng.liu@intel.com>
From: Maxime Coquelin <maxime.coquelin@redhat.com>
Message-ID: <54aee9bd-ae85-9565-0bcd-c30346c9fa38@redhat.com>
Date: Thu, 16 Mar 2017 10:39:00 +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: <20170316074311.GR18844@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.16
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16
(mx1.redhat.com [10.5.110.29]); Thu, 16 Mar 2017 09:39:05 +0000 (UTC)
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 <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 16 Mar 2017 09:39:05 -0000
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?
Cheers,
Maxime