From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [patch] xen udev rule interfering with openvpn Date: Sat, 12 May 2012 23:30:37 +0100 Message-ID: <1336861837.3891.29.camel@dagon.hellion.org.uk> References: <1334658395.23948.6.camel@zakaz.uk.xensource.com> <1334817587.11493.44.camel@dagon.hellion.org.uk> <1334912603.28331.2.camel@zakaz.uk.xensource.com> <20369.15528.270106.567037@mariner.uk.xensource.com> <1334918900.28331.47.camel@zakaz.uk.xensource.com> <20369.16555.46229.798603@mariner.uk.xensource.com> <1334919613.28331.53.camel@zakaz.uk.xensource.com> <20369.17085.330843.561841@mariner.uk.xensource.com> <1335347949.28015.19.camel@zakaz.uk.xensource.com> <20375.52677.287182.934829@mariner.uk.xensource.com> <1335348880.28015.24.camel@zakaz.uk.xensource.com> <1335358736.28015.41.camel@zakaz.uk.xensource.com> <20397.10224.96552.711065@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Teck Choon Giam Cc: "xen-devel@lists.xen.org" , Roger Pau Monne , Ian Jackson , M A Young List-Id: xen-devel@lists.xenproject.org On Sat, 2012-05-12 at 23:00 +0100, Teck Choon Giam wrote: > On Sat, May 12, 2012 at 3:29 PM, Teck Choon Giam > wrote: > > On Sat, May 12, 2012 at 7:53 AM, Teck Choon Giam > > wrote: > >> On Fri, May 11, 2012 at 10:53 PM, Ian Jackson wrote: > >>> Ian Campbell writes ("Re: [Xen-devel] [patch] xen udev rule interfering with openvpn"): > >>>> libxl/xend: name tap devices vifX.Y-emu > >>> > >>> Committed-by: Ian Jackson > >> > >> This is my backport version which excludes the following: > >> > >> Lastly also move libxl__device_* to a better place in the header, otherwise the > >> comment about evgen stuff isn't next to the associated functions (noticed jsut > >> because I was going to add nic_devname near to the setdefault functions) > >> > >> I have tested with xm and xl with and without vifname set in domU > >> config for hvmdomain and pvdomain. > > > > Sorry, when re-test one of the test case failed... xm create hvmdomain > > with vifname set. I must have missed certain test case :( > > The same test case failed for xen unstable latest changeset 25326:cd4dd23a831d. Oh dear. > My tests as below: Which ones fail? > 1. xm create pvdomain without vifname set > 2. xl create pvdomain without vifname set > 3. xm create hvmdomain without vifname set > 4. xl create hvmdomain without vifname set > 5. xm create pvdomain with vifname set > 6. xl create pvdomain with vifname set > 7. xm create hvmdomain with vifname set > 8. xl create hvmdomain with vifname set > > Thanks. > > Kindest regards, > > > > > > Kindest regards, > > Giam Teck Choon > > > > > > > >> > >> For your review and comments please. > >> > >> Thanks. > >> > >> Kindest regards, > >> Giam Teck Choon > >> > >> > >> > >> > >> libxl/xend: name tap devices vifX.Y-emu > >> > >> This prevents the udev scripts from operating on other tap devices (e.g. > >> openvpn etc) > >> > >> Correct the documentation for the "vifname" option which suggested it applied > >> to HVM tap devices only, which is not the case. > >> > >> Reported by Michael Young. > >> > >> Also fix the use of vifname with emulated devices. This is slightly complex. > >> The current hotplug scripts rely on being able to parse the "tapX.Y" (now > >> "vifX.Y-emu") name in order to locate the xenstore backend dir relating to the > >> corresponding vif. This is because we cannot inject our own environment vars > >> into the tap hotplug events. However this means that if the tap is initially > >> named with a user specified name (which will not match the expected scheme) we > >> fail to do anything useful with the device. So now we create the initial tap > >> device with the standard "vifX.Y-emu" name and the hotplug script will handle > >> the rename to the desired name. This is also how PV vif devices work -- they > >> are always created by netback with the name vifX.Y and renamed in the script. > >> > >> xen-unstable changeset: 25290:7a6dcecb1781 > >> Signed-off-by: Giam Teck Choon > >> --- > >> tools/hotplug/Linux/vif-common.sh | 15 +++++++++++++-- > >> tools/hotplug/Linux/xen-backend.rules | 2 +- > >> tools/libxl/libxl_dm.c | 17 ++++++----------- > >> tools/python/xen/xend/image.py | 6 +----- > >> 4 files changed, 21 insertions(+), 19 deletions(-) > >> > >> diff --git a/tools/hotplug/Linux/vif-common.sh > >> b/tools/hotplug/Linux/vif-common.sh > >> index c9c5d41..fff22bb 100644 > >> --- a/tools/hotplug/Linux/vif-common.sh > >> +++ b/tools/hotplug/Linux/vif-common.sh > >> @@ -85,12 +85,23 @@ elif [ "$type_if" = tap ]; then > >> : ${INTERFACE:?} > >> > >> # Get xenbus_path from device name. > >> - # The name is built like that: "tap${domid}.${devid}". > >> - dev_=${dev#tap} > >> + # The name is built like that: "vif${domid}.${devid}-emu". > >> + dev_=${dev#vif} > >> + dev_=${dev_%-emu} > >> domid=${dev_%.*} > >> devid=${dev_#*.} > >> > >> XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid" > >> + vifname=$(xenstore_read_default "$XENBUS_PATH/vifname" "") > >> + if [ "$vifname" ] > >> + then > >> + vifname="${vifname}-emu" > >> + if [ "$command" == "add" ] && ! ip link show "$vifname" >&/dev/null > >> + then > >> + do_or_die ip link set "$dev" name "$vifname" > >> + fi > >> + dev="$vifname" > >> + fi > >> fi > >> > >> ip=${ip:-} > >> diff --git a/tools/hotplug/Linux/xen-backend.rules > >> b/tools/hotplug/Linux/xen-backend.rules > >> index 40f2658..405387f 100644 > >> --- a/tools/hotplug/Linux/xen-backend.rules > >> +++ b/tools/hotplug/Linux/xen-backend.rules > >> @@ -13,4 +13,4 @@ KERNEL=="blktap-control", > >> NAME="xen/blktap-2/control", MODE="0600" > >> KERNEL=="gntdev", NAME="xen/%k", MODE="0600" > >> KERNEL=="pci_iomul", NAME="xen/%k", MODE="0600" > >> KERNEL=="tapdev[a-z]*", NAME="xen/blktap-2/tapdev%m", MODE="0600" > >> -SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add", > >> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap" > >> +SUBSYSTEM=="net", KERNEL=="vif*-emu", ACTION=="add", > >> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap" > >> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > >> index 1ffcc90..2c030ca 100644 > >> --- a/tools/libxl/libxl_dm.c > >> +++ b/tools/libxl/libxl_dm.c > >> @@ -134,11 +134,9 @@ static char ** > >> libxl_build_device_model_args_old(libxl__gc *gc, > >> char *smac = libxl__sprintf(gc, > >> "%02x:%02x:%02x:%02x:%02x:%02x", > >> vifs[i].mac[0], > >> vifs[i].mac[1], vifs[i].mac[2], > >> vifs[i].mac[3], > >> vifs[i].mac[4], vifs[i].mac[5]); > >> - char *ifname; > >> - if (!vifs[i].ifname) > >> - ifname = libxl__sprintf(gc, "tap%d.%d", > >> info->domid, vifs[i].devid); > >> - else > >> - ifname = vifs[i].ifname; > >> + const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu", > >> + info->domid, > >> + vifs[i].devid); > >> flexarray_vappend(dm_args, > >> "-net", libxl__sprintf(gc, > >> "nic,vlan=%d,macaddr=%s,model=%s", > >> vifs[i].devid, > >> smac, vifs[i].model), > >> @@ -271,12 +269,9 @@ static char ** > >> libxl_build_device_model_args_new(libxl__gc *gc, > >> char *smac = libxl__sprintf(gc, > >> "%02x:%02x:%02x:%02x:%02x:%02x", > >> vifs[i].mac[0], > >> vifs[i].mac[1], vifs[i].mac[2], > >> vifs[i].mac[3], > >> vifs[i].mac[4], vifs[i].mac[5]); > >> - char *ifname; > >> - if (!vifs[i].ifname) { > >> - ifname = libxl__sprintf(gc, "tap%d.%d", > >> info->domid, vifs[i].devid); > >> - } else { > >> - ifname = vifs[i].ifname; > >> - } > >> + const char *ifname = libxl__sprintf(gc, "vif%d.%d-emu", > >> + info->domid, > >> + vifs[i].devid); > >> flexarray_append(dm_args, "-net"); > >> flexarray_append(dm_args, libxl__sprintf(gc, > >> "nic,vlan=%d,macaddr=%s,model=%s", > >> vifs[i].devid, smac, vifs[i].model)); > >> diff --git a/tools/python/xen/xend/image.py b/tools/python/xen/xend/image.py > >> index f1464ac..3c8d80c 100644 > >> --- a/tools/python/xen/xend/image.py > >> +++ b/tools/python/xen/xend/image.py > >> @@ -917,11 +917,7 @@ class HVMImageHandler(ImageHandler): > >> ret.append("-net") > >> ret.append("nic,vlan=%d,macaddr=%s,model=%s" % > >> (nics, mac, model)) > >> - vifname = devinfo.get('vifname') > >> - if vifname: > >> - vifname = "tap-" + vifname > >> - else: > >> - vifname = "tap%d.%d" % (self.vm.getDomid(), nics-1) > >> + vifname = "vif%d.%d-emu" % (self.vm.getDomid(), nics-1) > >> ret.append("-net") > >> ret.append("tap,vlan=%d,ifname=%s,bridge=%s" % > >> (nics, vifname, bridge))