Set MTU in userspace rather than kernel.

Currently the kernel automatically sets the MTU of any internal
interfaces to the minimum of all attached interfaces because the Linux
bridge does this.  Userspace can do this with more knowledge and
flexibility.

Feature #7323

Signed-off-by: Justin Pettit <jpettit@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 232b265..0238c5f 100644 (file)
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -749,52 +749,6 @@ static void get_dp_stats(struct datapath *dp, struct ovs_dp_stats *stats)
        }
 }
 
-/* MTU of the dp pseudo-device: ETH_DATA_LEN or the minimum of the ports.
- * Called with RTNL lock.
- */
-int dp_min_mtu(const struct datapath *dp)
-{
-       struct vport *p;
-       int mtu = 0;
-
-       ASSERT_RTNL();
-
-       list_for_each_entry (p, &dp->port_list, node) {
-               int dev_mtu;
-
-               /* Skip any internal ports, since that's what we're trying to
-                * set. */
-               if (is_internal_vport(p))
-                       continue;
-
-               dev_mtu = vport_get_mtu(p);
-               if (!dev_mtu)
-                       continue;
-               if (!mtu || dev_mtu < mtu)
-                       mtu = dev_mtu;
-       }
-
-       return mtu ? mtu : ETH_DATA_LEN;
-}
-
-/* Sets the MTU of all datapath devices to the minimum of the ports
- * Called with RTNL lock.
- */
-void set_internal_devs_mtu(const struct datapath *dp)
-{
-       struct vport *p;
-       int mtu;
-
-       ASSERT_RTNL();
-
-       mtu = dp_min_mtu(dp);
-
-       list_for_each_entry (p, &dp->port_list, node) {
-               if (is_internal_vport(p))
-                       vport_set_mtu(p, mtu);
-       }
-}
-
 static const struct nla_policy flow_policy[OVS_FLOW_ATTR_MAX + 1] = {
        [OVS_FLOW_ATTR_KEY] = { .type = NLA_NESTED },
        [OVS_FLOW_ATTR_ACTIONS] = { .type = NLA_NESTED },
@@ -1740,7 +1694,6 @@ static int ovs_vport_cmd_new(struct sk_buff *skb, struct genl_info *info)
        if (IS_ERR(vport))
                goto exit_unlock;
 
-       set_internal_devs_mtu(dp);
        dp_sysfs_add_if(vport);
 
        err = change_vport(vport, a);

diff --git a/datapath/datapath.h b/datapath/datapath.h
index 4b5f7ec..ff8ac10 100644 (file)
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -150,8 +150,6 @@ extern int (*dp_ioctl_hook)(struct net_device *dev, struct ifreq *rq, int cmd);
 void dp_process_received_packet(struct vport *, struct sk_buff *);
 void dp_detach_port(struct vport *);
 int dp_upcall(struct datapath *, struct sk_buff *, const struct dp_upcall_info *);
-int dp_min_mtu(const struct datapath *dp);
-void set_internal_devs_mtu(const struct datapath *dp);
 
 struct datapath *get_dp(int dp_idx);
 const char *dp_name(const struct datapath *dp);

diff --git a/datapath/dp_notify.c b/datapath/dp_notify.c
index 7b3b219..942d628 100644 (file)
--- a/datapath/dp_notify.c
+++ b/datapath/dp_notify.c
@@ -59,11 +59,6 @@ static int dp_device_event(struct notifier_block *unused, unsigned long event,
                        dp_sysfs_add_if(vport);
                }
                break;
-
-       case NETDEV_CHANGEMTU:
-               if (!is_internal_dev(dev))
-                       set_internal_devs_mtu(dp);
-               break;
        }
        return NOTIFY_DONE;
 }

diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
index 82079bd..04f51eb 100644 (file)
--- a/datapath/vport-internal_dev.c
+++ b/datapath/vport-internal_dev.c
@@ -117,14 +117,9 @@ static const struct ethtool_ops internal_dev_ethtool_ops = {
 
 static int internal_dev_change_mtu(struct net_device *netdev, int new_mtu)
 {
-       struct vport *vport = internal_dev_get_vport(netdev);
-
        if (new_mtu < 68)
                return -EINVAL;
 
-       if (new_mtu > dp_min_mtu(vport->dp))
-               return -EINVAL;
-
        netdev->mtu = new_mtu;
        return 0;
 }
@@ -274,7 +269,6 @@ const struct vport_ops internal_vport_ops = {
        .flags          = VPORT_F_REQUIRED | VPORT_F_GEN_STATS | VPORT_F_FLOW,
        .create         = internal_dev_create,
        .destroy        = internal_dev_destroy,
-       .set_mtu        = netdev_set_mtu,
        .set_addr       = netdev_set_addr,
        .get_name       = netdev_get_name,
        .get_addr       = netdev_get_addr,

diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
index 478f271..7cd6c6b 100644 (file)
--- a/datapath/vport-netdev.c
+++ b/datapath/vport-netdev.c
@@ -189,12 +189,6 @@ static void netdev_destroy(struct vport *vport)
        vport_free(vport);
 }
 
-int netdev_set_mtu(struct vport *vport, int mtu)
-{
-       struct netdev_vport *netdev_vport = netdev_vport_priv(vport);
-       return dev_set_mtu(netdev_vport->dev, mtu);
-}
-
 int netdev_set_addr(struct vport *vport, const unsigned char *addr)
 {
        struct netdev_vport *netdev_vport = netdev_vport_priv(vport);
@@ -422,7 +416,6 @@ const struct vport_ops netdev_vport_ops = {
        .exit           = netdev_exit,
        .create         = netdev_create,
        .destroy        = netdev_destroy,
-       .set_mtu        = netdev_set_mtu,
        .set_addr       = netdev_set_addr,
        .get_name       = netdev_get_name,
        .get_addr       = netdev_get_addr,

diff --git a/datapath/vport-netdev.h b/datapath/vport-netdev.h
index 3f64767..7a743d2 100644 (file)
--- a/datapath/vport-netdev.h
+++ b/datapath/vport-netdev.h
@@ -25,7 +25,6 @@ netdev_vport_priv(const struct vport *vport)
        return vport_priv(vport);
 }
 
-int netdev_set_mtu(struct vport *, int mtu);
 int netdev_set_addr(struct vport *, const unsigned char *addr);
 const char *netdev_get_name(const struct vport *);
 const unsigned char *netdev_get_addr(const struct vport *);

diff --git a/datapath/vport.c b/datapath/vport.c
index b1418a4..ca802b4 100644 (file)
--- a/datapath/vport.c
+++ b/datapath/vport.c
@@ -284,36 +284,6 @@ void vport_del(struct vport *vport)
        vport->ops->destroy(vport);
 }
 
-/**
- *     vport_set_mtu - set device MTU (for kernel callers)
- *
- * @vport: vport on which to set MTU.
- * @mtu: New MTU.
- *
- * Sets the MTU of the given device.  Some devices may not support setting the
- * MTU, in which case the result will always be -EOPNOTSUPP.  RTNL lock must
- * be held.
- */
-int vport_set_mtu(struct vport *vport, int mtu)
-{
-       ASSERT_RTNL();
-
-       if (mtu < 68)
-               return -EINVAL;
-
-       if (vport->ops->set_mtu) {
-               int ret;
-
-               ret = vport->ops->set_mtu(vport, mtu);
-
-               if (!ret && !is_internal_vport(vport))
-                       set_internal_devs_mtu(vport->dp);
-
-               return ret;
-       } else
-               return -EOPNOTSUPP;
-}
-
 /**
  *     vport_set_addr - set device Ethernet address (for kernel callers)
  *

diff --git a/datapath/vport.h b/datapath/vport.h
index bbbb214..6c4da24 100644 (file)
--- a/datapath/vport.h
+++ b/datapath/vport.h
@@ -30,7 +30,6 @@ void vport_del(struct vport *);
 
 struct vport *vport_locate(const char *name);
 
-int vport_set_mtu(struct vport *, int mtu);
 int vport_set_addr(struct vport *, const unsigned char *);
 int vport_set_stats(struct vport *, struct rtnl_link_stats64 *);
 
@@ -156,7 +155,6 @@ struct vport_parms {
  * @get_options: Appends vport-specific attributes for the configuration of an
  * existing vport to a &struct sk_buff.  May be %NULL for a vport that does not
  * have any configuration.
- * @set_mtu: Set the device's MTU.  May be null if not supported.
  * @set_addr: Set the device's MAC address.  May be null if not supported.
  * @get_name: Get the device's name.
  * @get_addr: Get the device's MAC address.
@@ -190,7 +188,6 @@ struct vport_ops {
        int (*set_options)(struct vport *, struct nlattr *);
        int (*get_options)(const struct vport *, struct sk_buff *);
 
-       int (*set_mtu)(struct vport *, int mtu);
        int (*set_addr)(struct vport *, const unsigned char *);
 
        /* Called with rcu_read_lock or RTNL lock. */

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 3037bb2..f596abc 100644 (file)
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -93,6 +93,7 @@ struct ofport {
     struct ofp_phy_port opp;
     uint16_t ofp_port;          /* OpenFlow port number. */
     unsigned int change_seq;
+    int mtu;
 };
 
 /* An OpenFlow flow within a "struct ofproto".

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 7748ef1..1309b49 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -147,6 +147,7 @@ static int handle_flow_mod__(struct ofproto *, struct ofconn *,
 static void update_port(struct ofproto *, const char *devname);
 static int init_ports(struct ofproto *);
 static void reinit_ports(struct ofproto *);
+static void set_internal_devs_mtu(struct ofproto *);
 
 static void ofproto_unixctl_init(void);
 
@@ -1193,6 +1194,7 @@ ofport_install(struct ofproto *p,
 {
     const char *netdev_name = netdev_get_name(netdev);
     struct ofport *ofport;
+    int dev_mtu;
     int error;
 
     /* Create ofport. */
@@ -1211,6 +1213,13 @@ ofport_install(struct ofproto *p,
     hmap_insert(&p->ports, &ofport->hmap_node, hash_int(ofport->ofp_port, 0));
     shash_add(&p->port_by_name, netdev_name, ofport);
 
+    if (!netdev_get_mtu(netdev, &dev_mtu)) {
+        set_internal_devs_mtu(p);
+        ofport->mtu = dev_mtu;
+    } else {
+        ofport->mtu = 0;
+    }
+
     /* Let the ofproto_class initialize its private data. */
     error = p->ofproto_class->port_construct(ofport);
     if (error) {
@@ -1337,12 +1346,22 @@ update_port(struct ofproto *ofproto, const char *name)
         port = ofproto_get_port(ofproto, ofproto_port.ofp_port);
         if (port && !strcmp(netdev_get_name(port->netdev), name)) {
             struct netdev *old_netdev = port->netdev;
+            int dev_mtu;
 
             /* 'name' hasn't changed location.  Any properties changed? */
             if (!ofport_equal(&port->opp, &opp)) {
                 ofport_modified(port, &opp);
             }
 
+            /* If this is a non-internal port and the MTU changed, check
+             * if the datapath's MTU needs to be updated. */
+            if (strcmp(netdev_get_type(netdev), "internal")
+                    && !netdev_get_mtu(netdev, &dev_mtu)
+                    && port->mtu != dev_mtu) {
+                set_internal_devs_mtu(ofproto);
+                port->mtu = dev_mtu;
+            }
+
             /* Install the newly opened netdev in case it has changed.
              * Don't close the old netdev yet in case port_modified has to
              * remove a retained reference to it.*/
@@ -1398,6 +1417,52 @@ init_ports(struct ofproto *p)
 
     return 0;
 }
+
+/* Find the minimum MTU of all non-datapath devices attached to 'p'.
+ * Returns ETH_PAYLOAD_MAX or the minimum of the ports. */
+static int
+find_min_mtu(struct ofproto *p)
+{
+    struct ofport *ofport;
+    int mtu = 0;
+
+    HMAP_FOR_EACH (ofport, hmap_node, &p->ports) {
+        struct netdev *netdev = ofport->netdev;
+        int dev_mtu;
+
+        /* Skip any internal ports, since that's what we're trying to
+         * set. */
+        if (!strcmp(netdev_get_type(netdev), "internal")) {
+            continue;
+        }
+
+        if (netdev_get_mtu(netdev, &dev_mtu)) {
+            continue;
+        }
+        if (!mtu || dev_mtu < mtu) {
+            mtu = dev_mtu;
+        }
+    }
+
+    return mtu ? mtu: ETH_PAYLOAD_MAX;
+}
+
+/* Set the MTU of all datapath devices on 'p' to the minimum of the
+ * non-datapath ports. */
+static void
+set_internal_devs_mtu(struct ofproto *p)
+{
+    struct ofport *ofport;
+    int mtu = find_min_mtu(p);
+
+    HMAP_FOR_EACH (ofport, hmap_node, &p->ports) {
+        struct netdev *netdev = ofport->netdev;
+
+        if (!strcmp(netdev_get_type(netdev), "internal")) {
+            netdev_set_mtu(netdev, mtu);
+        }
+    }
+}
 \f
 static void
 ofproto_rule_destroy__(struct rule *rule)