Skip to contentSkip to conversation

Recent xl and network-route scripts

PATCH New W. Michael Petullo 5 participants 13 messages Feb 17, 2011 – Mar 16, 2011

lore
Share & tools
lore
#0 lore raw

I've been testing recent Xen 4.1 release candidates, and it seems that "xl" does not want to play nicely with the vif-route script.

First, the udev subsystem executes vif-setup after Xen creates the vif device. For some reason, this script does not have $script defined and so it defaults to running vif-bridge (even though xend-config.sxp specifies vif-route). Once I changed vif-setup by hand to run vif-route things went further. I'm not entirely sure how $script is supposed to be set, but it worked fine with xm.

Next, vif-route does not pick up $ip from my domain configuration's "vif = [ 'ip=w.x.y.z' ]." When using xm, vif-route finds $ip set and creates a route based on its value.

I have most recently tested this with Xen 4.1.0 RC4.

Signature · 3 lines
Mike

:wq
#1 lore raw
Quoted 7 lines · W. Michael Petullo

I've been testing recent Xen 4.1 release candidates, and it seems that "xl" does not want to play nicely with the vif-route script.

First, the udev subsystem executes vif-setup after Xen creates the vif device. For some reason, this script does not have $script defined and so it defaults to running vif-bridge (even though xend-config.sxp specifies vif-route).

I don't think xl/libxl reads/parses xend-config.sxp ..

Quoted 10 lines

Once I changed vif-setup by hand to run vif-route
things went further. I'm not entirely sure how $script is supposed to be set, but it worked fine with xm.

Next, vif-route does not pick up $ip from my domain configuration's "vif = [ 'ip=w.x.y.z' ]." When using xm, vif-route finds $ip set and creates a route based on its value.

I have most recently tested this with Xen 4.1.0 RC4.

-- Pasi

#2 lore raw
Quoted 10 lines · Pasi Kärkkäinen
Quoted 7 lines · W. Michael Petullo

I've been testing recent Xen 4.1 release candidates, and it seems that "xl" does not want to play nicely with the vif-route script.

First, the udev subsystem executes vif-setup after Xen creates the vif device. For some reason, this script does not have $script defined and so it defaults to running vif-bridge (even though xend-config.sxp specifies vif-route).

I don't think xl/libxl reads/parses xend-config.sxp ..

There is an /etc/xl.conf but I don't believe it contains a setting for the default vif script. It probably could/should though.

As a workaround I believe adding ",script=foo" to the vif entry might work?

Quoted 7 lines
Quoted 7 lines

Once I changed vif-setup by hand to run vif-route
things went further. I'm not entirely sure how $script is supposed to be set, but it worked fine with xm.

Next, vif-route does not pick up $ip from my domain configuration's "vif = [ 'ip=w.x.y.z' ]." When using xm, vif-route finds $ip set and creates a route based on its value.

It looks like xl parses the ip option but that libxl doesn't do anything with it.

I'll see if I can out a patch together for both of these issues shortly.

Ian.

#3 lore raw
Quoted 1 line · Ian Campbell

I'll see if I can out a patch together for both of these issues shortly.

Does this work for you?

I don't have a routed setup handy to try it myself but I have verified that the things appear in xenstore as I would expect.

You will need to edit/create /etc/xen/xl.conf and add:
vifscript = "vif-route"

You will also need to ensure your host network is setup appropriately since one difference between xm and xl is that xl does not attempt to reconfigure the host network instead leaving this up to distro provided tools etc.

Looking at the network-route script it seems that it should be sufficient to (assuming eth0 is your physical device):

	echo 1 >/proc/sys/net/ipv4/ip_forward
	echo 1 >/proc/sys/net/ipv4/conf/eth0/proxy_arp
which I think is equivalent to adding the following to /etc/sysctl.conf
and running "sysctl -f" (this method will persist over reboot too)
	sys.net.ipv4.ip_forward = 1
	sys.net.ipv4.conf.eth0.proxy_arp = 1

(Pasi, Todd: this would be a useful addition to the
http://wiki.xensource.com/xenwiki/MigrationGuideToXen4.1+
page)

If this doesn't help then it would be useful to see the result of
xenstore-ls /local/domain/0/backends/vif/<domid>/0 echo
xenstore-ls /local/domain/<domid>/device/vif/0
when the domain is running from both xl and xm.

Ian.

8<------------------------------------

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1297936972 0
# Node ID c529d4d7d79ba3fe69ffb6cbacac5c9e3e5cf246 # Parent 1728ed4bbec9e82ca13c2639c8e4ef8b4dc231b6 libxl/xl: enable support for routed network configurations.

