Message-ID: <CAP_N_Z9yi96YDW3gJdCFrPJpNhwpJnaT8gruk7JJSsBne8J-8Q@mail.gmail.com>
Date:   Mon, 12 Apr 2021 11:53:15 -0700
From:   "Jiang Wang ." <jiang.wang@...edance.com>
To:     Stefano Garzarella <sgarzare@...hat.com>
Cc:     Jorgen Hansen <jhansen@...are.com>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        Stefan Hajnoczi <stefanha@...hat.com>,
        "cong.wang@...edance.com" <cong.wang@...edance.com>,
        "duanxiongchun@...edance.com" <duanxiongchun@...edance.com>,
        "xieyongji@...edance.com" <xieyongji@...edance.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Colin Ian King <colin.king@...onical.com>,
        Norbert Slusarek <nslusarek@....net>,
        Andra Paraschiv <andraprs@...zon.com>,
        Alexander Popov <alex.popov@...ux.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: Re: [RFC] vsock: add multiple transports support for dgram

On Mon, Apr 12, 2021 at 7:04 AM Stefano Garzarella <sgarzare@...hat.com> wrote:
>
> Hi Jiang,
> thanks for re-starting the multi-transport support for dgram!

No problem.

> On Wed, Apr 07, 2021 at 11:25:36AM -0700, Jiang Wang . wrote:
> >On Wed, Apr 7, 2021 at 2:51 AM Jorgen Hansen <jhansen@...are.com> wrote:
> >>
> >>
> >> > On 6 Apr 2021, at 20:31, Jiang Wang <jiang.wang@...edance.com> wrote:
> >> >
> >> > From: "jiang.wang" <jiang.wang@...edance.com>
> >> >
> >> > Currently, only VMCI supports dgram sockets. To supported
> >> > nested VM use case, this patch removes transport_dgram and
> >> > uses transport_g2h and transport_h2g for dgram too.
>
> I agree on this part, I think that's the direction to go.
> transport_dgram was added as a shortcut.

Got it.

> >>
> >> Could you provide some background for introducing this change - are you
> >> looking at introducing datagrams for a different transport? VMCI datagrams
> >> already support the nested use case,
> >
> >Yes, I am trying to introduce datagram for virtio transport. I wrote a
> >spec patch for
> >virtio dgram support and also a code patch, but the code patch is still WIP.
> >When I wrote this commit message, I was thinking nested VM is the same as
> >multiple transport support. But now, I realize they are different.
> >Nested VMs may use
> >the same virtualization layer(KVM on KVM), or different virtualization layers
> >(KVM on ESXi). Thanks for letting me know that VMCI already supported nested
> >use cases. I think you mean VMCI on VMCI, right?
> >
> >> but if we need to support multiple datagram
> >> transports we need to rework how we administer port assignment for datagrams.
> >> One specific issue is that the vmci transport won’t receive any datagrams for a
> >> port unless the datagram socket has already been assigned the vmci transport
> >> and the port bound to the underlying VMCI device (see below for more details).
> >>
> >I see.
> >
> >> > The transport is assgined when sending every packet and
> >> > receiving every packet on dgram sockets.
> >>
> >> Is the intent that the same datagram socket can be used for sending packets both
> >> In the host to guest, and the guest to directions?
> >
> >Nope. One datagram socket will only send packets to one direction, either to the
> >host or to the guest. My above description is wrong. When sending packets, the
> >transport is assigned with the first packet (with auto_bind).
>
> I'm not sure this is right.
> The auto_bind on the first packet should only assign a local port to the
> socket, but does not affect the transport to be used.
>
> A user could send one packet to the nested guest and another to the host
> using the same socket, or am I wrong?

OK. I think you are right.

> >
> >The problem is when receiving packets. The listener can bind to the
> >VMADDR_CID_ANY
> >address. Then it is unclear which transport we should use. For stream
> >sockets, there will be a new socket for each connection, and transport
> >can be decided
> >at that time. For datagram sockets, I am not sure how to handle that.
>
> yes, this I think is the main problem, but maybe the sender one is even
> more complicated.
>
> Maybe we should remove the 1:1 association we have now between vsk and
> transport.

Yes, I thought about that too. One idea is to define two transports in vsk.
For sending pkt, we can pick the right transport when we get the packet, like
in virtio_transport_send_pkt_info(). For receiving pkts, we have to check
and call both
transports dequeue callbacks if the local cid is CID_ANY.

> At least for DGRAM, for connected sockets I think the association makes
> sense.

Yeah. For a connected socket, we will only use one transport.

> >For VMCI, does the same transport can be used for both receiving from
> >host and from
> >the guest?
>
> Yes, they're registered at different times, but it's the same transport.
>
> >
> >For virtio, the h2g and g2h transports are different,, so we have to
> >choose
> >one of them. My original thought is to wait until the first packet
> >arrives.
> >
> >Another idea is that we always bind to host addr and use h2g
> >transport because I think that might
> >be more common. If a listener wants to recv packets from the host, then
> >it
> >should bind to the guest addr instead of CID_ANY.
>
> Yes, I remember we discussed this idea, this would simplify the
> receiving, but there is still the issue of a user wanting to receive
> packets from both the nested guest and the host.

OK. Agree.

> >Any other suggestions?
> >
>
> I think one solution could be to remove the 1:1 association between
> DGRAM socket and transport.
>
> IIUC VMCI creates a skb for each received packet and queues it through
> sk_receive_skb() directly in the struct sock.
>
> Then the .dgram_dequeue() callback dequeues them using
> skb_recv_datagram().
>
> We can move these parts in the vsock core, and create some helpers to
> allow the transports to enqueue received DGRAM packets in the same way
> (and with the same format) directly in the struct sock.
>

I agree to use skbs (and move them to vscok core). We have another use case
which will need to use skb. But I am not sure how this helps with multiple
transport cases. Each transport has a dgram_dequeue callback. So we still
need to let vsk have multiple transports somehow. Could you elaborate how
using skb help with multiple transport support? Will that be similar to what I
mentioned above? Thanks.

Regards,

Jiang

> What do you think?
>
> Thanks,
> Stefano
>

Powered by blists - more mailing lists