virtio_nic: Fix packet transmission handling.

The problem can be seen when running nic_router_flood scenarion on arm
qemu_virt boards. With the amount of data this scenario tries to send
the driver quickly complains it has failed to push data into TX VirtIO
queue. After this warning message is printed nothing really happens and
after a while the test scenario fails.

The fact that we can't write all available data to the device is not
unexpected. VirtIO queue size is slected at initialization time and we
don't change it during driver lifetime. It can be tweaked via driver
config, but this does not change the fact that we'll always be able to
produce more data packets than we have free space in the VirtIO queue.

IMO the expected behavior of the driver in such case should be to:
1. Notify the device there is data to process.
2. Wait for the device to process at least part of it.
3. Retry sending queued packets.

One could expect returning Transmit_result::RETRY from _drv_transmit_pkt
would produce such result. Unfortunately it seems that Uplink_client_base
treats RETRY return value as indication of link being down. It'll retry
sending the packet only after the device notifies it the link is once
again up. This is the reason why nothing happens when running
nic_router_flood on top of virtio_nic driver. The link never goes down
in this case so once we fill the TX VirtIO queue and tell the base class
to retry the send, we'll be stuck waiting for link up change event
which will never arrive.

To fix this problem, when sending a packet to the device fails, do a
synchrnonus TX VirtIO queue flush (tell device there is data to process
and wait until its done with it).

With this fix in place nic_router_flood test scenario passes on both arm
qemu_virt boards.

Issue #4264
This commit is contained in:
Piotr Tworek 2021-09-21 22:13:42 +02:00 committed by Norman Feske
parent b7f66626c2
commit 880cd3a490

View File

@ -376,6 +376,16 @@ class Virtio_nic::Device : Noncopyable
_tx_vq.ack_all_transfers();
}
/**
* Tell the device we have some buffers for it to process and wait
* until its done with at least one of them.
*/
void tx_vq_flush()
{
_device.notify_buffers_available(TX_VQ);
while (!_tx_vq.has_used_buffers());
}
bool read_link_state()
{
/**
@ -652,15 +662,19 @@ class Genode::Uplink_client : public Virtio_nic::Device,
size_t conn_rx_pkt_size) override
{
rx_vq_ack_pkts();
if (tx_vq_write_pkt(conn_rx_pkt_base, conn_rx_pkt_size)) {
if (!tx_vq_write_pkt(conn_rx_pkt_base, conn_rx_pkt_size)) {
/*
* VirtIO transmit queue is full, flush it and retry sending the pkt.
*/
tx_vq_flush();
return Transmit_result::ACCEPTED;
} else {
warning("Failed to push packet into tx VirtIO queue!");
return Transmit_result::RETRY;
if (!tx_vq_write_pkt(conn_rx_pkt_base, conn_rx_pkt_size)) {
warning("Failed to send packet after flushing VirtIO queue!");
return Transmit_result::REJECTED;
}
}
return Transmit_result::ACCEPTED;
}
void _drv_finish_transmitted_pkts() override