Add "vifscript" option to xl.conf which configures the default vif script to use (default remains "vif-bridge")

Write each VIFs "ip" option to xenstore so the vif-route script can pick it up.

Reported by W. Michael Petullo <[email protected]>.

tools/examples/xl.conf +3 -0
diff -r 1728ed4bbec9 -r c529d4d7d79b tools/examples/xl.conf
@@ -6,3 +6,6 @@ autoballoon=0
 
 # full path of the lockfile used by xl during domain creation
 #lockfile="/var/lock/xl"
+
+# default vif script 
+#vifscript="vif-bridge"
tools/libxl/libxl.c +13 -0
diff -r 1728ed4bbec9 -r c529d4d7d79b tools/libxl/libxl.c
@@ -30,6 +30,8 @@
 #include <stdint.h>
 #include <inttypes.h>
 #include <assert.h>
+
+#include <arpa/inet.h>
 
 #include "libxl.h"
 #include "libxl_utils.h"
@@ -1123,6 +1125,7 @@ int libxl_device_nic_init(libxl_device_n
     nic_info->mac[5] = r[2];
     nic_info->ifname = NULL;
     nic_info->bridge = strdup("xenbr0");
+    nic_info->ip.s_addr = 0UL;
     if ( asprintf(&nic_info->script, "%s/vif-bridge",
                libxl_xen_script_dir_path()) < 0 )
         return ERROR_FAIL;
@@ -1182,6 +1185,16 @@ int libxl_device_nic_add(libxl_ctx *ctx,
     flexarray_append(back, libxl__sprintf(&gc, "%02x:%02x:%02x:%02x:%02x:%02x",
                                                  nic->mac[0], nic->mac[1], nic->mac[2],
                                                  nic->mac[3], nic->mac[4], nic->mac[5]));
+    if (nic->ip.s_addr != 0UL) {
+        char dst[INET_ADDRSTRLEN];
+        const char *addr = inet_ntop(AF_INET, &nic->ip.s_addr, &dst[0], INET_ADDRSTRLEN);
+        if (addr) {
+            flexarray_append(back, "ip");
+            flexarray_append(back, libxl__strdup(&gc, addr));
+        } else {
+            LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "Unable to format IP address");
+        }
+    }
     flexarray_append(back, "bridge");
     flexarray_append(back, libxl__strdup(&gc, nic->bridge));
     flexarray_append(back, "handle");
tools/libxl/xl.c +4 -0
diff -r 1728ed4bbec9 -r c529d4d7d79b tools/libxl/xl.c
@@ -35,6 +35,7 @@ xentoollog_logger_stdiostream *logger;
 xentoollog_logger_stdiostream *logger;
 int autoballoon = 1;
 char *lockfile;
+char *default_vifscript = NULL;
 
 static xentoollog_level minmsglevel = XTL_PROGRESS;
 
@@ -71,6 +72,9 @@ static void parse_global_config(const ch
             exit(1);
         }
     }
+
+    if (!xlu_cfg_get_string (config, "vifscript", &buf))
+        default_vifscript = strdup(buf);
 
     xlu_cfg_destroy(config);
 }
tools/libxl/xl.h +1 -0
diff -r 1728ed4bbec9 -r c529d4d7d79b tools/libxl/xl.h
@@ -101,5 +101,6 @@ extern xentoollog_logger_stdiostream *lo
 /* global options */
 extern int autoballoon;
 extern char *lockfile;
+extern char *default_vifscript;
 
 #endif /* XL_H */
