[libvirt] [RFC PATCH] Let network bridge MTU to be configured on XML

[Eduardo Habkost]

Hi,

Below is an incomplete patch for letting the MTU of a libvirt virtual
network to be configured on XML.

This implementation has an issue: it works when trying to set the MTU
below 1500, but the kernel bridge code doesn't let us to set the bridge
MTU above 1500 if there is no interface attached to the bridge yet. To
allow setting the bridge MTU higher than 1500, we need to save the
configured value somewhere and set it only when attaching tap interface
to it.

To solve that, I propose changing the way the network driver interface
expose things for the domain network code. Today we do the following
(src/qemu_conf.c):

 virNetworkPtr network;
 virDomainNetDefPtr net;
 char *brname;
 int tapfd;
 brname = virNetworkGetBridgeName(network);
 brAddTap(brctl, brname, &net->ifname, &tapfd)))

With this code, the only information that can flow between the network
driver and the bridge code is the bridge name.

I was going to suggest having something like this:

 virNetworkAddTap(network, &net->ifname, &tapfd);

However, we wouldn't be able to return an FD when using the 'remote'
network driver. But we can split the creation of the tap interface and
attaching it to the network bridge, like this:

 brNewTap(brctl, &net->ifname, &tapfd);
 virNetworkAttachInterface(network, net->ifname);

This way, the network driver code can do anything needed to attach the
new tap interface to the virtual network, including but not limited to
MTU setting.

Another alternative could be creating a virNetworkGetBridgeMtu()
function. That would solve our problem for the MTU settings, but in my
opinion, attaching interfaces to the network is a task for the network
driver. Having the qemu code calling brAddTap() directly was just a
shortcut we won't be able to use anymore.

What do you think?


---
 src/bridge.c               |   19 +++++++++++++++++++
 src/bridge.h               |    4 ++++
 src/libvirt_sym.version.in |    1 +
 src/network_conf.c         |    5 +++++
 src/network_conf.h         |    2 ++
 src/network_driver.c       |   10 ++++++++++
 6 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/src/bridge.c b/src/bridge.c
index 13d81bc..ed9023b 100644
--- a/src/bridge.c
+++ b/src/bridge.c
@@ -452,6 +452,25 @@ brAddTap(brControl *ctl,
     return errno;
 }
 
+
+/**
+ * brSetMtu
+ * @ctl: bridge control pointer
+ * @bridge: the bridge name
+ * @mtu: the MTU value to be set
+ *
+ * Sets the MTU of a bridge.
+ *
+ * Returns 0 in case of success or an errno code in case of failure.
+ */
+int
+brSetMtu(brControl *ctl,
+         const char *bridge,
+         int mtu)
+{
+    return ifSetMtu(ctl, bridge, mtu);
+}
+
 /**
  * brSetInterfaceUp:
  * @ctl: bridge control pointer
diff --git a/src/bridge.h b/src/bridge.h
index 7eb6166..194e96f 100644
--- a/src/bridge.h
+++ b/src/bridge.h
@@ -58,6 +58,10 @@ int     brDeleteInterface       (brControl *ctl,
                                  const char *bridge,
                                  const char *iface);
 
+int     brSetMtu                (brControl *ctl,
+                                 const char *bridge,
+                                 int mtu);
+
 int     brAddTap                (brControl *ctl,
                                  const char *bridge,
                                  char **ifname,
diff --git a/src/libvirt_sym.version.in b/src/libvirt_sym.version.in
index ec9ff3d..668b902 100644
--- a/src/libvirt_sym.version.in
+++ b/src/libvirt_sym.version.in
@@ -274,6 +274,7 @@ LIBVIRT_PRIVATE_ at VERSION@ {
   global:
 	/* bridge.h */
 	brAddBridge;
+	brSetMtu;
 	brAddInterface;
 	brAddTap;
 	brDeleteBridge;
diff --git a/src/network_conf.c b/src/network_conf.c
index 5add62e..b7ec87e 100644
--- a/src/network_conf.c
+++ b/src/network_conf.c
@@ -295,6 +295,7 @@ virNetworkDefParseXML(virConnectPtr conn,
 {
     virNetworkDefPtr def;
     char *tmp;
+    long mtu;
 
     if (VIR_ALLOC(def) < 0) {
         virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL);
@@ -332,6 +333,8 @@ virNetworkDefParseXML(virConnectPtr conn,
 
     /* Parse bridge information */
     def->bridge = virXPathString(conn, "string(./bridge[1]/@name)", ctxt);
+    if (virXPathLong(conn, "number(./bridge[1]/@mtu)", ctxt, &mtu) == 0)
+        def->bridgeMtu = mtu;
     tmp = virXPathString(conn, "string(./bridge[1]/@stp)", ctxt);
     def->stp = (tmp && STREQ(tmp, "off")) ? 0 : 1;
     VIR_FREE(tmp);
@@ -570,6 +573,8 @@ char *virNetworkDefFormat(virConnectPtr conn,
     virBufferAddLit(&buf, "  <bridge");
     if (def->bridge)
         virBufferEscapeString(&buf, " name='%s'", def->bridge);
+    if (def->bridgeMtu > 0)
+        virBufferVSprintf(&buf, " mtu='%d'", def->bridgeMtu);
     virBufferVSprintf(&buf, " stp='%s' forwardDelay='%ld' />\n",
                       def->stp ? "on" : "off",
                       def->delay);
diff --git a/src/network_conf.h b/src/network_conf.h
index 2614e47..a30cc8d 100644
--- a/src/network_conf.h
+++ b/src/network_conf.h
@@ -61,6 +61,8 @@ struct _virNetworkDef {
     char *name;
 
     char *bridge;       /* Name of bridge device */
+    int bridgeMtu;      /* MTU of the bridge */
+
     char *domain;
     unsigned long delay;   /* Bridge forward delay (ms) */
     int stp : 1; /* Spanning tree protocol */
diff --git a/src/network_driver.c b/src/network_driver.c
index 96d3ddf..10bdd10 100644
--- a/src/network_driver.c
+++ b/src/network_driver.c
@@ -684,6 +684,16 @@ static int networkStartNetworkDaemon(virConnectPtr conn,
         return -1;
     }
 
+    if (network->def->bridgeMtu> 0) {
+        if ((err = brSetMtu(driver->brctl, network->def->bridge, network->def->bridgeMtu))) {
+            networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                             _("cannot set MTU of bridge '%s' to %d: %s"),
+                             network->def->bridge, network->def->bridgeMtu,
+                             strerror(err));
+            goto err_delbr;
+        }
+    }
+
 
     if (brSetForwardDelay(driver->brctl, network->def->bridge, network->def->delay) < 0)
         goto err_delbr;
-- 
1.5.5.GIT

-- 
Eduardo