All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@redhat.com>
To: "Jiang Wang ." <jiang.wang@bytedance.com>
Cc: cong.wang@bytedance.com,
	Xiongchun Duan <duanxiongchun@bytedance.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	cohuck@redhat.com, virtualization@lists.linux-foundation.org,
	xieyongji@bytedance.com,
	Arseny Krasnov <arseny.krasnov@kaspersky.com>,
	asias@redhat.com
Subject: Re: [External] Re: [RFC v2] virtio-vsock: add description for datagram type
Date: Thu, 13 May 2021 15:57:52 +0100	[thread overview]
Message-ID: <YJ0+cClrEyamHt+y@stefanha-x1.localdomain> (raw)
In-Reply-To: <CAP_N_Z-OMqQtnV04Wpynr7GhZooz1iQQ+0To-P8xPEnA0+sZgQ@mail.gmail.com>


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

On Mon, Apr 12, 2021 at 03:39:36PM -0700, Jiang Wang . wrote:
> On Mon, Apr 12, 2021 at 6:50 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > On Thu, Apr 01, 2021 at 04:36:02AM +0000, jiang.wang wrote:
> > > +For the rx side, dgram also uses the \field{buf_alloc}. If it is full, the packet
> > > +is dropped by the receiver.
> >
> > UDP is connectionless so any number of other sources can send messages
> > to the same destination, causing buf_alloc's value to be unpredictable.
> > Can you explain how buf_alloc works with datagram sockets in more
> > detail?
> 
> In the linux kernel in my prototype, datagram sockets also use
> virtio_transport_inc_rx_pkt() and virtio_transport_dec_rx_pkt() to update
> vvs->rx_bytes and compare it with vvs->buf_alloc. virtio_transport_inc_rx_pkt
> is called when enqueuing the datagram packets.
> virtio_transport_dec_rx_pkt is called when dequeuing those packets.
> 
> If multiple sources send messages to the same destination, they will share
> the same buf_alloc. Do you want something with more control?
> Maybe somehow allocate a buffer for each remote CID and port? But I
> feel that is a little bit overkill. Any other suggestions?

The opposite, I want less control :). It's not possible to track buffer
space accurately with connectionless sockets, so let's not try.

A real example is the iperf3 networking benchmark where you need to set
target bitrate for UDP sockets because there is no flow control (buffer
space) mechanism in UDP. That's how UDP applications work and for
similar reasons I don't think it's possible to achieve flow control with
vsock's buf_alloc.

> > >  \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 sockets, VIRTIO_VSOCK_OP_RW data packets MUST only be transmitted when the peer has
> > > +sufficient free buffer space for the payload. For dgram sockets, VIRTIO_VSOCK_OP_RW data packets
> > > +MAY be transmitted when the peer buffer is full. Then the packet will be dropped by the receiver.
> > >
> > >  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 sockets, VIRTIO_VSOCK_OP_RW data packets MUST only be transmitted when the peer has
> > > +sufficient free buffer space for the payload. For dgram sockets, VIRTIO_VSOCK_OP_RW data packets
> > > +MAY be transmitted when the peer buffer is full. Then the packet will be dropped by the receiver.
> > >
> > >  All packets associated with a stream flow MUST contain valid information in
> > >  \field{buf_alloc} and \field{fwd_cnt} fields.
> > > @@ -203,14 +234,14 @@ \subsubsection{Receive and Transmit}\label{sec:Device Types / Socket Device / De
> > >  The \field{guest_cid} configuration field MUST be used as the source CID when
> > >  sending outgoing packets.
> > >
> > > -A VIRTIO_VSOCK_OP_RST reply MUST be sent if a packet is received with an
> > > +For stream sockets, A VIRTIO_VSOCK_OP_RST reply MUST be sent if a packet is received with an
> > >  unknown \field{type} value.
> >
> > What about datagram sockets? Please state what must happen and why.
> 
> I think datagram sockets should do the same thing as the stream to
> return a VIRTIO_VSOCK_OP_RST?
> Any other ideas?

Sounds good to me. I just wanted to check and request that you add that
to the spec.

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

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

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

      reply	other threads:[~2021-05-13 14:58 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-01  4:36 [RFC v2] virtio-vsock: add description for datagram type jiang.wang
2021-04-12 13:50 ` Stefan Hajnoczi
2021-04-12 14:21   ` Stefano Garzarella
2021-04-12 22:42     ` Jiang Wang .
2021-04-13 12:58       ` Stefano Garzarella
2021-04-13 13:16         ` Michael S. Tsirkin
2021-04-13 13:38           ` Stefano Garzarella
2021-04-13 13:50             ` Michael S. Tsirkin
2021-04-13 14:03               ` Stefano Garzarella
2021-04-13 19:58                 ` Michael S. Tsirkin
2021-04-13 22:00                   ` Jiang Wang .
2021-04-14  7:07                     ` Stefano Garzarella
2021-04-14  6:57                   ` Stefano Garzarella
2021-04-14  7:20                     ` Michael S. Tsirkin
2021-04-14  9:38                       ` Stefano Garzarella
2021-04-15  3:15                         ` Jiang Wang .
2021-05-04  3:40                           ` Jiang Wang .
2021-05-04 16:16                             ` Stefano Garzarella
2021-05-04 17:06                               ` Jiang Wang .
2021-05-05 10:49                                 ` Stefano Garzarella
2021-05-05 16:58                                   ` Jiang Wang .
2021-05-07 16:53                                     ` Jiang Wang .
2021-05-10 14:50                                       ` Stefano Garzarella
2021-05-13 23:26                                         ` Jiang Wang .
2021-05-14 15:17                                           ` Stefano Garzarella
2021-05-14 18:55                                             ` Jiang Wang .
2021-05-17 11:02                                               ` Stefano Garzarella
2021-05-18  6:33                                                 ` Jiang Wang .
2021-05-18 13:02                                                   ` Stefano Garzarella
2021-05-19  4:59                                                     ` Jiang Wang .
2021-06-09  4:31                                                       ` Jiang Wang .
2021-06-09  7:40                                                         ` Stefano Garzarella
2021-04-12 22:39   ` [External] " Jiang Wang .
2021-05-13 14:57     ` Stefan Hajnoczi [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YJ0+cClrEyamHt+y@stefanha-x1.localdomain \
    --to=stefanha@redhat.com \
    --cc=arseny.krasnov@kaspersky.com \
    --cc=asias@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=cong.wang@bytedance.com \
    --cc=duanxiongchun@bytedance.com \
    --cc=jiang.wang@bytedance.com \
    --cc=mst@redhat.com \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=xieyongji@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.