tools/libxl/xl_cmdimpl.c +5 -0
diff -r 1728ed4bbec9 -r c529d4d7d79b tools/libxl/xl_cmdimpl.c
@@ -782,6 +782,11 @@ static void parse_config_data(const char
             d_config->vifs = (libxl_device_nic *) realloc(d_config->vifs, sizeof (libxl_device_nic) * (d_config->num_vifs+1));
             nic = d_config->vifs + d_config->num_vifs;
             CHK_ERRNO( libxl_device_nic_init(nic, d_config->num_vifs) );
+
+            if (default_vifscript) {
+                free(nic->script);
+                nic->script = strdup(default_vifscript);
+            }
 
             p = strtok(buf2, ",");
             if (!p)
Applied
#4 lore raw

Ian Campbell writes ("Re: [Xen-devel] Recent xl and network-route scripts"):

Quoted 1 line

libxl/xl: enable support for routed network configurations.

Applied, thanks (sorry for apparently missing this one yesterday).

Ian.

#5 lore raw
Quoted 3 lines
Quoted 1 line

libxl/xl: enable support for routed network configurations.

Applied, thanks (sorry for apparently missing this one yesterday).

I finally had a change to test this and things work well. However, may I suggest changing the default xl.conf to:

#vifscript="/etc/xen/scripts/vif-bridge"

instead of:

#vifscript="vif-bridge"

The former pattern is what "vif-setup" needs.

Signature · 3 lines
Mike

:wq
#6 lore raw
Quoted 7 lines · W. Michael Petullo
Quoted 3 lines
Quoted 1 line

libxl/xl: enable support for routed network configurations.

Applied, thanks (sorry for apparently missing this one yesterday).

I finally had a change to test this and things work well. However, may I suggest changing the default xl.conf to:

Quoted 7 lines

#vifscript="/etc/xen/scripts/vif-bridge"

instead of:

#vifscript="vif-bridge"

The former pattern is what "vif-setup" needs.

Thanks, I sent out a patch last week "libxl: use full path to vif hotplug script script" which addresses this discrepancy.

I thought it has gone it but evidently not. The patch was part 1 of 2 in a series, I don't think 2/2 is suitable for 4.1 (which is likely why the whole series was missed), but the first patch most probably is.

Ian.

#7 lore raw

Ian Campbell writes ("Re: [Xen-devel] Recent xl and network-route scripts"):

Quoted 5 lines · W. Michael Petullo
Quoted 1 line

The former pattern is what "vif-setup" needs.

Thanks, I sent out a patch last week "libxl: use full path to vif hotplug script script" which addresses this discrepancy.

I think we should _allow_ the user to specify the complete path to a script, but not require them to. Your existing patch 1/2 doesn't do that.

How about something like:

- flexarray_append(back, nic->script);
+ flexarray_append(back, nic->script[0]=='/' ? nic->script :
+ libxl__sprintf(&gc, "%s/%s", libxl_xen_script_dir_path(), nic->script));

?

Quoted 3 lines

I thought it has gone it but evidently not. The patch was part 1 of 2 in a series, I don't think 2/2 is suitable for 4.1 (which is likely why the whole series was missed), but the first patch most probably is.

I didn't realise you intended it for 4.1, but I think this is an important enough compatibility problem that we should fix it along the lines I suggest above.

Ian.

#8 lore raw
Quoted 10 lines · Ian Jackson

Ian Campbell writes ("Re: [Xen-devel] Recent xl and network-route scripts"):

Quoted 5 lines · W. Michael Petullo
Quoted 1 line

The former pattern is what "vif-setup" needs.

Thanks, I sent out a patch last week "libxl: use full path to vif hotplug script script" which addresses this discrepancy.

I think we should _allow_ the user to specify the complete path to a script, but not require them to. Your existing patch 1/2 doesn't do that.

It's over and above whatr xend supports but I agree it's a good idea.

Review

Quoted 5 lines

How about something like:

(quoted diff) +2 -1
-    flexarray_append(back, nic->script);
+    flexarray_append(back, nic->script[0]=='/' ? nic->script :
+     libxl__sprintf(&gc, "%s/%s", libxl_xen_script_dir_path(), nic->script));

Assuming there's an "if (nic-script)" above this context I think makes sense.

Ian

#9 lore raw

Re: Recent xl and network-route scripts [and 1 more messages]

Ian Campbell writes ("Re: [Xen-devel] Recent xl and network-route scripts"):

Quoted 6 lines · Ian Jackson

Review

Quoted 5 lines

How about something like:

(quoted diff) +2 -1
-    flexarray_append(back, nic->script);
+    flexarray_append(back, nic->script[0]=='/' ? nic->script :
+     libxl__sprintf(&gc, "%s/%s", libxl_xen_script_dir_path(), nic->script));

I've applied the patch below.

Quoted 2 lines

Assuming there's an "if (nic-script)" above this context I think makes sense.

There wasn't, so I added one. Looking at the code I think it's not necessary but now is not the time to take the risk ...

Ian.

# HG changeset patch
# User Ian Jackson <[email protected]> # Date 1300293865 0
# Node ID 4ca13a170482edc642b8a7d8c68556c4194f11d0 # Parent 8074dff66e9791cef6be06dd9a24176a59f88c5e
libxl: provide full path to vif hotplug script script

This improves compatibility with xm style config files since xend does this (see NetifController.getDeviceDetails() in tools/python/xen/xend/server/netif.py) and tools/hotplug/Linux/vif-setup expects an absolute path to the script.

This patch supports both absolute and relative paths in the configuration. Also, if somehow nic->script==NULL, don't add write the entry to xenstore at all (rather than crashing due to the new check for absolute vs relative path).

tools/libxl/libxl.c +7 -2
diff -r 8074dff66e97 -r 4ca13a170482 tools/libxl/libxl.c
@@ -1234,8 +1234,13 @@ int libxl_device_nic_add(libxl_ctx *ctx,
     flexarray_append(back, "1");
     flexarray_append(back, "state");
     flexarray_append(back, libxl__sprintf(&gc, "%d", 1));
-    flexarray_append(back, "script");
-    flexarray_append(back, nic->script);
+    if (nic->script) {
+        flexarray_append(back, "script");
+        flexarray_append(back, nic->script[0]=='/' ? nic->script
+                         : libxl__sprintf(&gc, "%s/%s",
+                                          libxl_xen_script_dir_path(),
+                                          nic->script));
+    }
     flexarray_append(back, "mac");
     flexarray_append(back, libxl__sprintf(&gc, "%02x:%02x:%02x:%02x:%02x:%02x",
                                                  nic->mac[0], nic->mac[1], nic->mac[2],
#10 lore raw

PATCH 1/1 vif hotplug script configuration fixes

The follow short series makes it possible to use alternative vif hotplug scripts with tap devices. Previously the default vif-bridge hotplug script would be run for such devices even if /etc/xen/xl.conf, /etc/xen/xend-config.sxp or the domain cfg said otherwise.

The series also improves xl compatibility with the xm vif script syntax which does not require the user to enter an absolute path.

#11 lore raw

PATCH 1/1 libxl: use full path to vif hotplug script script

# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1299232872 0
# Node ID 61357fd45fd75880c2683fdc643bf2563659e37e # Parent 0126a6729e8d8cf19ae5fedab2a0c79570e3ddbb
libxl: use full path to vif hotplug script script.

This improves compatibility with xm styke config files since xend does this (see NetifController.getDeviceDetails() in tools/python/xen/xend/server/netif.py) and tools/hotplug/Linux/vif-setup expects an absolute path to the script.

tools/libxl/libxl.c +2 -1
diff -r 0126a6729e8d -r 61357fd45fd7 tools/libxl/libxl.c
@@ -1228,7 +1228,8 @@ int libxl_device_nic_add(libxl_ctx *ctx,
     flexarray_append(back, "state");
     flexarray_append(back, libxl__sprintf(&gc, "%d", 1));
     flexarray_append(back, "script");
-    flexarray_append(back, nic->script);
+    flexarray_append(back, libxl__sprintf(&gc, "%s/%s",
+                                                libxl_xen_script_dir_path(), nic->script));
     flexarray_append(back, "mac");
     flexarray_append(back, libxl__sprintf(&gc, "%02x:%02x:%02x:%02x:%02x:%02x",
                                                  nic->mac[0], nic->mac[1], nic->mac[2],
#12 lore raw

PATCH 1/1 tools/hotplug/Linux: read script for tapX.Y devices from xenstore

In reply to Ian Campbell

# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1299232872 0
# Node ID 37b2d5372baab8b2fac860f9a9fff7348f74153c
# Parent 61357fd45fd75880c2683fdc643bf2563659e37e tools/hotplug/Linux: read script for tapX.Y devices from xenstore.

This variable is not present in the $environment for tap hotplug events in the way it is for vif hotplug events. Therefore we need to look up the script in xenbus.

vif-common.sh already takes care of most of the environmental differences but we need the script much earlier.

tools/hotplug/Linux/vif-setup +14 -0
diff -r 61357fd45fd7 -r 37b2d5372baa tools/hotplug/Linux/vif-setup
@@ -1,4 +1,18 @@
 #!/bin/bash
+
+# tap devices do not get the same environment as vif devices so we may
+# need to discover the script from xenbus.
+#
+# Try and only match the tap<domid>.<devid> naming scheme which Xen
+# uses, to avoid interfering with other uses of tap devices.
+if [ -z "$script" ] && echo $* | grep -qw type_if=tap ; then
+    xbpath=$(echo ${INTERFACE} | sed -ne 's,tap\([0-9][0-9]\?\)\.\([0-9][0-9]\?\),backend/vif/\1/\2,p')
+    if [ -n "$xbpath" ] ; then
+	script=`xenstore-read "${xbpath}/script"`
+    fi
+
+    export script
+fi
 
 if test "$script"
 then