Re: [PATCH] virtio_net: Eliminate OOO packets during switching
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: 2024-06-17 13:25:18
Also in:
lkml, virtualization
On Fri, Jun 14, 2024 at 10:04:22PM +0000, Abhinav Jain wrote:
Disable the network device & turn off carrier before modifying the number of queue pairs. Process all the in-flight packets and then turn on carrier, followed by waking up all the queues on the network device.
Did you test that there's a workload with OOO and this patch actually prevents that?
Signed-off-by: Abhinav Jain <redacted>
quoted hunk ↗ jump to hunk
--- drivers/net/virtio_net.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 61a57d134544..d0a655a3b4c6 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c@@ -3447,7 +3447,6 @@ static void virtnet_get_drvinfo(struct net_device *dev, } -/* TODO: Eliminate OOO packets during switching */ static int virtnet_set_channels(struct net_device *dev, struct ethtool_channels *channels) {@@ -3471,6 +3470,15 @@ static int virtnet_set_channels(struct net_device *dev, if (vi->rq[0].xdp_prog) return -EINVAL; + /* Disable network device to prevent packet processing during + * the switch. + */ + netif_tx_disable(dev); + netif_carrier_off(dev);
Won't turning off carrier cause a lot of damage such as changing IP and so on?
+ + /* Make certain that all in-flight packets are processed. */ + synchronize_net(); +
The comment seems to say what the code does not do. Also, doing this under rtnl is a heavy weight operation.
quoted hunk ↗ jump to hunk
cpus_read_lock(); err = virtnet_set_queues(vi, queue_pairs); if (err) {@@ -3482,7 +3490,12 @@ static int virtnet_set_channels(struct net_device *dev, netif_set_real_num_tx_queues(dev, queue_pairs); netif_set_real_num_rx_queues(dev, queue_pairs); - err: + + /* Restart the network device */ + netif_carrier_on(dev); + netif_tx_wake_all_queues(dev); + +err: return err; }
Given the result is, presumably, improved performance with less packet loss due to OOO, I'd like to see some actual testing results, hopefully also measuring the effect on CPU load.
-- 2.34.1