[Xen-devel] Re: [PATCH v3] libvchan: interdomain communications library

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Mon, 2011-08-29 at 19:48 +0100, Daniel De Graaf wrote:
> diff --git a/tools/libvchan/Makefile b/tools/libvchan/Makefile
> new file mode 100644
> index 0000000..6cccf3e
> --- /dev/null
> +++ b/tools/libvchan/Makefile
> @@ -0,0 +1,56 @@
> +#
> +# tools/libvchan/Makefile
> +#
> +
> +XEN_ROOT = $(CURDIR)/../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +LIBVCHAN_OBJS = init.o io.o
> +NODE_OBJS = node.o
> +NODE2_OBJS = node-select.o
> +
> +LIBVCHAN_LIBS = $(LDLIBS_libxenstore)
> +$(LIBVCHAN_OBJS): CFLAGS += $(CFLAGS_libxenstore)
> +
> +MAJOR = 1.0
> +MINOR = 0
> +
> +CFLAGS += -I../include -I. -fPIC

Can you use foo.opic in your $(*_OBJS) instead of -fPIC? I think that's
how this is intended to work.

> +
> +.PHONY: all
> +all: libvchan.so vchan-node1 vchan-node2 libvchan.a
> +
> +libvchan.so: libvchan.so.$(MAJOR)
> +       ln -sf $< $@
> +
> +libvchan.so.$(MAJOR): libvchan.so.$(MAJOR).$(MINOR)
> +       ln -sf $< $@
> +
> +libvchan.so.$(MAJOR).$(MINOR): $(LIBVCHAN_OBJS)
> +       $(CC) $(LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,libvchan.so.$(MAJOR) 
> $(SHLIB_LDFLAGS) -o $@ $^ $(LIBVCHAN_LIBS)
> +
> +libvchan.a: $(LIBVCHAN_OBJS)
> +       $(AR) rcs libvchan.a $^
> +
> +vchan-node1: $(NODE_OBJS) libvchan.so
> +       $(CC) $(LDFLAGS) -o $@ $(NODE_OBJS) libvchan.so $(LDLIBS_libvchan)
> +
> +vchan-node2: $(NODE2_OBJS) libvchan.so
> +       $(CC) $(LDFLAGS) -o $@ $(NODE2_OBJS) libvchan.so $(LDLIBS_libvchan)
> +
> +.PHONY: install
> +install: all
> +       $(INSTALL_DIR) $(DESTDIR)$(LIBDIR)
> +       $(INSTALL_DIR) $(DESTDIR)$(INCLUDEDIR)
> +       $(INSTALL_PROG) libvchan.so.$(MAJOR).$(MINOR) $(DESTDIR)$(LIBDIR)

Perhaps the library should be libxenvchan since it is going in /usr/lib
and vchan is a bit generic?

> diff --git a/tools/libvchan/init.c b/tools/libvchan/init.c
> new file mode 100644
> index 0000000..b267ca7
> --- /dev/null
> +++ b/tools/libvchan/init.c
> @@ -0,0 +1,464 @@
> +/**
> + * @file
> + * @section AUTHORS
> + *
> + * Copyright (C) 2010  Rafal Wojtczuk  <rafal@xxxxxxxxxxxxxxxxxxxxxx>
> + *
> + *  Authors:
> + *       Rafal Wojtczuk  <rafal@xxxxxxxxxxxxxxxxxxxxxx>
> + *       Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> + *
> + * @section LICENSE
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; under version 2 of the License.

I don't have a problem with GPL rather than LGPL myself but I suppose we
should consider if it meets the needs of the potential users now while
the number of people who have touched the library is small enough that
we can ask them if they are happy to relicense.

[...]
> +static int init_gnt_srv(struct libvchan *ctrl)
> +{
> +       int pages_left = ctrl->read.order >= PAGE_SHIFT ? 1 << 
> (ctrl->read.order - PAGE_SHIFT) : 0;
> +       int pages_right = ctrl->write.order >= PAGE_SHIFT ? 1 << 
> (ctrl->write.order - PAGE_SHIFT) : 0;
> +       struct ioctl_gntalloc_alloc_gref *gref_info = NULL;
> +       int ring_fd = open("/dev/xen/gntalloc", O_RDWR);
> +       int ring_ref = -1;
> +       int err;
> +       void *ring, *area;
> +
> +       if (ring_fd < 0)
> +               return -1;
> +
> +       gref_info = malloc(sizeof(*gref_info) + max(pages_left, 
> pages_right)*sizeof(uint32_t));
> +
> +       gref_info->domid = ctrl->other_domain_id;
> +       gref_info->flags = GNTALLOC_FLAG_WRITABLE;
> +       gref_info->count = 1;
> +
> +       err = ioctl(ring_fd, IOCTL_GNTALLOC_ALLOC_GREF, gref_info);

Unless libvchan is going to be the only user of this interface we should
add helpful wrappers to libxc, like we do for gntdev and evtchn.

> +       if (err)
> +               goto out;
> +
> +       ring = mmap(NULL, PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, 
> ring_fd, gref_info->index);
> +
> +       if (ring == MAP_FAILED)
> +               goto out;
> +
> +       ctrl->ring = ring;
> +       ring_ref = gref_info->gref_ids[0];
> +
> +       memset(ring, 0, PAGE_SIZE);
> +
> +       ctrl->read.shr = &ctrl->ring->left;
> +       ctrl->write.shr = &ctrl->ring->right;
> +       ctrl->ring->left_order = ctrl->read.order;
> +       ctrl->ring->right_order = ctrl->write.order;
> +       ctrl->ring->cli_live = 2;
> +       ctrl->ring->srv_live = 1;
> +       ctrl->ring->debug = 0xabcd;

Makes a change from deafbeef I guess ;-)

> +#ifdef IOCTL_GNTALLOC_SET_UNMAP_NOTIFY
> +       {
> +               struct ioctl_gntalloc_unmap_notify arg;
> +               arg.index = gref_info->index + offsetof(struct 
> vchan_interface, srv_live);
> +               arg.action = UNMAP_NOTIFY_CLEAR_BYTE | 
> UNMAP_NOTIFY_SEND_EVENT;
> +               arg.event_channel_port = ctrl->event_port;
> +               ioctl(ring_fd, IOCTL_GNTALLOC_SET_UNMAP_NOTIFY, &arg);
> +       }
> +#endif

What is the fallback if this isn't available?

[...]
> static int init_xs_srv(struct libvchan *ctrl, int ring_ref)
> +{
> +       int ret = -1;
> +       struct xs_handle *xs;
> +       struct xs_permissions perms[2];
> +       char buf[64];
> +       char ref[16];
> +       char* domid_str = NULL;
> +       xs = xs_domain_open();
> +       if (!xs)
> +               goto fail;
> +       domid_str = xs_read(xs, 0, "domid", NULL);
> +       if (!domid_str)
> +               goto fail_xs_open;
> +
> +       // owner domain is us
> +       perms[0].id = atoi(domid_str);

It sucks a bit that xenstore doesn't appear to allow DOMNID_SELF here
but oh well.

> +       // permissions for domains not listed = none
> +       perms[0].perms = XS_PERM_NONE;
> +       // other domains
> +       perms[1].id = ctrl->other_domain_id;
> +       perms[1].perms = XS_PERM_READ;
> +
> +       snprintf(ref, sizeof ref, "%d", ring_ref);
> +       snprintf(buf, sizeof buf, "data/vchan/%d/ring-ref", 
> ctrl->device_number);
> +       if (!xs_write(xs, 0, buf, ref, strlen(ref)))
> +               goto fail_xs_open;
> +       if (!xs_set_permissions(xs, 0, buf, perms, 2))
> +               goto fail_xs_open;
> +
> +       snprintf(ref, sizeof ref, "%d", ctrl->event_port);
> +       snprintf(buf, sizeof buf, "data/vchan/%d/event-channel", 
> ctrl->device_number);
> +       if (!xs_write(xs, 0, buf, ref, strlen(ref)))
> +               goto fail_xs_open;
> +       if (!xs_set_permissions(xs, 0, buf, perms, 2))
> +               goto fail_xs_open;

Am I right that the intended usage model is that two domains can decide
to setup a connection without admin or toolstack involvement?

Do we need to arrange on the toolstack side that a suitable
vchan-specific directory (or directories) in xenstore exists with
suitable permissions to allow this to happen exists or do we think data
is an appropriate location? 

[...]
> +static int init_evt_cli(struct libvchan *ctrl)
> +{
> +       struct ioctl_evtchn_bind_interdomain bind;
> +       ctrl->event_fd = open("/dev/xen/evtchn", O_RDWR);
> +       if (ctrl->event_fd < 0)
> +               return -1;
> +
> +       bind.remote_domain = ctrl->other_domain_id;
> +       bind.remote_port = ctrl->event_port;
> +       ctrl->event_port = ioctl(ctrl->event_fd, 
> IOCTL_EVTCHN_BIND_INTERDOMAIN, &bind);
> +       if (ctrl->event_port < 0)
> +               return -1;
> +       return 0;
> +}

This appears to be reimplementing xc_evtchn_bind_interdomain. It should
use the provided infrastructure libraries instead.

> diff --git a/xen/include/public/io/libvchan.h 
> b/xen/include/public/io/libvchan.h
> new file mode 100644
> index 0000000..81db0e2
> --- /dev/null
> +++ b/xen/include/public/io/libvchan.h
> @@ -0,0 +1,209 @@
> +/**
> + * @file
> + * @section AUTHORS
> + *
> + * Copyright (C) 2010  Rafal Wojtczuk  <rafal@xxxxxxxxxxxxxxxxxxxxxx>
> + *
> + *  Authors:
> + *       Rafal Wojtczuk  <rafal@xxxxxxxxxxxxxxxxxxxxxx>
> + *       Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> + *
> + * @section LICENSE
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; under version 2 of the License.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> + *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + *
> + * @section DESCRIPTION
> + *
> + *  Originally borrowed from the Qubes OS Project, http://www.qubes-os.org,
> + *  this code has been substantially rewritten to use the gntdev and gntalloc
> + *  devices instead of raw MFNs and map_foreign_range.
> + *
> + *  This is a library for inter-domain communication.  A standard Xen ring
> + *  buffer is used, with a datagram-based interface built on top.  The grant
> + *  reference and event channels are shared in XenStore under the path
> + *  /local/domain/<domid>/data/vchan/<port>/{ring-ref,event-channel}

Is the peer's domid just implicit in port and/or the out of band setup?

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel