All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Wang <jasowang@redhat.com>
To: David Woodhouse <dwmw2@infradead.org>, netdev@vger.kernel.org
Cc: "Eugenio Pérez" <eperezma@redhat.com>
Subject: Re: [PATCH v2 4/4] vhost_net: Add self test with tun device
Date: Thu, 24 Jun 2021 14:12:36 +0800	[thread overview]
Message-ID: <4a5c6e49-ee50-3c0c-c8e6-04d85137cfb1@redhat.com> (raw)
In-Reply-To: <d6ad85649fd56d3e12e59085836326a09885593b.camel@infradead.org>


在 2021/6/24 上午12:12, David Woodhouse 写道:
> On Wed, 2021-06-23 at 12:02 +0800, Jason Wang wrote:
>> 在 2021/6/23 上午12:15, David Woodhouse 写道:
>>> From: David Woodhouse <dwmw@amazon.co.uk>
>>>
>>> This creates a tun device and brings it up, then finds out the link-local
>>> address the kernel automatically assigns to it.
>>>
>>> It sends a ping to that address, from a fake LL address of its own, and
>>> then waits for a response.
>>>
>>> If the virtio_net_hdr stuff is all working correctly, it gets a response
>>> and manages to understand it.
>>
>> I wonder whether it worth to bother the dependency like ipv6 or kernel
>> networking stack.
>>
>> How about simply use packet socket that is bound to tun to receive and
>> send packets?
>>
> I pondered that but figured that using the kernel's network stack
> wasn't too much of an additional dependency. We *could* use an
> AF_PACKET socket on the tun device and then drive both ends, but given
> that the kernel *automatically* assigns a link-local address when we
> bring the device up anyway, it seemed simple enough just to use ICMP.
> I also happened to have the ICMP generation/checking code lying around
> anyway in the same emacs instance, so it was reduced to a previously
> solved problem.


Ok.


>
> We *should* eventually expand this test case to attach an AF_PACKET
> device to the vhost-net, instead of using a tun device as the back end.
> (Although I don't really see *why* vhost is limited to AF_PACKET. Why
> *can't* I attach anything else, like an AF_UNIX socket, to vhost-net?)


It's just because nobody wrote the code. And we're lacking the real use 
case.

Vhost_net is bascially used for accepting packet from userspace to the 
kernel networking stack.

Using AF_UNIX makes it looks more like a case of inter process 
communication (without vnet header it won't be used efficiently by VM). 
In this case, using io_uring is much more suitable.

Or thinking in another way, instead of depending on the vhost_net, we 
can expose TUN/TAP socket to userspace then io_uring could be used for 
the OpenConnect case as well?


>
>
>>> +	/*
>>> +	 * I just want to map the *whole* of userspace address space. But
>>> +	 * from userspace I don't know what that is. On x86_64 it would be:
>>> +	 *
>>> +	 * vmem->regions[0].guest_phys_addr = 4096;
>>> +	 * vmem->regions[0].memory_size = 0x7fffffffe000;
>>> +	 * vmem->regions[0].userspace_addr = 4096;
>>> +	 *
>>> +	 * For now, just ensure we put everything inside a single BSS region.
>>> +	 */
>>> +	vmem->regions[0].guest_phys_addr = (uint64_t)&rings;
>>> +	vmem->regions[0].userspace_addr = (uint64_t)&rings;
>>> +	vmem->regions[0].memory_size = sizeof(rings);
>>
>> Instead of doing tricks like this, we can do it in another way:
>>
>> 1) enable device IOTLB
>> 2) wait for the IOTLB miss request (iova, len) and update identity
>> mapping accordingly
>>
>> This should work for all the archs (with some performance hit).
> Ick. For my actual application (OpenConnect) I'm either going to suck
> it up and put in the arch-specific limits like in the comment above, or
> I'll fix things to do the VHOST_F_IDENTITY_MAPPING thing we're talking
> about elsewhere.


The feature could be useful for the case of vhost-vDPA as well.


>   (Probably the former, since if I'm requiring kernel
> changes then I have grander plans around extending AF_TLS to do DTLS,
> then hooking that directly up to the tun socket via BPF and a sockmap
> without the data frames ever going to userspace at all.)


Ok, I guess we need to make sockmap works for tun socket.


>
> For this test case, a hard-coded single address range in BSS is fine.
>
> I've now added !IFF_NO_PI support to the test case, but as noted it
> fails just like the other ones I'd already marked with #if 0, which is
> because vhost-net pulls some value for 'sock_hlen' out of its posterior
> based on some assumption around the vhost features. And then expects
> sock_recvmsg() to return precisely that number of bytes more than the
> value it peeks in the skb at the head of the sock's queue.
>
> I think I can fix *all* those test cases by making tun_get_socket()
> take an extra 'int *' argument, and use that to return the *actual*
> value of sock_hlen. Here's the updated test case in the meantime:


It would be better if you can post a new version of the whole series to 
ease the reviewing.

