virtio_net: Revert "virtio_net: set the default max ring size by find_vqs()"
authorMichael S. Tsirkin <mst@redhat.com>
Tue, 16 Aug 2022 05:36:27 +0000 (01:36 -0400)
committerMichael S. Tsirkin <mst@redhat.com>
Tue, 16 Aug 2022 05:38:28 +0000 (01:38 -0400)
This reverts commit 762faee5a2678559d3dc09d95f8f2c54cd0466a7.

This has been reported to trip up guests on GCP (Google Cloud).
The reason is that virtio_find_vqs_ctx_size is broken on legacy
devices. We can in theory fix virtio_find_vqs_ctx_size but
in fact the patch itself has several other issues:

- It treats unknown speed as < 10G
- It leaves userspace no way to find out the ring size set by hypervisor
- It tests speed when link is down
- It ignores the virtio spec advice:
        Both \field{speed} and \field{duplex} can change, thus the driver
        is expected to re-read these values after receiving a
        configuration change notification.
- It is not clear the performance impact has been tested properly

Revert the patch for now.

Reported-by: Andres Freund <andres@anarazel.de>
Link: https://lore.kernel.org/r/20220814212610.GA3690074%40roeck-us.net
Link: https://lore.kernel.org/r/20220815070203.plwjx7b3cyugpdt7%40awork3.anarazel.de
Link: https://lore.kernel.org/r/3df6bb82-1951-455d-a768-e9e1513eb667%40www.fastmail.com
Link: https://lore.kernel.org/r/FCDC5DDE-3CDD-4B8A-916F-CA7D87B547CE%40anarazel.de
Fixes: 762faee5a267 ("virtio_net: set the default max ring size by find_vqs()")
Cc: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Cc: Jason Wang <jasowang@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Tested-by: Andres Freund <andres@anarazel.de>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Message-Id: <20220816053602.173815-2-mst@redhat.com>

drivers/net/virtio_net.c

index d934774..ece00b8 100644 (file)
@@ -3432,29 +3432,6 @@ static unsigned int mergeable_min_buf_len(struct virtnet_info *vi, struct virtqu
                   (unsigned int)GOOD_PACKET_LEN);
 }
 
-static void virtnet_config_sizes(struct virtnet_info *vi, u32 *sizes)
-{
-       u32 i, rx_size, tx_size;
-
-       if (vi->speed == SPEED_UNKNOWN || vi->speed < SPEED_10000) {
-               rx_size = 1024;
-               tx_size = 1024;
-
-       } else if (vi->speed < SPEED_40000) {
-               rx_size = 1024 * 4;
-               tx_size = 1024 * 4;
-
-       } else {
-               rx_size = 1024 * 8;
-               tx_size = 1024 * 8;
-       }
-
-       for (i = 0; i < vi->max_queue_pairs; i++) {
-               sizes[rxq2vq(i)] = rx_size;
-               sizes[txq2vq(i)] = tx_size;
-       }
-}
-
 static int virtnet_find_vqs(struct virtnet_info *vi)
 {
        vq_callback_t **callbacks;
@@ -3462,7 +3439,6 @@ static int virtnet_find_vqs(struct virtnet_info *vi)
        int ret = -ENOMEM;
        int i, total_vqs;
        const char **names;
-       u32 *sizes;
        bool *ctx;
 
        /* We expect 1 RX virtqueue followed by 1 TX virtqueue, followed by
@@ -3490,15 +3466,10 @@ static int virtnet_find_vqs(struct virtnet_info *vi)
                ctx = NULL;
        }
 
-       sizes = kmalloc_array(total_vqs, sizeof(*sizes), GFP_KERNEL);
-       if (!sizes)
-               goto err_sizes;
-
        /* Parameters for control virtqueue, if any */
        if (vi->has_cvq) {
                callbacks[total_vqs - 1] = NULL;
                names[total_vqs - 1] = "control";
-               sizes[total_vqs - 1] = 64;
        }
 
        /* Allocate/initialize parameters for send/receive virtqueues */
@@ -3513,10 +3484,8 @@ static int virtnet_find_vqs(struct virtnet_info *vi)
                        ctx[rxq2vq(i)] = true;
        }
 
-       virtnet_config_sizes(vi, sizes);
-
-       ret = virtio_find_vqs_ctx_size(vi->vdev, total_vqs, vqs, callbacks,
-                                      names, sizes, ctx, NULL);
+       ret = virtio_find_vqs_ctx(vi->vdev, total_vqs, vqs, callbacks,
+                                 names, ctx, NULL);
        if (ret)
                goto err_find;
 
@@ -3536,8 +3505,6 @@ static int virtnet_find_vqs(struct virtnet_info *vi)
 
 
 err_find:
-       kfree(sizes);
-err_sizes:
        kfree(ctx);
 err_ctx:
        kfree(names);
@@ -3897,9 +3864,6 @@ static int virtnet_probe(struct virtio_device *vdev)
                vi->curr_queue_pairs = num_online_cpus();
        vi->max_queue_pairs = max_queue_pairs;
 
-       virtnet_init_settings(dev);
-       virtnet_update_settings(vi);
-
        /* Allocate/initialize the rx/tx queues, and invoke find_vqs */
        err = init_vqs(vi);
        if (err)
@@ -3912,6 +3876,8 @@ static int virtnet_probe(struct virtio_device *vdev)
        netif_set_real_num_tx_queues(dev, vi->curr_queue_pairs);
        netif_set_real_num_rx_queues(dev, vi->curr_queue_pairs);
 
+       virtnet_init_settings(dev);
+
        if (virtio_has_feature(vdev, VIRTIO_NET_F_STANDBY)) {
                vi->failover = net_failover_create(vi->dev);
                if (IS_ERR(vi->failover)) {