[virtio-comment] [RFC PATCH v1] virtio-vsock: use C style defines for constants

[virtio-comment] [RFC PATCH v1] virtio-vsock: use C style defines for constants

[Arseny Krasnov]

This:
1) Replaces enums with C style defines.
2) Adds defines for some constants.

Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>
---
 virtio-vsock.tex | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/virtio-vsock.tex b/virtio-vsock.tex
index da7e641..62ab6b3 100644
--- a/virtio-vsock.tex
+++ b/virtio-vsock.tex
@@ -86,23 +86,18 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op
 operation constants:
 
 \begin{lstlisting}
-enum {
-	VIRTIO_VSOCK_OP_INVALID = 0,
-
-	/* Connect operations */
-	VIRTIO_VSOCK_OP_REQUEST = 1,
-	VIRTIO_VSOCK_OP_RESPONSE = 2,
-	VIRTIO_VSOCK_OP_RST = 3,
-	VIRTIO_VSOCK_OP_SHUTDOWN = 4,
-
-	/* To send payload */
-	VIRTIO_VSOCK_OP_RW = 5,
-
-	/* Tell the peer our credit info */
-	VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6,
-	/* Request the peer to send the credit info to us */
-	VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7,
-};
+#define VIRTIO_VSOCK_OP_INVALID        0
+/* Connect operations */
+#define VIRTIO_VSOCK_OP_REQUEST        1
+#define VIRTIO_VSOCK_OP_RESPONSE       2
+#define VIRTIO_VSOCK_OP_RST            3
+#define VIRTIO_VSOCK_OP_SHUTDOWN       4
+/* To send payload */
+#define VIRTIO_VSOCK_OP_RW             5
+/* Tell the peer our credit info */
+#define VIRTIO_VSOCK_OP_CREDIT_UPDATE  6
+/* Request the peer to send the credit info to us */
+#define VIRTIO_VSOCK_OP_CREDIT_REQUEST 7
 \end{lstlisting}
 
 \subsubsection{Virtqueue Flow Control}\label{sec:Device Types / Socket Device / Device Operation / Virtqueue Flow Control}
