* [virtio-comment] [RFC PATCH v1 0/1] virtio-vsock: introduce SEQPACKET description @ 2021-02-18 6:07 Arseny Krasnov 2021-02-18 6:08 ` [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description Arseny Krasnov 2021-02-22 10:16 ` Stefano Garzarella 0 siblings, 2 replies; 22+ messages in thread From: Arseny Krasnov @ 2021-02-18 6:07 UTC (permalink / raw) To: cohuck, virtio-comment, Stefan Hajnoczi, Stefano Garzarella, Michael S. Tsirkin, Jason Wang, David S. Miller, Jakub Kicinski, Arseny Krasnov, Jorgen Hansen, Norbert Slusarek, Colin Ian King, Andra Paraschiv Cc: virtualization, oxffffaa This patchset adds description of SOCK_SEQPACKET socket's type of virtio vsock. Here is implementation: https://lkml.org/lkml/2021/2/18/24 Arseny Krasnov(1): virtio-vsock: add SOCK_SEQPACKET description virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- 1 files changed, 37 insertions(+), 3 deletions(-) TODO: - for messages identification I use header's field called 'msg_cnt'. May be it is better to call it 'msg_id' or 'msg_num', because it is used as ID, but implemented as counter. - in current version of specification, some values of headers' fields still unnamed, for example type of socket (stream or seqpacket), then shutdown flags. Spec says that for stream socket value of 'type' must be 1. For receive shutdown bit 0 is set in 'flags', for send shutdown bit 1 is set in 'flags'. But in Linux these unnamed ones and zeroes are implemented as enums, so may be it will be ok to add such enums in specification (as 'enum virtio_vsock_event_id'). Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> -- 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/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description 2021-02-18 6:07 [virtio-comment] [RFC PATCH v1 0/1] virtio-vsock: introduce SEQPACKET description Arseny Krasnov @ 2021-02-18 6:08 ` Arseny Krasnov 2021-02-24 9:32 ` Stefano Garzarella 2021-03-16 13:49 ` Michael S. Tsirkin 2021-02-22 10:16 ` Stefano Garzarella 1 sibling, 2 replies; 22+ messages in thread From: Arseny Krasnov @ 2021-02-18 6:08 UTC (permalink / raw) To: cohuck, virtio-comment, Stefan Hajnoczi, Stefano Garzarella, Michael S. Tsirkin, Jason Wang, David S. Miller, Jakub Kicinski, Arseny Krasnov, Jorgen Hansen, Andra Paraschiv, Norbert Slusarek, Colin Ian King Cc: virtualization, oxffffaa Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> --- virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/virtio-vsock.tex b/virtio-vsock.tex index da7e641..1ee8f99 100644 --- a/virtio-vsock.tex +++ b/virtio-vsock.tex @@ -102,6 +102,11 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6, /* Request the peer to send the credit info to us */ VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7, + + /* Message begin for SOCK_SEQPACKET */ + VIRTIO_VSOCK_OP_SEQ_BEGIN = 8, + /* Message end for SOCK_SEQPACKET */ + VIRTIO_VSOCK_OP_SEQ_END = 9, }; \end{lstlisting} @@ -140,11 +145,11 @@ \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 stream and seqpacket sockets are supported. \field{type} is 1 for +stream socket types. \field{type} is 2 for seqpacket socket types. Stream sockets provide in-order, guaranteed, connection-oriented delivery -without message boundaries. +without message boundaries. Seqpacket sockets also provide message boundaries. \subsubsection{Buffer Space Management}\label{sec:Device Types / Socket Device / Device Operation / Buffer Space Management} \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of @@ -240,6 +245,35 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O destination) address tuple for a new connection while the other peer is still processing the old connection. +\subsubsection{Seqpacket Sockets}\label{sec:Device Types / Socket Device / Device Operation / Seqpacket Sockets} + +Seqpacket sockets differ from stream sockets only in data transmission way: in +stream sockets all data is sent using only VIRTIO_VSOCK_OP_RW packets. In +seqpacket sockets, to provide message boundaries, every sequence of +VIRTIO_VSOCK_OP_RW packets of each message is headed with +VIRTIO_VSOCK_OP_SEQ_BEGIN and tailed with VIRTIO_VSOCK_OP_SEQ_END packets. +Both VIRTIO_VSOCK_OP_SEQ_BEGIN and VIRTIO_VSOCK_OP_SEQ_END packets carry +special header in payload: + +\begin{lstlisting} +struct virtio_vsock_seq_hdr { + __le32 msg_cnt; + __le32 msg_len; +}; +\end{lstlisting} + +\field{msg_cnt} is per socket and incremented by 1 on every send of +VIRTIO_VSOCK_OP_SEQ_BEGIN or VIRTIO_VSOCK_OP_SEQ_END. \field{msg_len} contains +message length. This header is used to check integrity of each message: message +is valid if length of data in VIRTIO_VSOCK_OP_RW packets is equal to +\field{msg_len} of prepending VIRTIO_VSOCK_OP_SEQ_BEGIN and \field{msg_cnt} of +VIRTIO_VSOCK_OP_SEQ_END differs from \field{msg_cnt} of +VIRTIO_VSOCK_OP_SEQ_BEGIN by 1. + +POSIX MSG_EOR flag is supported by special value of \field{flags} in packet's +header. If this flag is set for message, then all VIRTIO_VSOCK_OP_RW packets +of this message have bit 0 is set to 1 in \field{flags}. + \subsubsection{Device Events}\label{sec:Device Types / Socket Device / Device Operation / Device Events} Certain events are communicated by the device to the driver using the event -- 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/ ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description 2021-02-18 6:08 ` [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description Arseny Krasnov @ 2021-02-24 9:32 ` Stefano Garzarella 2021-03-16 13:49 ` Michael S. Tsirkin 1 sibling, 0 replies; 22+ messages in thread From: Stefano Garzarella @ 2021-02-24 9:32 UTC (permalink / raw) To: Arseny Krasnov Cc: cohuck, virtio-comment, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, David S. Miller, Jakub Kicinski, Jorgen Hansen, Andra Paraschiv, Norbert Slusarek, Colin Ian King, virtualization, oxffffaa On Thu, Feb 18, 2021 at 09:08:23AM +0300, Arseny Krasnov wrote: >Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> >--- > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- > 1 file changed, 37 insertions(+), 3 deletions(-) > >diff --git a/virtio-vsock.tex b/virtio-vsock.tex >index da7e641..1ee8f99 100644 >--- a/virtio-vsock.tex >+++ b/virtio-vsock.tex >@@ -102,6 +102,11 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op > VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6, > /* Request the peer to send the credit info to us */ > VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7, >+ >+ /* Message begin for SOCK_SEQPACKET */ >+ VIRTIO_VSOCK_OP_SEQ_BEGIN = 8, >+ /* Message end for SOCK_SEQPACKET */ >+ VIRTIO_VSOCK_OP_SEQ_END = 9, > }; > \end{lstlisting} > >@@ -140,11 +145,11 @@ \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 stream and seqpacket sockets are supported. \field{type} is 1 for >+stream socket types. \field{type} is 2 for seqpacket socket types. > > Stream sockets provide in-order, guaranteed, connection-oriented delivery >-without message boundaries. >+without message boundaries. Seqpacket sockets also provide message boundaries. > > \subsubsection{Buffer Space Management}\label{sec:Device Types / Socket Device / Device Operation / Buffer Space Management} > \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of >@@ -240,6 +245,35 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O > destination) address tuple for a new connection while the other peer is still > processing the old connection. > >+\subsubsection{Seqpacket Sockets}\label{sec:Device Types / Socket Device / Device Operation / Seqpacket Sockets} >+ >+Seqpacket sockets differ from stream sockets only in data transmission way: in >+stream sockets all data is sent using only VIRTIO_VSOCK_OP_RW packets. In >+seqpacket sockets, to provide message boundaries, every sequence of >+VIRTIO_VSOCK_OP_RW packets of each message is headed with ^ Since this is a spec, I think we should use MUST when something must be respected by the peer, for example here we can say "MUST be headed" >+VIRTIO_VSOCK_OP_SEQ_BEGIN and tailed with VIRTIO_VSOCK_OP_SEQ_END packets. >+Both VIRTIO_VSOCK_OP_SEQ_BEGIN and VIRTIO_VSOCK_OP_SEQ_END packets carry ^ Same here "MUST carry" and in the rest of the patch. >+special header in payload: >+ >+\begin{lstlisting} >+struct virtio_vsock_seq_hdr { >+ __le32 msg_cnt; >+ __le32 msg_len; >+}; >+\end{lstlisting} >+ >+\field{msg_cnt} is per socket and incremented by 1 on every send of >+VIRTIO_VSOCK_OP_SEQ_BEGIN or VIRTIO_VSOCK_OP_SEQ_END. \field{msg_len} contains >+message length. This header is used to check integrity of each message: message >+is valid if length of data in VIRTIO_VSOCK_OP_RW packets is equal to >+\field{msg_len} of prepending VIRTIO_VSOCK_OP_SEQ_BEGIN and \field{msg_cnt} of >+VIRTIO_VSOCK_OP_SEQ_END differs from \field{msg_cnt} of >+VIRTIO_VSOCK_OP_SEQ_BEGIN by 1. If we replace msg_cnt with msg_id, I think we should have the same 'msg_id' for VIRTIO_VSOCK_OP_SEQ_BEGIN or VIRTIO_VSOCK_OP_SEQ_END. If you want to use 'msg_cnt' and you increment it between BEGIN and END, we should consider the overflow case. >+ >+POSIX MSG_EOR flag is supported by special value of \field{flags} in packet's >+header. If this flag is set for message, then all VIRTIO_VSOCK_OP_RW packets >+of this message have bit 0 is set to 1 in \field{flags}. Maybe we need to define what MSG_EOR means. Thanks, Stefano >+ > \subsubsection{Device Events}\label{sec:Device Types / Socket Device / Device Operation / Device Events} > > Certain events are communicated by the device to the driver using the event >-- >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/ > 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/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description @ 2021-02-24 9:32 ` Stefano Garzarella 0 siblings, 0 replies; 22+ messages in thread From: Stefano Garzarella @ 2021-02-24 9:32 UTC (permalink / raw) To: Arseny Krasnov Cc: Andra Paraschiv, Michael S. Tsirkin, cohuck, Colin Ian King, oxffffaa, Norbert Slusarek, Stefan Hajnoczi, virtio-comment, Jakub Kicinski, virtualization, David S. Miller, Jorgen Hansen On Thu, Feb 18, 2021 at 09:08:23AM +0300, Arseny Krasnov wrote: >Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> >--- > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- > 1 file changed, 37 insertions(+), 3 deletions(-) > >diff --git a/virtio-vsock.tex b/virtio-vsock.tex >index da7e641..1ee8f99 100644 >--- a/virtio-vsock.tex >+++ b/virtio-vsock.tex >@@ -102,6 +102,11 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op > VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6, > /* Request the peer to send the credit info to us */ > VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7, >+ >+ /* Message begin for SOCK_SEQPACKET */ >+ VIRTIO_VSOCK_OP_SEQ_BEGIN = 8, >+ /* Message end for SOCK_SEQPACKET */ >+ VIRTIO_VSOCK_OP_SEQ_END = 9, > }; > \end{lstlisting} > >@@ -140,11 +145,11 @@ \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 stream and seqpacket sockets are supported. \field{type} is 1 for >+stream socket types. \field{type} is 2 for seqpacket socket types. > > Stream sockets provide in-order, guaranteed, connection-oriented delivery >-without message boundaries. >+without message boundaries. Seqpacket sockets also provide message boundaries. > > \subsubsection{Buffer Space Management}\label{sec:Device Types / Socket Device / Device Operation / Buffer Space Management} > \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of >@@ -240,6 +245,35 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O > destination) address tuple for a new connection while the other peer is still > processing the old connection. > >+\subsubsection{Seqpacket Sockets}\label{sec:Device Types / Socket Device / Device Operation / Seqpacket Sockets} >+ >+Seqpacket sockets differ from stream sockets only in data transmission way: in >+stream sockets all data is sent using only VIRTIO_VSOCK_OP_RW packets. In >+seqpacket sockets, to provide message boundaries, every sequence of >+VIRTIO_VSOCK_OP_RW packets of each message is headed with ^ Since this is a spec, I think we should use MUST when something must be respected by the peer, for example here we can say "MUST be headed" >+VIRTIO_VSOCK_OP_SEQ_BEGIN and tailed with VIRTIO_VSOCK_OP_SEQ_END packets. >+Both VIRTIO_VSOCK_OP_SEQ_BEGIN and VIRTIO_VSOCK_OP_SEQ_END packets carry ^ Same here "MUST carry" and in the rest of the patch. >+special header in payload: >+ >+\begin{lstlisting} >+struct virtio_vsock_seq_hdr { >+ __le32 msg_cnt; >+ __le32 msg_len; >+}; >+\end{lstlisting} >+ >+\field{msg_cnt} is per socket and incremented by 1 on every send of >+VIRTIO_VSOCK_OP_SEQ_BEGIN or VIRTIO_VSOCK_OP_SEQ_END. \field{msg_len} contains >+message length. This header is used to check integrity of each message: message >+is valid if length of data in VIRTIO_VSOCK_OP_RW packets is equal to >+\field{msg_len} of prepending VIRTIO_VSOCK_OP_SEQ_BEGIN and \field{msg_cnt} of >+VIRTIO_VSOCK_OP_SEQ_END differs from \field{msg_cnt} of >+VIRTIO_VSOCK_OP_SEQ_BEGIN by 1. If we replace msg_cnt with msg_id, I think we should have the same 'msg_id' for VIRTIO_VSOCK_OP_SEQ_BEGIN or VIRTIO_VSOCK_OP_SEQ_END. If you want to use 'msg_cnt' and you increment it between BEGIN and END, we should consider the overflow case. >+ >+POSIX MSG_EOR flag is supported by special value of \field{flags} in packet's >+header. If this flag is set for message, then all VIRTIO_VSOCK_OP_RW packets >+of this message have bit 0 is set to 1 in \field{flags}. Maybe we need to define what MSG_EOR means. Thanks, Stefano >+ > \subsubsection{Device Events}\label{sec:Device Types / Socket Device / Device Operation / Device Events} > > Certain events are communicated by the device to the driver using the event >-- >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/ > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description 2021-02-24 9:32 ` Stefano Garzarella @ 2021-03-03 12:08 ` Cornelia Huck -1 siblings, 0 replies; 22+ messages in thread From: Cornelia Huck @ 2021-03-03 12:08 UTC (permalink / raw) To: Stefano Garzarella Cc: Arseny Krasnov, virtio-comment, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, David S. Miller, Jakub Kicinski, Jorgen Hansen, Andra Paraschiv, Norbert Slusarek, Colin Ian King, virtualization, oxffffaa On Wed, 24 Feb 2021 10:32:00 +0100 Stefano Garzarella <sgarzare@redhat.com> wrote: > On Thu, Feb 18, 2021 at 09:08:23AM +0300, Arseny Krasnov wrote: > >Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> > >--- > > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- > > 1 file changed, 37 insertions(+), 3 deletions(-) > > > >diff --git a/virtio-vsock.tex b/virtio-vsock.tex > >index da7e641..1ee8f99 100644 > >--- a/virtio-vsock.tex > >+++ b/virtio-vsock.tex > >@@ -102,6 +102,11 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op > > VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6, > > /* Request the peer to send the credit info to us */ > > VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7, > >+ > >+ /* Message begin for SOCK_SEQPACKET */ > >+ VIRTIO_VSOCK_OP_SEQ_BEGIN = 8, > >+ /* Message end for SOCK_SEQPACKET */ > >+ VIRTIO_VSOCK_OP_SEQ_END = 9, > > }; > > \end{lstlisting} > > > >@@ -140,11 +145,11 @@ \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 stream and seqpacket sockets are supported. \field{type} is 1 for > >+stream socket types. \field{type} is 2 for seqpacket socket types. > > > > Stream sockets provide in-order, guaranteed, connection-oriented delivery > >-without message boundaries. > >+without message boundaries. Seqpacket sockets also provide message boundaries. > > > > \subsubsection{Buffer Space Management}\label{sec:Device Types / Socket Device / Device Operation / Buffer Space Management} > > \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of > >@@ -240,6 +245,35 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O > > destination) address tuple for a new connection while the other peer is still > > processing the old connection. > > > >+\subsubsection{Seqpacket Sockets}\label{sec:Device Types / Socket Device / Device Operation / Seqpacket Sockets} > >+ > >+Seqpacket sockets differ from stream sockets only in data transmission way: in > >+stream sockets all data is sent using only VIRTIO_VSOCK_OP_RW packets. In > >+seqpacket sockets, to provide message boundaries, every sequence of > >+VIRTIO_VSOCK_OP_RW packets of each message is headed with > ^ > Since this is a spec, I think we should use MUST when something must be > respected by the peer, for example here we can say "MUST be headed" > > >+VIRTIO_VSOCK_OP_SEQ_BEGIN and tailed with VIRTIO_VSOCK_OP_SEQ_END packets. > >+Both VIRTIO_VSOCK_OP_SEQ_BEGIN and VIRTIO_VSOCK_OP_SEQ_END packets carry > ^ > Same here "MUST carry" and in the rest of the patch. Actually, MUST and friends are really for normative sections; I'd advise to have a description of how this feature works and then some device/driver normative clauses with MUST statements (like "the device MUST reject <malformed packets>" or so). 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/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description @ 2021-03-03 12:08 ` Cornelia Huck 0 siblings, 0 replies; 22+ messages in thread From: Cornelia Huck @ 2021-03-03 12:08 UTC (permalink / raw) To: Stefano Garzarella Cc: Andra Paraschiv, Michael S. Tsirkin, Colin Ian King, oxffffaa, Norbert Slusarek, Stefan Hajnoczi, virtio-comment, Jakub Kicinski, Arseny Krasnov, virtualization, David S. Miller, Jorgen Hansen On Wed, 24 Feb 2021 10:32:00 +0100 Stefano Garzarella <sgarzare@redhat.com> wrote: > On Thu, Feb 18, 2021 at 09:08:23AM +0300, Arseny Krasnov wrote: > >Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> > >--- > > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- > > 1 file changed, 37 insertions(+), 3 deletions(-) > > > >diff --git a/virtio-vsock.tex b/virtio-vsock.tex > >index da7e641..1ee8f99 100644 > >--- a/virtio-vsock.tex > >+++ b/virtio-vsock.tex > >@@ -102,6 +102,11 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op > > VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6, > > /* Request the peer to send the credit info to us */ > > VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7, > >+ > >+ /* Message begin for SOCK_SEQPACKET */ > >+ VIRTIO_VSOCK_OP_SEQ_BEGIN = 8, > >+ /* Message end for SOCK_SEQPACKET */ > >+ VIRTIO_VSOCK_OP_SEQ_END = 9, > > }; > > \end{lstlisting} > > > >@@ -140,11 +145,11 @@ \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 stream and seqpacket sockets are supported. \field{type} is 1 for > >+stream socket types. \field{type} is 2 for seqpacket socket types. > > > > Stream sockets provide in-order, guaranteed, connection-oriented delivery > >-without message boundaries. > >+without message boundaries. Seqpacket sockets also provide message boundaries. > > > > \subsubsection{Buffer Space Management}\label{sec:Device Types / Socket Device / Device Operation / Buffer Space Management} > > \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of > >@@ -240,6 +245,35 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O > > destination) address tuple for a new connection while the other peer is still > > processing the old connection. > > > >+\subsubsection{Seqpacket Sockets}\label{sec:Device Types / Socket Device / Device Operation / Seqpacket Sockets} > >+ > >+Seqpacket sockets differ from stream sockets only in data transmission way: in > >+stream sockets all data is sent using only VIRTIO_VSOCK_OP_RW packets. In > >+seqpacket sockets, to provide message boundaries, every sequence of > >+VIRTIO_VSOCK_OP_RW packets of each message is headed with > ^ > Since this is a spec, I think we should use MUST when something must be > respected by the peer, for example here we can say "MUST be headed" > > >+VIRTIO_VSOCK_OP_SEQ_BEGIN and tailed with VIRTIO_VSOCK_OP_SEQ_END packets. > >+Both VIRTIO_VSOCK_OP_SEQ_BEGIN and VIRTIO_VSOCK_OP_SEQ_END packets carry > ^ > Same here "MUST carry" and in the rest of the patch. Actually, MUST and friends are really for normative sections; I'd advise to have a description of how this feature works and then some device/driver normative clauses with MUST statements (like "the device MUST reject <malformed packets>" or so). _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description 2021-03-03 12:08 ` Cornelia Huck @ 2021-03-03 12:35 ` Stefano Garzarella -1 siblings, 0 replies; 22+ messages in thread From: Stefano Garzarella @ 2021-03-03 12:35 UTC (permalink / raw) To: Cornelia Huck Cc: Arseny Krasnov, virtio-comment, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, David S. Miller, Jakub Kicinski, Jorgen Hansen, Andra Paraschiv, Norbert Slusarek, Colin Ian King, virtualization, oxffffaa On Wed, Mar 03, 2021 at 01:08:43PM +0100, Cornelia Huck wrote: >On Wed, 24 Feb 2021 10:32:00 +0100 >Stefano Garzarella <sgarzare@redhat.com> wrote: > >> On Thu, Feb 18, 2021 at 09:08:23AM +0300, Arseny Krasnov wrote: >> >Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> >> >--- >> > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- >> > 1 file changed, 37 insertions(+), 3 deletions(-) >> > >> >diff --git a/virtio-vsock.tex b/virtio-vsock.tex >> >index da7e641..1ee8f99 100644 >> >--- a/virtio-vsock.tex >> >+++ b/virtio-vsock.tex >> >@@ -102,6 +102,11 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op >> > VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6, >> > /* Request the peer to send the credit info to us */ >> > VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7, >> >+ >> >+ /* Message begin for SOCK_SEQPACKET */ >> >+ VIRTIO_VSOCK_OP_SEQ_BEGIN = 8, >> >+ /* Message end for SOCK_SEQPACKET */ >> >+ VIRTIO_VSOCK_OP_SEQ_END = 9, >> > }; >> > \end{lstlisting} >> > >> >@@ -140,11 +145,11 @@ \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 stream and seqpacket sockets are supported. \field{type} is 1 for >> >+stream socket types. \field{type} is 2 for seqpacket socket types. >> > >> > Stream sockets provide in-order, guaranteed, connection-oriented delivery >> >-without message boundaries. >> >+without message boundaries. Seqpacket sockets also provide message boundaries. >> > >> > \subsubsection{Buffer Space Management}\label{sec:Device Types / Socket Device / Device Operation / Buffer Space Management} >> > \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of >> >@@ -240,6 +245,35 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O >> > destination) address tuple for a new connection while the other peer is still >> > processing the old connection. >> > >> >+\subsubsection{Seqpacket Sockets}\label{sec:Device Types / Socket Device / Device Operation / Seqpacket Sockets} >> >+ >> >+Seqpacket sockets differ from stream sockets only in data transmission way: in >> >+stream sockets all data is sent using only VIRTIO_VSOCK_OP_RW packets. In >> >+seqpacket sockets, to provide message boundaries, every sequence of >> >+VIRTIO_VSOCK_OP_RW packets of each message is headed with >> ^ >> Since this is a spec, I think we should use MUST when something must be >> respected by the peer, for example here we can say "MUST be headed" >> >> >+VIRTIO_VSOCK_OP_SEQ_BEGIN and tailed with VIRTIO_VSOCK_OP_SEQ_END packets. >> >+Both VIRTIO_VSOCK_OP_SEQ_BEGIN and VIRTIO_VSOCK_OP_SEQ_END packets carry >> ^ >> Same here "MUST carry" and in the rest of the patch. > >Actually, MUST and friends are really for normative sections; I'd >advise to have a description of how this feature works and then some >device/driver normative clauses with MUST statements (like "the device >MUST reject <malformed packets>" or so). > Okay, got it. Thanks for the clarification, Stefano 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/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description @ 2021-03-03 12:35 ` Stefano Garzarella 0 siblings, 0 replies; 22+ messages in thread From: Stefano Garzarella @ 2021-03-03 12:35 UTC (permalink / raw) To: Cornelia Huck Cc: Andra Paraschiv, Michael S. Tsirkin, Colin Ian King, oxffffaa, Norbert Slusarek, Stefan Hajnoczi, virtio-comment, Jakub Kicinski, Arseny Krasnov, virtualization, David S. Miller, Jorgen Hansen On Wed, Mar 03, 2021 at 01:08:43PM +0100, Cornelia Huck wrote: >On Wed, 24 Feb 2021 10:32:00 +0100 >Stefano Garzarella <sgarzare@redhat.com> wrote: > >> On Thu, Feb 18, 2021 at 09:08:23AM +0300, Arseny Krasnov wrote: >> >Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> >> >--- >> > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- >> > 1 file changed, 37 insertions(+), 3 deletions(-) >> > >> >diff --git a/virtio-vsock.tex b/virtio-vsock.tex >> >index da7e641..1ee8f99 100644 >> >--- a/virtio-vsock.tex >> >+++ b/virtio-vsock.tex >> >@@ -102,6 +102,11 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op >> > VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6, >> > /* Request the peer to send the credit info to us */ >> > VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7, >> >+ >> >+ /* Message begin for SOCK_SEQPACKET */ >> >+ VIRTIO_VSOCK_OP_SEQ_BEGIN = 8, >> >+ /* Message end for SOCK_SEQPACKET */ >> >+ VIRTIO_VSOCK_OP_SEQ_END = 9, >> > }; >> > \end{lstlisting} >> > >> >@@ -140,11 +145,11 @@ \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 stream and seqpacket sockets are supported. \field{type} is 1 for >> >+stream socket types. \field{type} is 2 for seqpacket socket types. >> > >> > Stream sockets provide in-order, guaranteed, connection-oriented delivery >> >-without message boundaries. >> >+without message boundaries. Seqpacket sockets also provide message boundaries. >> > >> > \subsubsection{Buffer Space Management}\label{sec:Device Types / Socket Device / Device Operation / Buffer Space Management} >> > \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of >> >@@ -240,6 +245,35 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O >> > destination) address tuple for a new connection while the other peer is still >> > processing the old connection. >> > >> >+\subsubsection{Seqpacket Sockets}\label{sec:Device Types / Socket Device / Device Operation / Seqpacket Sockets} >> >+ >> >+Seqpacket sockets differ from stream sockets only in data transmission way: in >> >+stream sockets all data is sent using only VIRTIO_VSOCK_OP_RW packets. In >> >+seqpacket sockets, to provide message boundaries, every sequence of >> >+VIRTIO_VSOCK_OP_RW packets of each message is headed with >> ^ >> Since this is a spec, I think we should use MUST when something must be >> respected by the peer, for example here we can say "MUST be headed" >> >> >+VIRTIO_VSOCK_OP_SEQ_BEGIN and tailed with VIRTIO_VSOCK_OP_SEQ_END packets. >> >+Both VIRTIO_VSOCK_OP_SEQ_BEGIN and VIRTIO_VSOCK_OP_SEQ_END packets carry >> ^ >> Same here "MUST carry" and in the rest of the patch. > >Actually, MUST and friends are really for normative sections; I'd >advise to have a description of how this feature works and then some >device/driver normative clauses with MUST statements (like "the device >MUST reject <malformed packets>" or so). > Okay, got it. Thanks for the clarification, Stefano _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description 2021-03-03 12:08 ` Cornelia Huck @ 2021-03-03 16:52 ` Michael S. Tsirkin -1 siblings, 0 replies; 22+ messages in thread From: Michael S. Tsirkin @ 2021-03-03 16:52 UTC (permalink / raw) To: Cornelia Huck Cc: Stefano Garzarella, Arseny Krasnov, virtio-comment, Stefan Hajnoczi, Jason Wang, David S. Miller, Jakub Kicinski, Jorgen Hansen, Andra Paraschiv, Norbert Slusarek, Colin Ian King, virtualization, oxffffaa On Wed, Mar 03, 2021 at 01:08:43PM +0100, Cornelia Huck wrote: > On Wed, 24 Feb 2021 10:32:00 +0100 > Stefano Garzarella <sgarzare@redhat.com> wrote: > > > On Thu, Feb 18, 2021 at 09:08:23AM +0300, Arseny Krasnov wrote: > > >Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> > > >--- > > > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- > > > 1 file changed, 37 insertions(+), 3 deletions(-) > > > > > >diff --git a/virtio-vsock.tex b/virtio-vsock.tex > > >index da7e641..1ee8f99 100644 > > >--- a/virtio-vsock.tex > > >+++ b/virtio-vsock.tex > > >@@ -102,6 +102,11 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op > > > VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6, > > > /* Request the peer to send the credit info to us */ > > > VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7, > > >+ > > >+ /* Message begin for SOCK_SEQPACKET */ > > >+ VIRTIO_VSOCK_OP_SEQ_BEGIN = 8, > > >+ /* Message end for SOCK_SEQPACKET */ > > >+ VIRTIO_VSOCK_OP_SEQ_END = 9, > > > }; > > > \end{lstlisting} > > > > > >@@ -140,11 +145,11 @@ \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 stream and seqpacket sockets are supported. \field{type} is 1 for > > >+stream socket types. \field{type} is 2 for seqpacket socket types. > > > > > > Stream sockets provide in-order, guaranteed, connection-oriented delivery > > >-without message boundaries. > > >+without message boundaries. Seqpacket sockets also provide message boundaries. > > > > > > \subsubsection{Buffer Space Management}\label{sec:Device Types / Socket Device / Device Operation / Buffer Space Management} > > > \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of > > >@@ -240,6 +245,35 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O > > > destination) address tuple for a new connection while the other peer is still > > > processing the old connection. > > > > > >+\subsubsection{Seqpacket Sockets}\label{sec:Device Types / Socket Device / Device Operation / Seqpacket Sockets} > > >+ > > >+Seqpacket sockets differ from stream sockets only in data transmission way: in > > >+stream sockets all data is sent using only VIRTIO_VSOCK_OP_RW packets. In > > >+seqpacket sockets, to provide message boundaries, every sequence of > > >+VIRTIO_VSOCK_OP_RW packets of each message is headed with > > ^ > > Since this is a spec, I think we should use MUST when something must be > > respected by the peer, for example here we can say "MUST be headed" > > > > >+VIRTIO_VSOCK_OP_SEQ_BEGIN and tailed with VIRTIO_VSOCK_OP_SEQ_END packets. > > >+Both VIRTIO_VSOCK_OP_SEQ_BEGIN and VIRTIO_VSOCK_OP_SEQ_END packets carry > > ^ > > Same here "MUST carry" and in the rest of the patch. > > Actually, MUST and friends are really for normative sections; I'd > advise to have a description of how this feature works and then some > device/driver normative clauses with MUST statements (like "the device > MUST reject <malformed packets>" or so). I agree we do want normative sections but please don't add MUST etc elsewhere. Also vague text saying "malformed" isn't all that helpful if it's a MUST. How does driver know for sure it's malformed? easy to miss some requirement. Therefore easiest thing it just to do some copy-pasting. E.g. You start with above and add a normative section saying: Driver MUST use XYZ in VIRTIO_VSOCK_OP_SEQ_END packets. We typically don't specify behaviour when out of spec, if we should here then please make a separate chapter for this explaining the how and the why. -- MST 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/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description @ 2021-03-03 16:52 ` Michael S. Tsirkin 0 siblings, 0 replies; 22+ messages in thread From: Michael S. Tsirkin @ 2021-03-03 16:52 UTC (permalink / raw) To: Cornelia Huck Cc: Andra Paraschiv, Colin Ian King, oxffffaa, Norbert Slusarek, Stefan Hajnoczi, virtio-comment, Jakub Kicinski, Arseny Krasnov, virtualization, David S. Miller, Jorgen Hansen On Wed, Mar 03, 2021 at 01:08:43PM +0100, Cornelia Huck wrote: > On Wed, 24 Feb 2021 10:32:00 +0100 > Stefano Garzarella <sgarzare@redhat.com> wrote: > > > On Thu, Feb 18, 2021 at 09:08:23AM +0300, Arseny Krasnov wrote: > > >Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> > > >--- > > > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- > > > 1 file changed, 37 insertions(+), 3 deletions(-) > > > > > >diff --git a/virtio-vsock.tex b/virtio-vsock.tex > > >index da7e641..1ee8f99 100644 > > >--- a/virtio-vsock.tex > > >+++ b/virtio-vsock.tex > > >@@ -102,6 +102,11 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op > > > VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6, > > > /* Request the peer to send the credit info to us */ > > > VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7, > > >+ > > >+ /* Message begin for SOCK_SEQPACKET */ > > >+ VIRTIO_VSOCK_OP_SEQ_BEGIN = 8, > > >+ /* Message end for SOCK_SEQPACKET */ > > >+ VIRTIO_VSOCK_OP_SEQ_END = 9, > > > }; > > > \end{lstlisting} > > > > > >@@ -140,11 +145,11 @@ \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 stream and seqpacket sockets are supported. \field{type} is 1 for > > >+stream socket types. \field{type} is 2 for seqpacket socket types. > > > > > > Stream sockets provide in-order, guaranteed, connection-oriented delivery > > >-without message boundaries. > > >+without message boundaries. Seqpacket sockets also provide message boundaries. > > > > > > \subsubsection{Buffer Space Management}\label{sec:Device Types / Socket Device / Device Operation / Buffer Space Management} > > > \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of > > >@@ -240,6 +245,35 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O > > > destination) address tuple for a new connection while the other peer is still > > > processing the old connection. > > > > > >+\subsubsection{Seqpacket Sockets}\label{sec:Device Types / Socket Device / Device Operation / Seqpacket Sockets} > > >+ > > >+Seqpacket sockets differ from stream sockets only in data transmission way: in > > >+stream sockets all data is sent using only VIRTIO_VSOCK_OP_RW packets. In > > >+seqpacket sockets, to provide message boundaries, every sequence of > > >+VIRTIO_VSOCK_OP_RW packets of each message is headed with > > ^ > > Since this is a spec, I think we should use MUST when something must be > > respected by the peer, for example here we can say "MUST be headed" > > > > >+VIRTIO_VSOCK_OP_SEQ_BEGIN and tailed with VIRTIO_VSOCK_OP_SEQ_END packets. > > >+Both VIRTIO_VSOCK_OP_SEQ_BEGIN and VIRTIO_VSOCK_OP_SEQ_END packets carry > > ^ > > Same here "MUST carry" and in the rest of the patch. > > Actually, MUST and friends are really for normative sections; I'd > advise to have a description of how this feature works and then some > device/driver normative clauses with MUST statements (like "the device > MUST reject <malformed packets>" or so). I agree we do want normative sections but please don't add MUST etc elsewhere. Also vague text saying "malformed" isn't all that helpful if it's a MUST. How does driver know for sure it's malformed? easy to miss some requirement. Therefore easiest thing it just to do some copy-pasting. E.g. You start with above and add a normative section saying: Driver MUST use XYZ in VIRTIO_VSOCK_OP_SEQ_END packets. We typically don't specify behaviour when out of spec, if we should here then please make a separate chapter for this explaining the how and the why. -- MST _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description 2021-03-03 16:52 ` Michael S. Tsirkin @ 2021-03-03 17:08 ` Cornelia Huck -1 siblings, 0 replies; 22+ messages in thread From: Cornelia Huck @ 2021-03-03 17:08 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Stefano Garzarella, Arseny Krasnov, virtio-comment, Stefan Hajnoczi, Jason Wang, David S. Miller, Jakub Kicinski, Jorgen Hansen, Andra Paraschiv, Norbert Slusarek, Colin Ian King, virtualization, oxffffaa On Wed, 3 Mar 2021 11:52:43 -0500 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Wed, Mar 03, 2021 at 01:08:43PM +0100, Cornelia Huck wrote: > > On Wed, 24 Feb 2021 10:32:00 +0100 > > Stefano Garzarella <sgarzare@redhat.com> wrote: > > > > > On Thu, Feb 18, 2021 at 09:08:23AM +0300, Arseny Krasnov wrote: > > > >Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> > > > >--- > > > > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- > > > > 1 file changed, 37 insertions(+), 3 deletions(-) > > > > > > > >diff --git a/virtio-vsock.tex b/virtio-vsock.tex > > > >index da7e641..1ee8f99 100644 > > > >--- a/virtio-vsock.tex > > > >+++ b/virtio-vsock.tex > > > >@@ -102,6 +102,11 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op > > > > VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6, > > > > /* Request the peer to send the credit info to us */ > > > > VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7, > > > >+ > > > >+ /* Message begin for SOCK_SEQPACKET */ > > > >+ VIRTIO_VSOCK_OP_SEQ_BEGIN = 8, > > > >+ /* Message end for SOCK_SEQPACKET */ > > > >+ VIRTIO_VSOCK_OP_SEQ_END = 9, > > > > }; > > > > \end{lstlisting} > > > > > > > >@@ -140,11 +145,11 @@ \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 stream and seqpacket sockets are supported. \field{type} is 1 for > > > >+stream socket types. \field{type} is 2 for seqpacket socket types. > > > > > > > > Stream sockets provide in-order, guaranteed, connection-oriented delivery > > > >-without message boundaries. > > > >+without message boundaries. Seqpacket sockets also provide message boundaries. > > > > > > > > \subsubsection{Buffer Space Management}\label{sec:Device Types / Socket Device / Device Operation / Buffer Space Management} > > > > \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of > > > >@@ -240,6 +245,35 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O > > > > destination) address tuple for a new connection while the other peer is still > > > > processing the old connection. > > > > > > > >+\subsubsection{Seqpacket Sockets}\label{sec:Device Types / Socket Device / Device Operation / Seqpacket Sockets} > > > >+ > > > >+Seqpacket sockets differ from stream sockets only in data transmission way: in > > > >+stream sockets all data is sent using only VIRTIO_VSOCK_OP_RW packets. In > > > >+seqpacket sockets, to provide message boundaries, every sequence of > > > >+VIRTIO_VSOCK_OP_RW packets of each message is headed with > > > ^ > > > Since this is a spec, I think we should use MUST when something must be > > > respected by the peer, for example here we can say "MUST be headed" > > > > > > >+VIRTIO_VSOCK_OP_SEQ_BEGIN and tailed with VIRTIO_VSOCK_OP_SEQ_END packets. > > > >+Both VIRTIO_VSOCK_OP_SEQ_BEGIN and VIRTIO_VSOCK_OP_SEQ_END packets carry > > > ^ > > > Same here "MUST carry" and in the rest of the patch. > > > > Actually, MUST and friends are really for normative sections; I'd > > advise to have a description of how this feature works and then some > > device/driver normative clauses with MUST statements (like "the device > > MUST reject <malformed packets>" or so). > > I agree we do want normative sections but please don't add MUST etc elsewhere. > Also vague text saying "malformed" isn't all that helpful if it's a > MUST. How does driver know for sure it's malformed? easy to miss > some requirement. Actually, I intended "<malformed packet>" to be a placeholder for a precise description... > Therefore easiest thing it just to do some copy-pasting. > > E.g. You start with above and add a normative section saying: > Driver MUST use XYZ in VIRTIO_VSOCK_OP_SEQ_END packets. > > We typically don't specify behaviour when out of spec, > if we should here then please make a separate chapter > for this explaining the how and the why. > I think it makes sense if we want to be concrete on what should happen for out-of-spec operation (e.g. in cases where ignoring it is preferable to actively rejecting it.) 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/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description @ 2021-03-03 17:08 ` Cornelia Huck 0 siblings, 0 replies; 22+ messages in thread From: Cornelia Huck @ 2021-03-03 17:08 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Andra Paraschiv, Colin Ian King, oxffffaa, Norbert Slusarek, Stefan Hajnoczi, virtio-comment, Jakub Kicinski, Arseny Krasnov, virtualization, David S. Miller, Jorgen Hansen On Wed, 3 Mar 2021 11:52:43 -0500 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Wed, Mar 03, 2021 at 01:08:43PM +0100, Cornelia Huck wrote: > > On Wed, 24 Feb 2021 10:32:00 +0100 > > Stefano Garzarella <sgarzare@redhat.com> wrote: > > > > > On Thu, Feb 18, 2021 at 09:08:23AM +0300, Arseny Krasnov wrote: > > > >Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> > > > >--- > > > > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- > > > > 1 file changed, 37 insertions(+), 3 deletions(-) > > > > > > > >diff --git a/virtio-vsock.tex b/virtio-vsock.tex > > > >index da7e641..1ee8f99 100644 > > > >--- a/virtio-vsock.tex > > > >+++ b/virtio-vsock.tex > > > >@@ -102,6 +102,11 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op > > > > VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6, > > > > /* Request the peer to send the credit info to us */ > > > > VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7, > > > >+ > > > >+ /* Message begin for SOCK_SEQPACKET */ > > > >+ VIRTIO_VSOCK_OP_SEQ_BEGIN = 8, > > > >+ /* Message end for SOCK_SEQPACKET */ > > > >+ VIRTIO_VSOCK_OP_SEQ_END = 9, > > > > }; > > > > \end{lstlisting} > > > > > > > >@@ -140,11 +145,11 @@ \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 stream and seqpacket sockets are supported. \field{type} is 1 for > > > >+stream socket types. \field{type} is 2 for seqpacket socket types. > > > > > > > > Stream sockets provide in-order, guaranteed, connection-oriented delivery > > > >-without message boundaries. > > > >+without message boundaries. Seqpacket sockets also provide message boundaries. > > > > > > > > \subsubsection{Buffer Space Management}\label{sec:Device Types / Socket Device / Device Operation / Buffer Space Management} > > > > \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of > > > >@@ -240,6 +245,35 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O > > > > destination) address tuple for a new connection while the other peer is still > > > > processing the old connection. > > > > > > > >+\subsubsection{Seqpacket Sockets}\label{sec:Device Types / Socket Device / Device Operation / Seqpacket Sockets} > > > >+ > > > >+Seqpacket sockets differ from stream sockets only in data transmission way: in > > > >+stream sockets all data is sent using only VIRTIO_VSOCK_OP_RW packets. In > > > >+seqpacket sockets, to provide message boundaries, every sequence of > > > >+VIRTIO_VSOCK_OP_RW packets of each message is headed with > > > ^ > > > Since this is a spec, I think we should use MUST when something must be > > > respected by the peer, for example here we can say "MUST be headed" > > > > > > >+VIRTIO_VSOCK_OP_SEQ_BEGIN and tailed with VIRTIO_VSOCK_OP_SEQ_END packets. > > > >+Both VIRTIO_VSOCK_OP_SEQ_BEGIN and VIRTIO_VSOCK_OP_SEQ_END packets carry > > > ^ > > > Same here "MUST carry" and in the rest of the patch. > > > > Actually, MUST and friends are really for normative sections; I'd > > advise to have a description of how this feature works and then some > > device/driver normative clauses with MUST statements (like "the device > > MUST reject <malformed packets>" or so). > > I agree we do want normative sections but please don't add MUST etc elsewhere. > Also vague text saying "malformed" isn't all that helpful if it's a > MUST. How does driver know for sure it's malformed? easy to miss > some requirement. Actually, I intended "<malformed packet>" to be a placeholder for a precise description... > Therefore easiest thing it just to do some copy-pasting. > > E.g. You start with above and add a normative section saying: > Driver MUST use XYZ in VIRTIO_VSOCK_OP_SEQ_END packets. > > We typically don't specify behaviour when out of spec, > if we should here then please make a separate chapter > for this explaining the how and the why. > I think it makes sense if we want to be concrete on what should happen for out-of-spec operation (e.g. in cases where ignoring it is preferable to actively rejecting it.) _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description 2021-02-18 6:08 ` [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description Arseny Krasnov @ 2021-03-16 13:49 ` Michael S. Tsirkin 2021-03-16 13:49 ` Michael S. Tsirkin 1 sibling, 0 replies; 22+ messages in thread From: Michael S. Tsirkin @ 2021-03-16 13:49 UTC (permalink / raw) To: Arseny Krasnov Cc: cohuck, virtio-comment, Stefan Hajnoczi, Stefano Garzarella, Jason Wang, David S. Miller, Jakub Kicinski, Jorgen Hansen, Andra Paraschiv, Norbert Slusarek, Colin Ian King, virtualization, oxffffaa On Thu, Feb 18, 2021 at 09:08:23AM +0300, Arseny Krasnov wrote: > Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> > --- > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- > 1 file changed, 37 insertions(+), 3 deletions(-) > > diff --git a/virtio-vsock.tex b/virtio-vsock.tex > index da7e641..1ee8f99 100644 > --- a/virtio-vsock.tex > +++ b/virtio-vsock.tex > @@ -102,6 +102,11 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op > VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6, > /* Request the peer to send the credit info to us */ > VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7, > + > + /* Message begin for SOCK_SEQPACKET */ > + VIRTIO_VSOCK_OP_SEQ_BEGIN = 8, > + /* Message end for SOCK_SEQPACKET */ > + VIRTIO_VSOCK_OP_SEQ_END = 9, > }; > \end{lstlisting} > > @@ -140,11 +145,11 @@ \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 stream and seqpacket sockets are supported. \field{type} is 1 for > +stream socket types. \field{type} is 2 for seqpacket socket types. > > Stream sockets provide in-order, guaranteed, connection-oriented delivery > -without message boundaries. > +without message boundaries. Seqpacket sockets also provide message boundaries. > > \subsubsection{Buffer Space Management}\label{sec:Device Types / Socket Device / Device Operation / Buffer Space Management} > \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of > @@ -240,6 +245,35 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O > destination) address tuple for a new connection while the other peer is still > processing the old connection. > > +\subsubsection{Seqpacket Sockets}\label{sec:Device Types / Socket Device / Device Operation / Seqpacket Sockets} > + > +Seqpacket sockets differ from stream sockets only in data transmission way: in > +stream sockets all data is sent using only VIRTIO_VSOCK_OP_RW packets. In > +seqpacket sockets, to provide message boundaries, every sequence of > +VIRTIO_VSOCK_OP_RW packets of each message is headed with > +VIRTIO_VSOCK_OP_SEQ_BEGIN and tailed with VIRTIO_VSOCK_OP_SEQ_END packets. > +Both VIRTIO_VSOCK_OP_SEQ_BEGIN and VIRTIO_VSOCK_OP_SEQ_END packets carry > +special header in payload: > + > +\begin{lstlisting} > +struct virtio_vsock_seq_hdr { > + __le32 msg_cnt; > + __le32 msg_len; > +}; > +\end{lstlisting} header in what sense? is there more payload beyond that? is header part of the payload or not? does this replace virtio_vsock_hdr? > + > +\field{msg_cnt} is per socket and incremented by 1 on every send of > +VIRTIO_VSOCK_OP_SEQ_BEGIN or VIRTIO_VSOCK_OP_SEQ_END. \field{msg_len} contains > +message length. This header is used to check integrity of each message: message > +is valid if length of data in VIRTIO_VSOCK_OP_RW packets is equal to > +\field{msg_len} of prepending VIRTIO_VSOCK_OP_SEQ_BEGIN and \field{msg_cnt} of > +VIRTIO_VSOCK_OP_SEQ_END differs from \field{msg_cnt} of > +VIRTIO_VSOCK_OP_SEQ_BEGIN by 1. > + > +POSIX MSG_EOR flag is supported by special value of \field{flags} in packet's > +header. If this flag is set for message, then all VIRTIO_VSOCK_OP_RW packets > +of this message have bit 0 is set to 1 in \field{flags}. > + > \subsubsection{Device Events}\label{sec:Device Types / Socket Device / Device Operation / Device Events} > > Certain events are communicated by the device to the driver using the event So if I understand it correctly, receiver must maintain VIRTIO_VSOCK_OP_SEQ_BEGIN/VIRTIO_VSOCK_OP_SEQ_END in its buffer, correct? If so these need to be accounted for as part of the flow control math. This calls for more changes in the spec, e.g. VIRTIO_VSOCK_OP_RW data packets MUST only be transmitted when the peer has sufficient free buffer space for the payload. would be VIRTIO_VSOCK_OP_RW data packets as well as VIRTIO_VSOCK_OP_SEQ_BEGIN and VIRTIO_VSOCK_OP_SEQ_END message boundary packets MUST only be transmitted when the peer has sufficient free buffer space for the payload. If not then how do we limit host memory use? Also, what is considered payload here size? the header? are we worried about wasting buffer space? the definition of payload and header is important for this: \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of stream sockets. The guest and the device publish how much buffer space is available per socket. Only payload bytes are counted and header bytes are not included. This facilitates flow control so data is never dropped. > -- > 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/ 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/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description @ 2021-03-16 13:49 ` Michael S. Tsirkin 0 siblings, 0 replies; 22+ messages in thread From: Michael S. Tsirkin @ 2021-03-16 13:49 UTC (permalink / raw) To: Arseny Krasnov Cc: Andra Paraschiv, cohuck, Colin Ian King, oxffffaa, Norbert Slusarek, Stefan Hajnoczi, virtio-comment, Jakub Kicinski, virtualization, David S. Miller, Jorgen Hansen On Thu, Feb 18, 2021 at 09:08:23AM +0300, Arseny Krasnov wrote: > Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> > --- > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- > 1 file changed, 37 insertions(+), 3 deletions(-) > > diff --git a/virtio-vsock.tex b/virtio-vsock.tex > index da7e641..1ee8f99 100644 > --- a/virtio-vsock.tex > +++ b/virtio-vsock.tex > @@ -102,6 +102,11 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op > VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6, > /* Request the peer to send the credit info to us */ > VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7, > + > + /* Message begin for SOCK_SEQPACKET */ > + VIRTIO_VSOCK_OP_SEQ_BEGIN = 8, > + /* Message end for SOCK_SEQPACKET */ > + VIRTIO_VSOCK_OP_SEQ_END = 9, > }; > \end{lstlisting} > > @@ -140,11 +145,11 @@ \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 stream and seqpacket sockets are supported. \field{type} is 1 for > +stream socket types. \field{type} is 2 for seqpacket socket types. > > Stream sockets provide in-order, guaranteed, connection-oriented delivery > -without message boundaries. > +without message boundaries. Seqpacket sockets also provide message boundaries. > > \subsubsection{Buffer Space Management}\label{sec:Device Types / Socket Device / Device Operation / Buffer Space Management} > \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of > @@ -240,6 +245,35 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O > destination) address tuple for a new connection while the other peer is still > processing the old connection. > > +\subsubsection{Seqpacket Sockets}\label{sec:Device Types / Socket Device / Device Operation / Seqpacket Sockets} > + > +Seqpacket sockets differ from stream sockets only in data transmission way: in > +stream sockets all data is sent using only VIRTIO_VSOCK_OP_RW packets. In > +seqpacket sockets, to provide message boundaries, every sequence of > +VIRTIO_VSOCK_OP_RW packets of each message is headed with > +VIRTIO_VSOCK_OP_SEQ_BEGIN and tailed with VIRTIO_VSOCK_OP_SEQ_END packets. > +Both VIRTIO_VSOCK_OP_SEQ_BEGIN and VIRTIO_VSOCK_OP_SEQ_END packets carry > +special header in payload: > + > +\begin{lstlisting} > +struct virtio_vsock_seq_hdr { > + __le32 msg_cnt; > + __le32 msg_len; > +}; > +\end{lstlisting} header in what sense? is there more payload beyond that? is header part of the payload or not? does this replace virtio_vsock_hdr? > + > +\field{msg_cnt} is per socket and incremented by 1 on every send of > +VIRTIO_VSOCK_OP_SEQ_BEGIN or VIRTIO_VSOCK_OP_SEQ_END. \field{msg_len} contains > +message length. This header is used to check integrity of each message: message > +is valid if length of data in VIRTIO_VSOCK_OP_RW packets is equal to > +\field{msg_len} of prepending VIRTIO_VSOCK_OP_SEQ_BEGIN and \field{msg_cnt} of > +VIRTIO_VSOCK_OP_SEQ_END differs from \field{msg_cnt} of > +VIRTIO_VSOCK_OP_SEQ_BEGIN by 1. > + > +POSIX MSG_EOR flag is supported by special value of \field{flags} in packet's > +header. If this flag is set for message, then all VIRTIO_VSOCK_OP_RW packets > +of this message have bit 0 is set to 1 in \field{flags}. > + > \subsubsection{Device Events}\label{sec:Device Types / Socket Device / Device Operation / Device Events} > > Certain events are communicated by the device to the driver using the event So if I understand it correctly, receiver must maintain VIRTIO_VSOCK_OP_SEQ_BEGIN/VIRTIO_VSOCK_OP_SEQ_END in its buffer, correct? If so these need to be accounted for as part of the flow control math. This calls for more changes in the spec, e.g. VIRTIO_VSOCK_OP_RW data packets MUST only be transmitted when the peer has sufficient free buffer space for the payload. would be VIRTIO_VSOCK_OP_RW data packets as well as VIRTIO_VSOCK_OP_SEQ_BEGIN and VIRTIO_VSOCK_OP_SEQ_END message boundary packets MUST only be transmitted when the peer has sufficient free buffer space for the payload. If not then how do we limit host memory use? Also, what is considered payload here size? the header? are we worried about wasting buffer space? the definition of payload and header is important for this: \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of stream sockets. The guest and the device publish how much buffer space is available per socket. Only payload bytes are counted and header bytes are not included. This facilitates flow control so data is never dropped. > -- > 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/ _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description 2021-03-16 13:49 ` Michael S. Tsirkin (?) @ 2021-03-17 9:23 ` Arseny Krasnov -1 siblings, 0 replies; 22+ messages in thread From: Arseny Krasnov @ 2021-03-17 9:23 UTC (permalink / raw) To: Michael S. Tsirkin Cc: cohuck@redhat.com, virtio-comment@lists.oasis-open.org, Stefan Hajnoczi, Stefano Garzarella, Jason Wang, David S. Miller, Jakub Kicinski, Jorgen Hansen, Andra Paraschiv, Norbert Slusarek, Colin Ian King, virtualization@lists.linux-foundation.org, oxffffaa@gmail.com On 16.03.2021 16:49, Michael S. Tsirkin wrote: > On Thu, Feb 18, 2021 at 09:08:23AM +0300, Arseny Krasnov wrote: >> Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> >> --- >> virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- >> 1 file changed, 37 insertions(+), 3 deletions(-) >> >> diff --git a/virtio-vsock.tex b/virtio-vsock.tex >> index da7e641..1ee8f99 100644 >> --- a/virtio-vsock.tex >> +++ b/virtio-vsock.tex >> @@ -102,6 +102,11 @@ \subsection{Device Operation}\label{sec:Device Types / Socket Device / Device Op >> VIRTIO_VSOCK_OP_CREDIT_UPDATE = 6, >> /* Request the peer to send the credit info to us */ >> VIRTIO_VSOCK_OP_CREDIT_REQUEST = 7, >> + >> + /* Message begin for SOCK_SEQPACKET */ >> + VIRTIO_VSOCK_OP_SEQ_BEGIN = 8, >> + /* Message end for SOCK_SEQPACKET */ >> + VIRTIO_VSOCK_OP_SEQ_END = 9, >> }; >> \end{lstlisting} >> >> @@ -140,11 +145,11 @@ \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 stream and seqpacket sockets are supported. \field{type} is 1 for >> +stream socket types. \field{type} is 2 for seqpacket socket types. >> >> Stream sockets provide in-order, guaranteed, connection-oriented delivery >> -without message boundaries. >> +without message boundaries. Seqpacket sockets also provide message boundaries. >> >> \subsubsection{Buffer Space Management}\label{sec:Device Types / Socket Device / Device Operation / Buffer Space Management} >> \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of >> @@ -240,6 +245,35 @@ \subsubsection{Stream Sockets}\label{sec:Device Types / Socket Device / Device O >> destination) address tuple for a new connection while the other peer is still >> processing the old connection. >> >> +\subsubsection{Seqpacket Sockets}\label{sec:Device Types / Socket Device / Device Operation / Seqpacket Sockets} >> + >> +Seqpacket sockets differ from stream sockets only in data transmission way: in >> +stream sockets all data is sent using only VIRTIO_VSOCK_OP_RW packets. In >> +seqpacket sockets, to provide message boundaries, every sequence of >> +VIRTIO_VSOCK_OP_RW packets of each message is headed with >> +VIRTIO_VSOCK_OP_SEQ_BEGIN and tailed with VIRTIO_VSOCK_OP_SEQ_END packets. >> +Both VIRTIO_VSOCK_OP_SEQ_BEGIN and VIRTIO_VSOCK_OP_SEQ_END packets carry >> +special header in payload: >> + >> +\begin{lstlisting} >> +struct virtio_vsock_seq_hdr { >> + __le32 msg_cnt; >> + __le32 msg_len; >> +}; >> +\end{lstlisting} > header in what sense? is there more payload beyond that? is header > part of the payload or not? does this replace virtio_vsock_hdr? Ok, this can be renamed to "special data structure in paylod". And no extra payload are allowed beyond or before it. > >> + >> +\field{msg_cnt} is per socket and incremented by 1 on every send of >> +VIRTIO_VSOCK_OP_SEQ_BEGIN or VIRTIO_VSOCK_OP_SEQ_END. \field{msg_len} contains >> +message length. This header is used to check integrity of each message: message >> +is valid if length of data in VIRTIO_VSOCK_OP_RW packets is equal to >> +\field{msg_len} of prepending VIRTIO_VSOCK_OP_SEQ_BEGIN and \field{msg_cnt} of >> +VIRTIO_VSOCK_OP_SEQ_END differs from \field{msg_cnt} of >> +VIRTIO_VSOCK_OP_SEQ_BEGIN by 1. >> + >> +POSIX MSG_EOR flag is supported by special value of \field{flags} in packet's >> +header. If this flag is set for message, then all VIRTIO_VSOCK_OP_RW packets >> +of this message have bit 0 is set to 1 in \field{flags}. >> + >> \subsubsection{Device Events}\label{sec:Device Types / Socket Device / Device Operation / Device Events} >> >> Certain events are communicated by the device to the driver using the event > So if I understand it correctly, receiver must maintain > VIRTIO_VSOCK_OP_SEQ_BEGIN/VIRTIO_VSOCK_OP_SEQ_END in its buffer, > correct? If so these need to be accounted for as part of > the flow control math. This calls for more changes in the spec, > e.g. > > VIRTIO_VSOCK_OP_RW data packets MUST only be transmitted when the peer has > sufficient free buffer space for the payload. > > would be > > VIRTIO_VSOCK_OP_RW data packets as well as VIRTIO_VSOCK_OP_SEQ_BEGIN and > VIRTIO_VSOCK_OP_SEQ_END message boundary packets MUST only be > transmitted when the peer has sufficient free buffer space for the > payload. Ack > > If not then how do we limit host memory use? Both VIRTIO_VSOCK_OP_SEQ_BEGIN/VIRTIO_VSOCK_OP_SEQ_END are accounted by flow control logic(e.g. size of 'virtio_vsock_seq_hdr'). I'll add that 'virtio_vsock_seq_hdr' in payload of such packets must never be fragmented. > > > Also, what is considered payload here size? the header? are we > worried about wasting buffer space? > > the definition of payload and header is important for this: > > \field{buf_alloc} and \field{fwd_cnt} are used for buffer space management of > stream sockets. The guest and the device publish how much buffer space is > available per socket. Only payload bytes are counted and header bytes are not > included. This facilitates flow control so data is never dropped. > > > > > >> -- >> 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/ > 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/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* [virtio-comment] Re: [RFC PATCH v1 0/1] virtio-vsock: introduce SEQPACKET description 2021-02-18 6:07 [virtio-comment] [RFC PATCH v1 0/1] virtio-vsock: introduce SEQPACKET description Arseny Krasnov @ 2021-02-22 10:16 ` Stefano Garzarella 2021-02-22 10:16 ` Stefano Garzarella 1 sibling, 0 replies; 22+ messages in thread From: Stefano Garzarella @ 2021-02-22 10:16 UTC (permalink / raw) To: Arseny Krasnov Cc: cohuck, virtio-comment, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, David S. Miller, Jakub Kicinski, Jorgen Hansen, Norbert Slusarek, Colin Ian King, Andra Paraschiv, virtualization, oxffffaa On Thu, Feb 18, 2021 at 09:07:12AM +0300, Arseny Krasnov wrote: >This patchset adds description of SOCK_SEQPACKET socket's type >of virtio vsock. > >Here is implementation: >https://lkml.org/lkml/2021/2/18/24 > > Arseny Krasnov(1): > virtio-vsock: add SOCK_SEQPACKET description > > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- > 1 files changed, 37 insertions(+), 3 deletions(-) > >TODO: >- for messages identification I use header's field called 'msg_cnt'. > May be it is better to call it 'msg_id' or 'msg_num', because it > is used as ID, but implemented as counter. If we use it only as an identifier, I think 'msg_id' is preferable and we shouldn't mention in the specs that it's a counter, since it's just a possible implementation. Instead if we use the counter for some control, for example if we lost a packet, then maybe msg_cnt is better. But since the channel shouldn't lose packets, I don't think this is the case. > >- in current version of specification, some values of headers' fields > still unnamed, for example type of socket (stream or seqpacket), then > shutdown flags. Spec says that for stream socket value of 'type' > must be 1. For receive shutdown bit 0 is set in 'flags', for send > shutdown bit 1 is set in 'flags'. But in Linux these unnamed ones and > zeroes are implemented as enums, so may be it will be ok to add such > enums in specification (as 'enum virtio_vsock_event_id'). Since we have an enumerate for VIRTIO_VSOCK_OP_* values, I think we can add enums for socket type and maybe 'flags'. Thanks, Stefano 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/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH v1 0/1] virtio-vsock: introduce SEQPACKET description @ 2021-02-22 10:16 ` Stefano Garzarella 0 siblings, 0 replies; 22+ messages in thread From: Stefano Garzarella @ 2021-02-22 10:16 UTC (permalink / raw) To: Arseny Krasnov Cc: Andra Paraschiv, Michael S. Tsirkin, cohuck, Colin Ian King, oxffffaa, Norbert Slusarek, Stefan Hajnoczi, virtio-comment, Jakub Kicinski, virtualization, David S. Miller, Jorgen Hansen On Thu, Feb 18, 2021 at 09:07:12AM +0300, Arseny Krasnov wrote: >This patchset adds description of SOCK_SEQPACKET socket's type >of virtio vsock. > >Here is implementation: >https://lkml.org/lkml/2021/2/18/24 > > Arseny Krasnov(1): > virtio-vsock: add SOCK_SEQPACKET description > > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- > 1 files changed, 37 insertions(+), 3 deletions(-) > >TODO: >- for messages identification I use header's field called 'msg_cnt'. > May be it is better to call it 'msg_id' or 'msg_num', because it > is used as ID, but implemented as counter. If we use it only as an identifier, I think 'msg_id' is preferable and we shouldn't mention in the specs that it's a counter, since it's just a possible implementation. Instead if we use the counter for some control, for example if we lost a packet, then maybe msg_cnt is better. But since the channel shouldn't lose packets, I don't think this is the case. > >- in current version of specification, some values of headers' fields > still unnamed, for example type of socket (stream or seqpacket), then > shutdown flags. Spec says that for stream socket value of 'type' > must be 1. For receive shutdown bit 0 is set in 'flags', for send > shutdown bit 1 is set in 'flags'. But in Linux these unnamed ones and > zeroes are implemented as enums, so may be it will be ok to add such > enums in specification (as 'enum virtio_vsock_event_id'). Since we have an enumerate for VIRTIO_VSOCK_OP_* values, I think we can add enums for socket type and maybe 'flags'. Thanks, Stefano _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 22+ messages in thread
* [virtio-comment] Re: [MASSMAIL KLMS] [virtio-comment] Re: [RFC PATCH v1 0/1] virtio-vsock: introduce SEQPACKET description 2021-02-22 10:16 ` Stefano Garzarella (?) @ 2021-02-22 10:38 ` Arseny Krasnov -1 siblings, 0 replies; 22+ messages in thread From: Arseny Krasnov @ 2021-02-22 10:38 UTC (permalink / raw) To: Stefano Garzarella Cc: cohuck@redhat.com, virtio-comment@lists.oasis-open.org, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, David S. Miller, Jakub Kicinski, Jorgen Hansen, Norbert Slusarek, Colin Ian King, Andra Paraschiv, virtualization@lists.linux-foundation.org, oxffffaa@gmail.com On 22.02.2021 13:16, Stefano Garzarella wrote: > On Thu, Feb 18, 2021 at 09:07:12AM +0300, Arseny Krasnov wrote: >> This patchset adds description of SOCK_SEQPACKET socket's type >> of virtio vsock. >> >> Here is implementation: >> https://lkml.org/lkml/2021/2/18/24 >> >> Arseny Krasnov(1): >> virtio-vsock: add SOCK_SEQPACKET description >> >> virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- >> 1 files changed, 37 insertions(+), 3 deletions(-) >> >> TODO: >> - for messages identification I use header's field called 'msg_cnt'. >> May be it is better to call it 'msg_id' or 'msg_num', because it >> is used as ID, but implemented as counter. > If we use it only as an identifier, I think 'msg_id' is preferable and > we shouldn't mention in the specs that it's a counter, since it's just a > possible implementation. Well let it be 'msg_id', because it's used for packet drop control, and counter is one of implementations. > Instead if we use the counter for some control, for example if we lost a > packet, then maybe msg_cnt is better. > But since the channel shouldn't lose packets, I don't think this is the > case. > >> - in current version of specification, some values of headers' fields >> still unnamed, for example type of socket (stream or seqpacket), then >> shutdown flags. Spec says that for stream socket value of 'type' >> must be 1. For receive shutdown bit 0 is set in 'flags', for send >> shutdown bit 1 is set in 'flags'. But in Linux these unnamed ones and >> zeroes are implemented as enums, so may be it will be ok to add such >> enums in specification (as 'enum virtio_vsock_event_id'). > Since we have an enumerate for VIRTIO_VSOCK_OP_* values, I think we can > add enums for socket type and maybe 'flags'. Ack > > Thanks, > Stefano > > > 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/ > > 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/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [virtio-comment] Re: [RFC PATCH v1 0/1] virtio-vsock: introduce SEQPACKET description 2021-02-22 10:16 ` Stefano Garzarella @ 2021-03-16 13:50 ` Michael S. Tsirkin -1 siblings, 0 replies; 22+ messages in thread From: Michael S. Tsirkin @ 2021-03-16 13:50 UTC (permalink / raw) To: Stefano Garzarella Cc: Arseny Krasnov, cohuck, virtio-comment, Stefan Hajnoczi, Jason Wang, David S. Miller, Jakub Kicinski, Jorgen Hansen, Norbert Slusarek, Colin Ian King, Andra Paraschiv, virtualization, oxffffaa On Mon, Feb 22, 2021 at 11:16:54AM +0100, Stefano Garzarella wrote: > On Thu, Feb 18, 2021 at 09:07:12AM +0300, Arseny Krasnov wrote: > > This patchset adds description of SOCK_SEQPACKET socket's type > > of virtio vsock. > > > > Here is implementation: > > https://lkml.org/lkml/2021/2/18/24 > > > > Arseny Krasnov(1): > > virtio-vsock: add SOCK_SEQPACKET description > > > > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- > > 1 files changed, 37 insertions(+), 3 deletions(-) > > > > TODO: > > - for messages identification I use header's field called 'msg_cnt'. > > May be it is better to call it 'msg_id' or 'msg_num', because it > > is used as ID, but implemented as counter. > > If we use it only as an identifier, I think 'msg_id' is preferable and we > shouldn't mention in the specs that it's a counter, since it's just a > possible implementation. > Instead if we use the counter for some control, for example if we lost a > packet, then maybe msg_cnt is better. > But since the channel shouldn't lose packets, I don't think this is the > case. > > > > > - in current version of specification, some values of headers' fields > > still unnamed, for example type of socket (stream or seqpacket), then > > shutdown flags. Spec says that for stream socket value of 'type' > > must be 1. For receive shutdown bit 0 is set in 'flags', for send > > shutdown bit 1 is set in 'flags'. But in Linux these unnamed ones and > > zeroes are implemented as enums, so may be it will be ok to add such > > enums in specification (as 'enum virtio_vsock_event_id'). > > Since we have an enumerate for VIRTIO_VSOCK_OP_* values, I think we can add > enums for socket type and maybe 'flags'. We really need to switch enums to defines, for consistency. > Thanks, > Stefano > > > 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/ 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/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [virtio-comment] Re: [RFC PATCH v1 0/1] virtio-vsock: introduce SEQPACKET description @ 2021-03-16 13:50 ` Michael S. Tsirkin 0 siblings, 0 replies; 22+ messages in thread From: Michael S. Tsirkin @ 2021-03-16 13:50 UTC (permalink / raw) To: Stefano Garzarella Cc: Andra Paraschiv, cohuck, Colin Ian King, oxffffaa, Norbert Slusarek, Stefan Hajnoczi, virtio-comment, Jakub Kicinski, Arseny Krasnov, virtualization, David S. Miller, Jorgen Hansen On Mon, Feb 22, 2021 at 11:16:54AM +0100, Stefano Garzarella wrote: > On Thu, Feb 18, 2021 at 09:07:12AM +0300, Arseny Krasnov wrote: > > This patchset adds description of SOCK_SEQPACKET socket's type > > of virtio vsock. > > > > Here is implementation: > > https://lkml.org/lkml/2021/2/18/24 > > > > Arseny Krasnov(1): > > virtio-vsock: add SOCK_SEQPACKET description > > > > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- > > 1 files changed, 37 insertions(+), 3 deletions(-) > > > > TODO: > > - for messages identification I use header's field called 'msg_cnt'. > > May be it is better to call it 'msg_id' or 'msg_num', because it > > is used as ID, but implemented as counter. > > If we use it only as an identifier, I think 'msg_id' is preferable and we > shouldn't mention in the specs that it's a counter, since it's just a > possible implementation. > Instead if we use the counter for some control, for example if we lost a > packet, then maybe msg_cnt is better. > But since the channel shouldn't lose packets, I don't think this is the > case. > > > > > - in current version of specification, some values of headers' fields > > still unnamed, for example type of socket (stream or seqpacket), then > > shutdown flags. Spec says that for stream socket value of 'type' > > must be 1. For receive shutdown bit 0 is set in 'flags', for send > > shutdown bit 1 is set in 'flags'. But in Linux these unnamed ones and > > zeroes are implemented as enums, so may be it will be ok to add such > > enums in specification (as 'enum virtio_vsock_event_id'). > > Since we have an enumerate for VIRTIO_VSOCK_OP_* values, I think we can add > enums for socket type and maybe 'flags'. We really need to switch enums to defines, for consistency. > Thanks, > Stefano > > > 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/ _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [virtio-comment] Re: [RFC PATCH v1 0/1] virtio-vsock: introduce SEQPACKET description 2021-03-16 13:50 ` Michael S. Tsirkin @ 2021-03-16 14:08 ` Stefano Garzarella -1 siblings, 0 replies; 22+ messages in thread From: Stefano Garzarella @ 2021-03-16 14:08 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Arseny Krasnov, cohuck, virtio-comment, Stefan Hajnoczi, Jason Wang, David S. Miller, Jakub Kicinski, Jorgen Hansen, Norbert Slusarek, Colin Ian King, Andra Paraschiv, virtualization, oxffffaa On Tue, Mar 16, 2021 at 09:50:48AM -0400, Michael S. Tsirkin wrote: >On Mon, Feb 22, 2021 at 11:16:54AM +0100, Stefano Garzarella wrote: >> On Thu, Feb 18, 2021 at 09:07:12AM +0300, Arseny Krasnov wrote: >> > This patchset adds description of SOCK_SEQPACKET socket's type >> > of virtio vsock. >> > >> > Here is implementation: >> > https://lkml.org/lkml/2021/2/18/24 >> > >> > Arseny Krasnov(1): >> > virtio-vsock: add SOCK_SEQPACKET description >> > >> > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- >> > 1 files changed, 37 insertions(+), 3 deletions(-) >> > >> > TODO: >> > - for messages identification I use header's field called 'msg_cnt'. >> > May be it is better to call it 'msg_id' or 'msg_num', because it >> > is used as ID, but implemented as counter. >> >> If we use it only as an identifier, I think 'msg_id' is preferable and we >> shouldn't mention in the specs that it's a counter, since it's just a >> possible implementation. >> Instead if we use the counter for some control, for example if we lost a >> packet, then maybe msg_cnt is better. >> But since the channel shouldn't lose packets, I don't think this is the >> case. >> >> > >> > - in current version of specification, some values of headers' fields >> > still unnamed, for example type of socket (stream or seqpacket), then >> > shutdown flags. Spec says that for stream socket value of 'type' >> > must be 1. For receive shutdown bit 0 is set in 'flags', for send >> > shutdown bit 1 is set in 'flags'. But in Linux these unnamed ones and >> > zeroes are implemented as enums, so may be it will be ok to add such >> > enums in specification (as 'enum virtio_vsock_event_id'). >> >> Since we have an enumerate for VIRTIO_VSOCK_OP_* values, I think we can add >> enums for socket type and maybe 'flags'. > >We really need to switch enums to defines, for consistency. I fully agree, indeed at least in the virtio-vsock part, we use the enumerate as they were many defines, always redefining the assigned value. Using defines, we are forced to always define the value and I think that's fair! Thanks, Stefano 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/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [virtio-comment] Re: [RFC PATCH v1 0/1] virtio-vsock: introduce SEQPACKET description @ 2021-03-16 14:08 ` Stefano Garzarella 0 siblings, 0 replies; 22+ messages in thread From: Stefano Garzarella @ 2021-03-16 14:08 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Andra Paraschiv, cohuck, Colin Ian King, oxffffaa, Norbert Slusarek, Stefan Hajnoczi, virtio-comment, Jakub Kicinski, Arseny Krasnov, virtualization, David S. Miller, Jorgen Hansen On Tue, Mar 16, 2021 at 09:50:48AM -0400, Michael S. Tsirkin wrote: >On Mon, Feb 22, 2021 at 11:16:54AM +0100, Stefano Garzarella wrote: >> On Thu, Feb 18, 2021 at 09:07:12AM +0300, Arseny Krasnov wrote: >> > This patchset adds description of SOCK_SEQPACKET socket's type >> > of virtio vsock. >> > >> > Here is implementation: >> > https://lkml.org/lkml/2021/2/18/24 >> > >> > Arseny Krasnov(1): >> > virtio-vsock: add SOCK_SEQPACKET description >> > >> > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- >> > 1 files changed, 37 insertions(+), 3 deletions(-) >> > >> > TODO: >> > - for messages identification I use header's field called 'msg_cnt'. >> > May be it is better to call it 'msg_id' or 'msg_num', because it >> > is used as ID, but implemented as counter. >> >> If we use it only as an identifier, I think 'msg_id' is preferable and we >> shouldn't mention in the specs that it's a counter, since it's just a >> possible implementation. >> Instead if we use the counter for some control, for example if we lost a >> packet, then maybe msg_cnt is better. >> But since the channel shouldn't lose packets, I don't think this is the >> case. >> >> > >> > - in current version of specification, some values of headers' fields >> > still unnamed, for example type of socket (stream or seqpacket), then >> > shutdown flags. Spec says that for stream socket value of 'type' >> > must be 1. For receive shutdown bit 0 is set in 'flags', for send >> > shutdown bit 1 is set in 'flags'. But in Linux these unnamed ones and >> > zeroes are implemented as enums, so may be it will be ok to add such >> > enums in specification (as 'enum virtio_vsock_event_id'). >> >> Since we have an enumerate for VIRTIO_VSOCK_OP_* values, I think we can add >> enums for socket type and maybe 'flags'. > >We really need to switch enums to defines, for consistency. I fully agree, indeed at least in the virtio-vsock part, we use the enumerate as they were many defines, always redefining the assigned value. Using defines, we are forced to always define the value and I think that's fair! Thanks, Stefano _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2021-03-17 9:23 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-02-18 6:07 [virtio-comment] [RFC PATCH v1 0/1] virtio-vsock: introduce SEQPACKET description Arseny Krasnov 2021-02-18 6:08 ` [virtio-comment] [RFC PATCH v1 1/1] virtio-vsock: add SOCK_SEQPACKET description Arseny Krasnov 2021-02-24 9:32 ` Stefano Garzarella 2021-02-24 9:32 ` Stefano Garzarella 2021-03-03 12:08 ` Cornelia Huck 2021-03-03 12:08 ` Cornelia Huck 2021-03-03 12:35 ` Stefano Garzarella 2021-03-03 12:35 ` Stefano Garzarella 2021-03-03 16:52 ` Michael S. Tsirkin 2021-03-03 16:52 ` Michael S. Tsirkin 2021-03-03 17:08 ` Cornelia Huck 2021-03-03 17:08 ` Cornelia Huck 2021-03-16 13:49 ` Michael S. Tsirkin 2021-03-16 13:49 ` Michael S. Tsirkin 2021-03-17 9:23 ` Arseny Krasnov 2021-02-22 10:16 ` [virtio-comment] Re: [RFC PATCH v1 0/1] virtio-vsock: introduce SEQPACKET description Stefano Garzarella 2021-02-22 10:16 ` Stefano Garzarella 2021-02-22 10:38 ` [virtio-comment] Re: [MASSMAIL KLMS] [virtio-comment] " Arseny Krasnov 2021-03-16 13:50 ` Michael S. Tsirkin 2021-03-16 13:50 ` Michael S. Tsirkin 2021-03-16 14:08 ` Stefano Garzarella 2021-03-16 14:08 ` Stefano Garzarella
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.