Re: [MASSMAIL KLMS] [virtio-comment] Re: [RFC PATCH v1] virtio-vsock: use enums instead of hardcoded constants

[Cornelia Huck <cohuck@redhat.com>]

On Tue, 16 Mar 2021 06:35:11 +0300
Arseny Krasnov <arseny.krasnov@kaspersky.com> wrote:

> On 15.03.2021 18:53, Cornelia Huck wrote:
> > On Mon, 15 Mar 2021 11:35:41 +0300
> > Arseny Krasnov <arseny.krasnov@kaspersky.com> wrote:
> >  
> >> This replaces constants defined in "bit X set to Y" manner
> >> with enums from Linux kernel uapi headers.
> >> ---
> >>  virtio-vsock.tex | 29 ++++++++++++++++++++++-------
> >>  1 file changed, 22 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/virtio-vsock.tex b/virtio-vsock.tex
> >> index da7e641..a97a81e 100644
> >> --- a/virtio-vsock.tex
> >> +++ b/virtio-vsock.tex
> >> @@ -140,8 +140,14 @@ \subsubsection{Addressing}\label{sec:Device Types / Socket Device / Device Opera
> >>  consists of a (cid, port number) tuple. The header fields used for this are
> >>  \field{src_cid}, \field{src_port}, \field{dst_cid}, and \field{dst_port}.
> >>  
> >> -Currently only stream sockets are supported. \field{type} is 1 for stream
> >> -socket types.
> >> +Currently only stream sockets are supported. \field{type} is one of the
> >> +following constants:
> >> +
> >> +\begin{lstlisting}
> >> +enum virtio_vsock_type {
> >> +	VIRTIO_VSOCK_TYPE_STREAM = 1,
> >> +};
> >> +\end{lstlisting}
> >>  
> >>  Stream sockets provide in-order, guaranteed, connection-oriented delivery
> >>  without message boundaries.
> >> @@ -221,11 +227,20 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O
> >>  a listening socket does not exist on the destination or the destination has
> >>  insufficient resources to establish the connection.
> >>  
> >> -When a connected socket receives VIRTIO_VSOCK_OP_SHUTDOWN the header
> >> -\field{flags} field bit 0 indicates that the peer will not receive any more
> >> -data and bit 1 indicates that the peer will not send any more data.  These
> >> -hints are permanent once sent and successive packets with bits clear do not
> >> -reset them.
> >> +When a connected socket receives VIRTIO_VSOCK_OP_SHUTDOWN, header \field{flags}
> >> +field is one of the following constants:  
> > I don't think these two are required to be mutually exclusive (i.e.
> > both can be indicated at the same time?)  
> 
> I'm not sure, in Linux driver there are several cases when these constants are used
> 
> independently from each over, but I think, this is question to implementation of virtio
> 
> vsock protocol, while this patch is cosmetic for specification.

Yes, even if Linux doesn't use it like that, we allow it in the spec.
What about:

"When a connected socked receives VIRTIO_VSOCK_OP_SHUTDOWN, the header
\field{flags} contains a combination of one or more of the following
constants:"

?

> 
> >  
> >> +
> >> +\begin{lstlisting}
> >> +enum virtio_vsock_shutdown {
> >> +	VIRTIO_VSOCK_SHUTDOWN_RCV = 1,
> >> +	VIRTIO_VSOCK_SHUTDOWN_SEND = 2,
> >> +};
> >> +\end{lstlisting}
> >> +
> >> +VIRTIO_VSOCK_SHUTDOWN_RCV indicates that the peer will not receive any more
> >> +data and VIRTIO_VSOCK_SHUTDOWN_SEND indicates that the peer will not send
> >> +any more data.  These hints are permanent once sent and successive packets
> >> +with bits clear do not reset them.
> >>  
> >>  The VIRTIO_VSOCK_OP_RST packet aborts the connection process or forcibly
> >>  disconnects a connected socket.