* [PATCH 0 of 5] Remus/Libxl: Network buffering support
@ 2013-08-25 23:45 rshriram
2013-08-25 23:45 ` [PATCH 1 of 5] remus: add libnl3 dependency to autoconf scripts rshriram
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: rshriram @ 2013-08-25 23:45 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, ian.campbell, stefano.stabellini
This patch series adds support for network buffering in the Remus
codebase in libxl.
The series is organized as follows:
1/5 - changes to autoconf scripts to check for libnl3. Add linker flags
to libxl Makefile.
2/5 - External script to setup/teardown network buffering using libnl3's CLI.
This script will be invoked by libxl before starting Remus. The script's
main job is to bring up an IFB device with plug qdisc attached to it.
It then re-routes egress traffic from the guest's vif to the IFB device.
3/5 - Libxl code to invoke the external setup script, followed by
netlink related setup to obtain a handle on the output buffers attached
to each vif.
4/5 - Libxl interaction with network buffer module in the kernel via libnl3 API.
5/5 - xl cmdline switch to explicitly enable network buffering when starting remus.
Few things to note:
a) Based on previous email discussions, the setup/teardown task has been moved to a
hotplug style shell script which can be customized as desired, instead of
implementing it as C code inside libxl.
b) Libnl3 is not available on NetBSD. Nor is it available on CentOS (Linux).
So I have made network buffering support an optional feature so that it can be
disabled if desired.
c) NetBSD does not have libnl3. So I have put the setup script under
tools/hotplug/Linux folder.
thanks
shriram
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1 of 5] remus: add libnl3 dependency to autoconf scripts 2013-08-25 23:45 [PATCH 0 of 5] Remus/Libxl: Network buffering support rshriram @ 2013-08-25 23:45 ` rshriram 2013-08-26 9:06 ` Andrew Cooper 2013-08-27 10:56 ` Ian Campbell 2013-08-25 23:45 ` [PATCH 2 of 5] tools/hotplug: Remus network buffering setup scripts rshriram ` (3 subsequent siblings) 4 siblings, 2 replies; 14+ messages in thread From: rshriram @ 2013-08-25 23:45 UTC (permalink / raw) To: xen-devel; +Cc: ian.jackson, ian.campbell, stefano.stabellini # HG changeset patch # User Shriram Rajagopalan <rshriram@cs.ubc.ca> # Date 1377473602 25200 # Node ID 0ae8b121b67b3cb3a6b00f2cc97e98fec5758e20 # Parent 40b079bd57dea24c3b000f233ed123763483a4d5 remus: add libnl3 dependency to autoconf scripts Libnl3 is required for controlling Remus network buffering. This patch adds dependency on libnl3 (>= 3.2.8) to autoconf scripts. Also provide ability to configure tools without libnl3 support, that is without network buffering support. Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> diff -r 40b079bd57de -r 0ae8b121b67b config/Tools.mk.in --- a/config/Tools.mk.in Sun Aug 25 14:39:36 2013 -0700 +++ b/config/Tools.mk.in Sun Aug 25 16:33:22 2013 -0700 @@ -35,6 +35,7 @@ PTHREAD_LDFLAGS := @PTHREAD_LDFLAGS@ PTHREAD_LIBS := @PTHREAD_LIBS@ PTYFUNCS_LIBS := @PTYFUNCS_LIBS@ +LIBNL3_LIBS := @LIBNL3_LIBS@ # Download GIT repositories via HTTP or GIT's own protocol? # GIT's protocol is faster and more robust, when it works at all (firewalls @@ -54,6 +55,7 @@ CONFIG_QEMU_TRAD := @qemu_traditional CONFIG_QEMU_XEN := @qemu_xen@ CONFIG_XEND := @xend@ CONFIG_BLKTAP1 := @blktap1@ +CONFIG_REMUS_NETBUF := @remusnetbuf@ #System options ZLIB := @zlib@ diff -r 40b079bd57de -r 0ae8b121b67b tools/configure.ac --- a/tools/configure.ac Sun Aug 25 14:39:36 2013 -0700 +++ b/tools/configure.ac Sun Aug 25 16:33:22 2013 -0700 @@ -56,6 +56,8 @@ AX_ARG_DEFAULT_ENABLE([xsmpolicy], [Disa AX_ARG_DEFAULT_DISABLE([ovmf], [Enable OVMF]) AX_ARG_DEFAULT_ENABLE([rombios], [Disable ROM BIOS]) AX_ARG_DEFAULT_ENABLE([seabios], [Disable SeaBIOS]) +AX_ARG_DEFAULT_ENABLE([remusnetbuf], [ Enable/Disable Remus + network buffering support (requires libnl3)]) AX_ARG_DEFAULT_ENABLE([debug], [Disable debug build of tools]) AX_ARG_DEFAULT_ENABLE([xend], [Disable xend toolstack]) AX_ARG_DEFAULT_DISABLE([blktap1], [Disable blktap1 tools]) @@ -208,4 +210,29 @@ AC_SUBST(libiconv) # Checks for header files. AC_CHECK_HEADERS([yajl/yajl_version.h sys/eventfd.h]) +AS_IF([test "x$remusnetbuf" = "xy"], [ + +dnl Need libnl version 3.2.8 or higher for plug qdisc support + AS_IF( + [ pkg-config --exists 'libnl-3.0 >= 3.2.8 libnl-route-3.0 >= 3.2.8 libnl-cli-3.0 >= 3.2.8' ], + [], [ + AC_MSG_ERROR([Need libnl3, libnl-route-3 and libnl-cli-3 (>= 3.2.8) for Remus network buffering]) + ]) + + AC_CHECK_HEADER([netlink/route/qdisc/plug.h], [ + AC_CHECK_LIB(nl-3, nl_socket_alloc, [ + AC_CHECK_LIB(nl-route-3, rtnl_qdisc_alloc, [ + LIBNL3_LIBS="-lnl-3 -lnl-route-3" + AC_SUBST(LIBNL3_LIBS) + ], [ + AC_MSG_ERROR([Could not find libnl-route-3]) + ]) + ], [ + AC_MSG_ERROR([Could not find libnl3]) + ]) + ], [ + AC_MSG_ERROR([Could not find libnl3 dev headers (esp. plug.h)]) + ]) +], []) + AC_OUTPUT() diff -r 40b079bd57de -r 0ae8b121b67b tools/libxl/Makefile --- a/tools/libxl/Makefile Sun Aug 25 14:39:36 2013 -0700 +++ b/tools/libxl/Makefile Sun Aug 25 16:33:22 2013 -0700 @@ -20,7 +20,7 @@ LIBUUID_LIBS += -luuid endif LIBXL_LIBS = -LIBXL_LIBS = $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(LDLIBS_libblktapctl) $(PTYFUNCS_LIBS) $(LIBUUID_LIBS) +LIBXL_LIBS = $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(LDLIBS_libblktapctl) $(PTYFUNCS_LIBS) $(LIBUUID_LIBS) $(LIBNL3_LIBS) CFLAGS_LIBXL += $(CFLAGS_libxenctrl) CFLAGS_LIBXL += $(CFLAGS_libxenguest) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1 of 5] remus: add libnl3 dependency to autoconf scripts 2013-08-25 23:45 ` [PATCH 1 of 5] remus: add libnl3 dependency to autoconf scripts rshriram @ 2013-08-26 9:06 ` Andrew Cooper 2013-08-27 8:44 ` Ian Campbell 2013-08-27 10:56 ` Ian Campbell 1 sibling, 1 reply; 14+ messages in thread From: Andrew Cooper @ 2013-08-26 9:06 UTC (permalink / raw) To: rshriram; +Cc: stefano.stabellini, ian.jackson, ian.campbell, xen-devel On 26/08/2013 00:45, rshriram@cs.ubc.ca wrote: > # HG changeset patch > # User Shriram Rajagopalan <rshriram@cs.ubc.ca> > # Date 1377473602 25200 > # Node ID 0ae8b121b67b3cb3a6b00f2cc97e98fec5758e20 > # Parent 40b079bd57dea24c3b000f233ed123763483a4d5 > remus: add libnl3 dependency to autoconf scripts > > Libnl3 is required for controlling Remus network buffering. > This patch adds dependency on libnl3 (>= 3.2.8) to autoconf scripts. > Also provide ability to configure tools without libnl3 support, that > is without network buffering support. > > Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> As identified in the patch 0, libnl3 is not present for CentOS (so by implication RHEL and derivatives) and various BSDs, so enabling it by default and erroring out of autoconf if it cant be found is not really acceptable imo. It would be better to detect the presence/absence of an appropriate libnl3, and use that as the default. Then, the logic would be: "By default build in libnl3 support if it is available, but don't if it is not. If the user explicitlicy configures --with-remusnetbuf and libnl3 is not present, present an error" which is far more appropriate for an option which affects a large swathe of distributions. ~Andrew > > diff -r 40b079bd57de -r 0ae8b121b67b config/Tools.mk.in > --- a/config/Tools.mk.in Sun Aug 25 14:39:36 2013 -0700 > +++ b/config/Tools.mk.in Sun Aug 25 16:33:22 2013 -0700 > @@ -35,6 +35,7 @@ PTHREAD_LDFLAGS := @PTHREAD_LDFLAGS@ > PTHREAD_LIBS := @PTHREAD_LIBS@ > > PTYFUNCS_LIBS := @PTYFUNCS_LIBS@ > +LIBNL3_LIBS := @LIBNL3_LIBS@ > > # Download GIT repositories via HTTP or GIT's own protocol? > # GIT's protocol is faster and more robust, when it works at all (firewalls > @@ -54,6 +55,7 @@ CONFIG_QEMU_TRAD := @qemu_traditional > CONFIG_QEMU_XEN := @qemu_xen@ > CONFIG_XEND := @xend@ > CONFIG_BLKTAP1 := @blktap1@ > +CONFIG_REMUS_NETBUF := @remusnetbuf@ > > #System options > ZLIB := @zlib@ > diff -r 40b079bd57de -r 0ae8b121b67b tools/configure.ac > --- a/tools/configure.ac Sun Aug 25 14:39:36 2013 -0700 > +++ b/tools/configure.ac Sun Aug 25 16:33:22 2013 -0700 > @@ -56,6 +56,8 @@ AX_ARG_DEFAULT_ENABLE([xsmpolicy], [Disa > AX_ARG_DEFAULT_DISABLE([ovmf], [Enable OVMF]) > AX_ARG_DEFAULT_ENABLE([rombios], [Disable ROM BIOS]) > AX_ARG_DEFAULT_ENABLE([seabios], [Disable SeaBIOS]) > +AX_ARG_DEFAULT_ENABLE([remusnetbuf], [ Enable/Disable Remus > + network buffering support (requires libnl3)]) > AX_ARG_DEFAULT_ENABLE([debug], [Disable debug build of tools]) > AX_ARG_DEFAULT_ENABLE([xend], [Disable xend toolstack]) > AX_ARG_DEFAULT_DISABLE([blktap1], [Disable blktap1 tools]) > @@ -208,4 +210,29 @@ AC_SUBST(libiconv) > # Checks for header files. > AC_CHECK_HEADERS([yajl/yajl_version.h sys/eventfd.h]) > > +AS_IF([test "x$remusnetbuf" = "xy"], [ > + > +dnl Need libnl version 3.2.8 or higher for plug qdisc support > + AS_IF( > + [ pkg-config --exists 'libnl-3.0 >= 3.2.8 libnl-route-3.0 >= 3.2.8 libnl-cli-3.0 >= 3.2.8' ], > + [], [ > + AC_MSG_ERROR([Need libnl3, libnl-route-3 and libnl-cli-3 (>= 3.2.8) for Remus network buffering]) > + ]) > + > + AC_CHECK_HEADER([netlink/route/qdisc/plug.h], [ > + AC_CHECK_LIB(nl-3, nl_socket_alloc, [ > + AC_CHECK_LIB(nl-route-3, rtnl_qdisc_alloc, [ > + LIBNL3_LIBS="-lnl-3 -lnl-route-3" > + AC_SUBST(LIBNL3_LIBS) > + ], [ > + AC_MSG_ERROR([Could not find libnl-route-3]) > + ]) > + ], [ > + AC_MSG_ERROR([Could not find libnl3]) > + ]) > + ], [ > + AC_MSG_ERROR([Could not find libnl3 dev headers (esp. plug.h)]) > + ]) > +], []) > + > AC_OUTPUT() > diff -r 40b079bd57de -r 0ae8b121b67b tools/libxl/Makefile > --- a/tools/libxl/Makefile Sun Aug 25 14:39:36 2013 -0700 > +++ b/tools/libxl/Makefile Sun Aug 25 16:33:22 2013 -0700 > @@ -20,7 +20,7 @@ LIBUUID_LIBS += -luuid > endif > > LIBXL_LIBS = > -LIBXL_LIBS = $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(LDLIBS_libblktapctl) $(PTYFUNCS_LIBS) $(LIBUUID_LIBS) > +LIBXL_LIBS = $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(LDLIBS_libblktapctl) $(PTYFUNCS_LIBS) $(LIBUUID_LIBS) $(LIBNL3_LIBS) > > CFLAGS_LIBXL += $(CFLAGS_libxenctrl) > CFLAGS_LIBXL += $(CFLAGS_libxenguest) > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1 of 5] remus: add libnl3 dependency to autoconf scripts 2013-08-26 9:06 ` Andrew Cooper @ 2013-08-27 8:44 ` Ian Campbell 0 siblings, 0 replies; 14+ messages in thread From: Ian Campbell @ 2013-08-27 8:44 UTC (permalink / raw) To: Andrew Cooper; +Cc: rshriram, stefano.stabellini, ian.jackson, xen-devel On Mon, 2013-08-26 at 10:06 +0100, Andrew Cooper wrote: > On 26/08/2013 00:45, rshriram@cs.ubc.ca wrote: > > # HG changeset patch > > # User Shriram Rajagopalan <rshriram@cs.ubc.ca> > > # Date 1377473602 25200 > > # Node ID 0ae8b121b67b3cb3a6b00f2cc97e98fec5758e20 > > # Parent 40b079bd57dea24c3b000f233ed123763483a4d5 > > remus: add libnl3 dependency to autoconf scripts > > > > Libnl3 is required for controlling Remus network buffering. > > This patch adds dependency on libnl3 (>= 3.2.8) to autoconf scripts. > > Also provide ability to configure tools without libnl3 support, that > > is without network buffering support. > > > > Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> > > As identified in the patch 0, libnl3 is not present for CentOS (so by > implication RHEL and derivatives) and various BSDs, so enabling it by > default and erroring out of autoconf if it cant be found is not really > acceptable imo. > > It would be better to detect the presence/absence of an appropriate > libnl3, and use that as the default. Then, the logic would be: "By > default build in libnl3 support if it is available, but don't if it is > not. This is the minimum we should require IMHO > If the user explicitlicy configures --with-remusnetbuf and libnl3 > is not present, present an error" The presence of an option to force enable it is a stretch goal as far as I'm concerned. Ian. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1 of 5] remus: add libnl3 dependency to autoconf scripts 2013-08-25 23:45 ` [PATCH 1 of 5] remus: add libnl3 dependency to autoconf scripts rshriram 2013-08-26 9:06 ` Andrew Cooper @ 2013-08-27 10:56 ` Ian Campbell 2013-08-29 19:13 ` Shriram Rajagopalan 1 sibling, 1 reply; 14+ messages in thread From: Ian Campbell @ 2013-08-27 10:56 UTC (permalink / raw) To: rshriram; +Cc: stefano.stabellini, ian.jackson, xen-devel On Sun, 2013-08-25 at 16:45 -0700, rshriram@cs.ubc.ca wrote: > +AS_IF([test "x$remusnetbuf" = "xy"], [ > + > +dnl Need libnl version 3.2.8 or higher for plug qdisc support > + AS_IF( > + [ pkg-config --exists 'libnl-3.0 >= 3.2.8 libnl-route-3.0 >= 3.2.8 libnl-cli-3.0 >= 3.2.8' ], > + [], [ > + AC_MSG_ERROR([Need libnl3, libnl-route-3 and libnl-cli-3 (>= 3.2.8) for Remus network buffering]) > + ]) At least on Debian pkg-config ships pkg.m4 which contains handy looking autoconf macros for doing pkg-config stuff. Better to use them, gets you free cross compile support and general consistency with other projects. > + > + AC_CHECK_HEADER([netlink/route/qdisc/plug.h], [ > + AC_CHECK_LIB(nl-3, nl_socket_alloc, [ > + AC_CHECK_LIB(nl-route-3, rtnl_qdisc_alloc, [ > + LIBNL3_LIBS="-lnl-3 -lnl-route-3" > + AC_SUBST(LIBNL3_LIBS) > + ], [ > + AC_MSG_ERROR([Could not find libnl-route-3]) > + ]) > + ], [ > + AC_MSG_ERROR([Could not find libnl3]) > + ]) > + ], [ > + AC_MSG_ERROR([Could not find libnl3 dev headers (esp. plug.h)]) Is there any chance of this failing if pkg-config says it exists? IOW isn't this test redundant? Ian. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1 of 5] remus: add libnl3 dependency to autoconf scripts 2013-08-27 10:56 ` Ian Campbell @ 2013-08-29 19:13 ` Shriram Rajagopalan 0 siblings, 0 replies; 14+ messages in thread From: Shriram Rajagopalan @ 2013-08-29 19:13 UTC (permalink / raw) To: Ian Campbell; +Cc: Stefano Stabellini, Ian Jackson, xen-devel@lists.xen.org [-- Attachment #1.1: Type: text/plain, Size: 2021 bytes --] On Tue, Aug 27, 2013 at 6:56 AM, Ian Campbell <Ian.Campbell@citrix.com>wrote: > On Sun, 2013-08-25 at 16:45 -0700, rshriram@cs.ubc.ca wrote: > > +AS_IF([test "x$remusnetbuf" = "xy"], [ > > + > > +dnl Need libnl version 3.2.8 or higher for plug qdisc support > > + AS_IF( > > + [ pkg-config --exists 'libnl-3.0 >= 3.2.8 libnl-route-3.0 >= > 3.2.8 libnl-cli-3.0 >= 3.2.8' ], > > + [], [ > > + AC_MSG_ERROR([Need libnl3, libnl-route-3 and libnl-cli-3 (>= > 3.2.8) for Remus network buffering]) > > + ]) > > At least on Debian pkg-config ships pkg.m4 which contains handy looking > autoconf macros for doing pkg-config stuff. Better to use them, gets you > free cross compile support and general consistency with other projects. > > > + > > + AC_CHECK_HEADER([netlink/route/qdisc/plug.h], [ > > + AC_CHECK_LIB(nl-3, nl_socket_alloc, [ > > + AC_CHECK_LIB(nl-route-3, rtnl_qdisc_alloc, [ > > + LIBNL3_LIBS="-lnl-3 -lnl-route-3" > > + AC_SUBST(LIBNL3_LIBS) > > + ], [ > > + AC_MSG_ERROR([Could not find libnl-route-3]) > > + ]) > > + ], [ > > + AC_MSG_ERROR([Could not find libnl3]) > > + ]) > > + ], [ > > + AC_MSG_ERROR([Could not find libnl3 dev headers (esp. plug.h)]) > > Is there any chance of this failing if pkg-config says it exists? IOW > isn't this test redundant? > > Ian. > > Yeah, that is redundant. [my autoconf naiveté ]. I was under the impression that the ac_check_lib would automatically add the linker flags when compiling code, only to realize later that it didnt work for libxenlight.so. So I ended up explicitly using LIBNL3_LIBS (in the libxl makefile) will fix this and the other suggestions: I was thinking of getting rid of that config option altogether. Compile in libnl3 if available (and enable libxl_netbuffer.c). If not, issue a warning and compile libxl_nonetbuffer.c. shriram [-- Attachment #1.2: Type: text/html, Size: 2814 bytes --] [-- Attachment #2: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2 of 5] tools/hotplug: Remus network buffering setup scripts 2013-08-25 23:45 [PATCH 0 of 5] Remus/Libxl: Network buffering support rshriram 2013-08-25 23:45 ` [PATCH 1 of 5] remus: add libnl3 dependency to autoconf scripts rshriram @ 2013-08-25 23:45 ` rshriram 2013-08-25 23:45 ` [PATCH 3 of 5] tools/libxl: setup/teardown Remus network buffering rshriram ` (2 subsequent siblings) 4 siblings, 0 replies; 14+ messages in thread From: rshriram @ 2013-08-25 23:45 UTC (permalink / raw) To: xen-devel; +Cc: ian.jackson, ian.campbell, stefano.stabellini # HG changeset patch # User Shriram Rajagopalan <rshriram@cs.ubc.ca> # Date 1377473607 25200 # Node ID 4d24d36d30c37e4fb5985952db92c1e789c90894 # Parent 0ae8b121b67b3cb3a6b00f2cc97e98fec5758e20 tools/hotplug: Remus network buffering setup scripts This patch introduces remus-netbuf-setup hotplug script responsible for setting up and tearing down the necessary infrastructure required for network output buffering in Remus. This script is intended to be invoked by libxl for each guest interface, when starting or stopping Remus. Apart from returning success/failure indication via the usual hotplug entires in xenstore, this script also writes to xenstore, the name of the IFB device to be used to control the vif's network output. The script relies on libnl3 command line utilities to perform various setup/teardown functions. The script is confined to Linux platforms only since NetBSD does not seem to have libnl3. Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> diff -r 0ae8b121b67b -r 4d24d36d30c3 tools/hotplug/Linux/Makefile --- a/tools/hotplug/Linux/Makefile Sun Aug 25 16:33:22 2013 -0700 +++ b/tools/hotplug/Linux/Makefile Sun Aug 25 16:33:27 2013 -0700 @@ -16,6 +16,7 @@ XEN_SCRIPTS += network-nat vif-nat XEN_SCRIPTS += vif-openvswitch XEN_SCRIPTS += vif2 XEN_SCRIPTS += vif-setup +XEN_SCRIPTS-$(CONFIG_REMUS_NETBUF) += remus-netbuf-setup XEN_SCRIPTS += block XEN_SCRIPTS += block-enbd block-nbd XEN_SCRIPTS-$(CONFIG_BLKTAP1) += blktap diff -r 0ae8b121b67b -r 4d24d36d30c3 tools/hotplug/Linux/remus-netbuf-setup --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tools/hotplug/Linux/remus-netbuf-setup Sun Aug 25 16:33:27 2013 -0700 @@ -0,0 +1,186 @@ +#!/bin/bash +#============================================================================ +# ${XEN_SCRIPT_DIR}/remus-netbuf-setup +# +# Script for attaching a network buffer to the specified vif (in any mode). +# The hotplugging system will call this script when starting remus via libxl +# API, libxl_domain_remus_start. +# +# Usage: +# remus-netbuf-setup (setup|teardown) +# +# Environment vars: +# vifname vif interface name (required). +# XENBUS_PATH path in Xenstore, where the IFB device details will be stored +# or read from (required). +# (libxl passes /libxl/<domid>/remus/netbuf/<devid>) +# +# Written to the store: (setup operation) +# XENBUS_PATH/ifb=<ifbdevName> the IFB device serving +# as the intermediate buffer through which the interface's network output +# can be controlled. +# +# Read from the store: (teardown operation) +# XENBUS_PATH/ifb the IFB device serving as the output buffer +# controller for the vif +# +# To install a network buffer on a guest vif (vif1.0) using ifb (ifb0) +# we need to do the following +# +# ip link set dev ifb0 up +# tc qdisc add dev vif1.0 ingress +# tc filter add dev vif1.0 parent ffff: proto ip \ +# prio 10 u32 match u32 0 0 action mirred egress redirect dev ifb0 +# nl-qdisc-add --dev=ifb0 --parent root plug +# nl-qdisc-add --dev=ifb0 --parent root --update plug --limit=10000000 +# (10MB limit on buffer) +# +# So order of operations when installing a network buffer on vif1.0 +# 1. find a free ifb and bring up the device +# 2. redirect traffic from vif1.0 to ifb: +# 2.1 add ingress qdisc to vif1.0 (to capture outgoing packets from guest) +# 2.2 use tc filter command with actions mirred egress + redirect +# 3. install plug_qdisc on ifb device, with which we can buffer/release +# guest's network output from vif1.0 +# +# + +#============================================================================ + +# Unlike other vif scripts, vif-common is not needed here as it executes vif +#specific setup code such as renaming. +dir=$(dirname "$0") +. "$dir/xen-hotplug-common.sh" + +findCommand "$@" + +if [ "$command" != "setup" -a "$command" != "teardown" ] +then + echo "Invalid command: $command" + log err "Invalid command: $command" + exit 1 +fi + +evalVariables "$@" + +: ${vifname?} +: ${XENBUS_PATH?} +IFB=$(xenstore_read_default "$XENBUS_PATH/ifb" "") + +check_libnl_tools() { + if ! command -v nl-qdisc-list > /dev/null 2>&1; then + fatal "Unable to find nl-qdisc-list tool" + fi + if ! command -v nl-qdisc-add > /dev/null 2>&1; then + fatal "Unable to find nl-qdisc-add tool" + fi + if ! command -v nl-qdisc-delete > /dev/null 2>&1; then + fatal "Unable to find nl-qdisc-delete tool" + fi +} + +# We only check for modules. We dont load them. +# User/Admin is supposed to load ifb during boot time, +# ensuring that there are enough free ifbs in the system. +# Other modules will be loaded automatically by tc commands. +check_modules() { + for m in ifb sch_plug sch_ingress act_mirred cls_u32 + do + if ! modinfo $m > /dev/null 2>&1; then + fatal "Unable to find $m kernel module" + fi + done +} + +setup_ifb() { + + for ifb in `ifconfig -a -s|egrep ^ifb|cut -d ' ' -f1` + do + local installed=`nl-qdisc-list -d $ifb` + [ -n "$installed" ] && continue + IFB="$ifb" + break + done + + if [ -z "$IFB" ] + then + fatal "Unable to find a free IFB device for $vifname" + fi + + do_or_die ip link set dev "$IFB" up +} + +redirect_vif_traffic() { + local vif=$1 + local ifb=$2 + + do_or_die tc qdisc add dev "$vif" ingress + + tc filter add dev "$vif" parent ffff: proto ip prio 10 \ + u32 match u32 0 0 action mirred egress redirect dev "$ifb" >/dev/null 2>&1 + + if [ $? -ne 0 ] + then + do_without_error tc qdisc del dev "$vif" ingress + fatal "Failed to redirect traffic from $vif to $ifb" + fi +} + +add_plug_qdisc() { + local vif=$1 + local ifb=$2 + + nl-qdisc-add --dev="$ifb" --parent root plug >/dev/null 2>&1 + if [ $? -ne 0 ] + then + do_without_error tc qdisc del dev "$vif" ingress + fatal "Failed to add plug qdisc to $ifb" + fi + + #set ifb buffering limit in bytes. Its okay if this command + nl-qdisc-add --dev="$ifb" --parent root \ + --update plug --limit=10000000 >/dev/null 2>&1 +} + +teardown_netbuf() { + local vif=$1 + local ifb=$2 + + if [ "$ifb" ]; then + do_without_error ip link set dev "$ifb" down + do_without_error nl-qdisc-delete --dev="$ifb" --parent root plug >/dev/null 2>&1 + xenstore-rm -t "$XENBUS_PATH/ifb" 2>/dev/null || true + fi + do_without_error tc qdisc del dev "$vif" ingress + xenstore-rm -t "$XENBUS_PATH/hotplug-status" 2>/dev/null || true +} + +xs_write_failed() { + local vif=$1 + local ifb=$2 + teardown_netbuf "$vifname" "$IFB" + fatal "failed to write ifb name to xenstore" +} + +case "$command" in + setup) + check_libnl_tools + check_modules + setup_ifb + redirect_vif_traffic "$vifname" "$IFB" + add_plug_qdisc "$vifname" "$IFB" + #not using xenstore_write that automatically exits on error + # because we need to cleanup + _xenstore_write "$XENBUS_PATH/ifb" "$IFB" || xs_write_failed "$vifname" "$IFB" + ;; + teardown) + teardown_netbuf "$vifname" "$IFB" + ;; +esac + +log debug "Successful remus-netbuf-setup $command for $vifname, ifb $IFB." + +if [ "$command" = "setup" ] +then + success +fi ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3 of 5] tools/libxl: setup/teardown Remus network buffering 2013-08-25 23:45 [PATCH 0 of 5] Remus/Libxl: Network buffering support rshriram 2013-08-25 23:45 ` [PATCH 1 of 5] remus: add libnl3 dependency to autoconf scripts rshriram 2013-08-25 23:45 ` [PATCH 2 of 5] tools/hotplug: Remus network buffering setup scripts rshriram @ 2013-08-25 23:45 ` rshriram 2013-08-26 22:13 ` Andrew Cooper 2013-08-25 23:45 ` [PATCH 4 of 5] tools/libxl: Control network buffering in remus callbacks rshriram 2013-08-25 23:45 ` [PATCH 5 of 5] tools/xl: Remus - Network buffering cmdline switch rshriram 4 siblings, 1 reply; 14+ messages in thread From: rshriram @ 2013-08-25 23:45 UTC (permalink / raw) To: xen-devel; +Cc: ian.jackson, ian.campbell, stefano.stabellini # HG changeset patch # User Shriram Rajagopalan <rshriram@cs.ubc.ca> # Date 1377473609 25200 # Node ID 4b23104828c09218aa7f9fbde1578bb6706e13d6 # Parent 4d24d36d30c37e4fb5985952db92c1e789c90894 tools/libxl: setup/teardown Remus network buffering Setup/teardown remus network buffering for a given guest, when libxl_domain_remus_start API is invoked. This patch does the following during setup: a) call the hotplug script for each vif to setup its network buffer b) establish a dedicated remus context containing libnl related state (netlink sockets, qdisc caches, etc.,) c) Obtain handles to plug qdiscs installed on the IFB devices chosen by the hotplug scripts. During teardown, the hotplug scripts are called to remove the IFB devices. This is followed by releasing netlink resources. Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> diff -r 4d24d36d30c3 -r 4b23104828c0 tools/libxl/Makefile --- a/tools/libxl/Makefile Sun Aug 25 16:33:27 2013 -0700 +++ b/tools/libxl/Makefile Sun Aug 25 16:33:29 2013 -0700 @@ -40,6 +40,13 @@ LIBXL_OBJS-y += libxl_blktap2.o else LIBXL_OBJS-y += libxl_noblktap2.o endif + +ifeq ($(CONFIG_REMUS_NETBUF),y) +LIBXL_OBJS-y += libxl_netbuffer.o +else +LIBXL_OBJS-y += libxl_nonetbuffer.o +endif + LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpuid.o libxl_noarch.o LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_noarch.o diff -r 4d24d36d30c3 -r 4b23104828c0 tools/libxl/libxl.c --- a/tools/libxl/libxl.c Sun Aug 25 16:33:27 2013 -0700 +++ b/tools/libxl/libxl.c Sun Aug 25 16:33:29 2013 -0700 @@ -692,8 +692,9 @@ libxl_vminfo * libxl_list_vm(libxl_ctx * return ptr; } -static void remus_failover_cb(libxl__egc *egc, - libxl__domain_suspend_state *dss, int rc); +static void remus_replication_failure_cb(libxl__egc *egc, + libxl__domain_suspend_state *dss, + int rc); /* TODO: Explicit Checkpoint acknowledgements via recv_fd. */ int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info, @@ -712,18 +713,44 @@ int libxl_domain_remus_start(libxl_ctx * GCNEW(dss); dss->ao = ao; - dss->callback = remus_failover_cb; + dss->callback = remus_replication_failure_cb; dss->domid = domid; dss->fd = send_fd; /* TODO do something with recv_fd */ dss->type = type; dss->live = 1; dss->debug = 0; - dss->remus = info; assert(info); - /* TBD: Remus setup - i.e. attach qdisc, enable disk buffering, etc */ + GCNEW(dss->remus_ctx); + + /* convenience shorthand */ + libxl__remus_ctx *remus_ctx = dss->remus_ctx; + remus_ctx->blackhole = info->blackhole; + remus_ctx->interval = info->interval; + remus_ctx->compression = info->compression; + + /* Setup network buffering before invoking domain_suspend */ + if (info->netbuf) { + if (info->netbufscript) { + remus_ctx->netbufscript = + libxl__strdup(gc, info->netbufscript); + } else { + remus_ctx->netbufscript = + libxl__sprintf(gc, "%s/remus-netbuf-setup", + libxl__xen_script_dir_path()); + } + + if (libxl__remus_netbuf_setup(gc, domid, remus_ctx)) { + LOG(ERROR, "Remus: failed to setup network buffering" + " for guest with domid %u", domid); + rc = ERROR_FAIL; + goto out; + } + } + + /* TBD: enable disk buffering */ /* Point of no return */ libxl__domain_suspend(egc, dss); @@ -733,8 +760,9 @@ int libxl_domain_remus_start(libxl_ctx * return AO_ABORT(rc); } -static void remus_failover_cb(libxl__egc *egc, - libxl__domain_suspend_state *dss, int rc) +static void remus_replication_failure_cb(libxl__egc *egc, + libxl__domain_suspend_state *dss, + int rc) { STATE_AO_GC(dss->ao); /* @@ -743,9 +771,10 @@ static void remus_failover_cb(libxl__egc * from sending checkpoints. */ - /* TBD: Remus cleanup - i.e. detach qdisc, release other - * resources. - */ + /* Teardown the network buffers and release netlink resources. */ + if (dss->remus_ctx && dss->remus_ctx->netbuf_ctx) + libxl__remus_netbuf_teardown(gc, dss->domid, dss->remus_ctx); + libxl__ao_complete(egc, ao, rc); } diff -r 4d24d36d30c3 -r 4b23104828c0 tools/libxl/libxl_dom.c --- a/tools/libxl/libxl_dom.c Sun Aug 25 16:33:27 2013 -0700 +++ b/tools/libxl/libxl_dom.c Sun Aug 25 16:33:29 2013 -0700 @@ -1259,7 +1259,7 @@ static void remus_checkpoint_dm_saved(li /* REMUS TODO: Wait for disk and memory ack, release network buffer */ /* REMUS TODO: make this asynchronous */ assert(!rc); /* REMUS TODO handle this error properly */ - usleep(dss->interval * 1000); + usleep(dss->remus_ctx->interval * 1000); libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, 1); } @@ -1277,7 +1277,6 @@ void libxl__domain_suspend(libxl__egc *e const libxl_domain_type type = dss->type; const int live = dss->live; const int debug = dss->debug; - const libxl_domain_remus_info *const r_info = dss->remus; libxl__srm_save_autogen_callbacks *const callbacks = &dss->shs.callbacks.save.a; @@ -1312,9 +1311,8 @@ void libxl__domain_suspend(libxl__egc *e dss->guest_responded = 0; dss->dm_savefile = libxl__device_model_savefile(gc, domid); - if (r_info != NULL) { - dss->interval = r_info->interval; - if (r_info->compression) + if (dss->remus_ctx != NULL) { + if (dss->remus_ctx->compression) dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS; } @@ -1335,7 +1333,7 @@ void libxl__domain_suspend(libxl__egc *e } memset(callbacks, 0, sizeof(*callbacks)); - if (r_info != NULL) { + if (dss->remus_ctx != NULL) { callbacks->suspend = libxl__remus_domain_suspend_callback; callbacks->postcopy = libxl__remus_domain_resume_callback; callbacks->checkpoint = libxl__remus_domain_checkpoint_callback; diff -r 4d24d36d30c3 -r 4b23104828c0 tools/libxl/libxl_internal.h --- a/tools/libxl/libxl_internal.h Sun Aug 25 16:33:27 2013 -0700 +++ b/tools/libxl/libxl_internal.h Sun Aug 25 16:33:29 2013 -0700 @@ -2242,6 +2242,29 @@ typedef struct libxl__logdirty_switch { libxl__ev_time timeout; } libxl__logdirty_switch; +typedef struct libxl__remus_ctx { + /* checkpoint interval */ + int interval; + int blackhole; + int compression; + /* Script to setup/teardown network buffers */ + const char *netbufscript; + /* Opaque context containing network buffer related stuff */ + void *netbuf_ctx; +} libxl__remus_ctx; + +_hidden int libxl__remus_netbuf_setup(libxl__gc *gc, uint32_t domid, + libxl__remus_ctx *remus_ctx); + +_hidden int libxl__remus_netbuf_teardown(libxl__gc *gc, uint32_t domid, + libxl__remus_ctx *remus_ctx); + +_hidden int libxl__remus_netbuf_start_new_epoch(libxl__gc *gc, uint32_t domid, + libxl__remus_ctx *remus_ctx); + +_hidden int libxl__remus_netbuf_release_prev_epoch(libxl__gc *gc, uint32_t domid, + libxl__remus_ctx *remus_ctx); + struct libxl__domain_suspend_state { /* set by caller of libxl__domain_suspend */ libxl__ao *ao; @@ -2252,7 +2275,7 @@ struct libxl__domain_suspend_state { libxl_domain_type type; int live; int debug; - const libxl_domain_remus_info *remus; + libxl__remus_ctx *remus_ctx; /* private */ xc_evtchn *xce; /* event channel handle */ int suspend_eventchn; @@ -2260,7 +2283,6 @@ struct libxl__domain_suspend_state { int xcflags; int guest_responded; const char *dm_savefile; - int interval; /* checkpoint interval (for Remus) */ libxl__save_helper_state shs; libxl__logdirty_switch logdirty; /* private for libxl__domain_save_device_model */ diff -r 4d24d36d30c3 -r 4b23104828c0 tools/libxl/libxl_netbuffer.c --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tools/libxl/libxl_netbuffer.c Sun Aug 25 16:33:29 2013 -0700 @@ -0,0 +1,434 @@ +/* + * Copyright (C) 2013 + * Author Shriram Rajagopalan <rshriram@cs.ubc.ca> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation; version 2.1 only. with the special + * exception on linking described in file 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 Lesser General Public License for more details. + */ + +#include "libxl_osdeps.h" /* must come before any other headers */ + +#include "libxl_internal.h" + +#include <netlink/cache.h> +#include <netlink/socket.h> +#include <netlink/attr.h> +#include <netlink/route/link.h> +#include <netlink/route/route.h> +#include <netlink/route/qdisc.h> +#include <netlink/route/qdisc/plug.h> + +typedef struct libxl__remus_netbuf_ctx { + struct rtnl_qdisc **netbuf_qdisc_list; + struct nl_sock *nlsock; + struct nl_cache *qdisc_cache; + char **vif_list; + char **ifb_list; + uint32_t num_netbufs; + uint32_t unused; +} libxl__remus_netbuf_ctx; + +/* If the device has a vifname, then use that instead of + * the vifX.Y format. + */ +static char *get_vifname(libxl__gc *gc, uint32_t domid, + libxl_device_nic *nic) +{ + char *vifname = NULL; + vifname = libxl__xs_read(gc, XBT_NULL, + libxl__sprintf(gc, + "%s/backend/vif/%d/%d/vifname", + libxl__xs_get_dompath(gc, 0), + domid, nic->devid)); + if (!vifname) { + vifname = (char *)libxl__device_nic_devname(gc, domid, + nic->devid, + nic->nictype); + } + + return vifname; +} + +static char **get_guest_vif_list(libxl__gc *gc, uint32_t domid, + int *num_vifs) +{ + libxl_device_nic *nics; + int nb, i = 0; + char **vif_list = NULL; + + nics = libxl_device_nic_list(CTX, domid, &nb); + if (!nics) { + *num_vifs = 0; + return NULL; + } + + vif_list = libxl__calloc(gc, nb, sizeof(char *)); + for (i = 0; i < nb; ++i) { + vif_list[i] = get_vifname(gc, domid, &nics[i]); + libxl_device_nic_dispose(&nics[i]); + } + free(nics); + + *num_vifs = nb; + return vif_list; +} + +static void netbuf_script_child_death_cb(libxl__egc *egc, + libxl__ev_child *child, + pid_t pid, int status) +{ + /* No-op. hotplug-error will be read in setup/teardown_ifb */ + return; +} + +/* the script needs the following env & args + * $vifname + * $XENBUS_PATH (/libxl/<domid>/remus/netbuf/<devid>/) + * setup/teardown as command line arg. + * In return, the script writes the name of IFB device to be used for + * output buffering into XENBUS_PATH/ifb + * + * The setup and teardown ops are synchronous, as they are executed during + * the context of an asynchronous API call (libxl_domain_remus_start). + */ +static int libxl__exec_netbuf_script(libxl__gc *gc, uint32_t domid, + int devid, char *vif, char *script, + char *op) +{ + int arraysize, nr; + char **env = NULL, **args = NULL; + pid_t pid; + libxl__ev_child childw_out; + + arraysize = 5; nr = 0; + GCNEW_ARRAY(env, arraysize); + env[nr++] = "vifname"; + env[nr++] = vif; + env[nr++] = "XENBUS_PATH"; + env[nr++] = GCSPRINTF("%s/remus/netbuf/%d", + libxl__xs_libxl_path(gc, domid), devid); + env[nr++] = NULL; + assert(nr <= arraysize); + + arraysize = 3; nr = 0; + GCNEW_ARRAY(args, arraysize); + args[nr++] = script; + args[nr++] = op; + args[nr++] = NULL; + assert(nr == arraysize); + + libxl__ev_child_init(&childw_out); + LOG(DEBUG, "Calling netbuf script: %s %s", script, op); + + /* Fork and exec netbuf script */ + pid = libxl__ev_child_fork(gc, &childw_out, netbuf_script_child_death_cb); + if (pid == -1) { + LOG(ERROR, "unable to fork netbuf script %s", script); + return ERROR_FAIL; + } + + if (!pid) { + /* child: Launch netbuf script */ + libxl__exec(gc, -1, -1, -1, args[0], args, env); + /* notreached */ + abort(); + } + + assert(libxl__ev_child_inuse(&childw_out)); + + return 0; +} + +static int script_done_cb(libxl__gc *gc, uint32_t domid, + const char *state, void *userdata) +{ + return 0; /* No-op callback */ +} + +static int libxl__netbuf_script_setup(libxl__gc *gc, uint32_t domid, + int devid, char *vif, char *script, + char **ifb) +{ + int rc; + char *out_path_base, *hotplug_error; + + rc = libxl__exec_netbuf_script(gc, domid, devid, vif, + script, "setup"); + if (rc) return rc; + + out_path_base = GCSPRINTF("%s/remus/netbuf/%d", + libxl__xs_libxl_path(gc, domid), devid); + + /* Now wait for the result (XENBUS_PATH/hotplug-status). + * It doesnt matter what the result is. If the hotplug-status path + * appears, then we are good to go. + */ + rc = libxl__wait_for_offspring(gc, domid, LIBXL_HOTPLUG_TIMEOUT, + script, + /* path */ + GCSPRINTF("%s/hotplug-status", + out_path_base), + NULL /* state */, + NULL, script_done_cb, NULL); + if (rc) { + LOG(ERROR, "unable to wait for %s setup to complete", script); + return ERROR_FAIL; + } + + hotplug_error = libxl__xs_read(gc, XBT_NULL, + GCSPRINTF("%s/hotplug-error", + out_path_base)); + if (hotplug_error) { + LOG(ERROR, "netbuf script %s setup failed: %s", + script, hotplug_error); + return ERROR_FAIL; + } + + *ifb = libxl__xs_read(gc, XBT_NULL, + GCSPRINTF("%s/remus/netbuf/%d/ifb", + libxl__xs_libxl_path(gc, domid), + devid)); + if (!(*ifb)) { + LOG(ERROR, "Cannot get ifb dev name for domain %u dev %s", + domid, vif); + return ERROR_FAIL; + } + + return 0; +} + +static int libxl__netbuf_script_teardown(libxl__gc *gc, uint32_t domid, + int devid, char *vif, + char *script) +{ + /* Nothing to wait for during tear down */ + return libxl__exec_netbuf_script(gc, domid, devid, vif, + script, "teardown"); +} + +/* Scan through the list of vifs belonging to domid and + * invoke the netbufscript to setup the IFB device & plug qdisc + * for each vif. Then scan through the list of IFB devices to obtain + * a handle on the plug qdisc installed on these IFB devices. + * Network output buffering is controlled via these qdiscs. + */ +int libxl__remus_netbuf_setup(libxl__gc *gc, uint32_t domid, + libxl__remus_ctx *remus_ctx) +{ + int i, ret, ifindex, num_netbufs = 0; + struct rtnl_link *ifb = NULL; + struct rtnl_qdisc *qdisc = NULL; + libxl__remus_netbuf_ctx *netbuf_ctx = NULL; + + /* If the domain backend is a stubdom, do nothing. We dont + * support stubdom network buffering yet. + */ + if (libxl_get_stubdom_id(CTX, domid)) { + LOG(ERROR, "Network buffering is not supported with stubdoms"); + return ERROR_FAIL; + } + + netbuf_ctx = libxl__zalloc(gc, sizeof(libxl__remus_netbuf_ctx)); + netbuf_ctx->vif_list = get_guest_vif_list(gc, domid, &num_netbufs); + if (!num_netbufs) return 0; + + netbuf_ctx->ifb_list = libxl__calloc(gc, num_netbufs, + sizeof(char *)); + + /* convenience vars */ + char **vif_list = netbuf_ctx->vif_list; + char **ifb_list = netbuf_ctx->ifb_list; + + for (i = 0; i < num_netbufs; ++i) { + + /* The setup script does the following + * find a free IFB to act as buffer for the vif. + * set up the plug qdisc on the IFB. + */ + ret = libxl__netbuf_script_setup(gc, domid, i, vif_list[i], + (char *) remus_ctx->netbufscript, + &ifb_list[i]); + if (ret) { + LOG(ERROR, "Failed to setup ifb device for dev %s", + vif_list[i]); + return ERROR_FAIL; + } + + LOG(DEBUG, "dev %s will be buffered at ifb %s", vif_list[i], + ifb_list[i]); + } + + /* Now that we have brought up IFB devices with plug qdisc for + * each vif, lets get a netlink handle on the plug qdisc for use + * during checkpointing. + */ + netbuf_ctx->nlsock = nl_socket_alloc(); + if (!netbuf_ctx->nlsock) { + LOG(ERROR, "cannot allocate nl socket"); + return ERROR_FAIL; + } + + ret = nl_connect(netbuf_ctx->nlsock, NETLINK_ROUTE); + if (ret) { + LOG(ERROR, "failed to open netlink socket: %s", + nl_geterror(ret)); + return ERROR_FAIL; + } + + /* get list of all qdiscs installed on network devs. */ + ret = rtnl_qdisc_alloc_cache(netbuf_ctx->nlsock, + &netbuf_ctx->qdisc_cache); + if (ret) { + LOG(ERROR, "failed to allocate qdisc cache: %s", + nl_geterror(ret)); + goto end; + } + + /* list of handles to plug qdiscs */ + netbuf_ctx->netbuf_qdisc_list = + libxl__calloc(gc, num_netbufs, + sizeof(struct rtnl_qdisc *)); + + ifb = rtnl_link_alloc(); + + for (i = 0; i < num_netbufs; ++i) { + + /* get a handle to the IFB interface */ + ret = rtnl_link_get_kernel(netbuf_ctx->nlsock, 0, + ifb_list[i], &ifb); + if (ret) { + LOG(ERROR, "cannot obtain handle for %s: %s", ifb_list[i], + nl_geterror(ret)); + goto end; + } + + ifindex = rtnl_link_get_ifindex(ifb); + if (!ifindex) { + LOG(ERROR, "interface %s has no index", ifb_list[i]); + goto end; + } + + /* Get a reference to the root qdisc installed on the IFB, by + * querying the qdisc list we obtained earlier. The netbufscript + * sets up the plug qdisc as the root qdisc, so we dont have to + * search the entire qdisc tree on the IFB dev. + + * There is no need to explicitly free this qdisc as its just a + * reference from the qdisc cache we allocated earlier. + */ + qdisc = rtnl_qdisc_get_by_parent(netbuf_ctx->qdisc_cache, ifindex, + TC_H_ROOT); + + if (qdisc) { + const char *tc_kind = rtnl_tc_get_kind(TC_CAST(qdisc)); + /* Sanity check: Ensure that the root qdisc is a plug qdisc. */ + if (!tc_kind || strcmp(tc_kind, "plug")) { + LOG(ERROR, "plug qdisc is not installed on %s", ifb_list[i]); + goto end; + } + netbuf_ctx->netbuf_qdisc_list[i] = qdisc; + } else { + LOG(ERROR, "Cannot get qdisc handle from ifb %s", ifb_list[i]); + goto end; + } + } + + rtnl_link_put(ifb); + + netbuf_ctx->num_netbufs = num_netbufs; + + /* Update remus_ctx to point to the newly setup netbuffer context */ + remus_ctx->netbuf_ctx = netbuf_ctx; + return 0; + + end: + if (ifb) rtnl_link_put(ifb); + if (netbuf_ctx->qdisc_cache) nl_cache_free(netbuf_ctx->qdisc_cache); + if (netbuf_ctx->nlsock) nl_close(netbuf_ctx->nlsock); + return ERROR_FAIL; +} + +/* Note: This function will be called in the same gc context as + * libxl__remus_netbuf_setup, created during the libxl_domain_remus_start + * API call. + */ +int libxl__remus_netbuf_teardown(libxl__gc *gc, uint32_t domid, + libxl__remus_ctx *remus_ctx) +{ + libxl__remus_netbuf_ctx *netbuf_ctx = remus_ctx->netbuf_ctx; + char **vif_list = NULL, **ifb_list = NULL; + int i; + + if (!(remus_ctx && netbuf_ctx && netbuf_ctx->num_netbufs)) + return 0; + + nl_cache_free(netbuf_ctx->qdisc_cache); + nl_close(netbuf_ctx->nlsock); + + vif_list = netbuf_ctx->vif_list; + ifb_list = netbuf_ctx->ifb_list; + + for (i = 0; i < netbuf_ctx->num_netbufs; ++i) + libxl__netbuf_script_teardown(gc, domid, i, vif_list[i], + (char *) remus_ctx->netbufscript); + + return 0; +} + +#define TC_BUFFER_START 1 +#define TC_BUFFER_RELEASE 2 +static int remus_netbuf_op(libxl__gc *gc, uint32_t domid, + libxl__remus_ctx *remus_ctx, + int buffer_op) +{ + int i, ret; + libxl__remus_netbuf_ctx *netbuf_ctx = remus_ctx->netbuf_ctx; + + for (i = 0; i < netbuf_ctx->num_netbufs; ++i) { + if (buffer_op == TC_BUFFER_START) + ret = rtnl_qdisc_plug_buffer(netbuf_ctx->netbuf_qdisc_list[i]); + else + ret = rtnl_qdisc_plug_release_one(netbuf_ctx->netbuf_qdisc_list[i]); + + if (!ret) + ret = rtnl_qdisc_add(netbuf_ctx->nlsock, + netbuf_ctx->netbuf_qdisc_list[i], + NLM_F_REQUEST); + if (ret) { + LOG(ERROR, "Remus: cannot do netbuf op %s on %s:%s", + ((buffer_op == TC_BUFFER_START) ? + "start_new_epoch" : "release_prev_epoch"), + netbuf_ctx->ifb_list[i], nl_geterror(ret)); + return ERROR_FAIL; + } + } + + return 0; +} + +int libxl__remus_netbuf_start_new_epoch(libxl__gc *gc, uint32_t domid, + libxl__remus_ctx *remus_ctx) +{ + return remus_netbuf_op(gc, domid, remus_ctx, TC_BUFFER_START); +} + +int libxl__remus_netbuf_release_prev_epoch(libxl__gc *gc, uint32_t domid, + libxl__remus_ctx *remus_ctx) +{ + return remus_netbuf_op(gc, domid, remus_ctx, TC_BUFFER_RELEASE); +} + +/* + * Local variables: + * mode: C + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff -r 4d24d36d30c3 -r 4b23104828c0 tools/libxl/libxl_nonetbuffer.c --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tools/libxl/libxl_nonetbuffer.c Sun Aug 25 16:33:29 2013 -0700 @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2013 + * Author Shriram Rajagopalan <rshriram@cs.ubc.ca> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation; version 2.1 only. with the special + * exception on linking described in file 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 Lesser General Public License for more details. + */ + +#include "libxl_osdeps.h" /* must come before any other headers */ + +#include "libxl_internal.h" + +/* Remus network buffer related stubs */ +int libxl__remus_netbuf_setup(libxl__gc *gc, uint32_t domid, + libxl__remus_ctx *remus_ctx) +{ + LOG(ERROR, "Remus: No support for network buffering"); + return ERROR_FAIL; +} + +int libxl__remus_netbuf_teardown(libxl__gc *gc, uint32_t domid, + libxl__remus_ctx *remus_ctx) +{ + return 0; /* No point complaining in a teardown call. */ +} + +int libxl__remus_netbuf_start_new_epoch(libxl__gc *gc, uint32_t domid, + libxl__remus_ctx *remus_ctx) +{ + LOG(ERROR, "Remus: No support for network buffering"); + return ERROR_FAIL; +} + +int libxl__remus_netbuf_release_prev_epoch(libxl__gc *gc, uint32_t domid, + libxl__remus_ctx *remus_ctx) +{ + LOG(ERROR, "Remus: No support for network buffering"); + return ERROR_FAIL; +} + +/* + * Local variables: + * mode: C + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff -r 4d24d36d30c3 -r 4b23104828c0 tools/libxl/libxl_types.idl --- a/tools/libxl/libxl_types.idl Sun Aug 25 16:33:27 2013 -0700 +++ b/tools/libxl/libxl_types.idl Sun Aug 25 16:33:29 2013 -0700 @@ -526,6 +526,8 @@ libxl_domain_remus_info = Struct("domain ("interval", integer), ("blackhole", bool), ("compression", bool), + ("netbuf", bool), + ("netbufscript", string), ]) libxl_event_type = Enumeration("event_type", [ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3 of 5] tools/libxl: setup/teardown Remus network buffering 2013-08-25 23:45 ` [PATCH 3 of 5] tools/libxl: setup/teardown Remus network buffering rshriram @ 2013-08-26 22:13 ` Andrew Cooper 0 siblings, 0 replies; 14+ messages in thread From: Andrew Cooper @ 2013-08-26 22:13 UTC (permalink / raw) To: rshriram; +Cc: stefano.stabellini, ian.jackson, ian.campbell, xen-devel On 26/08/2013 00:45, rshriram@cs.ubc.ca wrote: > # HG changeset patch > # User Shriram Rajagopalan <rshriram@cs.ubc.ca> > # Date 1377473609 25200 > # Node ID 4b23104828c09218aa7f9fbde1578bb6706e13d6 > # Parent 4d24d36d30c37e4fb5985952db92c1e789c90894 > tools/libxl: setup/teardown Remus network buffering > > Setup/teardown remus network buffering for a given guest, when > libxl_domain_remus_start API is invoked. > > This patch does the following during setup: > a) call the hotplug script for each vif to setup its network buffer > > b) establish a dedicated remus context containing libnl related > state (netlink sockets, qdisc caches, etc.,) > > c) Obtain handles to plug qdiscs installed on the IFB devices > chosen by the hotplug scripts. > > During teardown, the hotplug scripts are called to remove the IFB > devices. This is followed by releasing netlink resources. > > Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> > > diff -r 4d24d36d30c3 -r 4b23104828c0 tools/libxl/Makefile > --- a/tools/libxl/Makefile Sun Aug 25 16:33:27 2013 -0700 > +++ b/tools/libxl/Makefile Sun Aug 25 16:33:29 2013 -0700 > @@ -40,6 +40,13 @@ LIBXL_OBJS-y += libxl_blktap2.o > else > LIBXL_OBJS-y += libxl_noblktap2.o > endif > + > +ifeq ($(CONFIG_REMUS_NETBUF),y) > +LIBXL_OBJS-y += libxl_netbuffer.o > +else > +LIBXL_OBJS-y += libxl_nonetbuffer.o > +endif > + > LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o > LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpuid.o libxl_noarch.o > LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_noarch.o > diff -r 4d24d36d30c3 -r 4b23104828c0 tools/libxl/libxl.c > --- a/tools/libxl/libxl.c Sun Aug 25 16:33:27 2013 -0700 > +++ b/tools/libxl/libxl.c Sun Aug 25 16:33:29 2013 -0700 > @@ -692,8 +692,9 @@ libxl_vminfo * libxl_list_vm(libxl_ctx * > return ptr; > } > > -static void remus_failover_cb(libxl__egc *egc, > - libxl__domain_suspend_state *dss, int rc); > +static void remus_replication_failure_cb(libxl__egc *egc, > + libxl__domain_suspend_state *dss, > + int rc); > > /* TODO: Explicit Checkpoint acknowledgements via recv_fd. */ > int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info, > @@ -712,18 +713,44 @@ int libxl_domain_remus_start(libxl_ctx * > > GCNEW(dss); > dss->ao = ao; > - dss->callback = remus_failover_cb; > + dss->callback = remus_replication_failure_cb; > dss->domid = domid; > dss->fd = send_fd; > /* TODO do something with recv_fd */ > dss->type = type; > dss->live = 1; > dss->debug = 0; > - dss->remus = info; > > assert(info); > > - /* TBD: Remus setup - i.e. attach qdisc, enable disk buffering, etc */ > + GCNEW(dss->remus_ctx); > + > + /* convenience shorthand */ > + libxl__remus_ctx *remus_ctx = dss->remus_ctx; > + remus_ctx->blackhole = info->blackhole; > + remus_ctx->interval = info->interval; > + remus_ctx->compression = info->compression; > + > + /* Setup network buffering before invoking domain_suspend */ > + if (info->netbuf) { > + if (info->netbufscript) { > + remus_ctx->netbufscript = > + libxl__strdup(gc, info->netbufscript); > + } else { > + remus_ctx->netbufscript = > + libxl__sprintf(gc, "%s/remus-netbuf-setup", > + libxl__xen_script_dir_path()); > + } > + > + if (libxl__remus_netbuf_setup(gc, domid, remus_ctx)) { > + LOG(ERROR, "Remus: failed to setup network buffering" > + " for guest with domid %u", domid); > + rc = ERROR_FAIL; > + goto out; > + } > + } > + > + /* TBD: enable disk buffering */ > > /* Point of no return */ > libxl__domain_suspend(egc, dss); > @@ -733,8 +760,9 @@ int libxl_domain_remus_start(libxl_ctx * > return AO_ABORT(rc); > } > > -static void remus_failover_cb(libxl__egc *egc, > - libxl__domain_suspend_state *dss, int rc) > +static void remus_replication_failure_cb(libxl__egc *egc, > + libxl__domain_suspend_state *dss, > + int rc) > { > STATE_AO_GC(dss->ao); > /* > @@ -743,9 +771,10 @@ static void remus_failover_cb(libxl__egc > * from sending checkpoints. > */ > > - /* TBD: Remus cleanup - i.e. detach qdisc, release other > - * resources. > - */ > + /* Teardown the network buffers and release netlink resources. */ > + if (dss->remus_ctx && dss->remus_ctx->netbuf_ctx) > + libxl__remus_netbuf_teardown(gc, dss->domid, dss->remus_ctx); > + > libxl__ao_complete(egc, ao, rc); > } > > diff -r 4d24d36d30c3 -r 4b23104828c0 tools/libxl/libxl_dom.c > --- a/tools/libxl/libxl_dom.c Sun Aug 25 16:33:27 2013 -0700 > +++ b/tools/libxl/libxl_dom.c Sun Aug 25 16:33:29 2013 -0700 > @@ -1259,7 +1259,7 @@ static void remus_checkpoint_dm_saved(li > /* REMUS TODO: Wait for disk and memory ack, release network buffer */ > /* REMUS TODO: make this asynchronous */ > assert(!rc); /* REMUS TODO handle this error properly */ > - usleep(dss->interval * 1000); > + usleep(dss->remus_ctx->interval * 1000); > libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, 1); > } > > @@ -1277,7 +1277,6 @@ void libxl__domain_suspend(libxl__egc *e > const libxl_domain_type type = dss->type; > const int live = dss->live; > const int debug = dss->debug; > - const libxl_domain_remus_info *const r_info = dss->remus; > libxl__srm_save_autogen_callbacks *const callbacks = > &dss->shs.callbacks.save.a; > > @@ -1312,9 +1311,8 @@ void libxl__domain_suspend(libxl__egc *e > dss->guest_responded = 0; > dss->dm_savefile = libxl__device_model_savefile(gc, domid); > > - if (r_info != NULL) { > - dss->interval = r_info->interval; > - if (r_info->compression) > + if (dss->remus_ctx != NULL) { > + if (dss->remus_ctx->compression) This can be reduced to: if (dss->remus_ctx && dss->remus_ctx->compression) ~Andrew > dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS; > } > > @@ -1335,7 +1333,7 @@ void libxl__domain_suspend(libxl__egc *e > } > > memset(callbacks, 0, sizeof(*callbacks)); > - if (r_info != NULL) { > + if (dss->remus_ctx != NULL) { > callbacks->suspend = libxl__remus_domain_suspend_callback; > callbacks->postcopy = libxl__remus_domain_resume_callback; > callbacks->checkpoint = libxl__remus_domain_checkpoint_callback; > diff -r 4d24d36d30c3 -r 4b23104828c0 tools/libxl/libxl_internal.h > --- a/tools/libxl/libxl_internal.h Sun Aug 25 16:33:27 2013 -0700 > +++ b/tools/libxl/libxl_internal.h Sun Aug 25 16:33:29 2013 -0700 > @@ -2242,6 +2242,29 @@ typedef struct libxl__logdirty_switch { > libxl__ev_time timeout; > } libxl__logdirty_switch; > > +typedef struct libxl__remus_ctx { > + /* checkpoint interval */ > + int interval; > + int blackhole; > + int compression; > + /* Script to setup/teardown network buffers */ > + const char *netbufscript; > + /* Opaque context containing network buffer related stuff */ > + void *netbuf_ctx; > +} libxl__remus_ctx; > + > +_hidden int libxl__remus_netbuf_setup(libxl__gc *gc, uint32_t domid, > + libxl__remus_ctx *remus_ctx); > + > +_hidden int libxl__remus_netbuf_teardown(libxl__gc *gc, uint32_t domid, > + libxl__remus_ctx *remus_ctx); > + > +_hidden int libxl__remus_netbuf_start_new_epoch(libxl__gc *gc, uint32_t domid, > + libxl__remus_ctx *remus_ctx); > + > +_hidden int libxl__remus_netbuf_release_prev_epoch(libxl__gc *gc, uint32_t domid, > + libxl__remus_ctx *remus_ctx); > + > struct libxl__domain_suspend_state { > /* set by caller of libxl__domain_suspend */ > libxl__ao *ao; > @@ -2252,7 +2275,7 @@ struct libxl__domain_suspend_state { > libxl_domain_type type; > int live; > int debug; > - const libxl_domain_remus_info *remus; > + libxl__remus_ctx *remus_ctx; > /* private */ > xc_evtchn *xce; /* event channel handle */ > int suspend_eventchn; > @@ -2260,7 +2283,6 @@ struct libxl__domain_suspend_state { > int xcflags; > int guest_responded; > const char *dm_savefile; > - int interval; /* checkpoint interval (for Remus) */ > libxl__save_helper_state shs; > libxl__logdirty_switch logdirty; > /* private for libxl__domain_save_device_model */ > diff -r 4d24d36d30c3 -r 4b23104828c0 tools/libxl/libxl_netbuffer.c > --- /dev/null Thu Jan 01 00:00:00 1970 +0000 > +++ b/tools/libxl/libxl_netbuffer.c Sun Aug 25 16:33:29 2013 -0700 > @@ -0,0 +1,434 @@ > +/* > + * Copyright (C) 2013 > + * Author Shriram Rajagopalan <rshriram@cs.ubc.ca> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU Lesser General Public License as published > + * by the Free Software Foundation; version 2.1 only. with the special > + * exception on linking described in file 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 Lesser General Public License for more details. > + */ > + > +#include "libxl_osdeps.h" /* must come before any other headers */ > + > +#include "libxl_internal.h" > + > +#include <netlink/cache.h> > +#include <netlink/socket.h> > +#include <netlink/attr.h> > +#include <netlink/route/link.h> > +#include <netlink/route/route.h> > +#include <netlink/route/qdisc.h> > +#include <netlink/route/qdisc/plug.h> > + > +typedef struct libxl__remus_netbuf_ctx { > + struct rtnl_qdisc **netbuf_qdisc_list; > + struct nl_sock *nlsock; > + struct nl_cache *qdisc_cache; > + char **vif_list; > + char **ifb_list; > + uint32_t num_netbufs; > + uint32_t unused; > +} libxl__remus_netbuf_ctx; > + > +/* If the device has a vifname, then use that instead of > + * the vifX.Y format. > + */ > +static char *get_vifname(libxl__gc *gc, uint32_t domid, > + libxl_device_nic *nic) > +{ > + char *vifname = NULL; > + vifname = libxl__xs_read(gc, XBT_NULL, > + libxl__sprintf(gc, > + "%s/backend/vif/%d/%d/vifname", > + libxl__xs_get_dompath(gc, 0), > + domid, nic->devid)); > + if (!vifname) { > + vifname = (char *)libxl__device_nic_devname(gc, domid, > + nic->devid, > + nic->nictype); > + } > + > + return vifname; > +} > + > +static char **get_guest_vif_list(libxl__gc *gc, uint32_t domid, > + int *num_vifs) > +{ > + libxl_device_nic *nics; > + int nb, i = 0; > + char **vif_list = NULL; > + > + nics = libxl_device_nic_list(CTX, domid, &nb); > + if (!nics) { > + *num_vifs = 0; > + return NULL; > + } > + > + vif_list = libxl__calloc(gc, nb, sizeof(char *)); > + for (i = 0; i < nb; ++i) { > + vif_list[i] = get_vifname(gc, domid, &nics[i]); > + libxl_device_nic_dispose(&nics[i]); > + } > + free(nics); > + > + *num_vifs = nb; > + return vif_list; > +} > + > +static void netbuf_script_child_death_cb(libxl__egc *egc, > + libxl__ev_child *child, > + pid_t pid, int status) > +{ > + /* No-op. hotplug-error will be read in setup/teardown_ifb */ > + return; > +} > + > +/* the script needs the following env & args > + * $vifname > + * $XENBUS_PATH (/libxl/<domid>/remus/netbuf/<devid>/) > + * setup/teardown as command line arg. > + * In return, the script writes the name of IFB device to be used for > + * output buffering into XENBUS_PATH/ifb > + * > + * The setup and teardown ops are synchronous, as they are executed during > + * the context of an asynchronous API call (libxl_domain_remus_start). > + */ > +static int libxl__exec_netbuf_script(libxl__gc *gc, uint32_t domid, > + int devid, char *vif, char *script, > + char *op) > +{ > + int arraysize, nr; > + char **env = NULL, **args = NULL; > + pid_t pid; > + libxl__ev_child childw_out; > + > + arraysize = 5; nr = 0; > + GCNEW_ARRAY(env, arraysize); > + env[nr++] = "vifname"; > + env[nr++] = vif; > + env[nr++] = "XENBUS_PATH"; > + env[nr++] = GCSPRINTF("%s/remus/netbuf/%d", > + libxl__xs_libxl_path(gc, domid), devid); > + env[nr++] = NULL; > + assert(nr <= arraysize); > + > + arraysize = 3; nr = 0; > + GCNEW_ARRAY(args, arraysize); > + args[nr++] = script; > + args[nr++] = op; > + args[nr++] = NULL; > + assert(nr == arraysize); > + > + libxl__ev_child_init(&childw_out); > + LOG(DEBUG, "Calling netbuf script: %s %s", script, op); > + > + /* Fork and exec netbuf script */ > + pid = libxl__ev_child_fork(gc, &childw_out, netbuf_script_child_death_cb); > + if (pid == -1) { > + LOG(ERROR, "unable to fork netbuf script %s", script); > + return ERROR_FAIL; > + } > + > + if (!pid) { > + /* child: Launch netbuf script */ > + libxl__exec(gc, -1, -1, -1, args[0], args, env); > + /* notreached */ > + abort(); > + } > + > + assert(libxl__ev_child_inuse(&childw_out)); > + > + return 0; > +} > + > +static int script_done_cb(libxl__gc *gc, uint32_t domid, > + const char *state, void *userdata) > +{ > + return 0; /* No-op callback */ > +} > + > +static int libxl__netbuf_script_setup(libxl__gc *gc, uint32_t domid, > + int devid, char *vif, char *script, > + char **ifb) > +{ > + int rc; > + char *out_path_base, *hotplug_error; > + > + rc = libxl__exec_netbuf_script(gc, domid, devid, vif, > + script, "setup"); > + if (rc) return rc; > + > + out_path_base = GCSPRINTF("%s/remus/netbuf/%d", > + libxl__xs_libxl_path(gc, domid), devid); > + > + /* Now wait for the result (XENBUS_PATH/hotplug-status). > + * It doesnt matter what the result is. If the hotplug-status path > + * appears, then we are good to go. > + */ > + rc = libxl__wait_for_offspring(gc, domid, LIBXL_HOTPLUG_TIMEOUT, > + script, > + /* path */ > + GCSPRINTF("%s/hotplug-status", > + out_path_base), > + NULL /* state */, > + NULL, script_done_cb, NULL); > + if (rc) { > + LOG(ERROR, "unable to wait for %s setup to complete", script); > + return ERROR_FAIL; > + } > + > + hotplug_error = libxl__xs_read(gc, XBT_NULL, > + GCSPRINTF("%s/hotplug-error", > + out_path_base)); > + if (hotplug_error) { > + LOG(ERROR, "netbuf script %s setup failed: %s", > + script, hotplug_error); > + return ERROR_FAIL; > + } > + > + *ifb = libxl__xs_read(gc, XBT_NULL, > + GCSPRINTF("%s/remus/netbuf/%d/ifb", > + libxl__xs_libxl_path(gc, domid), > + devid)); > + if (!(*ifb)) { > + LOG(ERROR, "Cannot get ifb dev name for domain %u dev %s", > + domid, vif); > + return ERROR_FAIL; > + } > + > + return 0; > +} > + > +static int libxl__netbuf_script_teardown(libxl__gc *gc, uint32_t domid, > + int devid, char *vif, > + char *script) > +{ > + /* Nothing to wait for during tear down */ > + return libxl__exec_netbuf_script(gc, domid, devid, vif, > + script, "teardown"); > +} > + > +/* Scan through the list of vifs belonging to domid and > + * invoke the netbufscript to setup the IFB device & plug qdisc > + * for each vif. Then scan through the list of IFB devices to obtain > + * a handle on the plug qdisc installed on these IFB devices. > + * Network output buffering is controlled via these qdiscs. > + */ > +int libxl__remus_netbuf_setup(libxl__gc *gc, uint32_t domid, > + libxl__remus_ctx *remus_ctx) > +{ > + int i, ret, ifindex, num_netbufs = 0; > + struct rtnl_link *ifb = NULL; > + struct rtnl_qdisc *qdisc = NULL; > + libxl__remus_netbuf_ctx *netbuf_ctx = NULL; > + > + /* If the domain backend is a stubdom, do nothing. We dont > + * support stubdom network buffering yet. > + */ > + if (libxl_get_stubdom_id(CTX, domid)) { > + LOG(ERROR, "Network buffering is not supported with stubdoms"); > + return ERROR_FAIL; > + } > + > + netbuf_ctx = libxl__zalloc(gc, sizeof(libxl__remus_netbuf_ctx)); > + netbuf_ctx->vif_list = get_guest_vif_list(gc, domid, &num_netbufs); > + if (!num_netbufs) return 0; > + > + netbuf_ctx->ifb_list = libxl__calloc(gc, num_netbufs, > + sizeof(char *)); > + > + /* convenience vars */ > + char **vif_list = netbuf_ctx->vif_list; > + char **ifb_list = netbuf_ctx->ifb_list; > + > + for (i = 0; i < num_netbufs; ++i) { > + > + /* The setup script does the following > + * find a free IFB to act as buffer for the vif. > + * set up the plug qdisc on the IFB. > + */ > + ret = libxl__netbuf_script_setup(gc, domid, i, vif_list[i], > + (char *) remus_ctx->netbufscript, > + &ifb_list[i]); > + if (ret) { > + LOG(ERROR, "Failed to setup ifb device for dev %s", > + vif_list[i]); > + return ERROR_FAIL; > + } > + > + LOG(DEBUG, "dev %s will be buffered at ifb %s", vif_list[i], > + ifb_list[i]); > + } > + > + /* Now that we have brought up IFB devices with plug qdisc for > + * each vif, lets get a netlink handle on the plug qdisc for use > + * during checkpointing. > + */ > + netbuf_ctx->nlsock = nl_socket_alloc(); > + if (!netbuf_ctx->nlsock) { > + LOG(ERROR, "cannot allocate nl socket"); > + return ERROR_FAIL; > + } > + > + ret = nl_connect(netbuf_ctx->nlsock, NETLINK_ROUTE); > + if (ret) { > + LOG(ERROR, "failed to open netlink socket: %s", > + nl_geterror(ret)); > + return ERROR_FAIL; > + } > + > + /* get list of all qdiscs installed on network devs. */ > + ret = rtnl_qdisc_alloc_cache(netbuf_ctx->nlsock, > + &netbuf_ctx->qdisc_cache); > + if (ret) { > + LOG(ERROR, "failed to allocate qdisc cache: %s", > + nl_geterror(ret)); > + goto end; > + } > + > + /* list of handles to plug qdiscs */ > + netbuf_ctx->netbuf_qdisc_list = > + libxl__calloc(gc, num_netbufs, > + sizeof(struct rtnl_qdisc *)); > + > + ifb = rtnl_link_alloc(); > + > + for (i = 0; i < num_netbufs; ++i) { > + > + /* get a handle to the IFB interface */ > + ret = rtnl_link_get_kernel(netbuf_ctx->nlsock, 0, > + ifb_list[i], &ifb); > + if (ret) { > + LOG(ERROR, "cannot obtain handle for %s: %s", ifb_list[i], > + nl_geterror(ret)); > + goto end; > + } > + > + ifindex = rtnl_link_get_ifindex(ifb); > + if (!ifindex) { > + LOG(ERROR, "interface %s has no index", ifb_list[i]); > + goto end; > + } > + > + /* Get a reference to the root qdisc installed on the IFB, by > + * querying the qdisc list we obtained earlier. The netbufscript > + * sets up the plug qdisc as the root qdisc, so we dont have to > + * search the entire qdisc tree on the IFB dev. > + > + * There is no need to explicitly free this qdisc as its just a > + * reference from the qdisc cache we allocated earlier. > + */ > + qdisc = rtnl_qdisc_get_by_parent(netbuf_ctx->qdisc_cache, ifindex, > + TC_H_ROOT); > + > + if (qdisc) { > + const char *tc_kind = rtnl_tc_get_kind(TC_CAST(qdisc)); > + /* Sanity check: Ensure that the root qdisc is a plug qdisc. */ > + if (!tc_kind || strcmp(tc_kind, "plug")) { > + LOG(ERROR, "plug qdisc is not installed on %s", ifb_list[i]); > + goto end; > + } > + netbuf_ctx->netbuf_qdisc_list[i] = qdisc; > + } else { > + LOG(ERROR, "Cannot get qdisc handle from ifb %s", ifb_list[i]); > + goto end; > + } > + } > + > + rtnl_link_put(ifb); > + > + netbuf_ctx->num_netbufs = num_netbufs; > + > + /* Update remus_ctx to point to the newly setup netbuffer context */ > + remus_ctx->netbuf_ctx = netbuf_ctx; > + return 0; > + > + end: > + if (ifb) rtnl_link_put(ifb); > + if (netbuf_ctx->qdisc_cache) nl_cache_free(netbuf_ctx->qdisc_cache); > + if (netbuf_ctx->nlsock) nl_close(netbuf_ctx->nlsock); > + return ERROR_FAIL; > +} > + > +/* Note: This function will be called in the same gc context as > + * libxl__remus_netbuf_setup, created during the libxl_domain_remus_start > + * API call. > + */ > +int libxl__remus_netbuf_teardown(libxl__gc *gc, uint32_t domid, > + libxl__remus_ctx *remus_ctx) > +{ > + libxl__remus_netbuf_ctx *netbuf_ctx = remus_ctx->netbuf_ctx; > + char **vif_list = NULL, **ifb_list = NULL; > + int i; > + > + if (!(remus_ctx && netbuf_ctx && netbuf_ctx->num_netbufs)) > + return 0; > + > + nl_cache_free(netbuf_ctx->qdisc_cache); > + nl_close(netbuf_ctx->nlsock); > + > + vif_list = netbuf_ctx->vif_list; > + ifb_list = netbuf_ctx->ifb_list; > + > + for (i = 0; i < netbuf_ctx->num_netbufs; ++i) > + libxl__netbuf_script_teardown(gc, domid, i, vif_list[i], > + (char *) remus_ctx->netbufscript); > + > + return 0; > +} > + > +#define TC_BUFFER_START 1 > +#define TC_BUFFER_RELEASE 2 > +static int remus_netbuf_op(libxl__gc *gc, uint32_t domid, > + libxl__remus_ctx *remus_ctx, > + int buffer_op) > +{ > + int i, ret; > + libxl__remus_netbuf_ctx *netbuf_ctx = remus_ctx->netbuf_ctx; > + > + for (i = 0; i < netbuf_ctx->num_netbufs; ++i) { > + if (buffer_op == TC_BUFFER_START) > + ret = rtnl_qdisc_plug_buffer(netbuf_ctx->netbuf_qdisc_list[i]); > + else > + ret = rtnl_qdisc_plug_release_one(netbuf_ctx->netbuf_qdisc_list[i]); > + > + if (!ret) > + ret = rtnl_qdisc_add(netbuf_ctx->nlsock, > + netbuf_ctx->netbuf_qdisc_list[i], > + NLM_F_REQUEST); > + if (ret) { > + LOG(ERROR, "Remus: cannot do netbuf op %s on %s:%s", > + ((buffer_op == TC_BUFFER_START) ? > + "start_new_epoch" : "release_prev_epoch"), > + netbuf_ctx->ifb_list[i], nl_geterror(ret)); > + return ERROR_FAIL; > + } > + } > + > + return 0; > +} > + > +int libxl__remus_netbuf_start_new_epoch(libxl__gc *gc, uint32_t domid, > + libxl__remus_ctx *remus_ctx) > +{ > + return remus_netbuf_op(gc, domid, remus_ctx, TC_BUFFER_START); > +} > + > +int libxl__remus_netbuf_release_prev_epoch(libxl__gc *gc, uint32_t domid, > + libxl__remus_ctx *remus_ctx) > +{ > + return remus_netbuf_op(gc, domid, remus_ctx, TC_BUFFER_RELEASE); > +} > + > +/* > + * Local variables: > + * mode: C > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff -r 4d24d36d30c3 -r 4b23104828c0 tools/libxl/libxl_nonetbuffer.c > --- /dev/null Thu Jan 01 00:00:00 1970 +0000 > +++ b/tools/libxl/libxl_nonetbuffer.c Sun Aug 25 16:33:29 2013 -0700 > @@ -0,0 +1,54 @@ > +/* > + * Copyright (C) 2013 > + * Author Shriram Rajagopalan <rshriram@cs.ubc.ca> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU Lesser General Public License as published > + * by the Free Software Foundation; version 2.1 only. with the special > + * exception on linking described in file 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 Lesser General Public License for more details. > + */ > + > +#include "libxl_osdeps.h" /* must come before any other headers */ > + > +#include "libxl_internal.h" > + > +/* Remus network buffer related stubs */ > +int libxl__remus_netbuf_setup(libxl__gc *gc, uint32_t domid, > + libxl__remus_ctx *remus_ctx) > +{ > + LOG(ERROR, "Remus: No support for network buffering"); > + return ERROR_FAIL; > +} > + > +int libxl__remus_netbuf_teardown(libxl__gc *gc, uint32_t domid, > + libxl__remus_ctx *remus_ctx) > +{ > + return 0; /* No point complaining in a teardown call. */ > +} > + > +int libxl__remus_netbuf_start_new_epoch(libxl__gc *gc, uint32_t domid, > + libxl__remus_ctx *remus_ctx) > +{ > + LOG(ERROR, "Remus: No support for network buffering"); > + return ERROR_FAIL; > +} > + > +int libxl__remus_netbuf_release_prev_epoch(libxl__gc *gc, uint32_t domid, > + libxl__remus_ctx *remus_ctx) > +{ > + LOG(ERROR, "Remus: No support for network buffering"); > + return ERROR_FAIL; > +} > + > +/* > + * Local variables: > + * mode: C > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff -r 4d24d36d30c3 -r 4b23104828c0 tools/libxl/libxl_types.idl > --- a/tools/libxl/libxl_types.idl Sun Aug 25 16:33:27 2013 -0700 > +++ b/tools/libxl/libxl_types.idl Sun Aug 25 16:33:29 2013 -0700 > @@ -526,6 +526,8 @@ libxl_domain_remus_info = Struct("domain > ("interval", integer), > ("blackhole", bool), > ("compression", bool), > + ("netbuf", bool), > + ("netbufscript", string), > ]) > > libxl_event_type = Enumeration("event_type", [ > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4 of 5] tools/libxl: Control network buffering in remus callbacks 2013-08-25 23:45 [PATCH 0 of 5] Remus/Libxl: Network buffering support rshriram ` (2 preceding siblings ...) 2013-08-25 23:45 ` [PATCH 3 of 5] tools/libxl: setup/teardown Remus network buffering rshriram @ 2013-08-25 23:45 ` rshriram 2013-08-26 22:28 ` Andrew Cooper 2013-08-25 23:45 ` [PATCH 5 of 5] tools/xl: Remus - Network buffering cmdline switch rshriram 4 siblings, 1 reply; 14+ messages in thread From: rshriram @ 2013-08-25 23:45 UTC (permalink / raw) To: xen-devel; +Cc: ian.jackson, ian.campbell, stefano.stabellini # HG changeset patch # User Shriram Rajagopalan <rshriram@cs.ubc.ca> # Date 1377473611 25200 # Node ID c6804ccfe660cb9c373c5f53a8996d0443316684 # Parent 4b23104828c09218aa7f9fbde1578bb6706e13d6 tools/libxl: Control network buffering in remus callbacks This patch constitutes the core network buffering logic. and does the following: a) create a new network buffer when the domain is suspended (remus_domain_suspend_callback) b) release the previous network buffer pertaining to the committed checkpoint (remus_domain_checkpoint_dm_saved) Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> diff -r 4b23104828c0 -r c6804ccfe660 tools/libxl/libxl_dom.c --- a/tools/libxl/libxl_dom.c Sun Aug 25 16:33:29 2013 -0700 +++ b/tools/libxl/libxl_dom.c Sun Aug 25 16:33:31 2013 -0700 @@ -1213,12 +1213,28 @@ int libxl__toolstack_save(uint32_t domid /*----- remus callbacks -----*/ +/* REMUS TODO: Issue disk checkpoint reqs. */ static int libxl__remus_domain_suspend_callback(void *data) { - /* REMUS TODO: Issue disk and network checkpoint reqs. */ - return libxl__domain_suspend_common_callback(data); + libxl__save_helper_state *shs = data; + libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs); + libxl__remus_ctx *remus_ctx = dss->remus_ctx; + int is_suspended = 0; + STATE_AO_GC(dss->ao); + + is_suspended = libxl__domain_suspend_common_callback(data); + if (!remus_ctx->netbuf_ctx) return is_suspended; + + if (is_suspended) { + if (libxl__remus_netbuf_start_new_epoch(gc, dss->domid, + remus_ctx)) + return !is_suspended; + } + + return is_suspended; } +/* REMUS TODO: Deal with disk. */ static int libxl__remus_domain_resume_callback(void *data) { libxl__save_helper_state *shs = data; @@ -1229,7 +1245,6 @@ static int libxl__remus_domain_resume_ca if (libxl__domain_resume(gc, dss->domid, /* Fast Suspend */1)) return 0; - /* REMUS TODO: Deal with disk. Start a new network output buffer */ return 1; } @@ -1256,10 +1271,34 @@ static void libxl__remus_domain_checkpoi static void remus_checkpoint_dm_saved(libxl__egc *egc, libxl__domain_suspend_state *dss, int rc) { - /* REMUS TODO: Wait for disk and memory ack, release network buffer */ - /* REMUS TODO: make this asynchronous */ - assert(!rc); /* REMUS TODO handle this error properly */ - usleep(dss->remus_ctx->interval * 1000); + /* REMUS TODO: Wait for disk and explicit memory ack (through restore + callback from remote) before releasing network buffer. */ + libxl__remus_ctx *remus_ctx = dss->remus_ctx; + struct timespec epoch; + int ret; + STATE_AO_GC(dss->ao); + + if (rc) { + LOG(ERROR, "Failed to save device model. Terminating Remus.."); + libxl__xc_domain_saverestore_async_callback_done(egc, + &dss->shs, rc); + return; + } + + if (remus_ctx->netbuf_ctx) { + ret = libxl__remus_netbuf_release_prev_epoch(gc, dss->domid, + remus_ctx); + if (ret) { + libxl__xc_domain_saverestore_async_callback_done(egc, + &dss->shs, + ret); + return; + } + } + + epoch.tv_sec = remus_ctx->interval / 1000; /* interval is in ms */ + epoch.tv_nsec = remus_ctx->interval * 1000L * 1000L; + nanosleep(&epoch, 0); libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, 1); } ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4 of 5] tools/libxl: Control network buffering in remus callbacks 2013-08-25 23:45 ` [PATCH 4 of 5] tools/libxl: Control network buffering in remus callbacks rshriram @ 2013-08-26 22:28 ` Andrew Cooper 2013-08-29 21:49 ` Shriram Rajagopalan 0 siblings, 1 reply; 14+ messages in thread From: Andrew Cooper @ 2013-08-26 22:28 UTC (permalink / raw) To: rshriram; +Cc: stefano.stabellini, ian.jackson, ian.campbell, xen-devel On 26/08/2013 00:45, rshriram@cs.ubc.ca wrote: > # HG changeset patch > # User Shriram Rajagopalan <rshriram@cs.ubc.ca> > # Date 1377473611 25200 > # Node ID c6804ccfe660cb9c373c5f53a8996d0443316684 > # Parent 4b23104828c09218aa7f9fbde1578bb6706e13d6 > tools/libxl: Control network buffering in remus callbacks > > This patch constitutes the core network buffering logic. > and does the following: > a) create a new network buffer when the domain is suspended > (remus_domain_suspend_callback) > b) release the previous network buffer pertaining to the > committed checkpoint (remus_domain_checkpoint_dm_saved) > > Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> > > diff -r 4b23104828c0 -r c6804ccfe660 tools/libxl/libxl_dom.c > --- a/tools/libxl/libxl_dom.c Sun Aug 25 16:33:29 2013 -0700 > +++ b/tools/libxl/libxl_dom.c Sun Aug 25 16:33:31 2013 -0700 > @@ -1213,12 +1213,28 @@ int libxl__toolstack_save(uint32_t domid > > /*----- remus callbacks -----*/ > > +/* REMUS TODO: Issue disk checkpoint reqs. */ Why does this comment need to move? > static int libxl__remus_domain_suspend_callback(void *data) > { > - /* REMUS TODO: Issue disk and network checkpoint reqs. */ > - return libxl__domain_suspend_common_callback(data); > + libxl__save_helper_state *shs = data; > + libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs); > + libxl__remus_ctx *remus_ctx = dss->remus_ctx; > + int is_suspended = 0; > + STATE_AO_GC(dss->ao); > + > + is_suspended = libxl__domain_suspend_common_callback(data); > + if (!remus_ctx->netbuf_ctx) split to new line please. > return is_suspended; > + > + if (is_suspended) { > + if (libxl__remus_netbuf_start_new_epoch(gc, dss->domid, > + remus_ctx)) > + return !is_suspended; > + } > + > + return is_suspended; is_suspended is logically a boolean, so should be bool_t. Unhelpfully, libxl__domain_suspend_common_callback() returns an int, although its implementation strictly returns 0 on failure and 1 on success. IMO, It would probably be best to have "bool_t is_suspended" set to "!!libxl__domain_suspend_common_callback()", at which the subsequent return !is_suspended doesn't look as suspect, although that is just a matter of style. ~Andrew > } > > +/* REMUS TODO: Deal with disk. */ > static int libxl__remus_domain_resume_callback(void *data) > { > libxl__save_helper_state *shs = data; > @@ -1229,7 +1245,6 @@ static int libxl__remus_domain_resume_ca > if (libxl__domain_resume(gc, dss->domid, /* Fast Suspend */1)) > return 0; > > - /* REMUS TODO: Deal with disk. Start a new network output buffer */ > return 1; > } > > @@ -1256,10 +1271,34 @@ static void libxl__remus_domain_checkpoi > static void remus_checkpoint_dm_saved(libxl__egc *egc, > libxl__domain_suspend_state *dss, int rc) > { > - /* REMUS TODO: Wait for disk and memory ack, release network buffer */ > - /* REMUS TODO: make this asynchronous */ > - assert(!rc); /* REMUS TODO handle this error properly */ > - usleep(dss->remus_ctx->interval * 1000); > + /* REMUS TODO: Wait for disk and explicit memory ack (through restore > + callback from remote) before releasing network buffer. */ > + libxl__remus_ctx *remus_ctx = dss->remus_ctx; > + struct timespec epoch; > + int ret; > + STATE_AO_GC(dss->ao); > + > + if (rc) { > + LOG(ERROR, "Failed to save device model. Terminating Remus.."); > + libxl__xc_domain_saverestore_async_callback_done(egc, > + &dss->shs, rc); > + return; > + } > + > + if (remus_ctx->netbuf_ctx) { > + ret = libxl__remus_netbuf_release_prev_epoch(gc, dss->domid, > + remus_ctx); > + if (ret) { > + libxl__xc_domain_saverestore_async_callback_done(egc, > + &dss->shs, > + ret); > + return; > + } > + } > + > + epoch.tv_sec = remus_ctx->interval / 1000; /* interval is in ms */ > + epoch.tv_nsec = remus_ctx->interval * 1000L * 1000L; > + nanosleep(&epoch, 0); > libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, 1); > } > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4 of 5] tools/libxl: Control network buffering in remus callbacks 2013-08-26 22:28 ` Andrew Cooper @ 2013-08-29 21:49 ` Shriram Rajagopalan 2013-08-30 8:42 ` Andrew Cooper 0 siblings, 1 reply; 14+ messages in thread From: Shriram Rajagopalan @ 2013-08-29 21:49 UTC (permalink / raw) To: Andrew Cooper Cc: Stefano Stabellini, Ian Jackson, Ian Campbell, xen-devel@lists.xen.org [-- Attachment #1.1: Type: text/plain, Size: 2594 bytes --] On Mon, Aug 26, 2013 at 6:28 PM, Andrew Cooper <andrew.cooper3@citrix.com>wrote: > On 26/08/2013 00:45, rshriram@cs.ubc.ca wrote: > > # HG changeset patch > > # User Shriram Rajagopalan <rshriram@cs.ubc.ca> > > # Date 1377473611 25200 > > # Node ID c6804ccfe660cb9c373c5f53a8996d0443316684 > > # Parent 4b23104828c09218aa7f9fbde1578bb6706e13d6 > > tools/libxl: Control network buffering in remus callbacks > > > > This patch constitutes the core network buffering logic. > > and does the following: > > a) create a new network buffer when the domain is suspended > > (remus_domain_suspend_callback) > > b) release the previous network buffer pertaining to the > > committed checkpoint (remus_domain_checkpoint_dm_saved) > > > > Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> > > > > diff -r 4b23104828c0 -r c6804ccfe660 tools/libxl/libxl_dom.c > > --- a/tools/libxl/libxl_dom.c Sun Aug 25 16:33:29 2013 -0700 > > +++ b/tools/libxl/libxl_dom.c Sun Aug 25 16:33:31 2013 -0700 > > @@ -1213,12 +1213,28 @@ int libxl__toolstack_save(uint32_t domid > > > > /*----- remus callbacks -----*/ > > > > +/* REMUS TODO: Issue disk checkpoint reqs. */ > > Why does this comment need to move? > > > static int libxl__remus_domain_suspend_callback(void *data) > > { > > - /* REMUS TODO: Issue disk and network checkpoint reqs. */ > > - return libxl__domain_suspend_common_callback(data); > > + libxl__save_helper_state *shs = data; > > + libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs); > > + libxl__remus_ctx *remus_ctx = dss->remus_ctx; > > + int is_suspended = 0; > > + STATE_AO_GC(dss->ao); > > + > > + is_suspended = libxl__domain_suspend_common_callback(data); > > + if (!remus_ctx->netbuf_ctx) > > split to new line please. > > > return is_suspended; > > + > > + if (is_suspended) { > > + if (libxl__remus_netbuf_start_new_epoch(gc, dss->domid, > > + remus_ctx)) > > + return !is_suspended; > > + } > > + > > + return is_suspended; > > is_suspended is logically a boolean, so should be bool_t. Unhelpfully, > libxl__domain_suspend_common_callback() returns an int, although its > implementation strictly returns 0 on failure and 1 on success. > > IMO, It would probably be best to have "bool_t is_suspended" set to > "!!libxl__domain_suspend_common_callback()", at which the subsequent > return !is_suspended doesn't look as suspect, although that is just a > matter of style. > > Did you mean bool (not bool_t) ? Because I didnt see a typedef in libxl folder. [-- Attachment #1.2: Type: text/html, Size: 3549 bytes --] [-- Attachment #2: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4 of 5] tools/libxl: Control network buffering in remus callbacks 2013-08-29 21:49 ` Shriram Rajagopalan @ 2013-08-30 8:42 ` Andrew Cooper 0 siblings, 0 replies; 14+ messages in thread From: Andrew Cooper @ 2013-08-30 8:42 UTC (permalink / raw) To: rshriram Cc: Stefano Stabellini, Ian Jackson, Ian Campbell, xen-devel@lists.xen.org [-- Attachment #1.1: Type: text/plain, Size: 3131 bytes --] On 29/08/13 22:49, Shriram Rajagopalan wrote: > On Mon, Aug 26, 2013 at 6:28 PM, Andrew Cooper > <andrew.cooper3@citrix.com <mailto:andrew.cooper3@citrix.com>> wrote: > > On 26/08/2013 00:45, rshriram@cs.ubc.ca > <mailto:rshriram@cs.ubc.ca> wrote: > > # HG changeset patch > > # User Shriram Rajagopalan <rshriram@cs.ubc.ca > <mailto:rshriram@cs.ubc.ca>> > > # Date 1377473611 25200 > > # Node ID c6804ccfe660cb9c373c5f53a8996d0443316684 > > # Parent 4b23104828c09218aa7f9fbde1578bb6706e13d6 > > tools/libxl: Control network buffering in remus callbacks > > > > This patch constitutes the core network buffering logic. > > and does the following: > > a) create a new network buffer when the domain is suspended > > (remus_domain_suspend_callback) > > b) release the previous network buffer pertaining to the > > committed checkpoint (remus_domain_checkpoint_dm_saved) > > > > Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca > <mailto:rshriram@cs.ubc.ca>> > > > > diff -r 4b23104828c0 -r c6804ccfe660 tools/libxl/libxl_dom.c > > --- a/tools/libxl/libxl_dom.c Sun Aug 25 16:33:29 2013 -0700 > > +++ b/tools/libxl/libxl_dom.c Sun Aug 25 16:33:31 2013 -0700 > > @@ -1213,12 +1213,28 @@ int libxl__toolstack_save(uint32_t domid > > > > /*----- remus callbacks -----*/ > > > > +/* REMUS TODO: Issue disk checkpoint reqs. */ > > Why does this comment need to move? > > > static int libxl__remus_domain_suspend_callback(void *data) > > { > > - /* REMUS TODO: Issue disk and network checkpoint reqs. */ > > - return libxl__domain_suspend_common_callback(data); > > + libxl__save_helper_state *shs = data; > > + libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, > shs); > > + libxl__remus_ctx *remus_ctx = dss->remus_ctx; > > + int is_suspended = 0; > > + STATE_AO_GC(dss->ao); > > + > > + is_suspended = libxl__domain_suspend_common_callback(data); > > + if (!remus_ctx->netbuf_ctx) > > split to new line please. > > > return is_suspended; > > + > > + if (is_suspended) { > > + if (libxl__remus_netbuf_start_new_epoch(gc, dss->domid, > > + remus_ctx)) > > + return !is_suspended; > > + } > > + > > + return is_suspended; > > is_suspended is logically a boolean, so should be bool_t. > Unhelpfully, > libxl__domain_suspend_common_callback() returns an int, although its > implementation strictly returns 0 on failure and 1 on success. > > IMO, It would probably be best to have "bool_t is_suspended" set to > "!!libxl__domain_suspend_common_callback()", at which the subsequent > return !is_suspended doesn't look as suspect, although that is just a > matter of style. > > > Did you mean bool (not bool_t) ? Because I didnt see a typedef in > libxl folder. > Indeed. Sorry. (I had Xen on the brain when reviewing, which does use bool_t rather than bool). ~Andrew [-- Attachment #1.2: Type: text/html, Size: 6238 bytes --] [-- Attachment #2: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 5 of 5] tools/xl: Remus - Network buffering cmdline switch 2013-08-25 23:45 [PATCH 0 of 5] Remus/Libxl: Network buffering support rshriram ` (3 preceding siblings ...) 2013-08-25 23:45 ` [PATCH 4 of 5] tools/libxl: Control network buffering in remus callbacks rshriram @ 2013-08-25 23:45 ` rshriram 4 siblings, 0 replies; 14+ messages in thread From: rshriram @ 2013-08-25 23:45 UTC (permalink / raw) To: xen-devel; +Cc: ian.jackson, ian.campbell, stefano.stabellini # HG changeset patch # User Shriram Rajagopalan <rshriram@cs.ubc.ca> # Date 1377473613 25200 # Node ID 496b22ccf0e2b7e48388b5a76298e76a911ba193 # Parent c6804ccfe660cb9c373c5f53a8996d0443316684 tools/xl: Remus - Network buffering cmdline switch Command line switch to 'xl remus' command, to enable network buffering. Pass on this flag to libxl so that it can act accordingly. Also update man pages to reflect the addition of a new option to 'xl remus' command. Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca> diff -r c6804ccfe660 -r 496b22ccf0e2 docs/man/xl.pod.1 --- a/docs/man/xl.pod.1 Sun Aug 25 16:33:31 2013 -0700 +++ b/docs/man/xl.pod.1 Sun Aug 25 16:33:33 2013 -0700 @@ -398,8 +398,7 @@ Print huge (!) amount of debug during th Enable Remus HA for domain. By default B<xl> relies on ssh as a transport mechanism between the two hosts. -N.B: Remus support in xl is still in experimental (proof-of-concept) phase. - There is no support for network or disk buffering at the moment. +N.B: There is no support for disk buffering at the moment. B<OPTIONS> @@ -418,6 +417,13 @@ Generally useful for debugging. Disable memory checkpoint compression. +=item B<-n> + +Enable network output buffering. The default script used to configure +network buffering is /etc/xen/scripts/remus-netbuf-setup. If you wish to +use a custom script, set the global variable "remus.default.netbufscript" +in /etc/xen/xl.conf to point to your script. + =item B<-s> I<sshcommand> Use <sshcommand> instead of ssh. String will be passed to sh. diff -r c6804ccfe660 -r 496b22ccf0e2 tools/libxl/xl.c --- a/tools/libxl/xl.c Sun Aug 25 16:33:31 2013 -0700 +++ b/tools/libxl/xl.c Sun Aug 25 16:33:33 2013 -0700 @@ -46,6 +46,7 @@ char *default_vifscript = NULL; char *default_bridge = NULL; char *default_gatewaydev = NULL; char *default_vifbackend = NULL; +char *default_remus_netbufscript = NULL; enum output_format default_output_format = OUTPUT_FORMAT_JSON; int claim_mode = 1; @@ -177,6 +178,11 @@ static void parse_global_config(const ch if (!xlu_cfg_get_long (config, "claim_mode", &l, 0)) claim_mode = l; + if (!xlu_cfg_get_string (config, "remus.default.netbufscript", &buf, 0)) { + free(default_remus_netbufscript); + default_remus_netbufscript = strdup(buf); + } + xlu_cfg_destroy(config); } diff -r c6804ccfe660 -r 496b22ccf0e2 tools/libxl/xl.h --- a/tools/libxl/xl.h Sun Aug 25 16:33:31 2013 -0700 +++ b/tools/libxl/xl.h Sun Aug 25 16:33:33 2013 -0700 @@ -152,6 +152,7 @@ extern char *default_vifscript; extern char *default_bridge; extern char *default_gatewaydev; extern char *default_vifbackend; +extern char *default_remus_netbufscript; extern char *blkdev_start; enum output_format { diff -r c6804ccfe660 -r 496b22ccf0e2 tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Sun Aug 25 16:33:31 2013 -0700 +++ b/tools/libxl/xl_cmdimpl.c Sun Aug 25 16:33:33 2013 -0700 @@ -7070,8 +7070,9 @@ int main_remus(int argc, char **argv) r_info.interval = 200; r_info.blackhole = 0; r_info.compression = 1; - - SWITCH_FOREACH_OPT(opt, "bui:s:e", NULL, "remus", 2) { + r_info.netbuf = 0; + + SWITCH_FOREACH_OPT(opt, "buni:s:e", NULL, "remus", 2) { case 'i': r_info.interval = atoi(optarg); break; @@ -7081,6 +7082,9 @@ int main_remus(int argc, char **argv) case 'u': r_info.compression = 0; break; + case 'n': + r_info.netbuf = 1; + break; case 's': ssh_command = optarg; break; @@ -7092,6 +7096,11 @@ int main_remus(int argc, char **argv) domid = find_domain(argv[optind]); host = argv[optind + 1]; + if (r_info.netbuf) { + if (default_remus_netbufscript) + r_info.netbufscript = strdup(default_remus_netbufscript); + } + if (r_info.blackhole) { send_fd = open("/dev/null", O_RDWR, 0644); if (send_fd < 0) { @@ -7126,13 +7135,10 @@ int main_remus(int argc, char **argv) /* Point of no return */ rc = libxl_domain_remus_start(ctx, &r_info, domid, send_fd, recv_fd, 0); - /* If we are here, it means backup has failed/domain suspend failed. - * Try to resume the domain and exit gracefully. + /* If we are here, it means remus setup/domain susppend/backup has + * failed. Try to resume the domain and exit gracefully. * TODO: Split-Brain check. */ - fprintf(stderr, "remus sender: libxl_domain_suspend failed" - " (rc=%d)\n", rc); - if (rc == ERROR_GUEST_TIMEDOUT) fprintf(stderr, "Failed to suspend domain at primary.\n"); else { diff -r c6804ccfe660 -r 496b22ccf0e2 tools/libxl/xl_cmdtable.c --- a/tools/libxl/xl_cmdtable.c Sun Aug 25 16:33:31 2013 -0700 +++ b/tools/libxl/xl_cmdtable.c Sun Aug 25 16:33:33 2013 -0700 @@ -481,6 +481,7 @@ struct cmd_spec cmd_table[] = { "-i MS Checkpoint domain memory every MS milliseconds (def. 200ms).\n" "-b Replicate memory checkpoints to /dev/null (blackhole)\n" "-u Disable memory checkpoint compression.\n" + "-n Enable network output buffering.\n" "-s <sshcommand> Use <sshcommand> instead of ssh. String will be passed\n" " to sh. If empty, run <host> instead of \n" " ssh <host> xl migrate-receive -r [-e]\n" ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2013-08-30 8:42 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-08-25 23:45 [PATCH 0 of 5] Remus/Libxl: Network buffering support rshriram 2013-08-25 23:45 ` [PATCH 1 of 5] remus: add libnl3 dependency to autoconf scripts rshriram 2013-08-26 9:06 ` Andrew Cooper 2013-08-27 8:44 ` Ian Campbell 2013-08-27 10:56 ` Ian Campbell 2013-08-29 19:13 ` Shriram Rajagopalan 2013-08-25 23:45 ` [PATCH 2 of 5] tools/hotplug: Remus network buffering setup scripts rshriram 2013-08-25 23:45 ` [PATCH 3 of 5] tools/libxl: setup/teardown Remus network buffering rshriram 2013-08-26 22:13 ` Andrew Cooper 2013-08-25 23:45 ` [PATCH 4 of 5] tools/libxl: Control network buffering in remus callbacks rshriram 2013-08-26 22:28 ` Andrew Cooper 2013-08-29 21:49 ` Shriram Rajagopalan 2013-08-30 8:42 ` Andrew Cooper 2013-08-25 23:45 ` [PATCH 5 of 5] tools/xl: Remus - Network buffering cmdline switch rshriram
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.