All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hannes Reinecke <hare@suse.de>
To: Alvaro Karsz <alvaro.karsz@solid-run.com>
Cc: virtualization <virtualization@lists.linux-foundation.org>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: Virtio-net - add timeouts to control commands
Date: Wed, 24 Aug 2022 12:19:25 +0200	[thread overview]
Message-ID: <f0406458-319d-129b-655f-11343d99743d@suse.de> (raw)
In-Reply-To: <CAJs=3_AUiaDtyRTMcDd_DGuiKZVKuMUSZRUosQa7wU=5UZwtqw@mail.gmail.com>

On 8/24/22 11:42, Alvaro Karsz wrote:
> Hi Hannes,
> 
>> a) let the device do the timeout: pass in a timeout value with the
>> command, and allow the device to return an ETIMEDOUT error when the
>> timeout expires. Then it's up to the device to do the necessary timeout
>> handling; the server won't be involved at all (except for handling an
>> ETIMEDOUT error)
> 
> 
> This won't work if the device crashes.
> 
>>
>> b) implement an 'abort' command. With that the server controls the
>> timeout, and is allowed to send an 'abort' command when the timeout
>> expires. That requires the device to be able to abort commands (which
>> not all devices are able to), but avoids having to implement a timeout
>> handling in the device.
> 
> 
> I actually thought about this idea.
> This may work, but you'll still have a few moments when the server
> assumes that the command failed, and the network device assumes that
> it succeeded.
> So the server may still receive packets in an unexpected queue.
> 
> 
No. The server may only assume that the command failed until it gets the 
response for the abort command.
Before that the state of the command is undefined, and the server may 
not assume anything here.

And then we get into the fun topic of timing out aborts, which really 
can only be resolved if you have a fool-proof way of resetting the queue 
itself. But I guess virtio can do that (right?).

>>
>> I am very much in favour of having timeouts for virtio commands; we've
>> had several massive customer escalations which could have been solved if
>> we were able to set the command timeout in the VM.
>> As this was for virtio-scsi/virtio-block I would advocate to have a
>> generic virtio command timeout, not a protocol-specific one.
> 
> This may be difficult to implement.
> Especially when multiple commands may be queued at the same time, and
> the device can handle the commands in any order.
> We'll need to add identifiers for every command.
> 
Why, but of course. You cannot assume in-order delivery of the 
completions; in fact, that's the whole _point_ of having a queue-based 
I/O command delivery method.

> I'm actually referring here to the Linux kernel implementation of
> virtnet control commands, in which the server spins for a response.

Sheesh. Spinning for a response is never a good idea, as this means 
you'll end up with a non-interruptible command in the guest (essentially 
an ioctl into the hypervisor).

And really, identifying the command isn't hard. Each command already has 
an identifier (namely the virtio ring index), so if in doubt you can 
always use that.
To be foolproof, though, you might want to add a 'real' identifier (like 
a 32 or 64 bit command tag), which would even allow you to catch 
uninitialized / completed commands. Tends to be quite important when 
implementing an 'abort' command, as the command referred to by the 
'abort' command might have been completed by the time the hypervisor 
processes the abort command.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

  reply	other threads:[~2022-08-24 10:19 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-24  7:51 Virtio-net - add timeouts to control commands Alvaro Karsz
2022-08-24  9:06 ` Jason Wang
2022-08-24  9:16   ` Alvaro Karsz
2022-08-24  9:24     ` Hannes Reinecke
2022-08-24  9:48       ` Alvaro Karsz
2022-08-25  2:27     ` Jason Wang
2022-08-24  9:21   ` Hannes Reinecke
2022-08-24  9:42     ` Alvaro Karsz
2022-08-24 10:19       ` Hannes Reinecke [this message]
2022-08-24 10:31         ` Alvaro Karsz
2022-08-25  3:01       ` Jason Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f0406458-319d-129b-655f-11343d99743d@suse.de \
    --to=hare@suse.de \
    --cc=alvaro.karsz@solid-run.com \
    --cc=mst@redhat.com \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.