Re: [virtio-dev] RE: [virtio-comment] RE: [PATCH v13] virtio-net: support the virtqueue coalescing moderation

[Heng Qi <hengqi@linux.alibaba.com>]

å 2023/3/23 äå1:02, Parav Pandit åé:
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Wednesday, March 22, 2023 12:53 PM
> >
> > On Wed, Mar 22, 2023 at 04:49:58PM +0000, Parav Pandit wrote:
> > > > From: Michael S. Tsirkin <mst@redhat.com>
> > > > Sent: Wednesday, March 22, 2023 12:47 PM
> > > >
> > > > I agree with Cornelia here. Yes if devices do not want to trust
> > > > drivers then they will validate input but what exactly happens then is
> > currently up to device.
> > > > If we want to try and specify devices in all cases of out of spec
> > > > input that's a big project, certainly doable but I would rather not
> > > > connect it to this, rather boutique, feature.
> > > Both of your and Cornelia's comment is abstract to me.
> > > We cannot change past.
> > But we can make sure things are consistent. Currently we don't describe device
> > behaviour if driver is out of spec and I see 0 reasons to start doing it with
> > coalescing commands specifically.
> >
> > > For the new command of interest here, hen driver supplied incorrect values,
> > the device will return error.
> >
> > It might be easier for device to just set NEEDS_RESET and stop responding.
> This approach of treating all errors as a fatal category is completely the opposite of making the device and driver resilient to (recoverable) errors.
> We shouldn't go this route.
> Different discussion...
>
> > For
> > a hypervisor implementation that's often better than returning error since
> > device state is then preserved making things easier to debug.
> >
> > > How to implement is upto the device to figure out.
> >
> > what to do is also up to the device.
> Previously error code as not returned hence new command cannot return the error code is going backward.
>
> Returning the failure code is a way to indicate that the driver had a recoverable error.

I agree with you. Part of the specification [1] covered something we're 
talking about, e.g. if an untrusted driver sends a disabled vq, the 
device returns an error:

[1] +The device MUST respond to VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and 
VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET commands with VIRTIO_NET_ERR if the 
designated virtqueue is disabled.

Maybe we should modify [1] to:

"The device MUST respond to VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and 
VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET commands with VIRTIO_NET_ERR if the 
designated \field{vqn} is not the virtqueue number of an enabled 
transmit or receive virtqueue."


Thanks!


> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org