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 B8A571D8E for ; Mon, 4 Dec 2017 15:21:02 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C448E81E1A; Mon, 4 Dec 2017 14:21:01 +0000 (UTC) Received: from [10.36.112.49] (ovpn-112-49.ams2.redhat.com [10.36.112.49]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 43DBD5D98D; Mon, 4 Dec 2017 14:20:58 +0000 (UTC) To: Ladi Prosek Cc: dev@dpdk.org, yliu@fridaylinux.org, tiwei.bie@intel.com, jianfeng.tan@intel.com, Laszlo Ersek References: <20171204140900.7906-1-maxime.coquelin@redhat.com> <20171204140900.7906-2-maxime.coquelin@redhat.com> From: Maxime Coquelin Message-ID: Date: Mon, 4 Dec 2017 15:20:53 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Mon, 04 Dec 2017 14:21:01 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH 1/4] vhost: prevent features to be changed while device is running 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: Mon, 04 Dec 2017 14:21:03 -0000 On 12/04/2017 03:17 PM, Ladi Prosek wrote: > On Mon, Dec 4, 2017 at 3:08 PM, Maxime Coquelin > wrote: >> As section 2.2 of the Virtio spec states about features >> negotiation: >> "During device initialization, the driver reads this and tells >> the device the subset that it accepts. The only way to >> renegotiate is to reset the device." >> >> This patch implements a check to prevent illegal features change >> while the device is running. >> >> One exception is the VHOST_F_LOG_ALL feature bit, which is enabled >> when live-migration is initiated. But this feature is not negotiated >> with the Virtio driver, but directly with the Vhost master. >> >> Signed-off-by: Maxime Coquelin >> --- >> lib/librte_vhost/vhost_user.c | 17 ++++++++++++++++- >> 1 file changed, 16 insertions(+), 1 deletion(-) >> >> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c >> index f4c7ce462..f51055ab2 100644 >> --- a/lib/librte_vhost/vhost_user.c >> +++ b/lib/librte_vhost/vhost_user.c >> @@ -183,7 +183,22 @@ vhost_user_set_features(struct virtio_net *dev, uint64_t features) >> return -1; >> } >> >> - if ((dev->flags & VIRTIO_DEV_RUNNING) && dev->features != features) { >> + if (dev->features == features) >> + return 0; >> + >> + if (dev->flags & VIRTIO_DEV_RUNNING) { >> + /* >> + * Error out if master tries to change features while device is >> + * in running state. The exception being VHOST_F_LOG_ALL, which >> + * is enabled when the live-migration starts. >> + */ >> + if ((dev->features ^ features) & ~(1ULL >> VHOST_F_LOG_ALL)) { > The 1 should be shifted to the left: > > 1ULL << VHOST_F_LOG_ALL Oh right, of course. Thanks for spotting this. Maxime