@@ -143,6 +138,10 @@ \subsubsection{Addressing}\label{sec:Device Types / Socket Device / Device Opera
 Currently only stream sockets are supported. \field{type} is 1 for stream
 socket types.
 
+\begin{lstlisting}
+#define VIRTIO_VSOCK_TYPE_STREAM 1
+\end{lstlisting}
+
 Stream sockets provide in-order, guaranteed, connection-oriented delivery
 without message boundaries.
 
@@ -227,6 +226,11 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O
 hints are permanent once sent and successive packets with bits clear do not
 reset them.
 
+\begin{lstlisting}
+#define VIRTIO_VSOCK_SHUTDOWN_RECEIVE_BIT 0
+#define VIRTIO_VSOCK_SHUTDOWN_SEND_BIT    1
+\end{lstlisting}
+
 The VIRTIO_VSOCK_OP_RST packet aborts the connection process or forcibly
 disconnects a connected socket.
 
@@ -248,9 +252,7 @@ \subsubsection{Device Events}\label{sec:Device Types / Socket Device / Device Op
 The event buffer is as follows:
 
 \begin{lstlisting}
-enum virtio_vsock_event_id {
-        VIRTIO_VSOCK_EVENT_TRANSPORT_RESET = 0,
-};
+#define VIRTIO_VSOCK_EVENT_TRANSPORT_RESET 0
 
 struct virtio_vsock_event {
         le32 id;
-- 
2.25.1


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: [RFC PATCH v1] virtio-vsock: use C style defines for constants

[Stefan Hajnoczi]

[-- Attachment #1.1: Type: text/plain, Size: 3038 bytes --]

On Mon, Mar 22, 2021 at 09:37:59AM +0300, Arseny Krasnov wrote:
> This:
> 1) Replaces enums with C style defines.

Why?

> 2) Adds defines for some constants.

Definitions need to be referenced somewhere to explain their use. It's
not enough to add a constant, some text in the spec should mention that
constant. (The exception to this might be a group of constants where
individual constants don't need to be mentioned explicitly.)

> Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>
> ---
>  virtio-vsock.tex | 42 ++++++++++++++++++++++--------------------
>  1 file changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/virtio-vsock.tex b/virtio-vsock.tex
> index da7e641..62ab6b3 100644
> --- a/virtio-vsock.tex
> +++ b/virtio-vsock.tex
> @@ -86,23 +86,18 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op
>  operation constants:
>  
>  \begin{lstlisting}
> -enum {
> -	VIRTIO_VSOCK_OP_INVALID = 0,
> -
> -	/* Connect operations */
> -	VIRTIO_VSOCK_OP_REQUEST = 1,
> -	VIRTIO_VSOCK_OP_RESPONSE = 2,
> -	VIRTIO_VSOCK_OP_RST = 3,
> -	VIRTIO_VSOCK_OP_SHUTDOWN = 4,
> -
> -	/* To send payload */
> -	VIRTIO_VSOCK_OP_RW = 5,
> -
> -	/* Tell the peer our credit info */
> -	VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6,
> -	/* Request the peer to send the credit info to us */
> -	VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7,
> -};
> +#define VIRTIO_VSOCK_OP_INVALID        0
> +/* Connect operations */
> +#define VIRTIO_VSOCK_OP_REQUEST        1
> +#define VIRTIO_VSOCK_OP_RESPONSE       2
> +#define VIRTIO_VSOCK_OP_RST            3
> +#define VIRTIO_VSOCK_OP_SHUTDOWN       4
> +/* To send payload */
> +#define VIRTIO_VSOCK_OP_RW             5
> +/* Tell the peer our credit info */
> +#define VIRTIO_VSOCK_OP_CREDIT_UPDATE  6
> +/* Request the peer to send the credit info to us */
> +#define VIRTIO_VSOCK_OP_CREDIT_REQUEST 7
>  \end{lstlisting}
>  
>  \subsubsection{Virtqueue Flow Control}\label{sec:Device Types / Socket Device / Device Operation / Virtqueue Flow Control}
> @@ -143,6 +138,10 @@ \subsubsection{Addressing}\label{sec:Device Types / Socket Device / Device Opera
>  Currently only stream sockets are supported. \field{type} is 1 for stream
>  socket types.
>  
> +\begin{lstlisting}
> +#define VIRTIO_VSOCK_TYPE_STREAM 1
> +\end{lstlisting}
> +
>  Stream sockets provide in-order, guaranteed, connection-oriented delivery
>  without message boundaries.
>  
> @@ -227,6 +226,11 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O
>  hints are permanent once sent and successive packets with bits clear do not
>  reset them.
>  
> +\begin{lstlisting}
> +#define VIRTIO_VSOCK_SHUTDOWN_RECEIVE_BIT 0
> +#define VIRTIO_VSOCK_SHUTDOWN_SEND_BIT    1
> +\end{lstlisting}

The spec has no other _BIT constants. It uses bitmasks instead. I
suggest following that for consistency:

  #define VIRTIO_VSOCK_SHUTDOWN_RECEIVE (1 << 0)
  #define VIRTIO_VSOCK_SHUTDOWN_SEND    (1 << 1)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[virtio-comment] Re: [RFC PATCH v1] virtio-vsock: use C style defines for constants

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 3038 bytes --]

On Mon, Mar 22, 2021 at 09:37:59AM +0300, Arseny Krasnov wrote:
> This:
> 1) Replaces enums with C style defines.

Why?

> 2) Adds defines for some constants.

Definitions need to be referenced somewhere to explain their use. It's
not enough to add a constant, some text in the spec should mention that
constant. (The exception to this might be a group of constants where
individual constants don't need to be mentioned explicitly.)

> Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>
> ---
>  virtio-vsock.tex | 42 ++++++++++++++++++++++--------------------
>  1 file changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/virtio-vsock.tex b/virtio-vsock.tex
> index da7e641..62ab6b3 100644
> --- a/virtio-vsock.tex
> +++ b/virtio-vsock.tex
> @@ -86,23 +86,18 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op
>  operation constants:
>  
>  \begin{lstlisting}
> -enum {
> -	VIRTIO_VSOCK_OP_INVALID = 0,
> -
> -	/* Connect operations */
> -	VIRTIO_VSOCK_OP_REQUEST = 1,
> -	VIRTIO_VSOCK_OP_RESPONSE = 2,
> -	VIRTIO_VSOCK_OP_RST = 3,
> -	VIRTIO_VSOCK_OP_SHUTDOWN = 4,
> -
> -	/* To send payload */
> -	VIRTIO_VSOCK_OP_RW = 5,
> -
> -	/* Tell the peer our credit info */
> -	VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6,
> -	/* Request the peer to send the credit info to us */
> -	VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7,
> -};
> +#define VIRTIO_VSOCK_OP_INVALID        0
> +/* Connect operations */
> +#define VIRTIO_VSOCK_OP_REQUEST        1
> +#define VIRTIO_VSOCK_OP_RESPONSE       2
> +#define VIRTIO_VSOCK_OP_RST            3
> +#define VIRTIO_VSOCK_OP_SHUTDOWN       4
> +/* To send payload */
> +#define VIRTIO_VSOCK_OP_RW             5
> +/* Tell the peer our credit info */
> +#define VIRTIO_VSOCK_OP_CREDIT_UPDATE  6
> +/* Request the peer to send the credit info to us */
> +#define VIRTIO_VSOCK_OP_CREDIT_REQUEST 7
>  \end{lstlisting}
>  
>  \subsubsection{Virtqueue Flow Control}\label{sec:Device Types / Socket Device / Device Operation / Virtqueue Flow Control}
> @@ -143,6 +138,10 @@ \subsubsection{Addressing}\label{sec:Device Types / Socket Device / Device Opera
>  Currently only stream sockets are supported. \field{type} is 1 for stream
>  socket types.
>  
> +\begin{lstlisting}
> +#define VIRTIO_VSOCK_TYPE_STREAM 1
> +\end{lstlisting}
> +
>  Stream sockets provide in-order, guaranteed, connection-oriented delivery
>  without message boundaries.
>  
> @@ -227,6 +226,11 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O
>  hints are permanent once sent and successive packets with bits clear do not
>  reset them.
>  
> +\begin{lstlisting}
> +#define VIRTIO_VSOCK_SHUTDOWN_RECEIVE_BIT 0
> +#define VIRTIO_VSOCK_SHUTDOWN_SEND_BIT    1
> +\end{lstlisting}

The spec has no other _BIT constants. It uses bitmasks instead. I
suggest following that for consistency:

  #define VIRTIO_VSOCK_SHUTDOWN_RECEIVE (1 << 0)
  #define VIRTIO_VSOCK_SHUTDOWN_SEND    (1 << 1)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC PATCH v1] virtio-vsock: use C style defines for constants

[Michael S. Tsirkin]

On Mon, Mar 22, 2021 at 11:52:22AM +0000, Stefan Hajnoczi wrote:
> On Mon, Mar 22, 2021 at 09:37:59AM +0300, Arseny Krasnov wrote:
> > This:
> > 1) Replaces enums with C style defines.
> 
> Why?

I am guessing this is referencing
[PATCH] introduction: document #define syntax


> > 2) Adds defines for some constants.
> 
> Definitions need to be referenced somewhere to explain their use. It's
> not enough to add a constant, some text in the spec should mention that
> constant. (The exception to this might be a group of constants where
> individual constants don't need to be mentioned explicitly.)
> 
> > Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>
> > ---
> >  virtio-vsock.tex | 42 ++++++++++++++++++++++--------------------
> >  1 file changed, 22 insertions(+), 20 deletions(-)
> > 
> > diff --git a/virtio-vsock.tex b/virtio-vsock.tex
> > index da7e641..62ab6b3 100644
> > --- a/virtio-vsock.tex
> > +++ b/virtio-vsock.tex
> > @@ -86,23 +86,18 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op
> >  operation constants:
> >  
> >  \begin{lstlisting}
> > -enum {
> > -	VIRTIO_VSOCK_OP_INVALID = 0,
> > -
> > -	/* Connect operations */
> > -	VIRTIO_VSOCK_OP_REQUEST = 1,
> > -	VIRTIO_VSOCK_OP_RESPONSE = 2,
> > -	VIRTIO_VSOCK_OP_RST = 3,
> > -	VIRTIO_VSOCK_OP_SHUTDOWN = 4,
> > -
> > -	/* To send payload */
> > -	VIRTIO_VSOCK_OP_RW = 5,
> > -
> > -	/* Tell the peer our credit info */
> > -	VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6,
> > -	/* Request the peer to send the credit info to us */
> > -	VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7,
> > -};
> > +#define VIRTIO_VSOCK_OP_INVALID        0
> > +/* Connect operations */
> > +#define VIRTIO_VSOCK_OP_REQUEST        1
> > +#define VIRTIO_VSOCK_OP_RESPONSE       2
> > +#define VIRTIO_VSOCK_OP_RST            3
> > +#define VIRTIO_VSOCK_OP_SHUTDOWN       4
> > +/* To send payload */
> > +#define VIRTIO_VSOCK_OP_RW             5
> > +/* Tell the peer our credit info */
> > +#define VIRTIO_VSOCK_OP_CREDIT_UPDATE  6
> > +/* Request the peer to send the credit info to us */
> > +#define VIRTIO_VSOCK_OP_CREDIT_REQUEST 7
> >  \end{lstlisting}
> >  
> >  \subsubsection{Virtqueue Flow Control}\label{sec:Device Types / Socket Device / Device Operation / Virtqueue Flow Control}
> > @@ -143,6 +138,10 @@ \subsubsection{Addressing}\label{sec:Device Types / Socket Device / Device Opera
> >  Currently only stream sockets are supported. \field{type} is 1 for stream

So e.g.

Currently only stream sockets are supported. \field{type} is 1 (VIRTIO_VSOCK_TYPE_STREAM) for stream


> >  socket types.
> >  
> > +\begin{lstlisting}
> > +#define VIRTIO_VSOCK_TYPE_STREAM 1
> > +\end{lstlisting}
> > +
> >  Stream sockets provide in-order, guaranteed, connection-oriented delivery
> >  without message boundaries.
> >  
> > @@ -227,6 +226,11 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O
> >  hints are permanent once sent and successive packets with bits clear do not
> >  reset them.
> >  
> > +\begin{lstlisting}
> > +#define VIRTIO_VSOCK_SHUTDOWN_RECEIVE_BIT 0
> > +#define VIRTIO_VSOCK_SHUTDOWN_SEND_BIT    1
> > +\end{lstlisting}
> 
> The spec has no other _BIT constants.

True. Sometimes there's an _F_ somewhere there instead.

Also, it is possible to put the defines near the flags
field. Compare, for example:

\begin{lstlisting}
struct virtq_desc {
        /* Address (guest-physical). */
        le64 addr;
        /* Length. */
        le32 len;

/* This marks a buffer as continuing via the next field. */
#define VIRTQ_DESC_F_NEXT   1
/* This marks a buffer as device write-only (otherwise device read-only). */
#define VIRTQ_DESC_F_WRITE     2
/* This means the buffer contains a list of buffer descriptors. */
#define VIRTQ_DESC_F_INDIRECT   4
        /* The flags as indicated above. */
        le16 flags;
        /* Next field if flags & NEXT */
        le16 next;
};
\end{lstlisting}


> It uses bitmasks instead.

Not universally.

> I
> suggest following that for consistency:
> 
>   #define VIRTIO_VSOCK_SHUTDOWN_RECEIVE (1 << 0)
>   #define VIRTIO_VSOCK_SHUTDOWN_SEND    (1 << 1)


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[virtio-comment] Re: [RFC PATCH v1] virtio-vsock: use C style defines for constants

[Michael S. Tsirkin]

On Mon, Mar 22, 2021 at 11:52:22AM +0000, Stefan Hajnoczi wrote:
> On Mon, Mar 22, 2021 at 09:37:59AM +0300, Arseny Krasnov wrote:
> > This:
> > 1) Replaces enums with C style defines.
> 
> Why?

I am guessing this is referencing
[PATCH] introduction: document #define syntax


> > 2) Adds defines for some constants.
> 
> Definitions need to be referenced somewhere to explain their use. It's
> not enough to add a constant, some text in the spec should mention that
> constant. (The exception to this might be a group of constants where
> individual constants don't need to be mentioned explicitly.)
> 
> > Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>
> > ---
> >  virtio-vsock.tex | 42 ++++++++++++++++++++++--------------------
> >  1 file changed, 22 insertions(+), 20 deletions(-)
> > 
> > diff --git a/virtio-vsock.tex b/virtio-vsock.tex
> > index da7e641..62ab6b3 100644
> > --- a/virtio-vsock.tex
> > +++ b/virtio-vsock.tex
> > @@ -86,23 +86,18 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op
> >  operation constants:
> >  
> >  \begin{lstlisting}
> > -enum {
> > -	VIRTIO_VSOCK_OP_INVALID = 0,
> > -
> > -	/* Connect operations */
> > -	VIRTIO_VSOCK_OP_REQUEST = 1,
> > -	VIRTIO_VSOCK_OP_RESPONSE = 2,
> > -	VIRTIO_VSOCK_OP_RST = 3,
> > -	VIRTIO_VSOCK_OP_SHUTDOWN = 4,
> > -
> > -	/* To send payload */
> > -	VIRTIO_VSOCK_OP_RW = 5,
> > -
> > -	/* Tell the peer our credit info */
> > -	VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6,
> > -	/* Request the peer to send the credit info to us */
> > -	VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7,
> > -};
> > +#define VIRTIO_VSOCK_OP_INVALID        0
> > +/* Connect operations */
> > +#define VIRTIO_VSOCK_OP_REQUEST        1
> > +#define VIRTIO_VSOCK_OP_RESPONSE       2
> > +#define VIRTIO_VSOCK_OP_RST            3
> > +#define VIRTIO_VSOCK_OP_SHUTDOWN       4
> > +/* To send payload */
> > +#define VIRTIO_VSOCK_OP_RW             5
> > +/* Tell the peer our credit info */
> > +#define VIRTIO_VSOCK_OP_CREDIT_UPDATE  6
> > +/* Request the peer to send the credit info to us */
> > +#define VIRTIO_VSOCK_OP_CREDIT_REQUEST 7
> >  \end{lstlisting}
> >  
> >  \subsubsection{Virtqueue Flow Control}\label{sec:Device Types / Socket Device / Device Operation / Virtqueue Flow Control}
> > @@ -143,6 +138,10 @@ \subsubsection{Addressing}\label{sec:Device Types / Socket Device / Device Opera
> >  Currently only stream sockets are supported. \field{type} is 1 for stream

So e.g.

Currently only stream sockets are supported. \field{type} is 1 (VIRTIO_VSOCK_TYPE_STREAM) for stream


> >  socket types.
> >  
> > +\begin{lstlisting}
> > +#define VIRTIO_VSOCK_TYPE_STREAM 1
> > +\end{lstlisting}
> > +
> >  Stream sockets provide in-order, guaranteed, connection-oriented delivery
> >  without message boundaries.
> >  
> > @@ -227,6 +226,11 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O
> >  hints are permanent once sent and successive packets with bits clear do not
> >  reset them.
> >  
> > +\begin{lstlisting}
> > +#define VIRTIO_VSOCK_SHUTDOWN_RECEIVE_BIT 0
> > +#define VIRTIO_VSOCK_SHUTDOWN_SEND_BIT    1
> > +\end{lstlisting}
> 
> The spec has no other _BIT constants.

True. Sometimes there's an _F_ somewhere there instead.

Also, it is possible to put the defines near the flags
field. Compare, for example:

\begin{lstlisting}
struct virtq_desc {
        /* Address (guest-physical). */
        le64 addr;
        /* Length. */
        le32 len;

/* This marks a buffer as continuing via the next field. */
#define VIRTQ_DESC_F_NEXT   1
/* This marks a buffer as device write-only (otherwise device read-only). */
#define VIRTQ_DESC_F_WRITE     2
/* This means the buffer contains a list of buffer descriptors. */
#define VIRTQ_DESC_F_INDIRECT   4
        /* The flags as indicated above. */
        le16 flags;
        /* Next field if flags & NEXT */
        le16 next;
};
\end{lstlisting}


> It uses bitmasks instead.

Not universally.

> I
> suggest following that for consistency:
> 
>   #define VIRTIO_VSOCK_SHUTDOWN_RECEIVE (1 << 0)
>   #define VIRTIO_VSOCK_SHUTDOWN_SEND    (1 << 1)



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: [RFC PATCH v1] virtio-vsock: use C style defines for constants

[Stefan Hajnoczi]

[-- Attachment #1.1: Type: text/plain, Size: 1048 bytes --]

On Mon, Mar 22, 2021 at 09:34:58AM -0400, Michael S. Tsirkin wrote:
> On Mon, Mar 22, 2021 at 11:52:22AM +0000, Stefan Hajnoczi wrote:
> > On Mon, Mar 22, 2021 at 09:37:59AM +0300, Arseny Krasnov wrote:
> > > @@ -227,6 +226,11 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O
> > >  hints are permanent once sent and successive packets with bits clear do not
> > >  reset them.
> > >  
> > > +\begin{lstlisting}
> > > +#define VIRTIO_VSOCK_SHUTDOWN_RECEIVE_BIT 0
> > > +#define VIRTIO_VSOCK_SHUTDOWN_SEND_BIT    1
> > > +\end{lstlisting}
> > 
> > The spec has no other _BIT constants.
> 
> True. Sometimes there's an _F_ somewhere there instead.

You're right, I missed the virtio-net bit constants:

  #define VIRTIO_NET_F_GUEST_TSO4 7

Compared to:

  #define VIRTQ_DESC_F_AVAIL     (1 << 7)

or

  #define VIRTQ_DESC_F_WRITE     2

It's an inconsistent mix :). Hard to tell them apart when they aren't
bitmask constants with '<<'.

Can we use '<<' for clarity on new constants?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[virtio-comment] Re: [RFC PATCH v1] virtio-vsock: use C style defines for constants

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1048 bytes --]

On Mon, Mar 22, 2021 at 09:34:58AM -0400, Michael S. Tsirkin wrote:
> On Mon, Mar 22, 2021 at 11:52:22AM +0000, Stefan Hajnoczi wrote:
> > On Mon, Mar 22, 2021 at 09:37:59AM +0300, Arseny Krasnov wrote:
> > > @@ -227,6 +226,11 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O
> > >  hints are permanent once sent and successive packets with bits clear do not
> > >  reset them.
> > >  
> > > +\begin{lstlisting}
> > > +#define VIRTIO_VSOCK_SHUTDOWN_RECEIVE_BIT 0
> > > +#define VIRTIO_VSOCK_SHUTDOWN_SEND_BIT    1
> > > +\end{lstlisting}
> > 
> > The spec has no other _BIT constants.
> 
> True. Sometimes there's an _F_ somewhere there instead.

You're right, I missed the virtio-net bit constants:

  #define VIRTIO_NET_F_GUEST_TSO4 7

Compared to:

  #define VIRTQ_DESC_F_AVAIL     (1 << 7)

or

  #define VIRTQ_DESC_F_WRITE     2

It's an inconsistent mix :). Hard to tell them apart when they aren't
bitmask constants with '<<'.

Can we use '<<' for clarity on new constants?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[virtio-comment] Re: [RFC PATCH v1] virtio-vsock: use C style defines for constants

[Arseny Krasnov]

On 22.03.2021 17:30, Stefan Hajnoczi wrote:
> On Mon, Mar 22, 2021 at 09:34:58AM -0400, Michael S. Tsirkin wrote:
>> On Mon, Mar 22, 2021 at 11:52:22AM +0000, Stefan Hajnoczi wrote:
>>> On Mon, Mar 22, 2021 at 09:37:59AM +0300, Arseny Krasnov wrote:
>>>> @@ -227,6 +226,11 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O
>>>>  hints are permanent once sent and successive packets with bits clear do not
>>>>  reset them.
>>>>  
>>>> +\begin{lstlisting}
>>>> +#define VIRTIO_VSOCK_SHUTDOWN_RECEIVE_BIT 0
>>>> +#define VIRTIO_VSOCK_SHUTDOWN_SEND_BIT    1
>>>> +\end{lstlisting}
>>> The spec has no other _BIT constants.
>> True. Sometimes there's an _F_ somewhere there instead.
> You're right, I missed the virtio-net bit constants:
>
>   #define VIRTIO_NET_F_GUEST_TSO4 7
>
> Compared to:
>
>   #define VIRTQ_DESC_F_AVAIL     (1 << 7)
>
> or
>
>   #define VIRTQ_DESC_F_WRITE     2
>
> It's an inconsistent mix :). Hard to tell them apart when they aren't
> bitmask constants with '<<'.
>
> Can we use '<<' for clarity on new constants?

So for bit constants, i can use:

#define VIRTIO_VSOCK_SHUTDOWN_RECEIVE (1 << 0)
#define VIRTIO_VSOCK_SHUTDOWN_SEND    (1 << 1)

for this case and all other next cases, when i'll add SEQPACKET
description in spec?


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: [virtio-comment] Re: [RFC PATCH v1] virtio-vsock: use C style defines for constants

[Stefan Hajnoczi]

[-- Attachment #1.1: Type: text/plain, Size: 1643 bytes --]

On Tue, Mar 23, 2021 at 09:18:34AM +0300, Arseny Krasnov wrote:
> 
> On 22.03.2021 17:30, Stefan Hajnoczi wrote:
> > On Mon, Mar 22, 2021 at 09:34:58AM -0400, Michael S. Tsirkin wrote:
> >> On Mon, Mar 22, 2021 at 11:52:22AM +0000, Stefan Hajnoczi wrote:
> >>> On Mon, Mar 22, 2021 at 09:37:59AM +0300, Arseny Krasnov wrote:
> >>>> @@ -227,6 +226,11 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O
> >>>>  hints are permanent once sent and successive packets with bits clear do not
> >>>>  reset them.
> >>>>  
> >>>> +\begin{lstlisting}
> >>>> +#define VIRTIO_VSOCK_SHUTDOWN_RECEIVE_BIT 0
> >>>> +#define VIRTIO_VSOCK_SHUTDOWN_SEND_BIT    1
> >>>> +\end{lstlisting}
> >>> The spec has no other _BIT constants.
> >> True. Sometimes there's an _F_ somewhere there instead.
> > You're right, I missed the virtio-net bit constants:
> >
> >   #define VIRTIO_NET_F_GUEST_TSO4 7
> >
> > Compared to:
> >
> >   #define VIRTQ_DESC_F_AVAIL     (1 << 7)
> >
> > or
> >
> >   #define VIRTQ_DESC_F_WRITE     2
> >
> > It's an inconsistent mix :). Hard to tell them apart when they aren't
> > bitmask constants with '<<'.
> >
> > Can we use '<<' for clarity on new constants?
> 
> So for bit constants, i can use:
> 
> #define VIRTIO_VSOCK_SHUTDOWN_RECEIVE (1 << 0)
> #define VIRTIO_VSOCK_SHUTDOWN_SEND    (1 << 1)
> 
> for this case and all other next cases, when i'll add SEQPACKET
> description in spec?

Personally I think that's clearest. Michael pointed out that the spec is
not consistent though, so I won't complain if you prefer to keep the
_BIT numbering.

Stefan

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [virtio-comment] Re: [RFC PATCH v1] virtio-vsock: use C style defines for constants

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1643 bytes --]

On Tue, Mar 23, 2021 at 09:18:34AM +0300, Arseny Krasnov wrote:
> 
> On 22.03.2021 17:30, Stefan Hajnoczi wrote:
> > On Mon, Mar 22, 2021 at 09:34:58AM -0400, Michael S. Tsirkin wrote:
> >> On Mon, Mar 22, 2021 at 11:52:22AM +0000, Stefan Hajnoczi wrote:
> >>> On Mon, Mar 22, 2021 at 09:37:59AM +0300, Arseny Krasnov wrote:
> >>>> @@ -227,6 +226,11 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O
> >>>>  hints are permanent once sent and successive packets with bits clear do not
> >>>>  reset them.
> >>>>  
> >>>> +\begin{lstlisting}
> >>>> +#define VIRTIO_VSOCK_SHUTDOWN_RECEIVE_BIT 0
> >>>> +#define VIRTIO_VSOCK_SHUTDOWN_SEND_BIT    1
> >>>> +\end{lstlisting}
> >>> The spec has no other _BIT constants.
> >> True. Sometimes there's an _F_ somewhere there instead.
> > You're right, I missed the virtio-net bit constants:
> >
> >   #define VIRTIO_NET_F_GUEST_TSO4 7
> >
> > Compared to:
> >
> >   #define VIRTQ_DESC_F_AVAIL     (1 << 7)
> >
> > or
> >
> >   #define VIRTQ_DESC_F_WRITE     2
> >
> > It's an inconsistent mix :). Hard to tell them apart when they aren't
> > bitmask constants with '<<'.
> >
> > Can we use '<<' for clarity on new constants?
> 
> So for bit constants, i can use:
> 
> #define VIRTIO_VSOCK_SHUTDOWN_RECEIVE (1 << 0)
> #define VIRTIO_VSOCK_SHUTDOWN_SEND    (1 << 1)
> 
> for this case and all other next cases, when i'll add SEQPACKET
> description in spec?

Personally I think that's clearest. Michael pointed out that the spec is
not consistent though, so I won't complain if you prefer to keep the
_BIT numbering.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]