* [virtio-dev] RE: [virtio] [PATCH] virtio-net: clarify error handling for coalescing
2024-01-24 11:15 [virtio-dev] [PATCH] virtio-net: clarify error handling for coalescing Michael S. Tsirkin
@ 2024-01-24 11:23 ` Parav Pandit
2024-01-26 8:01 ` [virtio-dev] " Michael S. Tsirkin
2024-01-24 11:28 ` [virtio-dev] Re: [virtio-comment] " Heng Qi
2024-01-29 15:06 ` [virtio-dev] Re: [virtio] " Cornelia Huck
2 siblings, 1 reply; 7+ messages in thread
From: Parav Pandit @ 2024-01-24 11:23 UTC (permalink / raw)
To: Michael S. Tsirkin, virtio-comment@lists.oasis-open.org,
virtio-dev@lists.oasis-open.org
Cc: virtio@lists.oasis-open.org
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread* [virtio-dev] Re: [virtio] [PATCH] virtio-net: clarify error handling for coalescing
2024-01-24 11:23 ` [virtio-dev] RE: [virtio] " Parav Pandit
@ 2024-01-26 8:01 ` Michael S. Tsirkin
0 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2024-01-26 8:01 UTC (permalink / raw)
To: Parav Pandit
Cc: virtio-comment@lists.oasis-open.org,
virtio-dev@lists.oasis-open.org, virtio@lists.oasis-open.org
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [virtio-dev] Re: [virtio-comment] [PATCH] virtio-net: clarify error handling for coalescing
2024-01-24 11:15 [virtio-dev] [PATCH] virtio-net: clarify error handling for coalescing Michael S. Tsirkin
2024-01-24 11:23 ` [virtio-dev] RE: [virtio] " Parav Pandit
@ 2024-01-24 11:28 ` Heng Qi
2024-01-29 15:06 ` [virtio-dev] Re: [virtio] " Cornelia Huck
2 siblings, 0 replies; 7+ messages in thread
From: Heng Qi @ 2024-01-24 11:28 UTC (permalink / raw)
To: Michael S. Tsirkin, virtio-comment, virtio-dev; +Cc: virtio
在 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
^ permalink raw reply [flat|nested] 7+ messages in thread* [virtio-dev] Re: [virtio] [PATCH] virtio-net: clarify error handling for coalescing
2024-01-24 11:15 [virtio-dev] [PATCH] virtio-net: clarify error handling for coalescing Michael S. Tsirkin
2024-01-24 11:23 ` [virtio-dev] RE: [virtio] " Parav Pandit
2024-01-24 11:28 ` [virtio-dev] Re: [virtio-comment] " Heng Qi
@ 2024-01-29 15:06 ` Cornelia Huck
2024-01-29 16:26 ` Michael S. Tsirkin
2 siblings, 1 reply; 7+ messages in thread
From: Cornelia Huck @ 2024-01-29 15:06 UTC (permalink / raw)
To: Michael S. Tsirkin, virtio-comment, virtio-dev; +Cc: virtio
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
^ permalink raw reply [flat|nested] 7+ messages in thread* [virtio-dev] Re: [virtio] [PATCH] virtio-net: clarify error handling for coalescing
2024-01-29 15:06 ` [virtio-dev] Re: [virtio] " Cornelia Huck
@ 2024-01-29 16:26 ` Michael S. Tsirkin
2024-01-29 16:55 ` Cornelia Huck
0 siblings, 1 reply; 7+ messages in thread
From: Michael S. Tsirkin @ 2024-01-29 16:26 UTC (permalink / raw)
To: Cornelia Huck; +Cc: virtio-comment, virtio-dev, virtio
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
^ permalink raw reply [flat|nested] 7+ messages in thread* [virtio-dev] Re: [virtio] [PATCH] virtio-net: clarify error handling for coalescing
2024-01-29 16:26 ` Michael S. Tsirkin
@ 2024-01-29 16:55 ` Cornelia Huck
0 siblings, 0 replies; 7+ messages in thread
From: Cornelia Huck @ 2024-01-29 16:55 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtio-comment, virtio-dev, virtio
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
^ permalink raw reply [flat|nested] 7+ messages in thread