From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_RED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 26954C433ED for ; Tue, 18 May 2021 06:33:25 +0000 (UTC) Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 961C361261 for ; Tue, 18 May 2021 06:33:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 961C361261 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=bytedance.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=virtualization-bounces@lists.linux-foundation.org Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 4DAC2405C9; Tue, 18 May 2021 06:33:24 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id droZBpyWyMU5; Tue, 18 May 2021 06:33:23 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp4.osuosl.org (Postfix) with ESMTP id D24BF40594; Tue, 18 May 2021 06:33:22 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 9C4EAC000E; Tue, 18 May 2021 06:33:22 +0000 (UTC) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0BC82C0001 for ; Tue, 18 May 2021 06:33:21 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id BC40B83B33 for ; Tue, 18 May 2021 06:33:20 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp1.osuosl.org (amavisd-new); dkim=pass (2048-bit key) header.d=bytedance-com.20150623.gappssmtp.com Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id i6453TZweNQM for ; Tue, 18 May 2021 06:33:18 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.8.0 Received: from mail-oi1-x22d.google.com (mail-oi1-x22d.google.com [IPv6:2607:f8b0:4864:20::22d]) by smtp1.osuosl.org (Postfix) with ESMTPS id 91FEF83955 for ; Tue, 18 May 2021 06:33:18 +0000 (UTC) Received: by mail-oi1-x22d.google.com with SMTP id z3so8789009oib.5 for ; Mon, 17 May 2021 23:33:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=I/nOqogCBKj8Jo7Rsll9I66GD7iEvIt5ciCccCoZlEs=; b=efso70W53jkPpp58wUOr49Ntk4YT0Tdc8wXwaFC1cR7+cc0IFUuQkV6CbuSUcoxVMG 9CzgwIhjfiiFDfYRZNgqBRyHNFfZ3THqgEPS4PsK+k2FZCOapxeStrQV3Y7PqKsbcaXt 3X3uGqISsLdwjxItwKZHSJ4J85lcy+kW6feqTlSZD3No5J4orNDS+AXA0Ce2IFRF+Bip Q4Ip7mZ0TipDErgmLGeVDxctDJdSi1tyH/BgfpaMLuxsnkV5X+8oQimDusXXQ0jf06j9 KxT42ndCUuFTDGAQta6a4qjS5F/E8aoVGgq5GYlhLwbklK4ZpLO1bS6BQE48NzFiOnTC EHHw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=I/nOqogCBKj8Jo7Rsll9I66GD7iEvIt5ciCccCoZlEs=; b=BAKzaqpPqqgApmFp676j+n6bp/nYRgd2rMrL7TRqlgOKhVmOwDD1evHX1vqnLvh9lQ EXFW5WuwfsolVWgMKlpqafpmihe+BMibqqCF8LqbpkQfv7bbhYqTPyEsK++4fGO5JxoW 6Etu8DlVHcFyDacl/m3i0y0VBZduofREl68Ce3yQAQ9zVNrmJhqzPFyAmHtcXj47q82X IFG0UB4KrneN2R4jWDRZRlFDVyn/G+Ed6fkcymR259ZSqUdPcEsYyf6ZQYLv5JD5+0dY YCDUSha6hb0cLUvyolamBZjPYkTTwQcGW+Ybj6jMBm5VmAGOc+ParxcGupk4i08mfNKk rVyg== X-Gm-Message-State: AOAM5309X+xahHyuS4ytXd2ktXzv7Kb1yIqzoaL5yDpg0A9+ZP+xO5UR 7epJMdQcdyL+WMtywv7d1n3Puqy5W6fXItCq+wVVpw== X-Google-Smtp-Source: ABdhPJyXuN6Re7o2UA119O3TaXWxZasTalN/bhhBmv6HsX0gOMzmDNGBw2/q8yqAkZU4yOgXyl+7l4nc4N1/K3xiTYA= X-Received: by 2002:aca:d417:: with SMTP id l23mr2805816oig.97.1621319597357; Mon, 17 May 2021 23:33:17 -0700 (PDT) MIME-Version: 1.0 References: <20210504161651.3b6fhi64d7g3jui4@steredhat> <20210505104933.wgdn4gw56kle2mec@steredhat> <20210510145055.y7mxqaq4zggajz5a@steredhat> <20210514151701.6fp27qanjseom4tl@steredhat> <20210517110208.lippuk4rv57cn6hj@steredhat> In-Reply-To: <20210517110208.lippuk4rv57cn6hj@steredhat> From: "Jiang Wang ." Date: Mon, 17 May 2021 23:33:06 -0700 Message-ID: Subject: Re: Re: [RFC v2] virtio-vsock: add description for datagram type To: Stefano Garzarella Cc: cong.wang@bytedance.com, Xiongchun Duan , "Michael S. Tsirkin" , cohuck@redhat.com, virtualization@lists.linux-foundation.org, xieyongji@bytedance.com, Stefan Hajnoczi , Arseny Krasnov X-BeenThere: virtualization@lists.linux-foundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Linux virtualization List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: virtualization-bounces@lists.linux-foundation.org Sender: "Virtualization" On Mon, May 17, 2021 at 4:02 AM Stefano Garzarella 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 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. > > If there are no slots available, we can simply drop the packet. > In this way we don't have to put limits other than the available space > in the virtqueue. I considered this approach before. One question not clear to me is whether we should call virtqueue_kick for every dgram packet. If I am not wrong, each virtqueue_kick will lead to a vm_exit to the host. If we call virtqueue_kick() for each dgram packet, the performance might be bad when there are lots of packets. One idea is to set a threshold and a timer to call virtqueue_kick(). When there are lots of packets, we wait until a threshold. When there are few packets, the timer will make sure those packets will be delivered not too late. Any other ideas for virtqueue_kick? If the threshold plus timer is fine, I can go this direction too. > > > >In this way, the number of pending dgram pkts to be sent is limited. > >Then both stream and dgram sockets will compete to hold a lock > >for the tx virtqueue. Depending on the linux scheduler, this competition > >will be somewhat fair. As a result, dgram will not block stream completely. > >It will compete and share the virtqueue with stream, but stream > >can still send some pkts. > > > >Basically, the virtqueue becomes a first-come first-serve resource for > >the stream and dgram. When both stream and dgram applications > >have lots of data to send, dgram_send_pkt_list and send_pkt_list > >will still be a limited size, and each will have some chance to send out > >the data via virtqueue. Does this address your concern? > > > > > >> So having 2 dedicated queues could avoid a credit mechanism at all for > >> connection-less sockets, and simply the receiver discards packets that > >> it can't handle. > > > >With 2 dedicated queues, we still need some kind of accounting for > >the dgram. Like the dgram_send_pkt_size I mentioned above. Otherwise, > >it will cause OOM. It is not a real credit mechanism, but code is similar > >with or without 2 dedicated queues in my current prototype. > > I think in both cases we don't need an accounting, but we can drop > packets if the virtqueue is full. > > It's still not clear to me where OOM may occur. Can you elaborate on > this? OOM depending on the implementation details. If we keep dgram_send_pkt_list, and put no limitation, it may increase indefinitely and cause OOM. As long as we have some limitations or drop packets quickly, then we should be fine. > > > >For receiver discarding packets part, could you explain more? I think > >receiver discarding pkts code also is same with or without 2 dedicated > >queues. > > Yep. > > > Both will use can_enqueue = virtio_transport_inc_rx_pkt(vvs, pkt); > >to check if a packet should be discarded or not. > > I don't think we can use virtio_transport_inc_rx_pkt() for dgram. > This is based on the credit mechanism (e.g. buf_alloc) that works when > you have a connection oriented socket. > > With dgram you can have multiple transmitter for a single socket, so how > we can exchange that information? > In my view, we don't need to exchange that information. Buf_alloc for dgram is local to a receiving socket. The sending sockets do not know about that. For receiving socket, it just checks if there is still buffer available to decide to drop a packet or not. If multiple transmitter send to a single socket, they will share the same buf_alloc, and packets will be dropped randomly for those transmitters. > If we reuse the VMCI implementation with skbuff, the sk_receive_skb() > already checks if the sk_rcvbuf is full and discards the packet in this > case, so we don't need an internal rx_queue. OK. > Maybe someday we should convert stream to skbuff too, it would make our > lives easier. > Yeah. I remember the original virtio vsock patch did use skb, but somehow it did not use skb anymore when merging to the upstream, not sure why. Thanks, Jiang > Thanks, > Stefano > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization