Possible busy-loop on process_tx errors

[nirs]

This logic can lead to a busy-loop if process_tx() returns a persistent error other than NothingWritten. In such cases, tx_has_deferred_frame will be false, and if the guest continues to supply TX buffers (has_new_entries is true), this loop will repeatedly call process_tx() and fail. The loop should be broken on any error from process_tx() to avoid this.

            let result = self.process_tx();
            self.tx_has_deferred_frame =
                matches!(result, Err(TxError::Backend(WriteError::NothingWritten)));

            if let Err(e) = result {
                if !self.tx_has_deferred_frame {
                    log::error!("Failed to process tx: {e:?}");
                }
                break;
            }

            if !self.tx_q.queue.enable_notification(&self.mem).unwrap() {
                break;
            }

Originally posted by @gemini-code-assist[bot] in #556 (comment)

[nirs]

I think the behavior should be to drop frames that we cannot write and log the error. This seems to be the behavior in the current code.

If we break from the loop we may not woke up since we use edge triggered events. If the socket is writable and we cannot write we may never wake up which is not helpful.

[areporeporepo]

what do you think about enable_notification() before breaking, addressing the edge-triggered wakeup. tx_has_deferred_frame is also explicitly reset to false. Also willing to close #606 in favor of better design

[nirs]

@areporeporepo Lets try to answer these questions first:

• Which recoverable errors are possible?
• Which unrecoverable errors are possible?
• What is the wanted behavior for recoverable error?
• What is the wanted behavior for unrecoverable error?

[mtjhrc]

@nirs

@areporeporepo Lets try to answer these questions first:

Yeah good point

Which recoverable errors are possible?

If we are not counting EAGAIN/ENOBUFS etc. (partial sent or nothing send...), which we already handle, I can't really think of any... Maybe any error that is not explicitly handled is treated as recoverable?

Which unrecoverable errors are possible?

Not sure exactly which errno's this would give us (also we receive epoll events for this), but the backend process being killed or crashing is definitely something to consider.

What is the wanted behavior for recoverable error?

I suppose we could log a warning and drop the frame or try sending it later again.

What is the wanted behavior for unrecoverable error?

Basically stop the device processing, something like this:

• log a message at error level
• unsubscribe from epoll events for the backend socket
• maybe set the device status to DEVICE_NEEDS_RESET (or FAILED directly?) - I am not sure
• make sure device reset fails, because we cannot restart passt/gvproxy/vmnet sub-processes ourselves.