Re: [virtio-dev] RE: [virtio-comment] RE: [PATCH v13] virtio-net: support the virtqueue coalescing moderation
在 2023/3/23 上午1:02, Parav Pandit 写道:
From: Michael S. Tsirkin
Sent: Wednesday, March 22, 2023 12:53 PM
On Wed, Mar 22, 2023 at 04:49:58PM +, Parav Pandit wrote:
From: Michael S. Tsirkin
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-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [virtio-comment] RE: [PATCH v13] virtio-net: support the virtqueue coalescing moderation
> From: Michael S. Tsirkin > Sent: Wednesday, March 22, 2023 12:53 PM > > On Wed, Mar 22, 2023 at 04:49:58PM +, Parav Pandit wrote: > > > > > From: Michael S. Tsirkin > > > 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. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] RE: [PATCH v13] virtio-net: support the virtqueue coalescing moderation
On Wed, Mar 22, 2023 at 04:49:58PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > 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. 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. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] RE: [PATCH v13] virtio-net: support the virtqueue coalescing moderation
On Wed, Mar 22, 2023 at 04:46:36PM +, Parav Pandit wrote: > > > From: Cornelia Huck > > Sent: Wednesday, March 22, 2023 12:44 PM > > > > >> Well, I think we want to avoid having to add a normative statement > > >> for the device, so we need to be strict with what the driver is allowed > > >> to do. > > > Drivers are untrusted entities. > > > device normative statement is needed, it will do the checks anyway where > > > it > > is applying the config. > > > > But isn't that implementation specific? I.e. if the driver sends junk, the > > device > > needs to be able to deal with it in any case. > Not sure which part is implementation specific. > The device will deal with it and return an error code when supplied qid is > invalid (of cvq or of disabled vq). spec currently defines, generally, how devices behave if driver matches spec. We do not generally specify what happens if driver is out of spec. For example it's ok for device to just get wedged until reset. It's doable to change this but if yes we should do it over the board. And the benefit, IMHO, is limited. And doing it just for the coalescing commands would be super weird. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [virtio-comment] RE: [PATCH v13] virtio-net: support the virtqueue coalescing moderation
> From: Michael S. Tsirkin > 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. For the new command of interest here, hen driver supplied incorrect values, the device will return error. How to implement is upto the device to figure out. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] RE: [PATCH v13] virtio-net: support the virtqueue coalescing moderation
On Wed, Mar 22, 2023 at 05:44:27PM +0100, Cornelia Huck wrote:
> On Wed, Mar 22 2023, Parav Pandit wrote:
>
> >> From: Cornelia Huck
> >> Sent: Wednesday, March 22, 2023 12:37 PM
> >>
> >> On Wed, Mar 22 2023, Parav Pandit wrote:
> >>
> >> >> From: Cornelia Huck
> >> >> Sent: Wednesday, March 22, 2023 11:21 AM
> >> >>
> >> >> On Wed, Mar 22 2023, Heng Qi wrote:
> >> >>
> >> >> > +The driver MUST NOT set \field{vqn} to any value other than an
> >> >> > +enabled
> >> >> transmit or receive virtqueue number.
> >> >>
> >> > Why do you suggest a negative statement here?
> >> > How is it better than,
> >> > The driver MUST set ...
> >>
> >> So make it
> >>
> >> "The driver MUST set \field{vqn} to the virtqueue number of an enabled
> >> transmit or receive virtqueue." ?
> >>
> > Looks good.
> >
> >> > The device will anyway have to check and apply the parameter to the right
> >> virtqueue.
> >> > And if the vq is not enabled or vq is not tx or rx vq, it needs to fail
> >> > the
> >> command.
> >>
> >> Well, I think we want to avoid having to add a normative statement for the
> >> device, so we need to be strict with what the driver is allowed to do.
> > Drivers are untrusted entities.
> > device normative statement is needed, it will do the checks anyway where it
> > is applying the config.
>
> But isn't that implementation specific? I.e. if the driver sends junk,
> the device needs to be able to deal with it in any case.
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.
--
MST
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] RE: [virtio-comment] RE: [PATCH v13] virtio-net: support the virtqueue coalescing moderation
> From: Cornelia Huck > Sent: Wednesday, March 22, 2023 12:44 PM > > >> Well, I think we want to avoid having to add a normative statement > >> for the device, so we need to be strict with what the driver is allowed to > >> do. > > Drivers are untrusted entities. > > device normative statement is needed, it will do the checks anyway where it > is applying the config. > > But isn't that implementation specific? I.e. if the driver sends junk, the > device > needs to be able to deal with it in any case. Not sure which part is implementation specific. The device will deal with it and return an error code when supplied qid is invalid (of cvq or of disabled vq). - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [virtio-comment] RE: [PATCH v13] virtio-net: support the virtqueue coalescing moderation
On Wed, Mar 22 2023, Parav Pandit wrote:
>> From: Cornelia Huck
>> Sent: Wednesday, March 22, 2023 12:37 PM
>>
>> On Wed, Mar 22 2023, Parav Pandit wrote:
>>
>> >> From: Cornelia Huck
>> >> Sent: Wednesday, March 22, 2023 11:21 AM
>> >>
>> >> On Wed, Mar 22 2023, Heng Qi wrote:
>> >>
>> >> > +The driver MUST NOT set \field{vqn} to any value other than an
>> >> > +enabled
>> >> transmit or receive virtqueue number.
>> >>
>> > Why do you suggest a negative statement here?
>> > How is it better than,
>> > The driver MUST set ...
>>
>> So make it
>>
>> "The driver MUST set \field{vqn} to the virtqueue number of an enabled
>> transmit or receive virtqueue." ?
>>
> Looks good.
>
>> > The device will anyway have to check and apply the parameter to the right
>> virtqueue.
>> > And if the vq is not enabled or vq is not tx or rx vq, it needs to fail the
>> command.
>>
>> Well, I think we want to avoid having to add a normative statement for the
>> device, so we need to be strict with what the driver is allowed to do.
> Drivers are untrusted entities.
> device normative statement is needed, it will do the checks anyway where it
> is applying the config.
But isn't that implementation specific? I.e. if the driver sends junk,
the device needs to be able to deal with it in any case.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
