[virtio-dev] Re: [PATCH] virtio-vsock: add VIRTIO_VSOCK_F_DGRAM feature bit
On Wed, Sep 06, 2023 at 04:28:12PM +0200, Stefano Garzarella wrote:
> On Sat, Sep 02, 2023 at 04:35:25AM -0400, Michael S. Tsirkin wrote:
> > On Sat, Sep 02, 2023 at 04:56:42AM +0000, Bobby Eshleman wrote:
> > > On Fri, Sep 01, 2023 at 02:45:14PM +0200, Stefano Garzarella wrote:
> > > > On Tue, Aug 29, 2023 at 09:29:45PM +, Bobby Eshleman wrote:
> > > > > This adds support for datagrams to the virtio-vsock device.
> > > > >
> > > > > virtio-vsock already supports stream and seqpacket types. The existing
> > > > > message types and header fields are extended to support datagrams.
> > > > > Semantic differences between the flow types are stated, as well as any
> > > > > additional requirements for devices and drivers implementing this
> > > > > feature.
> > > > >
> > > > > Signed-off-by: Bobby Eshleman
> > > > > ---
> > > > > device-types/vsock/description.tex | 95 +++---
> > > > > 1 file changed, 88 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/device-types/vsock/description.tex
> > > > > b/device-types/vsock/description.tex
> > > > > index 7d91d159872f..638dca8e5da1 100644
> > > > > --- a/device-types/vsock/description.tex
> > > > > +++ b/device-types/vsock/description.tex
> > > > > @@ -20,6 +20,7 @@ \subsection{Feature bits}\label{sec:Device Types /
> > > > > Socket Device / Feature bits}
> > > > > \item[VIRTIO_VSOCK_F_STREAM (0)] stream socket type is supported.
> > > > > \item[VIRTIO_VSOCK_F_SEQPACKET (1)] seqpacket socket type is
> > > > > supported.
> > > > > \item[VIRTIO_VSOCK_F_NO_IMPLIED_STREAM (2)] stream socket type is not
> > > > > implied.
> > > > > +\item[VIRTIO_VSOCK_F_DGRAM (3)] datagram socket type is supported.
> > > > > \end{description}
> > > > >
> > > > > \drivernormative{\subsubsection}{Feature bits}{Device Types / Socket
> > > > > Device / Feature bits}
> > > > > @@ -167,17 +168,22 @@ \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 stream and seqpacket sockets are supported. \field{type}
> > > > > is 1 (VIRTIO_VSOCK_TYPE_STREAM)
> > > > > -for stream socket types, and 2 (VIRTIO_VSOCK_TYPE_SEQPACKET) for
> > > > > seqpacket socket types.
> > > > > +
> > > > > +Currently stream, seqpacket, and datagram sockets are supported.
> > > > > \field{type} is
> > > > > +1 (VIRTIO_VSOCK_TYPE_STREAM) for stream socket types, 2
> > > > > (VIRTIO_VSOCK_TYPE_SEQPACKET) for
> > > > > +seqpacket socket types, and 3 (VIRTIO_VSOCK_TYPE_DGRAM) for datagram
> > > > > socket types.
> > > > >
> > > > > \begin{lstlisting}
> > > > > #define VIRTIO_VSOCK_TYPE_STREAM1
> > > > > #define VIRTIO_VSOCK_TYPE_SEQPACKET 2
> > > > > +#define VIRTIO_VSOCK_TYPE_DGRAM 3
> > > > > \end{lstlisting}
> > > > >
> > > > > Stream sockets provide in-order, guaranteed, connection-oriented
> > > > > delivery
> > > > > without message boundaries. Seqpacket sockets provide in-order,
> > > > > guaranteed,
> > > > > -connection-oriented delivery with message and record boundaries.
> > > > > +connection-oriented delivery with message and record boundaries.
> > > > > Datagram
> > > > > +sockets provide connection-less, best-effort delivery of messages,
> > > > > with no
> > > > > +order or reliability guarantees.
> > > > >
> > > > > \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
> > > > > @@ -203,16 +209,19 @@ \subsubsection{Buffer Space
> > > > > Management}\label{sec:Device Types / Socket Device /
> > > > > previousl
Re: [virtio-dev] Re: [PATCH] virtio-vsock: add VIRTIO_VSOCK_F_DGRAM feature bit
On Wed, Sep 06, 2023 at 04:17:59AM -0400, Michael S. Tsirkin wrote:
> On Sat, Sep 02, 2023 at 09:24:26AM +0000, Bobby Eshleman wrote:
> > On Sat, Sep 02, 2023 at 04:41:28AM -0400, Michael S. Tsirkin wrote:
> > > On Sat, Sep 02, 2023 at 04:37:19AM +0000, Bobby Eshleman wrote:
> > > > On Fri, Sep 01, 2023 at 09:31:03AM -0400, Michael S. Tsirkin wrote:
> > > > > On Wed, Aug 30, 2023 at 12:43:03AM +, Bobby Eshleman wrote:
> > > > > > On Tue, Aug 29, 2023 at 06:21:35PM -0400, Michael S. Tsirkin wrote:
> > > > > > > On Tue, Aug 29, 2023 at 09:29:45PM +, Bobby Eshleman wrote:
> > > > > > > > This adds support for datagrams to the virtio-vsock device.
> > > > > > > >
> > > > > > > > virtio-vsock already supports stream and seqpacket types. The
> > > > > > > > existing
> > > > > > > > message types and header fields are extended to support
> > > > > > > > datagrams.
> > > > > > > > Semantic differences between the flow types are stated, as well
> > > > > > > > as any
> > > > > > > > additional requirements for devices and drivers implementing
> > > > > > > > this
> > > > > > > > feature.
> > > > > > > >
> > > > > > > > Signed-off-by: Bobby Eshleman
> > > > > > > > ---
> > > > > > > > device-types/vsock/description.tex | 95
> > > > > > > > +++---
> > > > > > > > 1 file changed, 88 insertions(+), 7 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/device-types/vsock/description.tex
> > > > > > > > b/device-types/vsock/description.tex
> > > > > > > > index 7d91d159872f..638dca8e5da1 100644
> > > > > > > > --- a/device-types/vsock/description.tex
> > > > > > > > +++ b/device-types/vsock/description.tex
> > > > > > > > @@ -20,6 +20,7 @@ \subsection{Feature bits}\label{sec:Device
> > > > > > > > Types / Socket Device / Feature bits}
> > > > > > > > \item[VIRTIO_VSOCK_F_STREAM (0)] stream socket type is
> > > > > > > > supported.
> > > > > > > > \item[VIRTIO_VSOCK_F_SEQPACKET (1)] seqpacket socket type is
> > > > > > > > supported.
> > > > > > > > \item[VIRTIO_VSOCK_F_NO_IMPLIED_STREAM (2)] stream socket type
> > > > > > > > is not implied.
> > > > > > > > +\item[VIRTIO_VSOCK_F_DGRAM (3)] datagram socket type is
> > > > > > > > supported.
> > > > > > > > \end{description}
> > > > > > > >
> > > > > > > > \drivernormative{\subsubsection}{Feature bits}{Device Types /
> > > > > > > > Socket Device / Feature bits}
> > > > > > > > @@ -167,17 +168,22 @@
> > > > > > > > \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 stream and seqpacket sockets are supported.
> > > > > > > > \field{type} is 1 (VIRTIO_VSOCK_TYPE_STREAM)
> > > > > > > > -for stream socket types, and 2 (VIRTIO_VSOCK_TYPE_SEQPACKET)
> > > > > > > > for seqpacket socket types.
> > > > > > > > +
> > > > > > > > +Currently stream, seqpacket, and datagram sockets are
> > > > > > > > supported. \field{type} is
> > > > > > > > +1 (VIRTIO_VSOCK_TYPE_STREAM) for stream socket types, 2
> > > > > > > > (VIRTIO_VSOCK_TYPE_SEQPACKET) for
> > > > > > > > +seqpacket socket types, and 3 (VIRTIO_VSOCK_TYPE_DGRAM) for
> > > > > > > > datagram socket types.
> > > > > > > >
> > > > > > > > \begin{lstlisting}
> > > > > > >
[virtio-dev] Re: [PATCH] virtio-vsock: add VIRTIO_VSOCK_F_DGRAM feature bit
On Sat, Sep 02, 2023 at 04:35:25AM -0400, Michael S. Tsirkin wrote:
> On Sat, Sep 02, 2023 at 04:56:42AM +0000, Bobby Eshleman wrote:
> > On Fri, Sep 01, 2023 at 02:45:14PM +0200, Stefano Garzarella wrote:
> > > On Tue, Aug 29, 2023 at 09:29:45PM +0000, Bobby Eshleman wrote:
> > > > This adds support for datagrams to the virtio-vsock device.
> > > >
> > > > virtio-vsock already supports stream and seqpacket types. The existing
> > > > message types and header fields are extended to support datagrams.
> > > > Semantic differences between the flow types are stated, as well as any
> > > > additional requirements for devices and drivers implementing this
> > > > feature.
> > > >
> > > > Signed-off-by: Bobby Eshleman
> > > > ---
> > > > device-types/vsock/description.tex | 95 +++---
> > > > 1 file changed, 88 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/device-types/vsock/description.tex
> > > > b/device-types/vsock/description.tex
> > > > index 7d91d159872f..638dca8e5da1 100644
> > > > --- a/device-types/vsock/description.tex
> > > > +++ b/device-types/vsock/description.tex
> > > > @@ -20,6 +20,7 @@ \subsection{Feature bits}\label{sec:Device Types /
> > > > Socket Device / Feature bits}
> > > > \item[VIRTIO_VSOCK_F_STREAM (0)] stream socket type is supported.
> > > > \item[VIRTIO_VSOCK_F_SEQPACKET (1)] seqpacket socket type is supported.
> > > > \item[VIRTIO_VSOCK_F_NO_IMPLIED_STREAM (2)] stream socket type is not
> > > > implied.
> > > > +\item[VIRTIO_VSOCK_F_DGRAM (3)] datagram socket type is supported.
> > > > \end{description}
> > > >
> > > > \drivernormative{\subsubsection}{Feature bits}{Device Types / Socket
> > > > Device / Feature bits}
> > > > @@ -167,17 +168,22 @@ \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 stream and seqpacket sockets are supported. \field{type} is
> > > > 1 (VIRTIO_VSOCK_TYPE_STREAM)
> > > > -for stream socket types, and 2 (VIRTIO_VSOCK_TYPE_SEQPACKET) for
> > > > seqpacket socket types.
> > > > +
> > > > +Currently stream, seqpacket, and datagram sockets are supported.
> > > > \field{type} is
> > > > +1 (VIRTIO_VSOCK_TYPE_STREAM) for stream socket types, 2
> > > > (VIRTIO_VSOCK_TYPE_SEQPACKET) for
> > > > +seqpacket socket types, and 3 (VIRTIO_VSOCK_TYPE_DGRAM) for datagram
> > > > socket types.
> > > >
> > > > \begin{lstlisting}
> > > > #define VIRTIO_VSOCK_TYPE_STREAM1
> > > > #define VIRTIO_VSOCK_TYPE_SEQPACKET 2
> > > > +#define VIRTIO_VSOCK_TYPE_DGRAM 3
> > > > \end{lstlisting}
> > > >
> > > > Stream sockets provide in-order, guaranteed, connection-oriented
> > > > delivery
> > > > without message boundaries. Seqpacket sockets provide in-order,
> > > > guaranteed,
> > > > -connection-oriented delivery with message and record boundaries.
> > > > +connection-oriented delivery with message and record boundaries.
> > > > Datagram
> > > > +sockets provide connection-less, best-effort delivery of messages,
> > > > with no
> > > > +order or reliability guarantees.
> > > >
> > > > \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
> > > > @@ -203,16 +209,19 @@ \subsubsection{Buffer Space
> > > > Management}\label{sec:Device Types / Socket Device /
> > > > previously receiving a VIRTIO_VSOCK_OP_CREDIT_REQUEST packet. This
> > > > allows
> > > > communicating updates any time a change in buffer space occurs.
> > > >
> > > > +\field{buf_alloc} and \field{fwd_cnt} are reserved for future use by
> > > > datagram
> > > > +sockets. These fields are not used for datagram buffer space
> > > > management.
> > > > +
> > >
[virtio-dev] Re: [PATCH] virtio-vsock: add VIRTIO_VSOCK_F_DGRAM feature bit
On Sat, Sep 02, 2023 at 04:41:28AM -0400, Michael S. Tsirkin wrote:
> On Sat, Sep 02, 2023 at 04:37:19AM +0000, Bobby Eshleman wrote:
> > On Fri, Sep 01, 2023 at 09:31:03AM -0400, Michael S. Tsirkin wrote:
> > > On Wed, Aug 30, 2023 at 12:43:03AM +0000, Bobby Eshleman wrote:
> > > > On Tue, Aug 29, 2023 at 06:21:35PM -0400, Michael S. Tsirkin wrote:
> > > > > On Tue, Aug 29, 2023 at 09:29:45PM +, Bobby Eshleman wrote:
> > > > > > This adds support for datagrams to the virtio-vsock device.
> > > > > >
> > > > > > virtio-vsock already supports stream and seqpacket types. The
> > > > > > existing
> > > > > > message types and header fields are extended to support datagrams.
> > > > > > Semantic differences between the flow types are stated, as well as
> > > > > > any
> > > > > > additional requirements for devices and drivers implementing this
> > > > > > feature.
> > > > > >
> > > > > > Signed-off-by: Bobby Eshleman
> > > > > > ---
> > > > > > device-types/vsock/description.tex | 95
> > > > > > +++---
> > > > > > 1 file changed, 88 insertions(+), 7 deletions(-)
> > > > > >
> > > > > > diff --git a/device-types/vsock/description.tex
> > > > > > b/device-types/vsock/description.tex
> > > > > > index 7d91d159872f..638dca8e5da1 100644
> > > > > > --- a/device-types/vsock/description.tex
> > > > > > +++ b/device-types/vsock/description.tex
> > > > > > @@ -20,6 +20,7 @@ \subsection{Feature bits}\label{sec:Device Types
> > > > > > / Socket Device / Feature bits}
> > > > > > \item[VIRTIO_VSOCK_F_STREAM (0)] stream socket type is supported.
> > > > > > \item[VIRTIO_VSOCK_F_SEQPACKET (1)] seqpacket socket type is
> > > > > > supported.
> > > > > > \item[VIRTIO_VSOCK_F_NO_IMPLIED_STREAM (2)] stream socket type is
> > > > > > not implied.
> > > > > > +\item[VIRTIO_VSOCK_F_DGRAM (3)] datagram socket type is supported.
> > > > > > \end{description}
> > > > > >
> > > > > > \drivernormative{\subsubsection}{Feature bits}{Device Types /
> > > > > > Socket Device / Feature bits}
> > > > > > @@ -167,17 +168,22 @@ \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 stream and seqpacket sockets are supported. \field{type}
> > > > > > is 1 (VIRTIO_VSOCK_TYPE_STREAM)
> > > > > > -for stream socket types, and 2 (VIRTIO_VSOCK_TYPE_SEQPACKET) for
> > > > > > seqpacket socket types.
> > > > > > +
> > > > > > +Currently stream, seqpacket, and datagram sockets are supported.
> > > > > > \field{type} is
> > > > > > +1 (VIRTIO_VSOCK_TYPE_STREAM) for stream socket types, 2
> > > > > > (VIRTIO_VSOCK_TYPE_SEQPACKET) for
> > > > > > +seqpacket socket types, and 3 (VIRTIO_VSOCK_TYPE_DGRAM) for
> > > > > > datagram socket types.
> > > > > >
> > > > > > \begin{lstlisting}
> > > > > > #define VIRTIO_VSOCK_TYPE_STREAM1
> > > > > > #define VIRTIO_VSOCK_TYPE_SEQPACKET 2
> > > > > > +#define VIRTIO_VSOCK_TYPE_DGRAM 3
> > > > > > \end{lstlisting}
> > > > > >
> > > > > > Stream sockets provide in-order, guaranteed, connection-oriented
> > > > > > delivery
> > > > > > without message boundaries. Seqpacket sockets provide in-order,
> > > > > > guaranteed,
> > > > > > -connection-oriented delivery with message and record boundaries.
> > > > > > +connection-oriented delivery with message and record boundaries.
> > > > > > Datagram
> > > > > > +sockets provide connection-less, best-effort delivery of messages,
> > > > > > with no
> > > > > > +order or reliabili
[virtio-dev] Re: [PATCH] virtio-vsock: add VIRTIO_VSOCK_F_DGRAM feature bit
On Fri, Sep 01, 2023 at 02:45:14PM +0200, Stefano Garzarella wrote:
> On Tue, Aug 29, 2023 at 09:29:45PM +0000, Bobby Eshleman wrote:
> > This adds support for datagrams to the virtio-vsock device.
> >
> > virtio-vsock already supports stream and seqpacket types. The existing
> > message types and header fields are extended to support datagrams.
> > Semantic differences between the flow types are stated, as well as any
> > additional requirements for devices and drivers implementing this
> > feature.
> >
> > Signed-off-by: Bobby Eshleman
> > ---
> > device-types/vsock/description.tex | 95 +++---
> > 1 file changed, 88 insertions(+), 7 deletions(-)
> >
> > diff --git a/device-types/vsock/description.tex
> > b/device-types/vsock/description.tex
> > index 7d91d159872f..638dca8e5da1 100644
> > --- a/device-types/vsock/description.tex
> > +++ b/device-types/vsock/description.tex
> > @@ -20,6 +20,7 @@ \subsection{Feature bits}\label{sec:Device Types / Socket
> > Device / Feature bits}
> > \item[VIRTIO_VSOCK_F_STREAM (0)] stream socket type is supported.
> > \item[VIRTIO_VSOCK_F_SEQPACKET (1)] seqpacket socket type is supported.
> > \item[VIRTIO_VSOCK_F_NO_IMPLIED_STREAM (2)] stream socket type is not
> > implied.
> > +\item[VIRTIO_VSOCK_F_DGRAM (3)] datagram socket type is supported.
> > \end{description}
> >
> > \drivernormative{\subsubsection}{Feature bits}{Device Types / Socket Device
> > / Feature bits}
> > @@ -167,17 +168,22 @@ \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 stream and seqpacket sockets are supported. \field{type} is 1
> > (VIRTIO_VSOCK_TYPE_STREAM)
> > -for stream socket types, and 2 (VIRTIO_VSOCK_TYPE_SEQPACKET) for seqpacket
> > socket types.
> > +
> > +Currently stream, seqpacket, and datagram sockets are supported.
> > \field{type} is
> > +1 (VIRTIO_VSOCK_TYPE_STREAM) for stream socket types, 2
> > (VIRTIO_VSOCK_TYPE_SEQPACKET) for
> > +seqpacket socket types, and 3 (VIRTIO_VSOCK_TYPE_DGRAM) for datagram
> > socket types.
> >
> > \begin{lstlisting}
> > #define VIRTIO_VSOCK_TYPE_STREAM1
> > #define VIRTIO_VSOCK_TYPE_SEQPACKET 2
> > +#define VIRTIO_VSOCK_TYPE_DGRAM 3
> > \end{lstlisting}
> >
> > Stream sockets provide in-order, guaranteed, connection-oriented delivery
> > without message boundaries. Seqpacket sockets provide in-order, guaranteed,
> > -connection-oriented delivery with message and record boundaries.
> > +connection-oriented delivery with message and record boundaries. Datagram
> > +sockets provide connection-less, best-effort delivery of messages, with no
> > +order or reliability guarantees.
> >
> > \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
> > @@ -203,16 +209,19 @@ \subsubsection{Buffer Space
> > Management}\label{sec:Device Types / Socket Device /
> > previously receiving a VIRTIO_VSOCK_OP_CREDIT_REQUEST packet. This allows
> > communicating updates any time a change in buffer space occurs.
> >
> > +\field{buf_alloc} and \field{fwd_cnt} are reserved for future use by
> > datagram
> > +sockets. These fields are not used for datagram buffer space management.
> > +
> > \drivernormative{\paragraph}{Device Operation: Buffer Space
> > Management}{Device Types / Socket Device / Device Operation / Buffer Space
> > Management}
> > -VIRTIO_VSOCK_OP_RW data packets MUST only be transmitted when the peer has
> > -sufficient free buffer space for the payload.
> > +For stream and seqpacket flows, VIRTIO_VSOCK_OP_RW data packets MUST only
> > be
> > +transmitted when the peer has sufficient free buffer space for the payload.
> >
> > All packets associated with a stream flow MUST contain valid information in
> > \field{buf_alloc} and \field{fwd_cnt} fields.
> >
> > \devicenormative{\paragraph}{Device Operation: Buffer Space
> > Management}{Device Types / Socket Device / Device Operation / Buffer
> > Space Management}
> > -VIRTIO_VSOCK_OP_RW data packets MUST only be transmitted when the peer has
> > -sufficient free buffer space for the payload.
> > +For stream and seqpacket flows, VIRTIO_VSOCK_OP_RW da
[virtio-dev] Re: [PATCH] virtio-vsock: add VIRTIO_VSOCK_F_DGRAM feature bit
On Fri, Sep 01, 2023 at 09:31:03AM -0400, Michael S. Tsirkin wrote:
> On Wed, Aug 30, 2023 at 12:43:03AM +0000, Bobby Eshleman wrote:
> > On Tue, Aug 29, 2023 at 06:21:35PM -0400, Michael S. Tsirkin wrote:
> > > On Tue, Aug 29, 2023 at 09:29:45PM +0000, Bobby Eshleman wrote:
> > > > This adds support for datagrams to the virtio-vsock device.
> > > >
> > > > virtio-vsock already supports stream and seqpacket types. The existing
> > > > message types and header fields are extended to support datagrams.
> > > > Semantic differences between the flow types are stated, as well as any
> > > > additional requirements for devices and drivers implementing this
> > > > feature.
> > > >
> > > > Signed-off-by: Bobby Eshleman
> > > > ---
> > > > device-types/vsock/description.tex | 95 +++---
> > > > 1 file changed, 88 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/device-types/vsock/description.tex
> > > > b/device-types/vsock/description.tex
> > > > index 7d91d159872f..638dca8e5da1 100644
> > > > --- a/device-types/vsock/description.tex
> > > > +++ b/device-types/vsock/description.tex
> > > > @@ -20,6 +20,7 @@ \subsection{Feature bits}\label{sec:Device Types /
> > > > Socket Device / Feature bits}
> > > > \item[VIRTIO_VSOCK_F_STREAM (0)] stream socket type is supported.
> > > > \item[VIRTIO_VSOCK_F_SEQPACKET (1)] seqpacket socket type is supported.
> > > > \item[VIRTIO_VSOCK_F_NO_IMPLIED_STREAM (2)] stream socket type is not
> > > > implied.
> > > > +\item[VIRTIO_VSOCK_F_DGRAM (3)] datagram socket type is supported.
> > > > \end{description}
> > > >
> > > > \drivernormative{\subsubsection}{Feature bits}{Device Types / Socket
> > > > Device / Feature bits}
> > > > @@ -167,17 +168,22 @@ \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 stream and seqpacket sockets are supported. \field{type} is
> > > > 1 (VIRTIO_VSOCK_TYPE_STREAM)
> > > > -for stream socket types, and 2 (VIRTIO_VSOCK_TYPE_SEQPACKET) for
> > > > seqpacket socket types.
> > > > +
> > > > +Currently stream, seqpacket, and datagram sockets are supported.
> > > > \field{type} is
> > > > +1 (VIRTIO_VSOCK_TYPE_STREAM) for stream socket types, 2
> > > > (VIRTIO_VSOCK_TYPE_SEQPACKET) for
> > > > +seqpacket socket types, and 3 (VIRTIO_VSOCK_TYPE_DGRAM) for datagram
> > > > socket types.
> > > >
> > > > \begin{lstlisting}
> > > > #define VIRTIO_VSOCK_TYPE_STREAM1
> > > > #define VIRTIO_VSOCK_TYPE_SEQPACKET 2
> > > > +#define VIRTIO_VSOCK_TYPE_DGRAM 3
> > > > \end{lstlisting}
> > > >
> > > > Stream sockets provide in-order, guaranteed, connection-oriented
> > > > delivery
> > > > without message boundaries. Seqpacket sockets provide in-order,
> > > > guaranteed,
> > > > -connection-oriented delivery with message and record boundaries.
> > > > +connection-oriented delivery with message and record boundaries.
> > > > Datagram
> > > > +sockets provide connection-less, best-effort delivery of messages,
> > > > with no
> > > > +order or reliability guarantees.
> > > >
> > > > \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
> > > > @@ -203,16 +209,19 @@ \subsubsection{Buffer Space
> > > > Management}\label{sec:Device Types / Socket Device /
> > > > previously receiving a VIRTIO_VSOCK_OP_CREDIT_REQUEST packet. This
> > > > allows
> > > > communicating updates any time a change in buffer space occurs.
> > > >
> > > > +\field{buf_alloc} and \field{fwd_cnt} are reserved for future use by
> > > > datagram
> > > > +sockets. These fields are not used for datagram buffer space
> > > > management.
> > >
[virtio-dev] Re: [PATCH] virtio-vsock: add VIRTIO_VSOCK_F_DGRAM feature bit
On Tue, Aug 29, 2023 at 06:21:35PM -0400, Michael S. Tsirkin wrote:
> On Tue, Aug 29, 2023 at 09:29:45PM +0000, Bobby Eshleman wrote:
> > This adds support for datagrams to the virtio-vsock device.
> >
> > virtio-vsock already supports stream and seqpacket types. The existing
> > message types and header fields are extended to support datagrams.
> > Semantic differences between the flow types are stated, as well as any
> > additional requirements for devices and drivers implementing this
> > feature.
> >
> > Signed-off-by: Bobby Eshleman
> > ---
> > device-types/vsock/description.tex | 95 +++---
> > 1 file changed, 88 insertions(+), 7 deletions(-)
> >
> > diff --git a/device-types/vsock/description.tex
> > b/device-types/vsock/description.tex
> > index 7d91d159872f..638dca8e5da1 100644
> > --- a/device-types/vsock/description.tex
> > +++ b/device-types/vsock/description.tex
> > @@ -20,6 +20,7 @@ \subsection{Feature bits}\label{sec:Device Types / Socket
> > Device / Feature bits}
> > \item[VIRTIO_VSOCK_F_STREAM (0)] stream socket type is supported.
> > \item[VIRTIO_VSOCK_F_SEQPACKET (1)] seqpacket socket type is supported.
> > \item[VIRTIO_VSOCK_F_NO_IMPLIED_STREAM (2)] stream socket type is not
> > implied.
> > +\item[VIRTIO_VSOCK_F_DGRAM (3)] datagram socket type is supported.
> > \end{description}
> >
> > \drivernormative{\subsubsection}{Feature bits}{Device Types / Socket
> > Device / Feature bits}
> > @@ -167,17 +168,22 @@ \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 stream and seqpacket sockets are supported. \field{type} is 1
> > (VIRTIO_VSOCK_TYPE_STREAM)
> > -for stream socket types, and 2 (VIRTIO_VSOCK_TYPE_SEQPACKET) for seqpacket
> > socket types.
> > +
> > +Currently stream, seqpacket, and datagram sockets are supported.
> > \field{type} is
> > +1 (VIRTIO_VSOCK_TYPE_STREAM) for stream socket types, 2
> > (VIRTIO_VSOCK_TYPE_SEQPACKET) for
> > +seqpacket socket types, and 3 (VIRTIO_VSOCK_TYPE_DGRAM) for datagram
> > socket types.
> >
> > \begin{lstlisting}
> > #define VIRTIO_VSOCK_TYPE_STREAM1
> > #define VIRTIO_VSOCK_TYPE_SEQPACKET 2
> > +#define VIRTIO_VSOCK_TYPE_DGRAM 3
> > \end{lstlisting}
> >
> > Stream sockets provide in-order, guaranteed, connection-oriented delivery
> > without message boundaries. Seqpacket sockets provide in-order, guaranteed,
> > -connection-oriented delivery with message and record boundaries.
> > +connection-oriented delivery with message and record boundaries. Datagram
> > +sockets provide connection-less, best-effort delivery of messages, with no
> > +order or reliability guarantees.
> >
> > \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
> > @@ -203,16 +209,19 @@ \subsubsection{Buffer Space
> > Management}\label{sec:Device Types / Socket Device /
> > previously receiving a VIRTIO_VSOCK_OP_CREDIT_REQUEST packet. This allows
> > communicating updates any time a change in buffer space occurs.
> >
> > +\field{buf_alloc} and \field{fwd_cnt} are reserved for future use by
> > datagram
> > +sockets. These fields are not used for datagram buffer space management.
>
> no limits on datagram size?
>
In the Linux proof-of-concept, it is 64KB. I can add that here too.
> also with no flow control at all there's a performance problem:
> imagine queue gets full. now buffers begin to be dropped.
> problem is, dropping is faster than delivering to application.
> so now application sees its packets consumed super quickly
> and starts sending even faster.
> not good for performance.
>
> yes datagram expects some way to drop packets but just disabling flow
> control completely is not the right thing to do I think.
>
On the LKML I discussed using congestion notification as a way to handle
this situation, but deferred it to a future feature bit. I can build
it in from the beginning though.
>
> > +
> > \drivernormative{\paragraph}{Device Operation: Buffer Space
> > Management}{Device Types / Socket Device / Device Operation / Buffer Space
> > Management}
> > -VIRTIO_VSOCK_OP_RW data packets MUST only be transmitted when
[virtio-dev] Re: [PATCH RFC net-next v2 0/4] virtio/vsock: support datagrams
On Fri, Apr 28, 2023 at 12:43:09PM +0200, Stefano Garzarella wrote: > On Sat, Apr 15, 2023 at 07:13:47AM +0000, Bobby Eshleman wrote: > > CC'ing virtio-dev@lists.oasis-open.org because this thread is starting > > to touch the spec. > > > > On Wed, Apr 19, 2023 at 12:00:17PM +0200, Stefano Garzarella wrote: > > > Hi Bobby, > > > > > > On Fri, Apr 14, 2023 at 11:18:40AM +, Bobby Eshleman wrote: > > > > CC'ing Cong. > > > > > > > > On Fri, Apr 14, 2023 at 12:25:56AM +, Bobby Eshleman wrote: > > > > > Hey all! > > > > > > > > > > This series introduces support for datagrams to virtio/vsock. > > > > > > Great! Thanks for restarting this work! > > > > > > > No problem! > > > > > > > > > > > > It is a spin-off (and smaller version) of this series from the summer: > > > > > > > > > > https://lore.kernel.org/all/cover.1660362668.git.bobby.eshle...@bytedance.com/ > > > > > > > > > > Please note that this is an RFC and should not be merged until > > > > > associated changes are made to the virtio specification, which will > > > > > follow after discussion from this series. > > > > > > > > > > This series first supports datagrams in a basic form for virtio, and > > > > > then optimizes the sendpath for all transports. > > > > > > > > > > The result is a very fast datagram communication protocol that > > > > > outperforms even UDP on multi-queue virtio-net w/ vhost on a variety > > > > > of multi-threaded workload samples. > > > > > > > > > > For those that are curious, some summary data comparing UDP and VSOCK > > > > > DGRAM (N=5): > > > > > > > > > > vCPUS: 16 > > > > > virtio-net queues: 16 > > > > > payload size: 4KB > > > > > Setup: bare metal + vm (non-nested) > > > > > > > > > > UDP: 287.59 MB/s > > > > > VSOCK DGRAM: 509.2 MB/s > > > > > > > > > > Some notes about the implementation... > > > > > > > > > > This datagram implementation forces datagrams to self-throttle > > > > > according > > > > > to the threshold set by sk_sndbuf. It behaves similar to the credits > > > > > used by streams in its effect on throughput and memory consumption, > > > > > but > > > > > it is not influenced by the receiving socket as credits are. > > > > > > So, sk_sndbuf influece the sender and sk_rcvbuf the receiver, right? > > > > > > > Correct. > > > > > We should check if VMCI behaves the same. > > > > > > > > > > > > > The device drops packets silently. There is room for improvement by > > > > > building into the device and driver some intelligence around how to > > > > > reduce frequency of kicking the virtqueue when packet loss is high. I > > > > > think there is a good discussion to be had on this. > > > > > > Can you elaborate a bit here? > > > > > > Do you mean some mechanism to report to the sender that a destination > > > (cid, port) is full so the packet will be dropped? > > > > > > > Correct. There is also the case of there being no receiver at all for > > this address since this case isn't rejected upon connect(). Ideally, > > such a socket (which will have 100% packet loss) will be throttled > > aggressively. > > > > Before we go down too far on this path, I also want to clarify that > > using UDP over vhost/virtio-net also has this property... this can be > > observed by using tcpdump to dump the UDP packets on the bridge network > > your VM is using. UDP packets sent to a garbage address can be seen on > > the host bridge (this is the nature of UDP, how is the host supposed to > > know the address eventually goes nowhere). I mention the above because I > > think it is possible for vsock to avoid this cost, given that it > > benefits from being point-to-point and g2h/h2g. > > > > If we're okay with vsock being on par, then the current series does > > that. I propose something below that can be added later and maybe > > negotiated as a feature bit too. > > I see and I agree on that, let's do it step by step. > If we can do it in t
[virtio-dev] Re: [PATCH RFC net-next v2 0/4] virtio/vsock: support datagrams
CC'ing virtio-dev@lists.oasis-open.org because this thread is starting to touch the spec. On Wed, Apr 19, 2023 at 12:00:17PM +0200, Stefano Garzarella wrote: > Hi Bobby, > > On Fri, Apr 14, 2023 at 11:18:40AM +, Bobby Eshleman wrote: > > CC'ing Cong. > > > > On Fri, Apr 14, 2023 at 12:25:56AM +, Bobby Eshleman wrote: > > > Hey all! > > > > > > This series introduces support for datagrams to virtio/vsock. > > Great! Thanks for restarting this work! > No problem! > > > > > > It is a spin-off (and smaller version) of this series from the summer: > > > > > > https://lore.kernel.org/all/cover.1660362668.git.bobby.eshle...@bytedance.com/ > > > > > > Please note that this is an RFC and should not be merged until > > > associated changes are made to the virtio specification, which will > > > follow after discussion from this series. > > > > > > This series first supports datagrams in a basic form for virtio, and > > > then optimizes the sendpath for all transports. > > > > > > The result is a very fast datagram communication protocol that > > > outperforms even UDP on multi-queue virtio-net w/ vhost on a variety > > > of multi-threaded workload samples. > > > > > > For those that are curious, some summary data comparing UDP and VSOCK > > > DGRAM (N=5): > > > > > > vCPUS: 16 > > > virtio-net queues: 16 > > > payload size: 4KB > > > Setup: bare metal + vm (non-nested) > > > > > > UDP: 287.59 MB/s > > > VSOCK DGRAM: 509.2 MB/s > > > > > > Some notes about the implementation... > > > > > > This datagram implementation forces datagrams to self-throttle according > > > to the threshold set by sk_sndbuf. It behaves similar to the credits > > > used by streams in its effect on throughput and memory consumption, but > > > it is not influenced by the receiving socket as credits are. > > So, sk_sndbuf influece the sender and sk_rcvbuf the receiver, right? > Correct. > We should check if VMCI behaves the same. > > > > > > > The device drops packets silently. There is room for improvement by > > > building into the device and driver some intelligence around how to > > > reduce frequency of kicking the virtqueue when packet loss is high. I > > > think there is a good discussion to be had on this. > > Can you elaborate a bit here? > > Do you mean some mechanism to report to the sender that a destination > (cid, port) is full so the packet will be dropped? > Correct. There is also the case of there being no receiver at all for this address since this case isn't rejected upon connect(). Ideally, such a socket (which will have 100% packet loss) will be throttled aggressively. Before we go down too far on this path, I also want to clarify that using UDP over vhost/virtio-net also has this property... this can be observed by using tcpdump to dump the UDP packets on the bridge network your VM is using. UDP packets sent to a garbage address can be seen on the host bridge (this is the nature of UDP, how is the host supposed to know the address eventually goes nowhere). I mention the above because I think it is possible for vsock to avoid this cost, given that it benefits from being point-to-point and g2h/h2g. If we're okay with vsock being on par, then the current series does that. I propose something below that can be added later and maybe negotiated as a feature bit too. > Can we adapt the credit mechanism? > I've thought about this a lot because the attraction of the approach for me would be that we could get the wait/buffer-limiting logic for free and without big changes to the protocol, but the problem is that the unreliable nature of datagrams means that the source's free-running tx_cnt will become out-of-sync with the destination's fwd_cnt upon packet loss. Imagine a source that initializes and starts sending packets before a destination socket even is created, the source's self-throttling will be dysfunctional because its tx_cnt will always far exceed the destination's fwd_cnt. We could play tricks with the meaning of the CREDIT_UPDATE message and fwd_cnt/buf_alloc fields, but I don't think we want to go down that path. I think that the best and simplest approach introduces a congestion notification (VIRTIO_VSOCK_OP_CN?). When a packet is dropped, the destination sends this notification. At a given repeated time period T, the source can check if it has received any notifications in the last T. If so, it halves its buffer allocation. If not, it doubles its buffer allocation unless it
Re: [virtio-dev] Re: [PATCH 5/6] virtio/vsock: add support for dgram
On Thu, Aug 18, 2022 at 08:35:48AM +, Arseniy Krasnov wrote:
> On Tue, 2022-08-16 at 09:58 +0000, Bobby Eshleman wrote:
> > On Wed, Aug 17, 2022 at 05:42:08AM +, Arseniy Krasnov wrote:
> > > On 17.08.2022 08:01, Arseniy Krasnov wrote:
> > > > On 16.08.2022 05:32, Bobby Eshleman wrote:
> > > > > CC'ing virtio-dev@lists.oasis-open.org
> > > > >
> > > > > On Mon, Aug 15, 2022 at 10:56:08AM -0700, Bobby Eshleman wrote:
> > > > > > This patch supports dgram in virtio and on the vhost side.
> > > > Hello,
> > > >
> > > > sorry, i don't understand, how this maintains message boundaries?
> > > > Or it
> > > > is unnecessary for SOCK_DGRAM?
> > > >
> > > > Thanks
> > > > > > Signed-off-by: Jiang Wang
> > > > > > Signed-off-by: Bobby Eshleman
> > > > > > ---
> > > > > > drivers/vhost/vsock.c | 2 +-
> > > > > > include/net/af_vsock.h | 2 +
> > > > > > include/uapi/linux/virtio_vsock.h | 1 +
> > > > > > net/vmw_vsock/af_vsock.c| 26 +++-
> > > > > > net/vmw_vsock/virtio_transport.c| 2 +-
> > > > > > net/vmw_vsock/virtio_transport_common.c | 173
> > > > > > ++--
> > > > > > 6 files changed, 186 insertions(+), 20 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> > > > > > index a5d1bdb786fe..3dc72a5647ca 100644
> > > > > > --- a/drivers/vhost/vsock.c
> > > > > > +++ b/drivers/vhost/vsock.c
> > > > > > @@ -925,7 +925,7 @@ static int __init vhost_vsock_init(void)
> > > > > > int ret;
> > > > > >
> > > > > > ret = vsock_core_register(&vhost_transport.transport,
> > > > > > - VSOCK_TRANSPORT_F_H2G);
> > > > > > + VSOCK_TRANSPORT_F_H2G |
> > > > > > VSOCK_TRANSPORT_F_DGRAM);
> > > > > > if (ret < 0)
> > > > > > return ret;
> > > > > >
> > > > > > diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> > > > > > index 1c53c4c4d88f..37e55c81e4df 100644
> > > > > > --- a/include/net/af_vsock.h
> > > > > > +++ b/include/net/af_vsock.h
> > > > > > @@ -78,6 +78,8 @@ struct vsock_sock {
> > > > > > s64 vsock_stream_has_data(struct vsock_sock *vsk);
> > > > > > s64 vsock_stream_has_space(struct vsock_sock *vsk);
> > > > > > struct sock *vsock_create_connected(struct sock *parent);
> > > > > > +int vsock_bind_stream(struct vsock_sock *vsk,
> > > > > > + struct sockaddr_vm *addr);
> > > > > >
> > > > > > / TRANSPORT /
> > > > > >
> > > > > > diff --git a/include/uapi/linux/virtio_vsock.h
> > > > > > b/include/uapi/linux/virtio_vsock.h
> > > > > > index 857df3a3a70d..0975b9c88292 100644
> > > > > > --- a/include/uapi/linux/virtio_vsock.h
> > > > > > +++ b/include/uapi/linux/virtio_vsock.h
> > > > > > @@ -70,6 +70,7 @@ struct virtio_vsock_hdr {
> > > > > > enum virtio_vsock_type {
> > > > > > VIRTIO_VSOCK_TYPE_STREAM = 1,
> > > > > > VIRTIO_VSOCK_TYPE_SEQPACKET = 2,
> > > > > > + VIRTIO_VSOCK_TYPE_DGRAM = 3,
> > > > > > };
> > > > > >
> > > > > > enum virtio_vsock_op {
> > > > > > diff --git a/net/vmw_vsock/af_vsock.c
> > > > > > b/net/vmw_vsock/af_vsock.c
> > > > > > index 1893f8aafa48..87e4ae1866d3 100644
> > > > > > --- a/net/vmw_vsock/af_vsock.c
> > > > > > +++ b/net/vmw_vsock/af_vsock.c
> > > > > > @@ -675,6 +675,19 @@ static int
> > > > > > __vsock_bind_connectible(struct vsock_sock *vsk,
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > +int vsock_bind_stream(struct vsock_sock *vsk,
> > > > > > + struct sockaddr_vm *addr)
> > > > > > +{
Re: [virtio-dev] Re: [PATCH 5/6] virtio/vsock: add support for dgram
On Wed, Aug 17, 2022 at 05:42:08AM +, Arseniy Krasnov wrote:
> On 17.08.2022 08:01, Arseniy Krasnov wrote:
> > On 16.08.2022 05:32, Bobby Eshleman wrote:
> >> CC'ing virtio-dev@lists.oasis-open.org
> >>
> >> On Mon, Aug 15, 2022 at 10:56:08AM -0700, Bobby Eshleman wrote:
> >>> This patch supports dgram in virtio and on the vhost side.
> > Hello,
> >
> > sorry, i don't understand, how this maintains message boundaries? Or it
> > is unnecessary for SOCK_DGRAM?
> >
> > Thanks
> >>>
> >>> Signed-off-by: Jiang Wang
> >>> Signed-off-by: Bobby Eshleman
> >>> ---
> >>> drivers/vhost/vsock.c | 2 +-
> >>> include/net/af_vsock.h | 2 +
> >>> include/uapi/linux/virtio_vsock.h | 1 +
> >>> net/vmw_vsock/af_vsock.c| 26 +++-
> >>> net/vmw_vsock/virtio_transport.c| 2 +-
> >>> net/vmw_vsock/virtio_transport_common.c | 173 ++--
> >>> 6 files changed, 186 insertions(+), 20 deletions(-)
> >>>
> >>> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> >>> index a5d1bdb786fe..3dc72a5647ca 100644
> >>> --- a/drivers/vhost/vsock.c
> >>> +++ b/drivers/vhost/vsock.c
> >>> @@ -925,7 +925,7 @@ static int __init vhost_vsock_init(void)
> >>> int ret;
> >>>
> >>> ret = vsock_core_register(&vhost_transport.transport,
> >>> - VSOCK_TRANSPORT_F_H2G);
> >>> + VSOCK_TRANSPORT_F_H2G |
> >>> VSOCK_TRANSPORT_F_DGRAM);
> >>> if (ret < 0)
> >>> return ret;
> >>>
> >>> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> >>> index 1c53c4c4d88f..37e55c81e4df 100644
> >>> --- a/include/net/af_vsock.h
> >>> +++ b/include/net/af_vsock.h
> >>> @@ -78,6 +78,8 @@ struct vsock_sock {
> >>> s64 vsock_stream_has_data(struct vsock_sock *vsk);
> >>> s64 vsock_stream_has_space(struct vsock_sock *vsk);
> >>> struct sock *vsock_create_connected(struct sock *parent);
> >>> +int vsock_bind_stream(struct vsock_sock *vsk,
> >>> + struct sockaddr_vm *addr);
> >>>
> >>> / TRANSPORT /
> >>>
> >>> diff --git a/include/uapi/linux/virtio_vsock.h
> >>> b/include/uapi/linux/virtio_vsock.h
> >>> index 857df3a3a70d..0975b9c88292 100644
> >>> --- a/include/uapi/linux/virtio_vsock.h
> >>> +++ b/include/uapi/linux/virtio_vsock.h
> >>> @@ -70,6 +70,7 @@ struct virtio_vsock_hdr {
> >>> enum virtio_vsock_type {
> >>> VIRTIO_VSOCK_TYPE_STREAM = 1,
> >>> VIRTIO_VSOCK_TYPE_SEQPACKET = 2,
> >>> + VIRTIO_VSOCK_TYPE_DGRAM = 3,
> >>> };
> >>>
> >>> enum virtio_vsock_op {
> >>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> >>> index 1893f8aafa48..87e4ae1866d3 100644
> >>> --- a/net/vmw_vsock/af_vsock.c
> >>> +++ b/net/vmw_vsock/af_vsock.c
> >>> @@ -675,6 +675,19 @@ static int __vsock_bind_connectible(struct
> >>> vsock_sock *vsk,
> >>> return 0;
> >>> }
> >>>
> >>> +int vsock_bind_stream(struct vsock_sock *vsk,
> >>> + struct sockaddr_vm *addr)
> >>> +{
> >>> + int retval;
> >>> +
> >>> + spin_lock_bh(&vsock_table_lock);
> >>> + retval = __vsock_bind_connectible(vsk, addr);
> >>> + spin_unlock_bh(&vsock_table_lock);
> >>> +
> >>> + return retval;
> >>> +}
> >>> +EXPORT_SYMBOL(vsock_bind_stream);
> >>> +
> >>> static int __vsock_bind_dgram(struct vsock_sock *vsk,
> >>> struct sockaddr_vm *addr)
> >>> {
> >>> @@ -2363,11 +2376,16 @@ int vsock_core_register(const struct
> >>> vsock_transport *t, int features)
> >>> }
> >>>
> >>> if (features & VSOCK_TRANSPORT_F_DGRAM) {
> >>> - if (t_dgram) {
> >>> - err = -EBUSY;
> >>> - goto err_busy;
> >>> + /* TODO: always chose the G2H variant over others, support
> >>> nesting later */
> >>> + if (features & VSOCK_TRANSPORT_F_G2H) {
Re: [virtio-dev] Re: [PATCH 5/6] virtio/vsock: add support for dgram
On Wed, Aug 17, 2022 at 05:01:00AM +, Arseniy Krasnov wrote:
> On 16.08.2022 05:32, Bobby Eshleman wrote:
> > CC'ing virtio-dev@lists.oasis-open.org
> >
> > On Mon, Aug 15, 2022 at 10:56:08AM -0700, Bobby Eshleman wrote:
> >> This patch supports dgram in virtio and on the vhost side.
> Hello,
>
> sorry, i don't understand, how this maintains message boundaries? Or it
> is unnecessary for SOCK_DGRAM?
>
> Thanks
If I understand your question, the length is included in the header, so
receivers always know that header start + header length + payload length
marks the message boundary.
> >>
> >> Signed-off-by: Jiang Wang
> >> Signed-off-by: Bobby Eshleman
> >> ---
> >> drivers/vhost/vsock.c | 2 +-
> >> include/net/af_vsock.h | 2 +
> >> include/uapi/linux/virtio_vsock.h | 1 +
> >> net/vmw_vsock/af_vsock.c| 26 +++-
> >> net/vmw_vsock/virtio_transport.c| 2 +-
> >> net/vmw_vsock/virtio_transport_common.c | 173 ++--
> >> 6 files changed, 186 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> >> index a5d1bdb786fe..3dc72a5647ca 100644
> >> --- a/drivers/vhost/vsock.c
> >> +++ b/drivers/vhost/vsock.c
> >> @@ -925,7 +925,7 @@ static int __init vhost_vsock_init(void)
> >>int ret;
> >>
> >>ret = vsock_core_register(&vhost_transport.transport,
> >> -VSOCK_TRANSPORT_F_H2G);
> >> +VSOCK_TRANSPORT_F_H2G |
> >> VSOCK_TRANSPORT_F_DGRAM);
> >>if (ret < 0)
> >>return ret;
> >>
> >> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> >> index 1c53c4c4d88f..37e55c81e4df 100644
> >> --- a/include/net/af_vsock.h
> >> +++ b/include/net/af_vsock.h
> >> @@ -78,6 +78,8 @@ struct vsock_sock {
> >> s64 vsock_stream_has_data(struct vsock_sock *vsk);
> >> s64 vsock_stream_has_space(struct vsock_sock *vsk);
> >> struct sock *vsock_create_connected(struct sock *parent);
> >> +int vsock_bind_stream(struct vsock_sock *vsk,
> >> +struct sockaddr_vm *addr);
> >>
> >> / TRANSPORT /
> >>
> >> diff --git a/include/uapi/linux/virtio_vsock.h
> >> b/include/uapi/linux/virtio_vsock.h
> >> index 857df3a3a70d..0975b9c88292 100644
> >> --- a/include/uapi/linux/virtio_vsock.h
> >> +++ b/include/uapi/linux/virtio_vsock.h
> >> @@ -70,6 +70,7 @@ struct virtio_vsock_hdr {
> >> enum virtio_vsock_type {
> >>VIRTIO_VSOCK_TYPE_STREAM = 1,
> >>VIRTIO_VSOCK_TYPE_SEQPACKET = 2,
> >> + VIRTIO_VSOCK_TYPE_DGRAM = 3,
> >> };
> >>
> >> enum virtio_vsock_op {
> >> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> >> index 1893f8aafa48..87e4ae1866d3 100644
> >> --- a/net/vmw_vsock/af_vsock.c
> >> +++ b/net/vmw_vsock/af_vsock.c
> >> @@ -675,6 +675,19 @@ static int __vsock_bind_connectible(struct vsock_sock
> >> *vsk,
> >>return 0;
> >> }
> >>
> >> +int vsock_bind_stream(struct vsock_sock *vsk,
> >> +struct sockaddr_vm *addr)
> >> +{
> >> + int retval;
> >> +
> >> + spin_lock_bh(&vsock_table_lock);
> >> + retval = __vsock_bind_connectible(vsk, addr);
> >> + spin_unlock_bh(&vsock_table_lock);
> >> +
> >> + return retval;
> >> +}
> >> +EXPORT_SYMBOL(vsock_bind_stream);
> >> +
> >> static int __vsock_bind_dgram(struct vsock_sock *vsk,
> >> struct sockaddr_vm *addr)
> >> {
> >> @@ -2363,11 +2376,16 @@ int vsock_core_register(const struct
> >> vsock_transport *t, int features)
> >>}
> >>
> >>if (features & VSOCK_TRANSPORT_F_DGRAM) {
> >> - if (t_dgram) {
> >> - err = -EBUSY;
> >> - goto err_busy;
> >> + /* TODO: always chose the G2H variant over others, support
> >> nesting later */
> >> + if (features & VSOCK_TRANSPORT_F_G2H) {
> >> + if (t_dgram)
> >> + pr_warn("virtio_vsock: t_dgram already set\n");
> >> + t_dgram = t;
> >> + }
> >> +
> >> +
[virtio-dev] Re: [PATCH 6/6] vsock_test: add tests for vsock dgram
CC'ing virtio-dev@lists.oasis-open.org
On Mon, Aug 15, 2022 at 10:56:09AM -0700, Bobby Eshleman wrote:
> From: Jiang Wang
>
> Added test cases for vsock dgram types.
>
> Signed-off-by: Jiang Wang
> ---
> tools/testing/vsock/util.c | 105 +
> tools/testing/vsock/util.h | 4 +
> tools/testing/vsock/vsock_test.c | 195 +++
> 3 files changed, 304 insertions(+)
>
> diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
> index 2acbb7703c6a..d2f5b223bf85 100644
> --- a/tools/testing/vsock/util.c
> +++ b/tools/testing/vsock/util.c
> @@ -260,6 +260,57 @@ void send_byte(int fd, int expected_ret, int flags)
> }
> }
>
> +/* Transmit one byte and check the return value.
> + *
> + * expected_ret:
> + * <0 Negative errno (for testing errors)
> + * 0 End-of-file
> + * 1 Success
> + */
> +void sendto_byte(int fd, const struct sockaddr *dest_addr, int len, int
> expected_ret,
> + int flags)
> +{
> + const uint8_t byte = 'A';
> + ssize_t nwritten;
> +
> + timeout_begin(TIMEOUT);
> + do {
> + nwritten = sendto(fd, &byte, sizeof(byte), flags, dest_addr,
> + len);
> + timeout_check("write");
> + } while (nwritten < 0 && errno == EINTR);
> + timeout_end();
> +
> + if (expected_ret < 0) {
> + if (nwritten != -1) {
> + fprintf(stderr, "bogus sendto(2) return value %zd\n",
> + nwritten);
> + exit(EXIT_FAILURE);
> + }
> + if (errno != -expected_ret) {
> + perror("write");
> + exit(EXIT_FAILURE);
> + }
> + return;
> + }
> +
> + if (nwritten < 0) {
> + perror("write");
> + exit(EXIT_FAILURE);
> + }
> + if (nwritten == 0) {
> + if (expected_ret == 0)
> + return;
> +
> + fprintf(stderr, "unexpected EOF while sending byte\n");
> + exit(EXIT_FAILURE);
> + }
> + if (nwritten != sizeof(byte)) {
> + fprintf(stderr, "bogus sendto(2) return value %zd\n", nwritten);
> + exit(EXIT_FAILURE);
> + }
> +}
> +
> /* Receive one byte and check the return value.
> *
> * expected_ret:
> @@ -313,6 +364,60 @@ void recv_byte(int fd, int expected_ret, int flags)
> }
> }
>
> +/* Receive one byte and check the return value.
> + *
> + * expected_ret:
> + * <0 Negative errno (for testing errors)
> + * 0 End-of-file
> + * 1 Success
> + */
> +void recvfrom_byte(int fd, struct sockaddr *src_addr, socklen_t *addrlen,
> + int expected_ret, int flags)
> +{
> + uint8_t byte;
> + ssize_t nread;
> +
> + timeout_begin(TIMEOUT);
> + do {
> + nread = recvfrom(fd, &byte, sizeof(byte), flags, src_addr,
> addrlen);
> + timeout_check("read");
> + } while (nread < 0 && errno == EINTR);
> + timeout_end();
> +
> + if (expected_ret < 0) {
> + if (nread != -1) {
> + fprintf(stderr, "bogus recvfrom(2) return value %zd\n",
> + nread);
> + exit(EXIT_FAILURE);
> + }
> + if (errno != -expected_ret) {
> + perror("read");
> + exit(EXIT_FAILURE);
> + }
> + return;
> + }
> +
> + if (nread < 0) {
> + perror("read");
> + exit(EXIT_FAILURE);
> + }
> + if (nread == 0) {
> + if (expected_ret == 0)
> + return;
> +
> + fprintf(stderr, "unexpected EOF while receiving byte\n");
> + exit(EXIT_FAILURE);
> + }
> + if (nread != sizeof(byte)) {
> + fprintf(stderr, "bogus recvfrom(2) return value %zd\n", nread);
> + exit(EXIT_FAILURE);
> + }
> + if (byte != 'A') {
> + fprintf(stderr, "unexpected byte read %c\n", byte);
> + exit(EXIT_FAILURE);
> + }
> +}
> +
> /* Run test cases. The program terminates if a failure occurs. */
> void run_tests(const struct test_case *test_cases,
> const struct test_opts *opts)
> diff --git a/tools/testing/vsock/util.h b
[virtio-dev] Re: [PATCH 5/6] virtio/vsock: add support for dgram
CC'ing virtio-dev@lists.oasis-open.org
On Mon, Aug 15, 2022 at 10:56:08AM -0700, Bobby Eshleman wrote:
> This patch supports dgram in virtio and on the vhost side.
>
> Signed-off-by: Jiang Wang
> Signed-off-by: Bobby Eshleman
> ---
> drivers/vhost/vsock.c | 2 +-
> include/net/af_vsock.h | 2 +
> include/uapi/linux/virtio_vsock.h | 1 +
> net/vmw_vsock/af_vsock.c| 26 +++-
> net/vmw_vsock/virtio_transport.c| 2 +-
> net/vmw_vsock/virtio_transport_common.c | 173 ++--
> 6 files changed, 186 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index a5d1bdb786fe..3dc72a5647ca 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -925,7 +925,7 @@ static int __init vhost_vsock_init(void)
> int ret;
>
> ret = vsock_core_register(&vhost_transport.transport,
> - VSOCK_TRANSPORT_F_H2G);
> + VSOCK_TRANSPORT_F_H2G |
> VSOCK_TRANSPORT_F_DGRAM);
> if (ret < 0)
> return ret;
>
> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> index 1c53c4c4d88f..37e55c81e4df 100644
> --- a/include/net/af_vsock.h
> +++ b/include/net/af_vsock.h
> @@ -78,6 +78,8 @@ struct vsock_sock {
> s64 vsock_stream_has_data(struct vsock_sock *vsk);
> s64 vsock_stream_has_space(struct vsock_sock *vsk);
> struct sock *vsock_create_connected(struct sock *parent);
> +int vsock_bind_stream(struct vsock_sock *vsk,
> + struct sockaddr_vm *addr);
>
> / TRANSPORT /
>
> diff --git a/include/uapi/linux/virtio_vsock.h
> b/include/uapi/linux/virtio_vsock.h
> index 857df3a3a70d..0975b9c88292 100644
> --- a/include/uapi/linux/virtio_vsock.h
> +++ b/include/uapi/linux/virtio_vsock.h
> @@ -70,6 +70,7 @@ struct virtio_vsock_hdr {
> enum virtio_vsock_type {
> VIRTIO_VSOCK_TYPE_STREAM = 1,
> VIRTIO_VSOCK_TYPE_SEQPACKET = 2,
> + VIRTIO_VSOCK_TYPE_DGRAM = 3,
> };
>
> enum virtio_vsock_op {
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index 1893f8aafa48..87e4ae1866d3 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -675,6 +675,19 @@ static int __vsock_bind_connectible(struct vsock_sock
> *vsk,
> return 0;
> }
>
> +int vsock_bind_stream(struct vsock_sock *vsk,
> + struct sockaddr_vm *addr)
> +{
> + int retval;
> +
> + spin_lock_bh(&vsock_table_lock);
> + retval = __vsock_bind_connectible(vsk, addr);
> + spin_unlock_bh(&vsock_table_lock);
> +
> + return retval;
> +}
> +EXPORT_SYMBOL(vsock_bind_stream);
> +
> static int __vsock_bind_dgram(struct vsock_sock *vsk,
> struct sockaddr_vm *addr)
> {
> @@ -2363,11 +2376,16 @@ int vsock_core_register(const struct vsock_transport
> *t, int features)
> }
>
> if (features & VSOCK_TRANSPORT_F_DGRAM) {
> - if (t_dgram) {
> - err = -EBUSY;
> - goto err_busy;
> + /* TODO: always chose the G2H variant over others, support
> nesting later */
> + if (features & VSOCK_TRANSPORT_F_G2H) {
> + if (t_dgram)
> + pr_warn("virtio_vsock: t_dgram already set\n");
> + t_dgram = t;
> + }
> +
> + if (!t_dgram) {
> + t_dgram = t;
> }
> - t_dgram = t;
> }
>
> if (features & VSOCK_TRANSPORT_F_LOCAL) {
> diff --git a/net/vmw_vsock/virtio_transport.c
> b/net/vmw_vsock/virtio_transport.c
> index 073314312683..d4526ca462d2 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
> @@ -850,7 +850,7 @@ static int __init virtio_vsock_init(void)
> return -ENOMEM;
>
> ret = vsock_core_register(&virtio_transport.transport,
> - VSOCK_TRANSPORT_F_G2H);
> + VSOCK_TRANSPORT_F_G2H |
> VSOCK_TRANSPORT_F_DGRAM);
> if (ret)
> goto out_wq;
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c
> b/net/vmw_vsock/virtio_transport_common.c
> index bdf16fff054f..aedb48728677 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -229,7 +229,9 @@ EXPORT_SYMBOL_GPL(virtio_transport_deliver_tap_pkt);
>
> static u16 virtio_transport_get_type(struct sock *sk)
> {
> - if (sk-&g
[virtio-dev] Re: [PATCH 4/6] virtio/vsock: add VIRTIO_VSOCK_F_DGRAM feature bit
CC'ing virtio-dev@lists.oasis-open.org
On Mon, Aug 15, 2022 at 10:56:07AM -0700, Bobby Eshleman wrote:
> This commit adds a feature bit for virtio vsock to support datagrams.
>
> Signed-off-by: Jiang Wang
> Signed-off-by: Bobby Eshleman
> ---
> drivers/vhost/vsock.c | 3 ++-
> include/uapi/linux/virtio_vsock.h | 1 +
> net/vmw_vsock/virtio_transport.c | 8 ++--
> 3 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index b20ddec2664b..a5d1bdb786fe 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -32,7 +32,8 @@
> enum {
> VHOST_VSOCK_FEATURES = VHOST_FEATURES |
> (1ULL << VIRTIO_F_ACCESS_PLATFORM) |
> -(1ULL << VIRTIO_VSOCK_F_SEQPACKET)
> +(1ULL << VIRTIO_VSOCK_F_SEQPACKET) |
> +(1ULL << VIRTIO_VSOCK_F_DGRAM)
> };
>
> enum {
> diff --git a/include/uapi/linux/virtio_vsock.h
> b/include/uapi/linux/virtio_vsock.h
> index 64738838bee5..857df3a3a70d 100644
> --- a/include/uapi/linux/virtio_vsock.h
> +++ b/include/uapi/linux/virtio_vsock.h
> @@ -40,6 +40,7 @@
>
> /* The feature bitmap for virtio vsock */
> #define VIRTIO_VSOCK_F_SEQPACKET 1 /* SOCK_SEQPACKET supported */
> +#define VIRTIO_VSOCK_F_DGRAM 2 /* Host support dgram vsock */
>
> struct virtio_vsock_config {
> __le64 guest_cid;
> diff --git a/net/vmw_vsock/virtio_transport.c
> b/net/vmw_vsock/virtio_transport.c
> index c6212eb38d3c..073314312683 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
> @@ -35,6 +35,7 @@ static struct virtio_transport virtio_transport; /* forward
> declaration */
> struct virtio_vsock {
> struct virtio_device *vdev;
> struct virtqueue *vqs[VSOCK_VQ_MAX];
> + bool has_dgram;
>
> /* Virtqueue processing is deferred to a workqueue */
> struct work_struct tx_work;
> @@ -709,7 +710,6 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
> }
>
> vsock->vdev = vdev;
> -
> vsock->rx_buf_nr = 0;
> vsock->rx_buf_max_nr = 0;
> atomic_set(&vsock->queued_replies, 0);
> @@ -726,6 +726,9 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
> if (virtio_has_feature(vdev, VIRTIO_VSOCK_F_SEQPACKET))
> vsock->seqpacket_allow = true;
>
> + if (virtio_has_feature(vdev, VIRTIO_VSOCK_F_DGRAM))
> + vsock->has_dgram = true;
> +
> vdev->priv = vsock;
>
> ret = virtio_vsock_vqs_init(vsock);
> @@ -820,7 +823,8 @@ static struct virtio_device_id id_table[] = {
> };
>
> static unsigned int features[] = {
> - VIRTIO_VSOCK_F_SEQPACKET
> + VIRTIO_VSOCK_F_SEQPACKET,
> + VIRTIO_VSOCK_F_DGRAM
> };
>
> static struct virtio_driver virtio_vsock_driver = {
> --
> 2.35.1
>
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH 3/6] vsock: add netdev to vhost/virtio vsock
CC'ing virtio-dev@lists.oasis-open.org
On Mon, Aug 15, 2022 at 10:56:06AM -0700, Bobby Eshleman wrote:
> In order to support usage of qdisc on vsock traffic, this commit
> introduces a struct net_device to vhost and virtio vsock.
>
> Two new devices are created, vhost-vsock for vhost and virtio-vsock
> for virtio. The devices are attached to the respective transports.
>
> To bypass the usage of the device, the user may "down" the associated
> network interface using common tools. For example, "ip link set dev
> virtio-vsock down" lets vsock bypass the net_device and qdisc entirely,
> simply using the FIFO logic of the prior implementation.
>
> For both hosts and guests, there is one device for all G2H vsock sockets
> and one device for all H2G vsock sockets. This makes sense for guests
> because the driver only supports a single vsock channel (one pair of
> TX/RX virtqueues), so one device and qdisc fits. For hosts, this may not
> seem ideal for some workloads. However, it is possible to use a
> multi-queue qdisc, where a given queue is responsible for a range of
> sockets. This seems to be a better solution than having one device per
> socket, which may yield a very large number of devices and qdiscs, all
> of which are dynamically being created and destroyed. Because of this
> dynamism, it would also require a complex policy management daemon, as
> devices would constantly be spun up and down as sockets were created and
> destroyed. To avoid this, one device and qdisc also applies to all H2G
> sockets.
>
> Signed-off-by: Bobby Eshleman
> ---
> drivers/vhost/vsock.c | 19 +++-
> include/linux/virtio_vsock.h| 10 +++
> net/vmw_vsock/virtio_transport.c| 19 +++-
> net/vmw_vsock/virtio_transport_common.c | 112 +++-
> 4 files changed, 152 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index f8601d93d94d..b20ddec2664b 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -927,13 +927,30 @@ static int __init vhost_vsock_init(void)
> VSOCK_TRANSPORT_F_H2G);
> if (ret < 0)
> return ret;
> - return misc_register(&vhost_vsock_misc);
> +
> + ret = virtio_transport_init(&vhost_transport, "vhost-vsock");
> + if (ret < 0)
> + goto out_unregister;
> +
> + ret = misc_register(&vhost_vsock_misc);
> + if (ret < 0)
> + goto out_transport_exit;
> + return ret;
> +
> +out_transport_exit:
> + virtio_transport_exit(&vhost_transport);
> +
> +out_unregister:
> + vsock_core_unregister(&vhost_transport.transport);
> + return ret;
> +
> };
>
> static void __exit vhost_vsock_exit(void)
> {
> misc_deregister(&vhost_vsock_misc);
> vsock_core_unregister(&vhost_transport.transport);
> + virtio_transport_exit(&vhost_transport);
> };
>
> module_init(vhost_vsock_init);
> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> index 9a37eddbb87a..5d7e7fbd75f8 100644
> --- a/include/linux/virtio_vsock.h
> +++ b/include/linux/virtio_vsock.h
> @@ -91,10 +91,20 @@ struct virtio_transport {
> /* This must be the first field */
> struct vsock_transport transport;
>
> + /* Used almost exclusively for qdisc */
> + struct net_device *dev;
> +
> /* Takes ownership of the packet */
> int (*send_pkt)(struct sk_buff *skb);
> };
>
> +int
> +virtio_transport_init(struct virtio_transport *t,
> + const char *name);
> +
> +void
> +virtio_transport_exit(struct virtio_transport *t);
> +
> ssize_t
> virtio_transport_stream_dequeue(struct vsock_sock *vsk,
> struct msghdr *msg,
> diff --git a/net/vmw_vsock/virtio_transport.c
> b/net/vmw_vsock/virtio_transport.c
> index 3bb293fd8607..c6212eb38d3c 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
> @@ -131,7 +131,9 @@ virtio_transport_send_pkt_work(struct work_struct *work)
>* the vq
>*/
> if (ret < 0) {
> - skb_queue_head(&vsock->send_pkt_queue, skb);
> + spin_lock_bh(&vsock->send_pkt_queue.lock);
> + __skb_queue_head(&vsock->send_pkt_queue, skb);
> + spin_unlock_bh(&vsock->send_pkt_queue.lock);
> break;
> }
>
> @@ -676,7 +678,9 @@ static void virtio_vsock_vqs_del(struct virtio_vsock
> *vsock)
>
[virtio-dev] Re: [PATCH 2/6] vsock: return errors other than -ENOMEM to socket
CC'ing virtio-dev@lists.oasis-open.org
On Mon, Aug 15, 2022 at 10:56:05AM -0700, Bobby Eshleman wrote:
> This commit allows vsock implementations to return errors
> to the socket layer other than -ENOMEM. One immediate effect
> of this is that upon the sk_sndbuf threshold being reached -EAGAIN
> will be returned and userspace may throttle appropriately.
>
> Resultingly, a known issue with uperf is resolved[1].
>
> Additionally, to preserve legacy behavior for non-virtio
> implementations, hyperv/vmci force errors to be -ENOMEM so that behavior
> is unchanged.
>
> [1]: https://gitlab.com/vsock/vsock/-/issues/1
>
> Signed-off-by: Bobby Eshleman
> ---
> include/linux/virtio_vsock.h| 3 +++
> net/vmw_vsock/af_vsock.c| 3 ++-
> net/vmw_vsock/hyperv_transport.c| 2 +-
> net/vmw_vsock/virtio_transport_common.c | 3 ---
> net/vmw_vsock/vmci_transport.c | 9 -
> 5 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> index 17ed01466875..9a37eddbb87a 100644
> --- a/include/linux/virtio_vsock.h
> +++ b/include/linux/virtio_vsock.h
> @@ -8,6 +8,9 @@
> #include
> #include
>
> +/* Threshold for detecting small packets to copy */
> +#define GOOD_COPY_LEN 128
> +
> enum virtio_vsock_metadata_flags {
> VIRTIO_VSOCK_METADATA_FLAGS_REPLY = BIT(0),
> VIRTIO_VSOCK_METADATA_FLAGS_TAP_DELIVERED = BIT(1),
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index e348b2d09eac..1893f8aafa48 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -1844,8 +1844,9 @@ static int vsock_connectible_sendmsg(struct socket
> *sock, struct msghdr *msg,
> written = transport->stream_enqueue(vsk,
> msg, len - total_written);
> }
> +
> if (written < 0) {
> - err = -ENOMEM;
> + err = written;
> goto out_err;
> }
>
> diff --git a/net/vmw_vsock/hyperv_transport.c
> b/net/vmw_vsock/hyperv_transport.c
> index fd98229e3db3..e99aea571f6f 100644
> --- a/net/vmw_vsock/hyperv_transport.c
> +++ b/net/vmw_vsock/hyperv_transport.c
> @@ -687,7 +687,7 @@ static ssize_t hvs_stream_enqueue(struct vsock_sock *vsk,
> struct msghdr *msg,
> if (bytes_written)
> ret = bytes_written;
> kfree(send_buf);
> - return ret;
> + return ret < 0 ? -ENOMEM : ret;
> }
>
> static s64 hvs_stream_has_data(struct vsock_sock *vsk)
> diff --git a/net/vmw_vsock/virtio_transport_common.c
> b/net/vmw_vsock/virtio_transport_common.c
> index 920578597bb9..d5780599fe93 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -23,9 +23,6 @@
> /* How long to wait for graceful shutdown of a connection */
> #define VSOCK_CLOSE_TIMEOUT (8 * HZ)
>
> -/* Threshold for detecting small packets to copy */
> -#define GOOD_COPY_LEN 128
> -
> static const struct virtio_transport *
> virtio_transport_get_ops(struct vsock_sock *vsk)
> {
> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> index b14f0ed7427b..c927a90dc859 100644
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
> @@ -1838,7 +1838,14 @@ static ssize_t vmci_transport_stream_enqueue(
> struct msghdr *msg,
> size_t len)
> {
> - return vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
> + int err;
> +
> + err = vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
> +
> + if (err < 0)
> + err = -ENOMEM;
> +
> + return err;
> }
>
> static s64 vmci_transport_stream_has_data(struct vsock_sock *vsk)
> --
> 2.35.1
>
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH 1/6] vsock: replace virtio_vsock_pkt with sk_buff
CC'ing virtio-dev@lists.oasis-open.org
On Mon, Aug 15, 2022 at 10:56:04AM -0700, Bobby Eshleman wrote:
> This patch replaces virtio_vsock_pkt with sk_buff.
>
> The benefit of this series includes:
>
> * The bug reported @ https://bugzilla.redhat.com/show_bug.cgi?id=2009935
> does not present itself when reasonable sk_sndbuf thresholds are set.
> * Using sock_alloc_send_skb() teaches VSOCK to respect
> sk_sndbuf for tunability.
> * Eliminates copying for vsock_deliver_tap().
> * sk_buff is required for future improvements, such as using socket map.
>
> Signed-off-by: Bobby Eshleman
> ---
> drivers/vhost/vsock.c | 214 +--
> include/linux/virtio_vsock.h| 60 ++-
> net/vmw_vsock/af_vsock.c| 1 +
> net/vmw_vsock/virtio_transport.c| 212 +-
> net/vmw_vsock/virtio_transport_common.c | 491
> net/vmw_vsock/vsock_loopback.c | 51 +--
> 6 files changed, 517 insertions(+), 512 deletions(-)
>
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index 368330417bde..f8601d93d94d 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -51,8 +51,7 @@ struct vhost_vsock {
> struct hlist_node hash;
>
> struct vhost_work send_pkt_work;
> - spinlock_t send_pkt_list_lock;
> - struct list_head send_pkt_list; /* host->guest pending packets */
> + struct sk_buff_head send_pkt_queue; /* host->guest pending packets */
>
> atomic_t queued_replies;
>
> @@ -108,7 +107,8 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
> vhost_disable_notify(&vsock->dev, vq);
>
> do {
> - struct virtio_vsock_pkt *pkt;
> + struct sk_buff *skb;
> + struct virtio_vsock_hdr *hdr;
> struct iov_iter iov_iter;
> unsigned out, in;
> size_t nbytes;
> @@ -116,31 +116,22 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
> int head;
> u32 flags_to_restore = 0;
>
> - spin_lock_bh(&vsock->send_pkt_list_lock);
> - if (list_empty(&vsock->send_pkt_list)) {
> - spin_unlock_bh(&vsock->send_pkt_list_lock);
> + skb = skb_dequeue(&vsock->send_pkt_queue);
> +
> + if (!skb) {
> vhost_enable_notify(&vsock->dev, vq);
> break;
> }
>
> - pkt = list_first_entry(&vsock->send_pkt_list,
> -struct virtio_vsock_pkt, list);
> - list_del_init(&pkt->list);
> - spin_unlock_bh(&vsock->send_pkt_list_lock);
> -
> head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
>&out, &in, NULL, NULL);
> if (head < 0) {
> - spin_lock_bh(&vsock->send_pkt_list_lock);
> - list_add(&pkt->list, &vsock->send_pkt_list);
> - spin_unlock_bh(&vsock->send_pkt_list_lock);
> + skb_queue_head(&vsock->send_pkt_queue, skb);
> break;
> }
>
> if (head == vq->num) {
> - spin_lock_bh(&vsock->send_pkt_list_lock);
> - list_add(&pkt->list, &vsock->send_pkt_list);
> - spin_unlock_bh(&vsock->send_pkt_list_lock);
> + skb_queue_head(&vsock->send_pkt_queue, skb);
>
> /* We cannot finish yet if more buffers snuck in while
>* re-enabling notify.
> @@ -153,26 +144,27 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
> }
>
> if (out) {
> - virtio_transport_free_pkt(pkt);
> + kfree_skb(skb);
> vq_err(vq, "Expected 0 output buffers, got %u\n", out);
> break;
> }
>
> iov_len = iov_length(&vq->iov[out], in);
> - if (iov_len < sizeof(pkt->hdr)) {
> - virtio_transport_free_pkt(pkt);
> + if (iov_len < sizeof(*hdr)) {
> + kfree_skb(skb);
> vq_err(vq, "Buffer len [%zu] too small\n", iov_len);
> break;
> }
>
> iov_iter_init(&iov_iter, READ, &vq->iov[out], in, iov_len);
> - payload_len = pkt->len -
[virtio-dev] Re: [PATCH 0/6] virtio/vsock: introduce dgrams, sk_buff, and qdisc
CC'ing virtio-dev@lists.oasis-open.org On Mon, Aug 15, 2022 at 10:56:03AM -0700, Bobby Eshleman wrote: > Hey everybody, > > This series introduces datagrams, packet scheduling, and sk_buff usage > to virtio vsock. > > The usage of struct sk_buff benefits users by a) preparing vsock to use > other related systems that require sk_buff, such as sockmap and qdisc, > b) supporting basic congestion control via sock_alloc_send_skb, and c) > reducing copying when delivering packets to TAP. > > The socket layer no longer forces errors to be -ENOMEM, as typically > userspace expects -EAGAIN when the sk_sndbuf threshold is reached and > messages are being sent with option MSG_DONTWAIT. > > The datagram work is based off previous patches by Jiang Wang[1]. > > The introduction of datagrams creates a transport layer fairness issue > where datagrams may freely starve streams of queue access. This happens > because, unlike streams, datagrams lack the transactions necessary for > calculating credits and throttling. > > Previous proposals introduce changes to the spec to add an additional > virtqueue pair for datagrams[1]. Although this solution works, using > Linux's qdisc for packet scheduling leverages already existing systems, > avoids the need to change the virtio specification, and gives additional > capabilities. The usage of SFQ or fq_codel, for example, may solve the > transport layer starvation problem. It is easy to imagine other use > cases as well. For example, services of varying importance may be > assigned different priorities, and qdisc will apply appropriate > priority-based scheduling. By default, the system default pfifo qdisc is > used. The qdisc may be bypassed and legacy queuing is resumed by simply > setting the virtio-vsock%d network device to state DOWN. This technique > still allows vsock to work with zero-configuration. > > In summary, this series introduces these major changes to vsock: > > - virtio vsock supports datagrams > - virtio vsock uses struct sk_buff instead of virtio_vsock_pkt > - Because virtio vsock uses sk_buff, it also uses sock_alloc_send_skb, > which applies the throttling threshold sk_sndbuf. > - The vsock socket layer supports returning errors other than -ENOMEM. > - This is used to return -EAGAIN when the sk_sndbuf threshold is > reached. > - virtio vsock uses a net_device, through which qdisc may be used. > - qdisc allows scheduling policies to be applied to vsock flows. > - Some qdiscs, like SFQ, may allow vsock to avoid transport layer > congestion. That is, > it may avoid datagrams from flooding out stream flows. The benefit > to this is that additional virtqueues are not needed for datagrams. > - The net_device and qdisc is bypassed by simply setting the > net_device state to DOWN. > > [1]: > https://lore.kernel.org/all/20210914055440.3121004-1-jiang.w...@bytedance.com/ > > Bobby Eshleman (5): > vsock: replace virtio_vsock_pkt with sk_buff > vsock: return errors other than -ENOMEM to socket > vsock: add netdev to vhost/virtio vsock > virtio/vsock: add VIRTIO_VSOCK_F_DGRAM feature bit > virtio/vsock: add support for dgram > > Jiang Wang (1): > vsock_test: add tests for vsock dgram > > drivers/vhost/vsock.c | 238 > include/linux/virtio_vsock.h| 73 ++- > include/net/af_vsock.h | 2 + > include/uapi/linux/virtio_vsock.h | 2 + > net/vmw_vsock/af_vsock.c| 30 +- > net/vmw_vsock/hyperv_transport.c| 2 +- > net/vmw_vsock/virtio_transport.c| 237 +--- > net/vmw_vsock/virtio_transport_common.c | 771 > net/vmw_vsock/vmci_transport.c | 9 +- > net/vmw_vsock/vsock_loopback.c | 51 +- > tools/testing/vsock/util.c | 105 > tools/testing/vsock/util.h | 4 + > tools/testing/vsock/vsock_test.c| 195 ++ > 13 files changed, 1176 insertions(+), 543 deletions(-) > > -- > 2.35.1 > - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
