[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] Re: [PATCH] bridge: set MTU of tap iface to the same value of the bridge



On Tue, Dec 02, 2008 at 10:15:51PM -0800, Chris Wright wrote:
> * Eduardo Habkost (ehabkost redhat com) wrote:
> > +static int brSetInterfaceMtu(brControl *ctl,
> > +                             const char *bridge,
> > +                             const char *ifname)
> > +{
> > +    int mtu = ifGetMtu(ctl, bridge);
> > +
> > +    fprintf(stderr, "mtu: %d\n", mtu);
> 
> Is that just a bit of leftover debugging?

Oops. Updated patch below.

---
From: Eduardo Habkost <ehabkost redhat com>
Date: Tue, 2 Dec 2008 16:37:00 -0200
Subject: [PATCH] bridge: set MTU of tap iface to the same value of the bridge

This is a follow-up to the RFC patch sent by Chris Wright, at:
https://www.redhat.com/archives/libvir-list/2008-November/msg00225.html

With this patch, instead of hardcoding the MTU to an high value, we set
the MTU of the tap interface to the same MTU value set on the bridge
interface. The current code uses the default tap MTU value and lowers
the bridge interface MTU without any need, killing performance.

I will send another patch later for allowing the tap MTU to be configured
on the XML, but the behavior added by this patch will be kept as the
default. The default behavior should be enough for most use cases where
a larger MTU is wanted for the bridged interfaces.

Signed-off-by: Eduardo Habkost <ehabkost redhat com>
---
 src/bridge.c |   96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 96 insertions(+), 0 deletions(-)

diff --git a/src/bridge.c b/src/bridge.c
index 4090163..13d81bc 100644
--- a/src/bridge.c
+++ b/src/bridge.c
@@ -276,6 +276,96 @@ brDeleteInterface(brControl *ctl ATTRIBUTE_UNUSED,
 #endif
 
 /**
+ * ifGetMtu
+ * @ctl: bridge control pointer
+ * @ifname: interface name get MTU for
+ *
+ * This function gets the @mtu value set for a given interface @ifname.
+ *
+ * Returns the MTU value in case of success.
+ * On error, returns -1 and sets errno accordingly
+ */
+static int ifGetMtu(brControl *ctl, const char *ifname)
+{
+    struct ifreq ifr;
+    int len;
+
+    if (!ctl || !ifname) {
+        errno = EINVAL;
+        return -1;
+    }
+
+    if ((len = strlen(ifname)) >=  BR_IFNAME_MAXLEN) {
+        errno = EINVAL;
+        return -1;
+    }
+
+    memset(&ifr, 0, sizeof(struct ifreq));
+
+    strncpy(ifr.ifr_name, ifname, len);
+    ifr.ifr_name[len] = '\0';
+
+    if (ioctl(ctl->fd, SIOCGIFMTU, &ifr))
+        return -1;
+
+    return ifr.ifr_mtu;
+
+}
+
+/**
+ * ifSetMtu:
+ * @ctl: bridge control pointer
+ * @ifname: interface name to set MTU for
+ * @mtu: MTU value
+ *
+ * This function sets the @mtu for a given interface @ifname.  Typically
+ * used on a tap device to set up for Jumbo Frames.
+ *
+ * Returns 0 in case of success or an errno code in case of failure.
+ */
+static int ifSetMtu(brControl *ctl, const char *ifname, int mtu)
+{
+    struct ifreq ifr;
+    int len;
+
+    if (!ctl || !ifname)
+        return EINVAL;
+
+    if ((len = strlen(ifname)) >=  BR_IFNAME_MAXLEN)
+        return EINVAL;
+
+    memset(&ifr, 0, sizeof(struct ifreq));
+
+    strncpy(ifr.ifr_name, ifname, len);
+    ifr.ifr_name[len] = '\0';
+    ifr.ifr_mtu = mtu;
+
+    return ioctl(ctl->fd, SIOCSIFMTU, &ifr) == 0 ? 0 : errno;
+}
+
+/**
+ * brSetInterfaceMtu
+ * @ctl: bridge control pointer
+ * @bridge: name of the bridge interface
+ * @ifname: name of the interface whose MTU we want to set
+ *
+ * Sets the interface mtu to the same MTU of the bridge
+ *
+ * Returns 0 in case of success or an errno code in case of failure.
+ */
+static int brSetInterfaceMtu(brControl *ctl,
+                             const char *bridge,
+                             const char *ifname)
+{
+    int mtu = ifGetMtu(ctl, bridge);
+
+    if (mtu < 0)
+        return errno;
+
+    return ifSetMtu(ctl, ifname, mtu);
+}
+
+/**
  * brAddTap:
  * @ctl: bridge control pointer
  * @bridge: the bridge name
@@ -334,6 +424,12 @@ brAddTap(brControl *ctl,
         }
 
         if (ioctl(fd, TUNSETIFF, &try) == 0) {
+            /* We need to set the interface MTU before adding it
+             * to the bridge, because the bridge will have its
+             * MTU adjusted automatically when we add the new interface.
+             */
+            if ((errno = brSetInterfaceMtu(ctl, bridge, try.ifr_name)))
+                goto error;
             if ((errno = brAddInterface(ctl, bridge, try.ifr_name)))
                 goto error;
             if ((errno = brSetInterfaceUp(ctl, try.ifr_name, 1)))
-- 
1.5.5.GIT

-- 
Eduardo


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]