[virtio-comment] [PATCH v3 0/2] virtio-net: Improve dev config layout

[virtio-comment] [PATCH v3 0/2] virtio-net: Improve dev config layout

[Parav Pandit]

This two patches improve dev configuration layout in two ways.

1. Define device configuration layout before describing it fields
2. Avoid duplicate description of its read only fields and whole
layout

Patch summary:
patch-1: consolidate read only field at one place in driver
requirements
patch-2: define device configuration layout before describing its
fields.

changelog:
v2->v3:
- split into two patches
- move read only description to driver requirements section

Parav Pandit (2):
  virtio-net: Describe dev cfg fields read only
  virtio-net: Define cfg fields before description

 device-types/net/description.tex | 48 ++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 21 deletions(-)

-- 
2.26.2


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

[PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Parav Pandit]

Device configuration fields are read only. Avoid duplicating this
description for multiple fields.

Instead describe it one time and do it in the driver requirements
section.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/161
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
changelog:
v2->v3:
- split as new patch
---
 device-types/net/description.tex | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index a197e1a..81e1135 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -156,10 +156,10 @@ \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network
 \subsection{Device configuration layout}\label{sec:Device Types / Network Device / Device configuration layout}
 \label{sec:Device Types / Block Device / Feature bits / Device configuration layout}
 
-Device configuration fields are listed below, they are read-only for a driver. The \field{mac} address field
+Device configuration fields are listed below. The \field{mac} address field
 always exists (though is only valid if VIRTIO_NET_F_MAC is set), and
 \field{status} only exists if VIRTIO_NET_F_STATUS is set. Two
-read-only bits (for the driver) are currently defined for the status field:
+bits (for the driver) are currently defined for the status field:
 VIRTIO_NET_S_LINK_UP and VIRTIO_NET_S_ANNOUNCE.
 
 \begin{lstlisting}
@@ -167,14 +167,14 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
 #define VIRTIO_NET_S_ANNOUNCE    2
 \end{lstlisting}
 
-The following driver-read-only field, \field{max_virtqueue_pairs} only exists if
+The following field, \field{max_virtqueue_pairs} only exists if
 VIRTIO_NET_F_MQ or VIRTIO_NET_F_RSS is set. This field specifies the maximum number
 of each of transmit and receive virtqueues (receiveq1\ldots receiveqN
 and transmitq1\ldots transmitqN respectively) that can be configured once at least one of these features
 is negotiated.
 
-The following driver-read-only field, \field{mtu} only exists if
-VIRTIO_NET_F_MTU is set. This field specifies the maximum MTU for the driver to
+The following field, \field{mtu} only exists if VIRTIO_NET_F_MTU
+is set. This field specifies the maximum MTU for the driver to
 use.
 
 The following two fields, \field{speed} and \field{duplex}, only
@@ -261,6 +261,8 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
 
 \drivernormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
 
+All the device configuration fields are read-only for the driver.
+
 A driver SHOULD negotiate VIRTIO_NET_F_MAC if the device offers it.
 If the driver negotiates the VIRTIO_NET_F_MAC feature, the driver MUST set
 the physical address of the NIC to \field{mac}.  Otherwise, it SHOULD
-- 
2.26.2

Re: [virtio-dev] [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[David Edmondson]

On Friday, 2023-02-17 at 17:45:28 +02, Parav Pandit wrote:
> Device configuration fields are read only. Avoid duplicating this
> description for multiple fields.
>
> Instead describe it one time and do it in the driver requirements
> section.
>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/161
> Signed-off-by: Parav Pandit <parav@nvidia.com>

Reviewed-by: David Edmondson <david.edmondson@oracle.com>

> ---
> changelog:
> v2->v3:
> - split as new patch
> ---
>  device-types/net/description.tex | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> index a197e1a..81e1135 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -156,10 +156,10 @@ \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network
>  \subsection{Device configuration layout}\label{sec:Device Types / Network Device / Device configuration layout}
>  \label{sec:Device Types / Block Device / Feature bits / Device configuration layout}
>  
> -Device configuration fields are listed below, they are read-only for a driver. The \field{mac} address field
> +Device configuration fields are listed below. The \field{mac} address field
>  always exists (though is only valid if VIRTIO_NET_F_MAC is set), and
>  \field{status} only exists if VIRTIO_NET_F_STATUS is set. Two
> -read-only bits (for the driver) are currently defined for the status field:
> +bits (for the driver) are currently defined for the status field:
>  VIRTIO_NET_S_LINK_UP and VIRTIO_NET_S_ANNOUNCE.
>  
>  \begin{lstlisting}
> @@ -167,14 +167,14 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>  #define VIRTIO_NET_S_ANNOUNCE    2
>  \end{lstlisting}
>  
> -The following driver-read-only field, \field{max_virtqueue_pairs} only exists if
> +The following field, \field{max_virtqueue_pairs} only exists if
>  VIRTIO_NET_F_MQ or VIRTIO_NET_F_RSS is set. This field specifies the maximum number
>  of each of transmit and receive virtqueues (receiveq1\ldots receiveqN
>  and transmitq1\ldots transmitqN respectively) that can be configured once at least one of these features
>  is negotiated.
>  
> -The following driver-read-only field, \field{mtu} only exists if
> -VIRTIO_NET_F_MTU is set. This field specifies the maximum MTU for the driver to
> +The following field, \field{mtu} only exists if VIRTIO_NET_F_MTU
> +is set. This field specifies the maximum MTU for the driver to
>  use.
>  
>  The following two fields, \field{speed} and \field{duplex}, only
> @@ -261,6 +261,8 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>  
>  \drivernormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
>  
> +All the device configuration fields are read-only for the driver.
> +
>  A driver SHOULD negotiate VIRTIO_NET_F_MAC if the device offers it.
>  If the driver negotiates the VIRTIO_NET_F_MAC feature, the driver MUST set
>  the physical address of the NIC to \field{mac}.  Otherwise, it SHOULD
-- 
I'm not the reason you're looking for redemption.

---------------------------------------------------------------------
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: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Cornelia Huck]

On Fri, Feb 17 2023, Parav Pandit <parav@nvidia.com> wrote:

