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

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

[Michael S. Tsirkin]

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.
+
 
 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


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

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

[Parav Pandit]

> 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?

> 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.

?

> +
> 
>  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> 


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

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

[Heng Qi]

在 2024/1/24 下午7:15, Michael S. Tsirkin 写道:
> 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.

Can we use "virtqueues" instead of "VQs"? Because "VQs" has not been 
used in the text.

With this,

Reviewed-by: Heng Qi <hengqi@linux.alibaba.com>

Thanks!

> +
>   
>   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.
>   


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

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

[Michael S. Tsirkin]

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> 


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

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

[Cornelia Huck]

On Wed, Jan 24 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> 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,

Hm, if we explicitly specify "coalescing parameters" here, should we add
it below as well? (We probably should keep the "the".)

> +the parameters SHOULD remain unchanged, for all VQs.
> +
>  
>  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.


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

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

[Michael S. Tsirkin]

On Mon, Jan 29, 2024 at 04:06:01PM +0100, Cornelia Huck wrote:
> On Wed, Jan 24 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > 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,
> 
> Hm, if we explicitly specify "coalescing parameters" here, should we add
> it below as well? (We probably should keep the "the".)

maybe "some of the" and in the next sentence "all of the".

so it's atomic: all or nothing.

> > +the parameters SHOULD remain unchanged, for all VQs.
> > +
> >  
> >  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.


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

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

[Cornelia Huck]

On Mon, Jan 29 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Jan 29, 2024 at 04:06:01PM +0100, Cornelia Huck wrote:
>> On Wed, Jan 24 2024, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> 
>> > 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,
>> 
>> Hm, if we explicitly specify "coalescing parameters" here, should we add
>> it below as well? (We probably should keep the "the".)
>
> maybe "some of the" and in the next sentence "all of the".
>
> so it's atomic: all or nothing.

Makes sense.

>
>> > +the parameters SHOULD remain unchanged, for all VQs.
>> > +
>> >  
>> >  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.

Also here ^ ?


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