Re: [PATCH 20/24] vhost-user: add vhost-user

On Mon, 12 Feb 2024 11:02:14 +0100 Laurent Vivier wrote:

On 2/12/24 03:49, David Gibson wrote:

...

...

On Fri, 2 Feb 2024 15:11:47 +0100 Laurent Vivier wrote:

...

add virtio and vhost-user functions to connect with QEMU.

$ ./passt --vhost-user

and

# qemu-system-x86_64 ... -m 4G \ -object memory-backend-memfd,id=memfd0,share=on,size=4G \ -numa node,memdev=memfd0 \ -chardev socket,id=chr0,path=/tmp/passt_1.socket \ -netdev vhost-user,id=netdev0,chardev=chr0 \ -device virtio-net,mac=9a:2b:2c:2d:2e:2f,netdev=netdev0 \ ...

Signed-off-by: Laurent Vivier --- conf.c | 20 ++++++++++++++-- passt.c | 7 ++++++ passt.h | 1 + tap.c | 73 ++++++++++++++++++++++++++++++++++++++++++--------------- tcp.c | 8 +++++-- udp.c | 6 +++-- 6 files changed, 90 insertions(+), 25 deletions(-) This would need a matching change in the man page, passt.1, at least documenting the --vhost-user option and adjusting descriptions about the guest communication interface (look for "UNIX domain" there).

...

diff --git a/conf.c b/conf.c index b6a2a1f0fdc3..40aa9519f8a6 100644 --- a/conf.c +++ b/conf.c @@ -44,6 +44,7 @@ #include "lineread.h" #include "isolation.h" #include "log.h" +#include "vhost_user.h"

/** * next_chunk - Return the next piece of a string delimited by a character @@ -735,9 +736,12 @@ static void print_usage(const char *name, int status) info( " -I, --ns-ifname NAME namespace interface name"); info( " default: same interface name as external one"); } else { - info( " -s, --socket PATH UNIX domain socket path"); + info( " -s, --socket, --socket-path PATH UNIX domain socket path"); I don't get the point of --socket-path. It's handled just like -s anyway, right? Why can't it just be -s / --socket? I believe the issue is that there's an expected command line interface for the vhost server, which uses --socket-path for, well, the socket

On Mon, Feb 12, 2024 at 12:19:01AM +0100, Stefano Brivio wrote: path. Hence adding an alias to the existing passt option.

...

...

info( " default: probe free path starting from " UNIX_SOCK_PATH, 1); + info( " --vhost-user Enable vhost-user mode"); + info( " UNIX domain socket is provided by -s option"); + info( " --print-capabilities print back-end capabilities in JSON format");

Instead of introducing a new option, couldn't we have these printed unconditionally with debug()? I guess it's debug-level stuff anyway. Likewise, I think this option is expected by the thing which starts the vhost server.

Yes, these parameters are defined in the vhost-user protocol specification:

https://qemu-project.gitlab.io/qemu/interop/vhost-user.html#backend-program-...

So, I went through the discussion from which these option names originated: https://patchwork.ozlabs.org/project/qemu-devel/cover/20180713130916.4153-1-... ...and I don't actually see the point for our case: libvirt and krunvm (pending) have a bit of code to start passt, and so do Podman and rootlesskit for pasta: https://gitlab.com/libvirt/libvirt/-/blob/20a5f77156bb0237269008351ddf285067... https://github.com/containers/krunvm/pull/56/commits/5fc42aa4dfde8608320e061... https://github.com/containers/common/blob/91e0fac33e22545f6e0d99d41d315075c0... https://github.com/rootless-containers/rootlesskit/blob/efee459a225b80ace3d5... where option names are happily open-coded (of course!). That is, I find rather unfeasible that any of these users would switch to some "unified" front-end that would ever need to start passt with a standardised command line. Anyway, okay, the cost isn't much as you already have the code changes, so I guess we can indulge in a bit of... compliance cult and keep all this. -- Stefano