Skip to content

Commit

Permalink
virtio_net: Revert "virtio_net: set the default max ring size by find…
Browse files Browse the repository at this point in the history
…_vqs()"

This reverts commit 762faee.

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 <[email protected]>
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: 762faee ("virtio_net: set the default max ring size by find_vqs()")
Cc: Xuan Zhuo <[email protected]>
Cc: Jason Wang <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
Tested-by: Andres Freund <[email protected]>
Tested-by: Guenter Roeck <[email protected]>
Message-Id: <[email protected]>
  • Loading branch information
mstsirkin committed Aug 16, 2022
1 parent 568035b commit 2e9ca76
Showing 1 changed file with 4 additions and 38 deletions.
42 changes: 4 additions & 38 deletions drivers/net/virtio_net.c
Original file line number Diff line number Diff line change
Expand Up @@ -3432,37 +3432,13 @@ 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;
struct virtqueue **vqs;
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
Expand Down Expand Up @@ -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 */
Expand All @@ -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;

Expand All @@ -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);
Expand Down Expand Up @@ -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)
Expand All @@ -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)) {
Expand Down

0 comments on commit 2e9ca76

Please sign in to comment.