From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yuanhan Liu Subject: Re: [PATCH 3/5] net/virtio-user: support to report net status Date: Wed, 29 Mar 2017 15:14:23 +0800 Message-ID: <20170329071423.GH18844@yliu-dev.sh.intel.com> References: <1488563803-87754-1-git-send-email-jianfeng.tan@intel.com> <1488563803-87754-4-git-send-email-jianfeng.tan@intel.com> <20170317065445.GH18844@yliu-dev.sh.intel.com> <20170329063301.GE18844@yliu-dev.sh.intel.com> <29d1e442-1c5b-6ba0-dbf1-9d35d36e37e1@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "dev@dpdk.org" , "david.marchand@6wind.com" To: "Tan, Jianfeng" Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id EAF2F2C55 for ; Wed, 29 Mar 2017 09:16:39 +0200 (CEST) Content-Disposition: inline In-Reply-To: <29d1e442-1c5b-6ba0-dbf1-9d35d36e37e1@intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Mar 29, 2017 at 03:07:28PM +0800, Tan, Jianfeng wrote: > > > On 3/29/2017 2:33 PM, Yuanhan Liu wrote: > >On Mon, Mar 27, 2017 at 07:46:32AM +0000, Tan, Jianfeng wrote: > >>>>diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c > >>>b/drivers/net/virtio/virtio_user/virtio_user_dev.c > >>>>index 9777d6b..cc6f557 100644 > >>>>--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c > >>>>+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c > >>>>@@ -176,6 +176,7 @@ virtio_user_start_device(struct virtio_user_dev > >>>*dev, uint8_t portid) > >>>> features &= ~(1ull << VIRTIO_NET_F_MAC); > >>>> /* Strip VIRTIO_NET_F_CTRL_VQ, as devices do not really need to > >>>know */ > >>>> features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ); > >>>>+ features &= ~(1ull << VIRTIO_NET_F_STATUS); > >>>> ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES, > >>>&features); > >>>> if (ret < 0) > >>>> goto error; > >>>>diff --git a/drivers/net/virtio/virtio_user_ethdev.c > >>>b/drivers/net/virtio/virtio_user_ethdev.c > >>>>index fa79419..fbdd0a8 100644 > >>>>--- a/drivers/net/virtio/virtio_user_ethdev.c > >>>>+++ b/drivers/net/virtio/virtio_user_ethdev.c > >>>>@@ -121,7 +121,8 @@ virtio_user_get_features(struct virtio_hw *hw) > >>>> struct virtio_user_dev *dev = virtio_user_get_dev(hw); > >>>> > >>>> /* unmask feature bits defined in vhost user protocol */ > >>>>- return dev->device_features & > >>>VIRTIO_PMD_SUPPORTED_GUEST_FEATURES; > >>>>+ return (dev->device_features | (1 << VIRTIO_NET_F_STATUS)) > >>>>+ & VIRTIO_PMD_SUPPORTED_GUEST_FEATURES; > >>>Why not handle the features at virtio_user_dev_init()? > >>You mean add VIRTIO_NET_F_STATUS when get_features from device? Yes, we could do that there. But we originally add device_features to only record features supported by device. > >> > >Aren't you adding the F_STATUS features to this device? > > > > For virtio driver, yes, we are adding F_STATUS feature so the it sees a > device supporting LSC. That means you are adding a device feature (F_STATUS) and reporting it to the driver that this feature is always on, no matter whether the device actually supports it or not? This looks wrong to me. > But for backend driver, we need hide this feature, it > happens when we set_features to the backend driver. The feature negotion of virtio-user seems broken to me. --yliu