On 3/29/2017 3:14 PM, Yuanhan Liu wrote:
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.
Why? IMO, device is not necessary to know this feature. For vhost-user,
except explicitly cutting down the unix connection, I don't see a
message of vhost-user protocol to report that.
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.
Why? vhost does not claim to support this feature, VHOST_SUPPORTED_FEATURES.
Thanks,
Jianfeng
