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 4A8503256 for ; Tue, 8 May 2018 09:37:16 +0200 (CEST) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 May 2018 00:37:15 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,377,1520924400"; d="scan'208";a="226666061" Received: from dpdk15.sh.intel.com ([10.67.111.77]) by fmsmga005.fm.intel.com with ESMTP; 08 May 2018 00:37:14 -0700 Date: Tue, 8 May 2018 15:44:22 +0800 From: Jiayu Hu To: Tiwei Bie Cc: dev@dpdk.org, zhiyong.yang@intel.com, maxime.coquelin@redhat.com Message-ID: <20180508074421.GA57422@dpdk15.sh.intel.com> References: <1525675806-22387-1-git-send-email-jiayu.hu@intel.com> <20180508024950.7rqyxjihdfilvovf@debian> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180508024950.7rqyxjihdfilvovf@debian> User-Agent: Mutt/1.7.1 (2016-10-04) Subject: Re: [dpdk-dev] [PATCH] net/virtio-user: fix feature setting with vhost-net backend 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: Tue, 08 May 2018 07:37:16 -0000 Hi Tiwei, On Tue, May 08, 2018 at 10:49:50AM +0800, Tiwei Bie wrote: > On Mon, May 07, 2018 at 02:50:06PM +0800, Jiayu Hu wrote: > > Currently, when the backend is vhost-net, which implies > > virtio-user works in client mode, virtio-user is assigned > > to the default feature VIRTIO_USER_SUPPORTED_FEATURES. > > However, virtio-user should request features from the > > backend in this case. > > > > Fixes: bd8f50a45d0f ("net/virtio-user: support server mode") > > Signed-off-by: Jiayu Hu > > --- > > drivers/net/virtio/virtio_user/virtio_user_dev.c | 15 +++++++-------- > > 1 file changed, 7 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c > > index 38b8bc9..f745163 100644 > > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c > > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c > > @@ -353,20 +353,22 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, > > return -1; > > } > > > > - if (dev->vhostfd >= 0) { > > + if (!dev->is_server) { > > Server mode also needs to run below code when the > connection to the backend is established. For server mode, the below code is called by virtio_user_start_device(), instead of virtio_user_dev_init(). Only client mode virtio-user needs to call it when init. > > > if (dev->ops->send_request(dev, VHOST_USER_SET_OWNER, > > - NULL) < 0) { > > + NULL) < 0) { > > There is no need to change the indent. Thanks, change in the next patch. > > > PMD_INIT_LOG(ERR, "set_owner fails: %s", > > - strerror(errno)); > > + strerror(errno)); > > There is no need to change the indent. ditto. > > > return -1; > > } > > > > if (dev->ops->send_request(dev, VHOST_USER_GET_FEATURES, > > - &dev->device_features) < 0) { > > + &dev->device_features) < 0) { > > There is no need to change the indent. ditto. > > > PMD_INIT_LOG(ERR, "get_features failed: %s", > > - strerror(errno)); > > + strerror(errno)); > > There is no need to change the indent. ditto. > > > return -1; > > } > > + if (dev->mac_specified) > > + dev->device_features |= (1ull << VIRTIO_NET_F_MAC); > > Why move above code? The server mode virtio-user is assigned the default features, which include VIRTIO_NET_F_MAC. So checking if support VIRTIO_NET_F_MAC is only necessary for client mode. Thanks, Jiayu > > > } else { > > /* We just pretend vhost-user can support all these features. > > * Note that this could be problematic that if some feature is > > @@ -376,9 +378,6 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, > > dev->device_features = VIRTIO_USER_SUPPORTED_FEATURES; > > } > > > > - if (dev->mac_specified) > > - dev->device_features |= (1ull << VIRTIO_NET_F_MAC); > > - > > if (cq) { > > /* device does not really need to know anything about CQ, > > * so if necessary, we just claim to support CQ > > -- > > 2.7.4 > >