> Device configuration fields are read only. Avoid duplicating this
> description for multiple fields.
>
> Instead describe it one time and do it in the driver requirements
> section.
>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/161
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> ---
> changelog:
> v2->v3:
> - split as new patch
> ---
>  device-types/net/description.tex | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> index a197e1a..81e1135 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -156,10 +156,10 @@ \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network
>  \subsection{Device configuration layout}\label{sec:Device Types / Network Device / Device configuration layout}
>  \label{sec:Device Types / Block Device / Feature bits / Device configuration layout}
>  
> -Device configuration fields are listed below, they are read-only for a driver. The \field{mac} address field
> +Device configuration fields are listed below. The \field{mac} address field

I would not remove this here, as I don't think we should move a simple
statement into the conformance section (see below.) It does makes sense
to remove the duplicate read-only annotations from the individual
fields.

>  always exists (though is only valid if VIRTIO_NET_F_MAC is set), and
>  \field{status} only exists if VIRTIO_NET_F_STATUS is set. Two
> -read-only bits (for the driver) are currently defined for the status field:
> +bits (for the driver) are currently defined for the status field:

What does "bits (for the driver)" mean? It made sense together with
"read-only", but I would drop "(for the driver)" as well.

>  VIRTIO_NET_S_LINK_UP and VIRTIO_NET_S_ANNOUNCE.
>  
>  \begin{lstlisting}
> @@ -167,14 +167,14 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>  #define VIRTIO_NET_S_ANNOUNCE    2
>  \end{lstlisting}
>  
> -The following driver-read-only field, \field{max_virtqueue_pairs} only exists if
> +The following field, \field{max_virtqueue_pairs} only exists if
>  VIRTIO_NET_F_MQ or VIRTIO_NET_F_RSS is set. This field specifies the maximum number
>  of each of transmit and receive virtqueues (receiveq1\ldots receiveqN
>  and transmitq1\ldots transmitqN respectively) that can be configured once at least one of these features
>  is negotiated.
>  
> -The following driver-read-only field, \field{mtu} only exists if
> -VIRTIO_NET_F_MTU is set. This field specifies the maximum MTU for the driver to
> +The following field, \field{mtu} only exists if VIRTIO_NET_F_MTU
> +is set. This field specifies the maximum MTU for the driver to
>  use.
>  
>  The following two fields, \field{speed} and \field{duplex}, only
> @@ -261,6 +261,8 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>  
>  \drivernormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
>  
> +All the device configuration fields are read-only for the driver.

Not sure if this makes a good normative clause, I would rather give the
driver something actionable:

"A driver SHOULD NOT try to write to any of the device configuration
fields."

> +
>  A driver SHOULD negotiate VIRTIO_NET_F_MAC if the device offers it.
>  If the driver negotiates the VIRTIO_NET_F_MAC feature, the driver MUST set
>  the physical address of the NIC to \field{mac}.  Otherwise, it SHOULD


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

Re: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Michael S. Tsirkin]

On Tue, Feb 21, 2023 at 04:37:16PM +0100, Cornelia Huck wrote:
> On Fri, Feb 17 2023, Parav Pandit <parav@nvidia.com> wrote:
> 
> > Device configuration fields are read only. Avoid duplicating this
> > description for multiple fields.
> >
> > Instead describe it one time and do it in the driver requirements
> > section.
> >
> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/161
> > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > ---
> > changelog:
> > v2->v3:
> > - split as new patch
> > ---
> >  device-types/net/description.tex | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> > index a197e1a..81e1135 100644
> > --- a/device-types/net/description.tex
> > +++ b/device-types/net/description.tex
> > @@ -156,10 +156,10 @@ \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network
> >  \subsection{Device configuration layout}\label{sec:Device Types / Network Device / Device configuration layout}
> >  \label{sec:Device Types / Block Device / Feature bits / Device configuration layout}
> >  
> > -Device configuration fields are listed below, they are read-only for a driver. The \field{mac} address field
> > +Device configuration fields are listed below. The \field{mac} address field
> 
> I would not remove this here, as I don't think we should move a simple
> statement into the conformance section (see below.) It does makes sense
> to remove the duplicate read-only annotations from the individual
> fields.
> 
> >  always exists (though is only valid if VIRTIO_NET_F_MAC is set), and
> >  \field{status} only exists if VIRTIO_NET_F_STATUS is set. Two
> > -read-only bits (for the driver) are currently defined for the status field:
> > +bits (for the driver) are currently defined for the status field:
> 
> What does "bits (for the driver)" mean? It made sense together with
> "read-only", but I would drop "(for the driver)" as well.

Ouch Parav are you making search and replace changes without
reading the result? Pls don't.


> >  VIRTIO_NET_S_LINK_UP and VIRTIO_NET_S_ANNOUNCE.
> >  
> >  \begin{lstlisting}
> > @@ -167,14 +167,14 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
> >  #define VIRTIO_NET_S_ANNOUNCE    2
> >  \end{lstlisting}
> >  
> > -The following driver-read-only field, \field{max_virtqueue_pairs} only exists if
> > +The following field, \field{max_virtqueue_pairs} only exists if
> >  VIRTIO_NET_F_MQ or VIRTIO_NET_F_RSS is set. This field specifies the maximum number
> >  of each of transmit and receive virtqueues (receiveq1\ldots receiveqN
> >  and transmitq1\ldots transmitqN respectively) that can be configured once at least one of these features
> >  is negotiated.
> >  
> > -The following driver-read-only field, \field{mtu} only exists if
> > -VIRTIO_NET_F_MTU is set. This field specifies the maximum MTU for the driver to
> > +The following field, \field{mtu} only exists if VIRTIO_NET_F_MTU
> > +is set. This field specifies the maximum MTU for the driver to
> >  use.
> >  
> >  The following two fields, \field{speed} and \field{duplex}, only
> > @@ -261,6 +261,8 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
> >  
> >  \drivernormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
> >  
> > +All the device configuration fields are read-only for the driver.
> 
> Not sure if this makes a good normative clause, I would rather give the
> driver something actionable:
> 
> "A driver SHOULD NOT try to write to any of the device configuration
> fields."

Agree it's not a normative statement as is.
MUST NOT actually - they were always read only.
And no need to "try" just don't write period.

> > +
> >  A driver SHOULD negotiate VIRTIO_NET_F_MAC if the device offers it.
> >  If the driver negotiates the VIRTIO_NET_F_MAC feature, the driver MUST set
> >  the physical address of the NIC to \field{mac}.  Otherwise, it SHOULD

