Re: [RFC v2] virtio-vsock: add description for datagram type

[Stefano Garzarella <sgarzare@redhat.com>]

On Tue, Jun 08, 2021 at 09:31:28PM -0700, Jiang Wang . wrote:
>On Tue, May 18, 2021 at 9:59 PM Jiang Wang . <jiang.wang@bytedance.com> wrote:
>>
>> On Tue, May 18, 2021 at 6:02 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>> >
>> > On Mon, May 17, 2021 at 11:33:06PM -0700, Jiang Wang . wrote:
>> > >On Mon, May 17, 2021 at 4:02 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>> > >>
>> > >> On Fri, May 14, 2021 at 11:55:29AM -0700, Jiang Wang . wrote:
>> > >> >On Fri, May 14, 2021 at 8:17 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>> > >> >> On Thu, May 13, 2021 at 04:26:03PM -0700, Jiang Wang . wrote:
>> > >>
>> > >> [...]
>> > >>
>> > >> >> >I see. I will add some limit to dgram packets. Also, when the
>> > >> >> >virtqueues
>> > >> >> >are shared between stream and dgram, both of them need to grab a lock
>> > >> >> >before using the virtqueue, so one will not completely block another one.
>> > >> >>
>> > >> >> I'm not worried about the concurrent access that we definitely need to
>> > >> >> handle with a lock, but more about the uncontrolled packet sending that
>> > >> >> dgram might have, flooding the queues and preventing others from
>> > >> >> communicating.
>> > >> >
>> > >> >That is a valid concern. Let me explain how I would handle that if we
>> > >> >don't add two new virtqueues. For dgram, I also add a dgram_send_pkt_list,
>> > >> >which is similar to send_pkt_list for stream (and seqpacket). But there
>> > >> >is one difference. The dgram_send_pkt_list has a maximum size setting,
>> > >> >and keep tracking how many pkts are in the list. The track number
>> > >> >(dgram_send_pkt_list_size) is  increased when a packet is added
>> > >> >to the list and is decreased when a packet
>> > >> >is removed from the list and added to the virtqueue. In
>> > >> >virtio_transport_send_pkt, if the current
>> > >> >dgram_send_pkt_list_size is equal
>> > >> >to the maximum ( let's say 128), then it will not add to the
>> > >> >dgram_send_pkt_list and return an error to the application.
>> > >>
>> > >> For stream socket, we have the send_pkt_list and the send worker because
>> > >> the virtqueue can be full and the transmitter needs to wait available
>> > >> slots, because we can't discard packets.
>> > >> For dgram I think we don't need this, so we can avoid the
>> > >> dgram_send_pkt_list and directly enqueue packets in the virtqueue.
>> > >>
>
>For the question of whether we need dgram_send_pkt_list, I tried to remove
>it and that has no problem with virtio vsock in the guest. But on the host, we
>still need to keep the dgram_send_pkt_list. The reason is that we cannot
>access virtqueue memory reliably in the syscall handling of an
>arbitrary process.
>The virtqueue memory is in the QEMU process virtual memory and may be
>paged out.

I see, I think in that case we can use the virtqueue size as limit for 
the dgram_send_pkt_list.

I mean for example if the virtqueue has 128 elements, we can queue at 
least 128 packets in the dgram_send_pkt_list.

If you have a better idea go ahead, we can discuss this implementation 
detail in the RFC linux series :-)

Thanks,
Stefano

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