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 <[email protected]> Acked-by: Jesse Gross <[email protected]>
mrangana · Sep 15, 2011 · 9197df7 · 9197df7
1 parent a89721b
commit 9197df7
Show file tree

Hide file tree

Showing 10 changed files with 66 additions and 101 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
+        }
+    }
+}
 
 static void
 ofproto_rule_destroy__(struct rule *rule)