RE: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Tuesday, February 21, 2023 12:42 PM
> >
> > What does "bits (for the driver)" mean? It made sense together with
> > "read-only", but I would drop "(for the driver)" as well.
> 
> Ouch Parav are you making search and replace changes without reading the
> result? Pls don't.
> 
It was wrong to keep the "for the driver".
I will fix this.

> 
> > >  VIRTIO_NET_S_LINK_UP and VIRTIO_NET_S_ANNOUNCE.
> > >
> > >  \begin{lstlisting}
> > > @@ -167,14 +167,14 @@ \subsection{Device configuration
> layout}\label{sec:Device Types / Network Device
> > >  #define VIRTIO_NET_S_ANNOUNCE    2
> > >  \end{lstlisting}
> > >
> > > -The following driver-read-only field, \field{max_virtqueue_pairs}
> > > only exists if
> > > +The following field, \field{max_virtqueue_pairs} only exists if
> > >  VIRTIO_NET_F_MQ or VIRTIO_NET_F_RSS is set. This field specifies
> > > the maximum number  of each of transmit and receive virtqueues
> > > (receiveq1\ldots receiveqN  and transmitq1\ldots transmitqN
> > > respectively) that can be configured once at least one of these features  is
> negotiated.
> > >
> > > -The following driver-read-only field, \field{mtu} only exists if
> > > -VIRTIO_NET_F_MTU is set. This field specifies the maximum MTU for
> > > the driver to
> > > +The following field, \field{mtu} only exists if VIRTIO_NET_F_MTU is
> > > +set. This field specifies the maximum MTU for the driver to
> > >  use.
> > >
> > >  The following two fields, \field{speed} and \field{duplex}, only @@
> > > -261,6 +261,8 @@ \subsection{Device configuration
> > > layout}\label{sec:Device Types / Network Device
> > >
> > >  \drivernormative{\subsubsection}{Device configuration
> > > layout}{Device Types / Network Device / Device configuration layout}
> > >
> > > +All the device configuration fields are read-only for the driver.
> >
> > Not sure if this makes a good normative clause, I would rather give
> > the driver something actionable:
> >
> > "A driver SHOULD NOT try to write to any of the device configuration
> > fields."
> 
> Agree it's not a normative statement as is.
> MUST NOT actually - they were always read only.
> And no need to "try" just don't write period.
> 
Saying driver must not write it, doesn't make it read only for the device.
Hence, it should be mentioned as read-only fields, so when the driver writes something to read-only fields, it can be considered as undefined behavior on such fields.


> > > +
> > >  A driver SHOULD negotiate VIRTIO_NET_F_MAC if the device offers it.
> > >  If the driver negotiates the VIRTIO_NET_F_MAC feature, the driver
> > > MUST set  the physical address of the NIC to \field{mac}.
> > > Otherwise, it SHOULD

Re: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Michael S. Tsirkin]

On Tue, Feb 21, 2023 at 05:50:09PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Tuesday, February 21, 2023 12:42 PM
> > >
> > > What does "bits (for the driver)" mean? It made sense together with
> > > "read-only", but I would drop "(for the driver)" as well.
> > 
> > Ouch Parav are you making search and replace changes without reading the
> > result? Pls don't.
> > 
> It was wrong to keep the "for the driver".
> I will fix this.
> 
> > 
> > > >  VIRTIO_NET_S_LINK_UP and VIRTIO_NET_S_ANNOUNCE.
> > > >
> > > >  \begin{lstlisting}
> > > > @@ -167,14 +167,14 @@ \subsection{Device configuration
> > layout}\label{sec:Device Types / Network Device
> > > >  #define VIRTIO_NET_S_ANNOUNCE    2
> > > >  \end{lstlisting}
> > > >
> > > > -The following driver-read-only field, \field{max_virtqueue_pairs}
> > > > only exists if
> > > > +The following field, \field{max_virtqueue_pairs} only exists if
> > > >  VIRTIO_NET_F_MQ or VIRTIO_NET_F_RSS is set. This field specifies
> > > > the maximum number  of each of transmit and receive virtqueues
> > > > (receiveq1\ldots receiveqN  and transmitq1\ldots transmitqN
> > > > respectively) that can be configured once at least one of these features  is
> > negotiated.
> > > >
> > > > -The following driver-read-only field, \field{mtu} only exists if
> > > > -VIRTIO_NET_F_MTU is set. This field specifies the maximum MTU for
> > > > the driver to
> > > > +The following field, \field{mtu} only exists if VIRTIO_NET_F_MTU is
> > > > +set. This field specifies the maximum MTU for the driver to
> > > >  use.
> > > >
> > > >  The following two fields, \field{speed} and \field{duplex}, only @@
> > > > -261,6 +261,8 @@ \subsection{Device configuration
> > > > layout}\label{sec:Device Types / Network Device
> > > >
> > > >  \drivernormative{\subsubsection}{Device configuration
> > > > layout}{Device Types / Network Device / Device configuration layout}
> > > >
> > > > +All the device configuration fields are read-only for the driver.
> > >
> > > Not sure if this makes a good normative clause, I would rather give
> > > the driver something actionable:
> > >
> > > "A driver SHOULD NOT try to write to any of the device configuration
> > > fields."
> > 
> > Agree it's not a normative statement as is.
> > MUST NOT actually - they were always read only.
> > And no need to "try" just don't write period.
> > 
> Saying driver must not write it, doesn't make it read only for the device.

no but this is not what your patch said either. It's read only for the
driver.

> Hence, it should be mentioned as read-only fields, so when the driver writes something to read-only fields, it can be considered as undefined behavior on such fields.
> 

In the description not in the normative statements. normative sections
just tell driver what it must and must not do, in the standard RFC
terms.

> > > > +
> > > >  A driver SHOULD negotiate VIRTIO_NET_F_MAC if the device offers it.
> > > >  If the driver negotiates the VIRTIO_NET_F_MAC feature, the driver
> > > > MUST set  the physical address of the NIC to \field{mac}.
> > > > Otherwise, it SHOULD

RE: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Tuesday, February 21, 2023 12:52 PM
> 
> On Tue, Feb 21, 2023 at 05:50:09PM +0000, Parav Pandit wrote:
> >
> >
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Tuesday, February 21, 2023 12:42 PM
> > > >
> > > > What does "bits (for the driver)" mean? It made sense together
> > > > with "read-only", but I would drop "(for the driver)" as well.
> > >
> > > Ouch Parav are you making search and replace changes without reading
> > > the result? Pls don't.
> > >
> > It was wrong to keep the "for the driver".
> > I will fix this.
> >
> > >
> > > > >  VIRTIO_NET_S_LINK_UP and VIRTIO_NET_S_ANNOUNCE.
> > > > >
> > > > >  \begin{lstlisting}
> > > > > @@ -167,14 +167,14 @@ \subsection{Device configuration
> > > layout}\label{sec:Device Types / Network Device
> > > > >  #define VIRTIO_NET_S_ANNOUNCE    2
> > > > >  \end{lstlisting}
> > > > >
> > > > > -The following driver-read-only field,
> > > > > \field{max_virtqueue_pairs} only exists if
> > > > > +The following field, \field{max_virtqueue_pairs} only exists if
> > > > >  VIRTIO_NET_F_MQ or VIRTIO_NET_F_RSS is set. This field
> > > > > specifies the maximum number  of each of transmit and receive
> > > > > virtqueues (receiveq1\ldots receiveqN  and transmitq1\ldots
> > > > > transmitqN
> > > > > respectively) that can be configured once at least one of these
> > > > > features  is
> > > negotiated.
> > > > >
> > > > > -The following driver-read-only field, \field{mtu} only exists
> > > > > if -VIRTIO_NET_F_MTU is set. This field specifies the maximum
> > > > > MTU for the driver to
> > > > > +The following field, \field{mtu} only exists if
> > > > > +VIRTIO_NET_F_MTU is set. This field specifies the maximum MTU
> > > > > +for the driver to
> > > > >  use.
> > > > >
> > > > >  The following two fields, \field{speed} and \field{duplex},
> > > > > only @@
> > > > > -261,6 +261,8 @@ \subsection{Device configuration
> > > > > layout}\label{sec:Device Types / Network Device
> > > > >
> > > > >  \drivernormative{\subsubsection}{Device configuration
> > > > > layout}{Device Types / Network Device / Device configuration
> > > > > layout}
> > > > >
> > > > > +All the device configuration fields are read-only for the driver.
> > > >
> > > > Not sure if this makes a good normative clause, I would rather
> > > > give the driver something actionable:
> > > >
> > > > "A driver SHOULD NOT try to write to any of the device
> > > > configuration fields."
> > >
> > > Agree it's not a normative statement as is.
> > > MUST NOT actually - they were always read only.
> > > And no need to "try" just don't write period.
> > >
> > Saying driver must not write it, doesn't make it read only for the device.
> 
> no but this is not what your patch said either. It's read only for the driver.
> 
It is read-only for the driver because the device doesn't treat them as writable.
Do you mean it should be better written as,

These fields are read-only for the driver, hence any writes by the driver to it will be ignored by the device.

?

> > Hence, it should be mentioned as read-only fields, so when the driver writes
> something to read-only fields, it can be considered as undefined behavior on
> such fields.
> >
> 
> In the description not in the normative statements. normative sections just tell
> driver what it must and must not do, in the standard RFC terms.
> 
Got it.
I will shift them as read-only in the description section.
And normative in the device and driver section.
Device section:
Any writes to config space fields is ignored by the device, because these are read-only fields for the driver.

Driver section:
Driver must not write to read-only fields.

Re: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Michael S. Tsirkin]

On Tue, Feb 21, 2023 at 05:59:52PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Tuesday, February 21, 2023 12:52 PM
> > 
> > On Tue, Feb 21, 2023 at 05:50:09PM +0000, Parav Pandit wrote:
> > >
> > >
> > > > From: Michael S. Tsirkin <mst@redhat.com>
> > > > Sent: Tuesday, February 21, 2023 12:42 PM
> > > > >
> > > > > What does "bits (for the driver)" mean? It made sense together
> > > > > with "read-only", but I would drop "(for the driver)" as well.
> > > >
> > > > Ouch Parav are you making search and replace changes without reading
> > > > the result? Pls don't.
> > > >
> > > It was wrong to keep the "for the driver".
> > > I will fix this.
> > >
> > > >
> > > > > >  VIRTIO_NET_S_LINK_UP and VIRTIO_NET_S_ANNOUNCE.
> > > > > >
> > > > > >  \begin{lstlisting}
> > > > > > @@ -167,14 +167,14 @@ \subsection{Device configuration
> > > > layout}\label{sec:Device Types / Network Device
> > > > > >  #define VIRTIO_NET_S_ANNOUNCE    2
> > > > > >  \end{lstlisting}
> > > > > >
> > > > > > -The following driver-read-only field,
> > > > > > \field{max_virtqueue_pairs} only exists if
> > > > > > +The following field, \field{max_virtqueue_pairs} only exists if
> > > > > >  VIRTIO_NET_F_MQ or VIRTIO_NET_F_RSS is set. This field
> > > > > > specifies the maximum number  of each of transmit and receive
> > > > > > virtqueues (receiveq1\ldots receiveqN  and transmitq1\ldots
> > > > > > transmitqN
> > > > > > respectively) that can be configured once at least one of these
> > > > > > features  is
> > > > negotiated.
> > > > > >
> > > > > > -The following driver-read-only field, \field{mtu} only exists
> > > > > > if -VIRTIO_NET_F_MTU is set. This field specifies the maximum
> > > > > > MTU for the driver to
> > > > > > +The following field, \field{mtu} only exists if
> > > > > > +VIRTIO_NET_F_MTU is set. This field specifies the maximum MTU
> > > > > > +for the driver to
> > > > > >  use.
> > > > > >
> > > > > >  The following two fields, \field{speed} and \field{duplex},
> > > > > > only @@
> > > > > > -261,6 +261,8 @@ \subsection{Device configuration
> > > > > > layout}\label{sec:Device Types / Network Device
> > > > > >
> > > > > >  \drivernormative{\subsubsection}{Device configuration
> > > > > > layout}{Device Types / Network Device / Device configuration
> > > > > > layout}
> > > > > >
> > > > > > +All the device configuration fields are read-only for the driver.
> > > > >
> > > > > Not sure if this makes a good normative clause, I would rather
> > > > > give the driver something actionable:
> > > > >
> > > > > "A driver SHOULD NOT try to write to any of the device
> > > > > configuration fields."
> > > >
> > > > Agree it's not a normative statement as is.
> > > > MUST NOT actually - they were always read only.
> > > > And no need to "try" just don't write period.
> > > >
> > > Saying driver must not write it, doesn't make it read only for the device.
> > 
> > no but this is not what your patch said either. It's read only for the driver.
> > 
> It is read-only for the driver because the device doesn't treat them as writable.
> Do you mean it should be better written as,
> 
> These fields are read-only for the driver, hence any writes by the driver to it will be ignored by the device.
> 
> ?

I just concur with Cornelia.

> > > Hence, it should be mentioned as read-only fields, so when the driver writes
> > something to read-only fields, it can be considered as undefined behavior on
> > such fields.
> > >
> > 
> > In the description not in the normative statements. normative sections just tell
> > driver what it must and must not do, in the standard RFC terms.
> > 
> Got it.
> I will shift them as read-only in the description section.
> And normative in the device and driver section.
> Device section:
> Any writes to config space fields is ignored by the device, because these are read-only fields for the driver.

writes is plural so "are ignored"

but more importantly use rfc terms in normative sections.

> 
> Driver section:
> Driver must not write to read-only fields.

[virtio-dev] Re: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Cornelia Huck]

On Tue, Feb 21 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Feb 21, 2023 at 05:59:52PM +0000, Parav Pandit wrote:
>> 
>> > From: Michael S. Tsirkin <mst@redhat.com>
>> > Sent: Tuesday, February 21, 2023 12:52 PM
>> > 
>> > On Tue, Feb 21, 2023 at 05:50:09PM +0000, Parav Pandit wrote:
>> > > Hence, it should be mentioned as read-only fields, so when the driver writes
>> > something to read-only fields, it can be considered as undefined behavior on
>> > such fields.
>> > >
>> > 
>> > In the description not in the normative statements. normative sections just tell
>> > driver what it must and must not do, in the standard RFC terms.
>> > 
>> Got it.
>> I will shift them as read-only in the description section.
>> And normative in the device and driver section.
>> Device section:
>> Any writes to config space fields is ignored by the device, because these are read-only fields for the driver.
>
> writes is plural so "are ignored"
>
> but more importantly use rfc terms in normative sections.

I don't think you need to talk about "read-only" in the normative
sections (that belongs to the descriptive sections.) I'd use

"The device MUST ignore any writes to config space fields by the
driver."

>
>> 
>> Driver section:
>> Driver must not write to read-only fields.

"The driver MUST NOT write to any config space field."


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

Re: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Michael S. Tsirkin]

On Wed, Feb 22, 2023 at 10:01:24AM +0100, Cornelia Huck wrote:
> On Tue, Feb 21 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Tue, Feb 21, 2023 at 05:59:52PM +0000, Parav Pandit wrote:
> >> 
> >> > From: Michael S. Tsirkin <mst@redhat.com>
> >> > Sent: Tuesday, February 21, 2023 12:52 PM
> >> > 
> >> > On Tue, Feb 21, 2023 at 05:50:09PM +0000, Parav Pandit wrote:
> >> > > Hence, it should be mentioned as read-only fields, so when the driver writes
> >> > something to read-only fields, it can be considered as undefined behavior on
> >> > such fields.
> >> > >
> >> > 
> >> > In the description not in the normative statements. normative sections just tell
> >> > driver what it must and must not do, in the standard RFC terms.
> >> > 
> >> Got it.
> >> I will shift them as read-only in the description section.
> >> And normative in the device and driver section.
> >> Device section:
> >> Any writes to config space fields is ignored by the device, because these are read-only fields for the driver.
> >
> > writes is plural so "are ignored"
> >
> > but more importantly use rfc terms in normative sections.
> 
> I don't think you need to talk about "read-only" in the normative
> sections (that belongs to the descriptive sections.) I'd use
> 
> "The device MUST ignore any writes to config space fields by the
> driver."

Hmm. Is this something we previously required for read only fields?


> >
> >> 
> >> Driver section:
> >> Driver must not write to read-only fields.
> 
> "The driver MUST NOT write to any config space field."

[virtio-comment] Re: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Cornelia Huck]

On Wed, Feb 22 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Feb 22, 2023 at 10:01:24AM +0100, Cornelia Huck wrote:
>> On Tue, Feb 21 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> 
>> > On Tue, Feb 21, 2023 at 05:59:52PM +0000, Parav Pandit wrote:
>> >> 
>> >> > From: Michael S. Tsirkin <mst@redhat.com>
>> >> > Sent: Tuesday, February 21, 2023 12:52 PM
>> >> > 
>> >> > On Tue, Feb 21, 2023 at 05:50:09PM +0000, Parav Pandit wrote:
>> >> > > Hence, it should be mentioned as read-only fields, so when the driver writes
>> >> > something to read-only fields, it can be considered as undefined behavior on
>> >> > such fields.
>> >> > >
>> >> > 
>> >> > In the description not in the normative statements. normative sections just tell
>> >> > driver what it must and must not do, in the standard RFC terms.
>> >> > 
>> >> Got it.
>> >> I will shift them as read-only in the description section.
>> >> And normative in the device and driver section.
>> >> Device section:
>> >> Any writes to config space fields is ignored by the device, because these are read-only fields for the driver.
>> >
>> > writes is plural so "are ignored"
>> >
>> > but more importantly use rfc terms in normative sections.
>> 
>> I don't think you need to talk about "read-only" in the normative
>> sections (that belongs to the descriptive sections.) I'd use
>> 
>> "The device MUST ignore any writes to config space fields by the
>> driver."
>
> Hmm. Is this something we previously required for read only fields?

So, better make it SHOULD?

(The only alternative to ignoring I see is breaking the device, and I
think ignoring is preferable.)

>
>
>> >
>> >> 
>> >> Driver section:
>> >> Driver must not write to read-only fields.
>> 
>> "The driver MUST NOT write to any config space field."


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Michael S. Tsirkin]

On Wed, Feb 22, 2023 at 01:07:07PM +0100, Cornelia Huck wrote:
> On Wed, Feb 22 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Feb 22, 2023 at 10:01:24AM +0100, Cornelia Huck wrote:
> >> On Tue, Feb 21 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >> 
> >> > On Tue, Feb 21, 2023 at 05:59:52PM +0000, Parav Pandit wrote:
> >> >> 
> >> >> > From: Michael S. Tsirkin <mst@redhat.com>
> >> >> > Sent: Tuesday, February 21, 2023 12:52 PM
> >> >> > 
> >> >> > On Tue, Feb 21, 2023 at 05:50:09PM +0000, Parav Pandit wrote:
> >> >> > > Hence, it should be mentioned as read-only fields, so when the driver writes
> >> >> > something to read-only fields, it can be considered as undefined behavior on
> >> >> > such fields.
> >> >> > >
> >> >> > 
> >> >> > In the description not in the normative statements. normative sections just tell
> >> >> > driver what it must and must not do, in the standard RFC terms.
> >> >> > 
> >> >> Got it.
> >> >> I will shift them as read-only in the description section.
> >> >> And normative in the device and driver section.
> >> >> Device section:
> >> >> Any writes to config space fields is ignored by the device, because these are read-only fields for the driver.
> >> >
> >> > writes is plural so "are ignored"
> >> >
> >> > but more importantly use rfc terms in normative sections.
> >> 
> >> I don't think you need to talk about "read-only" in the normative
> >> sections (that belongs to the descriptive sections.) I'd use
> >> 
> >> "The device MUST ignore any writes to config space fields by the
> >> driver."
> >
> > Hmm. Is this something we previously required for read only fields?
> 
> So, better make it SHOULD?
> 
> (The only alternative to ignoring I see is breaking the device, and I
> think ignoring is preferable.)

We can just skip adding a new requirement completely - we'll never get
there with a compliant driver.  This is what we do e.g. for MMIO.
Why not?
This has an advantage as this allows backing config with regular RAM.
Also I feel that since it always said "read only for driver" then
this implies a restriction on driver not the device.

> >
> >
> >> >
> >> >> 
> >> >> Driver section:
> >> >> Driver must not write to read-only fields.
> >> 
> >> "The driver MUST NOT write to any config space field."

[virtio-comment] Re: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Cornelia Huck]

On Wed, Feb 22 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Feb 22, 2023 at 01:07:07PM +0100, Cornelia Huck wrote:
>> On Wed, Feb 22 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> 
>> > On Wed, Feb 22, 2023 at 10:01:24AM +0100, Cornelia Huck wrote:
>> >> On Tue, Feb 21 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> >> 
>> >> > On Tue, Feb 21, 2023 at 05:59:52PM +0000, Parav Pandit wrote:
>> >> >> 
>> >> >> > From: Michael S. Tsirkin <mst@redhat.com>
>> >> >> > Sent: Tuesday, February 21, 2023 12:52 PM
>> >> >> > 
>> >> >> > On Tue, Feb 21, 2023 at 05:50:09PM +0000, Parav Pandit wrote:
>> >> >> > > Hence, it should be mentioned as read-only fields, so when the driver writes
>> >> >> > something to read-only fields, it can be considered as undefined behavior on
>> >> >> > such fields.
>> >> >> > >
>> >> >> > 
>> >> >> > In the description not in the normative statements. normative sections just tell
>> >> >> > driver what it must and must not do, in the standard RFC terms.
>> >> >> > 
>> >> >> Got it.
>> >> >> I will shift them as read-only in the description section.
>> >> >> And normative in the device and driver section.
>> >> >> Device section:
>> >> >> Any writes to config space fields is ignored by the device, because these are read-only fields for the driver.
>> >> >
>> >> > writes is plural so "are ignored"
>> >> >
>> >> > but more importantly use rfc terms in normative sections.
>> >> 
>> >> I don't think you need to talk about "read-only" in the normative
>> >> sections (that belongs to the descriptive sections.) I'd use
>> >> 
>> >> "The device MUST ignore any writes to config space fields by the
>> >> driver."
>> >
>> > Hmm. Is this something we previously required for read only fields?
>> 
>> So, better make it SHOULD?
>> 
>> (The only alternative to ignoring I see is breaking the device, and I
>> think ignoring is preferable.)
>
> We can just skip adding a new requirement completely - we'll never get
> there with a compliant driver.  This is what we do e.g. for MMIO.
> Why not?

That would be fine with me as well.

> This has an advantage as this allows backing config with regular RAM.
> Also I feel that since it always said "read only for driver" then
> this implies a restriction on driver not the device.

Indeed. The normative statement below should be enough to make that
"read-only for the driver" thing obvious.

>
>> >
>> >
>> >> >
>> >> >> 
>> >> >> Driver section:
>> >> >> Driver must not write to read-only fields.
>> >> 
>> >> "The driver MUST NOT write to any config space field."


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Parav Pandit]

> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Wednesday, February 22, 2023 8:27 AM

> > We can just skip adding a new requirement completely - we'll never get
> > there with a compliant driver.  This is what we do e.g. for MMIO.
> > Why not?
> 
> That would be fine with me as well.
> 
> > This has an advantage as this allows backing config with regular RAM.
> > Also I feel that since it always said "read only for driver" then this
> > implies a restriction on driver not the device.
> 
> Indeed. The normative statement below should be enough to make that "read-
> only for the driver" thing obvious.
> 
> >
> >> >
> >> >
> >> >> >
> >> >> >>
> >> >> >> Driver section:
> >> >> >> Driver must not write to read-only fields.
> >> >>
> >> >> "The driver MUST NOT write to any config space field."

I addressed the comments in v4 at [1].

[1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00523.html
Please review.

[PATCH v3 2/2] virtio-net: Define cfg fields before description

[Parav Pandit]

Currently some fields of the virtio_net_config structure are defined
before introducing the structure and some are defined after
introducing virtio_net_config.
Better to define the configuration layout first followed by
description of all the fields.

Device configuration fields are described in the section. Change wording
from 'listed' to 'described' as suggested in patch [1].

[1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00004.html

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/161
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
changelog:
v2->v3:
- split the patch for read only description as prepration patch
v1->v2:
- remove read-only wording from multiple places
v0->v1:
- Change wording about device configuration field introduction
- remove duplicate read-only wording for status field
- reword sentence to read it better
---
 device-types/net/description.tex | 40 ++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 81e1135..c4bb64e 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -156,11 +156,28 @@ \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network
 \subsection{Device configuration layout}\label{sec:Device Types / Network Device / Device configuration layout}
 \label{sec:Device Types / Block Device / Feature bits / Device configuration layout}
 
-Device configuration fields are listed below. The \field{mac} address field
-always exists (though is only valid if VIRTIO_NET_F_MAC is set), and
-\field{status} only exists if VIRTIO_NET_F_STATUS is set. Two
-bits (for the driver) are currently defined for the status field:
-VIRTIO_NET_S_LINK_UP and VIRTIO_NET_S_ANNOUNCE.
+The network device has the following device configuration layout.
+
+\begin{lstlisting}
+struct virtio_net_config {
+        u8 mac[6];
+        le16 status;
+        le16 max_virtqueue_pairs;
+        le16 mtu;
+        le32 speed;
+        u8 duplex;
+        u8 rss_max_key_size;
+        le16 rss_max_indirection_table_length;
+        le32 supported_hash_types;
+};
+\end{lstlisting}
+
+The \field{mac} address field always exists (although it is only
+valid if VIRTIO_NET_F_MAC is set).
+
+The \field{status} only exists if VIRTIO_NET_F_STATUS is set.
+Two bits are currently defined for the status field: VIRTIO_NET_S_LINK_UP
+and VIRTIO_NET_S_ANNOUNCE.
 
 \begin{lstlisting}
 #define VIRTIO_NET_S_LINK_UP     1
@@ -190,19 +207,6 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
 is expected to re-read these values after receiving a
 configuration change notification.
 
-\begin{lstlisting}
-struct virtio_net_config {
-        u8 mac[6];
-        le16 status;
-        le16 max_virtqueue_pairs;
-        le16 mtu;
-        le32 speed;
-        u8 duplex;
-        u8 rss_max_key_size;
-        le16 rss_max_indirection_table_length;
-        le32 supported_hash_types;
-};
-\end{lstlisting}
 The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
 It specifies the maximum supported length of RSS key in bytes.
 
-- 
2.26.2

Re: [virtio-comment] [PATCH v3 2/2] virtio-net: Define cfg fields before description

[David Edmondson]

On Friday, 2023-02-17 at 17:45:29 +02, Parav Pandit wrote:
> Currently some fields of the virtio_net_config structure are defined
> before introducing the structure and some are defined after
> introducing virtio_net_config.
> Better to define the configuration layout first followed by
> description of all the fields.
>
> Device configuration fields are described in the section. Change wording
> from 'listed' to 'described' as suggested in patch [1].
>
> [1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00004.html
>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/161
> Signed-off-by: Parav Pandit <parav@nvidia.com>

Reviewed-by: David Edmondson <david.edmondson@oracle.com>

> ---
> changelog:
> v2->v3:
> - split the patch for read only description as prepration patch
> v1->v2:
> - remove read-only wording from multiple places
> v0->v1:
> - Change wording about device configuration field introduction
> - remove duplicate read-only wording for status field
> - reword sentence to read it better
> ---
>  device-types/net/description.tex | 40 ++++++++++++++++++--------------
>  1 file changed, 22 insertions(+), 18 deletions(-)
>
> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> index 81e1135..c4bb64e 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -156,11 +156,28 @@ \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network
>  \subsection{Device configuration layout}\label{sec:Device Types / Network Device / Device configuration layout}
>  \label{sec:Device Types / Block Device / Feature bits / Device configuration layout}
>  
> -Device configuration fields are listed below. The \field{mac} address field
> -always exists (though is only valid if VIRTIO_NET_F_MAC is set), and
> -\field{status} only exists if VIRTIO_NET_F_STATUS is set. Two
> -bits (for the driver) are currently defined for the status field:
> -VIRTIO_NET_S_LINK_UP and VIRTIO_NET_S_ANNOUNCE.
> +The network device has the following device configuration layout.
> +
> +\begin{lstlisting}
> +struct virtio_net_config {
> +        u8 mac[6];
> +        le16 status;
> +        le16 max_virtqueue_pairs;
> +        le16 mtu;
> +        le32 speed;
> +        u8 duplex;
> +        u8 rss_max_key_size;
> +        le16 rss_max_indirection_table_length;
> +        le32 supported_hash_types;
> +};
> +\end{lstlisting}
> +
> +The \field{mac} address field always exists (although it is only
> +valid if VIRTIO_NET_F_MAC is set).
> +
> +The \field{status} only exists if VIRTIO_NET_F_STATUS is set.
> +Two bits are currently defined for the status field: VIRTIO_NET_S_LINK_UP
> +and VIRTIO_NET_S_ANNOUNCE.
>  
>  \begin{lstlisting}
>  #define VIRTIO_NET_S_LINK_UP     1
> @@ -190,19 +207,6 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>  is expected to re-read these values after receiving a
>  configuration change notification.
>  
> -\begin{lstlisting}
> -struct virtio_net_config {
> -        u8 mac[6];
> -        le16 status;
> -        le16 max_virtqueue_pairs;
> -        le16 mtu;
> -        le32 speed;
> -        u8 duplex;
> -        u8 rss_max_key_size;
> -        le16 rss_max_indirection_table_length;
> -        le32 supported_hash_types;
> -};
> -\end{lstlisting}
>  The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
>  It specifies the maximum supported length of RSS key in bytes.
-- 
Tell me sweet little lies.

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

RE: [PATCH v3 0/2] virtio-net: Improve dev config layout

[Parav Pandit]

> From: Parav Pandit <parav@nvidia.com>
> Sent: Friday, February 17, 2023 10:45 AM

> Patch summary:
> patch-1: consolidate read only field at one place in driver requirements
> patch-2: define device configuration layout before describing its fields.
> 
> changelog:
> v2->v3:
> - split into two patches
> - move read only description to driver requirements section

These 2 patches are reviewed.
Can you please open a ballot for it?
Github issue: https://lists.oasis-open.org/archives/virtio-dev/202302/msg00405.html
And the github description has the latest link this time. :)

[virtio-comment] RE: [PATCH v3 0/2] virtio-net: Improve dev config layout

[Parav Pandit]

Hi Cornelia,

> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Monday, March 6, 2023 9:32 AM
> To: Parav Pandit <parav@nvidia.com>; mst@redhat.com; virtio-dev@lists.oasis-
> open.org
> Cc: virtio-comment@lists.oasis-open.org; Shahaf Shuler <shahafs@nvidia.com>
> Subject: RE: [PATCH v3 0/2] virtio-net: Improve dev config layout
> 
> On Tue, Feb 21 2023, Parav Pandit <parav@nvidia.com> wrote:
> 
> >> From: Parav Pandit <parav@nvidia.com>
> >> Sent: Friday, February 17, 2023 10:45 AM
> >
> >> Patch summary:
> >> patch-1: consolidate read only field at one place in driver
> >> requirements
> >> patch-2: define device configuration layout before describing its fields.
> >>
> >> changelog:
> >> v2->v3:
> >> - split into two patches
> >> - move read only description to driver requirements section
> >
> > These 2 patches are reviewed.
> > Can you please open a ballot for it?
> > Github issue:
> > https://lists.oasis-open.org/archives/virtio-dev/202302/msg00405.html
> > And the github description has the latest link this time. :)
> 
> I think something went wrong here... the link above points to this series (i.e. not
> the latest one), and so does the github issue, and therefore the ballot. I think we
> should withdraw the ballot?

Please withdraw.

Right link is: [1].
Please create the new ballot with link [1].

[1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00556.html

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

[virtio-dev] RE: [PATCH v3 0/2] virtio-net: Improve dev config layout

[Parav Pandit]

Hi Cornelia,

> -----Original Message-----
> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Monday, March 6, 2023 9:32 AM
> To: Parav Pandit <parav@nvidia.com>; mst@redhat.com; virtio-dev@lists.oasis-
> open.org
> Cc: virtio-comment@lists.oasis-open.org; Shahaf Shuler <shahafs@nvidia.com>
> Subject: RE: [PATCH v3 0/2] virtio-net: Improve dev config layout
> 
> On Tue, Feb 21 2023, Parav Pandit <parav@nvidia.com> wrote:
> 
> >> From: Parav Pandit <parav@nvidia.com>
> >> Sent: Friday, February 17, 2023 10:45 AM
> >
> >> Patch summary:
> >> patch-1: consolidate read only field at one place in driver
> >> requirements
> >> patch-2: define device configuration layout before describing its fields.
> >>
> >> changelog:
> >> v2->v3:
> >> - split into two patches
> >> - move read only description to driver requirements section
> >
> > These 2 patches are reviewed.
> > Can you please open a ballot for it?
> > Github issue:
> > https://lists.oasis-open.org/archives/virtio-dev/202302/msg00405.html
> > And the github description has the latest link this time. :)
> 
> I think something went wrong here... the link above points to this series (i.e. not
> the latest one), and so does the github issue, and therefore the ballot. I think we
> should withdraw the ballot?

Please withdraw.

Right link is: [1].
Please create the new ballot with link [1].

[1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00556.html

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

Re: [virtio-comment] RE: [PATCH v3 0/2] virtio-net: Improve dev config layout

[Cornelia Huck]

On Tue, Mar 07 2023, Parav Pandit <parav@nvidia.com> wrote:

> Hi Cornelia,
>
>> -----Original Message-----
>> From: Cornelia Huck <cohuck@redhat.com>
>> Sent: Monday, March 6, 2023 9:32 AM
>> To: Parav Pandit <parav@nvidia.com>; mst@redhat.com; virtio-dev@lists.oasis-
>> open.org
>> Cc: virtio-comment@lists.oasis-open.org; Shahaf Shuler <shahafs@nvidia.com>
>> Subject: RE: [PATCH v3 0/2] virtio-net: Improve dev config layout
>> 
>> On Tue, Feb 21 2023, Parav Pandit <parav@nvidia.com> wrote:
>> 
>> >> From: Parav Pandit <parav@nvidia.com>
>> >> Sent: Friday, February 17, 2023 10:45 AM
>> >
>> >> Patch summary:
>> >> patch-1: consolidate read only field at one place in driver
>> >> requirements
>> >> patch-2: define device configuration layout before describing its fields.
>> >>
>> >> changelog:
>> >> v2->v3:
>> >> - split into two patches
>> >> - move read only description to driver requirements section
>> >
>> > These 2 patches are reviewed.
>> > Can you please open a ballot for it?
>> > Github issue:
>> > https://lists.oasis-open.org/archives/virtio-dev/202302/msg00405.html
>> > And the github description has the latest link this time. :)
>> 
>> I think something went wrong here... the link above points to this series (i.e. not
>> the latest one), and so does the github issue, and therefore the ballot. I think we
>> should withdraw the ballot?
>
> Please withdraw.
>
> Right link is: [1].
> Please create the new ballot with link [1].
>
> [1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00556.html

Done.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

[virtio-dev] Re: [virtio-comment] RE: [PATCH v3 0/2] virtio-net: Improve dev config layout

[Cornelia Huck]

On Tue, Mar 07 2023, Parav Pandit <parav@nvidia.com> wrote:

> Hi Cornelia,
>
>> -----Original Message-----
>> From: Cornelia Huck <cohuck@redhat.com>
>> Sent: Monday, March 6, 2023 9:32 AM
>> To: Parav Pandit <parav@nvidia.com>; mst@redhat.com; virtio-dev@lists.oasis-
>> open.org
>> Cc: virtio-comment@lists.oasis-open.org; Shahaf Shuler <shahafs@nvidia.com>
>> Subject: RE: [PATCH v3 0/2] virtio-net: Improve dev config layout
>> 
>> On Tue, Feb 21 2023, Parav Pandit <parav@nvidia.com> wrote:
>> 
>> >> From: Parav Pandit <parav@nvidia.com>
>> >> Sent: Friday, February 17, 2023 10:45 AM
>> >
>> >> Patch summary:
>> >> patch-1: consolidate read only field at one place in driver
>> >> requirements
>> >> patch-2: define device configuration layout before describing its fields.
>> >>
>> >> changelog:
>> >> v2->v3:
>> >> - split into two patches
>> >> - move read only description to driver requirements section
>> >
>> > These 2 patches are reviewed.
>> > Can you please open a ballot for it?
>> > Github issue:
>> > https://lists.oasis-open.org/archives/virtio-dev/202302/msg00405.html
>> > And the github description has the latest link this time. :)
>> 
>> I think something went wrong here... the link above points to this series (i.e. not
>> the latest one), and so does the github issue, and therefore the ballot. I think we
>> should withdraw the ballot?
>
> Please withdraw.
>
> Right link is: [1].
> Please create the new ballot with link [1].
>
> [1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00556.html

Done.


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