Re: [virtio] [PATCH] virtio-net: clarify error handling for coalescing

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Jan 24, 2024 at 11:23:08AM +0000, Parav Pandit wrote:
> 
> 
> > From: virtio@lists.oasis-open.org <virtio@lists.oasis-open.org> On Behalf Of
> > Michael S. Tsirkin
> > Sent: Wednesday, January 24, 2024 4:46 PM
> > To: virtio-comment@lists.oasis-open.org; virtio-dev@lists.oasis-open.org
> > Cc: virtio@lists.oasis-open.org
> 
> Can we please avoid posting to three different lists?

Yea I'll stop. Used to do it because of different archiving rules.
I'll stick to virtio-comment@lists.oasis-open.org


> > Subject: [virtio] [PATCH] virtio-net: clarify error handling for coalescing
> > 
> > This is not a huge deal since it's a SHOULD anyway, so make the new
> > requirement SHOULD too.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  device-types/net/description.tex | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/device-types/net/description.tex b/device-
> > types/net/description.tex
> > index aff5e08..d1d25fe 100644
> > --- a/device-types/net/description.tex
> > +++ b/device-types/net/description.tex
> > @@ -1792,7 +1792,11 @@ \subsubsection{Control
> > Virtqueue}\label{sec:Device Types / Network Device / Devi
> > 
> >  The device MUST ignore \field{reserved}.
> > 
> > -The device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
> > VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR if
> > it was not able to change the parameters.
> > +The device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
> > +VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR
> > if it was
> > +not able to change coalescing parameters. In this case, the parameters
> > +SHOULD remain unchanged, for all VQs.
> 
> Is the new added normative line without device is ok?
> 
> Or should it be written as
> ... In this case, the device SHOULD not update the parameters for all VQs.
> 
> ?

Your wording is confusing it can be intepreted to
mean just that not all parameters change but some might.
Generally we do not have to repeat "device" in each sentence.

> > +
> > 
> >  The device MUST respond to the VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET
> > command with VIRTIO_NET_ERR if it was not able to change the parameters.
> > 
> > --
> > MST
> For the functionality itself, ignoring above normative suggestion,
> 
> Reviewed-by: Parav Pandit <parav@nvidia.com>