Thanks



  reply	other threads:[~2021-06-24  6:13 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-19 13:33 [PATCH] net: tun: fix tun_xdp_one() for IFF_TUN mode David Woodhouse
2021-06-21  7:00 ` Jason Wang
2021-06-21 10:52   ` David Woodhouse
2021-06-21 14:50     ` David Woodhouse
2021-06-21 20:43       ` David Woodhouse
2021-06-22  4:52         ` Jason Wang
2021-06-22  7:24           ` David Woodhouse
2021-06-22  7:51             ` Jason Wang
2021-06-22  8:10               ` David Woodhouse
2021-06-22 11:36               ` David Woodhouse
2021-06-22  4:34       ` Jason Wang
2021-06-22  4:34     ` Jason Wang
2021-06-22  7:28       ` David Woodhouse
2021-06-22  8:00         ` Jason Wang
2021-06-22  8:29           ` David Woodhouse
2021-06-23  3:39             ` Jason Wang
2021-06-24 12:39               ` David Woodhouse
2021-06-22 16:15 ` [PATCH v2 1/4] " David Woodhouse
2021-06-22 16:15   ` [PATCH v2 2/4] net: tun: don't assume IFF_VNET_HDR in tun_xdp_one() tx path David Woodhouse
2021-06-23  3:46     ` Jason Wang
2021-06-22 16:15   ` [PATCH v2 3/4] vhost_net: validate virtio_net_hdr only if it exists David Woodhouse
2021-06-23  3:48     ` Jason Wang
2021-06-22 16:15   ` [PATCH v2 4/4] vhost_net: Add self test with tun device David Woodhouse
2021-06-23  4:02     ` Jason Wang
2021-06-23 16:12       ` David Woodhouse
2021-06-24  6:12         ` Jason Wang [this message]
2021-06-24 10:42           ` David Woodhouse
2021-06-25  2:55             ` Jason Wang
2021-06-25  7:54               ` David Woodhouse
2021-06-23  3:45   ` [PATCH v2 1/4] net: tun: fix tun_xdp_one() for IFF_TUN mode Jason Wang
2021-06-23  8:30     ` David Woodhouse
2021-06-23 13:52     ` David Woodhouse
2021-06-23 17:31       ` David Woodhouse
2021-06-23 22:52         ` David Woodhouse
2021-06-24  6:37           ` Jason Wang
2021-06-24  7:23             ` David Woodhouse
2021-06-24  6:18       ` Jason Wang
2021-06-24  7:05         ` David Woodhouse
2021-06-24 12:30 ` [PATCH v3 1/5] net: add header len parameter to tun_get_socket(), tap_get_socket() David Woodhouse
2021-06-24 12:30   ` [PATCH v3 2/5] net: tun: don't assume IFF_VNET_HDR in tun_xdp_one() tx path David Woodhouse
2021-06-25  6:58     ` Jason Wang
2021-06-24 12:30   ` [PATCH v3 3/5] vhost_net: remove virtio_net_hdr validation, let tun/tap do it themselves David Woodhouse
2021-06-25  7:33     ` Jason Wang
2021-06-25  8:37       ` David Woodhouse
2021-06-28  4:23         ` Jason Wang
2021-06-28 11:23           ` David Woodhouse
2021-06-28 23:29             ` David Woodhouse
2021-06-29  3:43               ` Jason Wang
2021-06-29  6:59                 ` David Woodhouse
2021-06-29 10:49                 ` David Woodhouse
2021-06-29 13:15                   ` David Woodhouse
2021-06-30  4:39                   ` Jason Wang
2021-06-30 10:02                     ` David Woodhouse
2021-07-01  4:13                       ` Jason Wang
2021-07-01 17:39                         ` David Woodhouse
2021-07-02  3:13                           ` Jason Wang
2021-07-02  8:08                             ` David Woodhouse
2021-07-02  8:50                               ` Jason Wang
2021-07-09 15:04                               ` Eugenio Perez Martin
2021-06-29  3:21             ` Jason Wang
2021-06-24 12:30   ` [PATCH v3 4/5] net: tun: fix tun_xdp_one() for IFF_TUN mode David Woodhouse
2021-06-25  7:41     ` Jason Wang
2021-06-25  8:51       ` David Woodhouse
2021-06-28  4:27         ` Jason Wang
2021-06-28 10:43           ` David Woodhouse
2021-06-25 18:43     ` Willem de Bruijn
2021-06-25 19:00       ` David Woodhouse
2021-06-24 12:30   ` [PATCH v3 5/5] vhost_net: Add self test with tun device David Woodhouse
2021-06-25  5:00   ` [PATCH v3 1/5] net: add header len parameter to tun_get_socket(), tap_get_socket() Jason Wang
2021-06-25  8:23     ` David Woodhouse
2021-06-28  4:22       ` Jason Wang
2021-06-25 18:13   ` Willem de Bruijn
2021-06-25 18:55     ` David Woodhouse

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=4a5c6e49-ee50-3c0c-c8e6-04d85137cfb1@redhat.com \
    --to=jasowang@redhat.com \
    --cc=dwmw2@infradead.org \
    --cc=eperezma@redhat.com \
    --cc=netdev@vger.kernel.org \
    /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.