net/virtio-user: fix wrongly get/set features

Before the commit 86d59b21468a ("net/virtio: support LRO"), features
in virtio PMD, is decided and properly set at device initialization
and will not be changed. But afterward, features could be changed in
virtio_dev_configure(), and will be re-negotiated if it's changed.

In virtio-user, device features is obtained at driver probe phase
only once, but we did not store it. So the added feature bits in
re-negotiation will fail.

To fix it, we store it down, and will be used to feature negotiation
either at device initialization phase or device configure phase.

Fixes: e9efa4d93821 ("net/virtio-user: add new virtual PCI driver")
Cc: stable@dpdk.org
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index 27d9a19..4feccf9 100644 (file)
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -70,6 +70,11 @@
         1ULL << VIRTIO_F_VERSION_1       |     \
         1ULL << VIRTIO_F_IOMMU_PLATFORM)
 
+#define VIRTIO_PMD_SUPPORTED_GUEST_FEATURES    \
+       (VIRTIO_PMD_DEFAULT_GUEST_FEATURES |    \
+        1u << VIRTIO_NET_F_GUEST_CSUM     |    \
+        1u << VIRTIO_NET_F_GUEST_TSO4     |    \
+        1u << VIRTIO_NET_F_GUEST_TSO6)
 /*
  * CQ function prototype
  */

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index e239e0e..0d7e17b 100644 (file)
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -148,12 +148,13 @@ virtio_user_start_device(struct virtio_user_dev *dev)
 
        /* Step 1: set features
         * Make sure VHOST_USER_F_PROTOCOL_FEATURES is added if mq is enabled,
-        * and VIRTIO_NET_F_MAC is stripped.
+        * VIRTIO_NET_F_MAC and VIRTIO_NET_F_CTRL_VQ is stripped.
         */
        features = dev->features;
        if (dev->max_queue_pairs > 1)
                features |= VHOST_USER_MQ;
        features &= ~(1ull << VIRTIO_NET_F_MAC);
+       features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
        ret = vhost_user_sock(dev->vhostfd, VHOST_USER_SET_FEATURES, &features);
        if (ret < 0)
                goto error;
@@ -228,29 +229,26 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
        }
 
        if (vhost_user_sock(dev->vhostfd, VHOST_USER_GET_FEATURES,
-                           &dev->features) < 0) {
+                           &dev->device_features) < 0) {
                PMD_INIT_LOG(ERR, "get_features failed: %s", strerror(errno));
                return -1;
        }
        if (dev->mac_specified)
-               dev->features |= (1ull << VIRTIO_NET_F_MAC);
+               dev->device_features |= (1ull << VIRTIO_NET_F_MAC);
 
-       if (!cq) {
-               dev->features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
-               /* Also disable features depends on VIRTIO_NET_F_CTRL_VQ */
-               dev->features &= ~(1ull << VIRTIO_NET_F_CTRL_RX);
-               dev->features &= ~(1ull << VIRTIO_NET_F_CTRL_VLAN);
-               dev->features &= ~(1ull << VIRTIO_NET_F_GUEST_ANNOUNCE);
-               dev->features &= ~(1ull << VIRTIO_NET_F_MQ);
-               dev->features &= ~(1ull << VIRTIO_NET_F_CTRL_MAC_ADDR);
-       } else {
-               /* vhost user backend does not need to know ctrl-q, so
-                * actually we need add this bit into features. However,
-                * DPDK vhost-user does send features with this bit, so we
-                * check it instead of OR it for now.
+       if (cq) {
+               /* device does not really need to know anything about CQ,
+                * so if necessary, we just claim to support CQ
                 */
-               if (!(dev->features & (1ull << VIRTIO_NET_F_CTRL_VQ)))
-                       PMD_INIT_LOG(INFO, "vhost does not support ctrl-q");
+               dev->device_features |= (1ull << VIRTIO_NET_F_CTRL_VQ);
+       } else {
+               dev->device_features &= ~(1ull << VIRTIO_NET_F_CTRL_VQ);
+               /* Also disable features depends on VIRTIO_NET_F_CTRL_VQ */
+               dev->device_features &= ~(1ull << VIRTIO_NET_F_CTRL_RX);
+               dev->device_features &= ~(1ull << VIRTIO_NET_F_CTRL_VLAN);
+               dev->device_features &= ~(1ull << VIRTIO_NET_F_GUEST_ANNOUNCE);
+               dev->device_features &= ~(1ull << VIRTIO_NET_F_MQ);
+               dev->device_features &= ~(1ull << VIRTIO_NET_F_CTRL_MAC_ADDR);
        }
 
        if (dev->max_queue_pairs > 1) {

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 33690b5..28fc788 100644 (file)
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -46,7 +46,10 @@ struct virtio_user_dev {
        uint32_t        max_queue_pairs;
        uint32_t        queue_pairs;
        uint32_t        queue_size;
-       uint64_t        features;
+       uint64_t        features; /* the negotiated features with driver,
+                                  * and will be sync with device
+                                  */
+       uint64_t        device_features; /* supported features by device */
        uint8_t         status;
        uint8_t         mac_addr[ETHER_ADDR_LEN];
        char            path[PATH_MAX];

diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 48a8c4f..fbf9df7 100644 (file)
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -117,7 +117,8 @@ virtio_user_get_features(struct virtio_hw *hw)
 {
        struct virtio_user_dev *dev = virtio_user_get_dev(hw);
 
-       return dev->features;
+       /* unmask feature bits defined in vhost user protocol */
+       return dev->device_features & VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
 }
 
 static void
@@ -125,7 +126,7 @@ virtio_user_set_features(struct virtio_hw *hw, uint64_t features)
 {
        struct virtio_user_dev *dev = virtio_user_get_dev(hw);
 
-       dev->features = features;
+       dev->features = features & dev->device_features;
 }
 
 static uint8_t