[ovs-dev] [PATCH v3] ofproto: Optimize internal device MTU update

[Pravin B Shelar]

Fixed according to comments from Ben.
v2-v3:
     - Fixed min mtu for non-internal devices.
     - Fixed update_mtu() comment.
v1-v2:
     - Do not allow larger mtu on internal device compared to
       non internal devices..

--8<--------------------------cut here-------------------------->8--

Internal device mtu does not influence mtu of other internal devices.
So skip MTU update to other devices when internal device mtu is changed.

Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
---
 ofproto/ofproto-provider.h |    1 +
 ofproto/ofproto.c          |   55 ++++++++++++++++++++++++++++----------------
 2 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 04c156a..e540850 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -81,6 +81,7 @@ struct ofproto {
      * widespread use, we will delete these interfaces. */
     unsigned long int *vlan_bitmap; /* 4096-bit bitmap of in-use VLANs. */
     bool vlans_changed;             /* True if new VLANs are in use. */
+    int min_mtu;                    /* Min MTU of non-internal ports. */
 };
 
 void ofproto_init_tables(struct ofproto *, int n_tables);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 6ce587d..a71ca58 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -196,7 +196,7 @@ static enum ofperr handle_flow_mod__(struct ofproto *, struct ofconn *,
 static uint64_t pick_datapath_id(const struct ofproto *);
 static uint64_t pick_fallback_dpid(void);
 static void ofproto_destroy__(struct ofproto *);
-static void set_internal_devs_mtu(struct ofproto *);
+static void update_mtu(struct ofproto *, struct ofport *);
 
 /* unixctl. */
 static void ofproto_unixctl_init(void);
@@ -387,6 +387,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     hmap_init(&ofproto->deletions);
     ofproto->vlan_bitmap = NULL;
     ofproto->vlans_changed = false;
+    ofproto->min_mtu = INT_MAX;
 
     error = ofproto->ofproto_class->construct(ofproto);
     if (error) {
@@ -1457,7 +1458,6 @@ ofport_install(struct ofproto *p,
 {
     const char *netdev_name = netdev_get_name(netdev);
     struct ofport *ofport;
-    int dev_mtu;
     int error;
 
     /* Create ofport. */
@@ -1476,12 +1476,7 @@ 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)) {
-        ofport->mtu = dev_mtu;
-        set_internal_devs_mtu(p);
-    } else {
-        ofport->mtu = 0;
-    }
+    update_mtu(p, ofport);
 
     /* Let the ofproto_class initialize its private data. */
     error = p->ofproto_class->port_construct(ofport);
@@ -1643,18 +1638,13 @@ 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->pp, &pp)) {
                 ofport_modified(port, &pp);
             }
 
-            if (!netdev_get_mtu(netdev, &dev_mtu) &&
-                port->mtu != dev_mtu) {
-                port->mtu = dev_mtu;
-                set_internal_devs_mtu(ofproto);
-            }
+            update_mtu(ofproto, port);
 
             /* Install the newly opened netdev in case it has changed.
              * Don't close the old netdev yet in case port_modified has to
@@ -1741,23 +1731,48 @@ find_min_mtu(struct ofproto *p)
     return mtu ? mtu: ETH_PAYLOAD_MAX;
 }
 
-/* Set the MTU of all datapath devices on 'p' to the minimum of the
- * non-datapath ports. */
+/* Update MTU of all datapath devices on 'p' to the minimum of the
+ * non-datapath ports in event of 'port' added or changed. */
 static void
-set_internal_devs_mtu(struct ofproto *p)
+update_mtu(struct ofproto *p, struct ofport *port)
 {
     struct ofport *ofport;
-    int mtu = find_min_mtu(p);
+    struct netdev *netdev = port->netdev;
+    int dev_mtu, old_min;
+
+    if (netdev_get_mtu(netdev, &dev_mtu)) {
+        port->mtu = 0;
+        return;
+    }
+    if (!strcmp(netdev_get_type(port->netdev), "internal")) {
+        if (dev_mtu > p->min_mtu) {
+           if (!netdev_set_mtu(port->netdev, p->min_mtu)) {
+               dev_mtu = p->min_mtu;
+           }
+        }
+        port->mtu = dev_mtu;
+        return;
+    }
+
+    /* For non-internal port find new min mtu. */
+    old_min = p->min_mtu;
+    port->mtu = dev_mtu;
+    p->min_mtu = find_min_mtu(p);
+    if (p->min_mtu == old_min) {
+        return;
+    }
 
     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);
-            ofport->mtu = mtu;
+            if (!netdev_set_mtu(netdev, p->min_mtu)) {
+                ofport->mtu = p->min_mtu;
+            }
         }
     }
 }
+
 
 static void
 ofproto_rule_destroy__(struct rule *rule)
-- 
1.7.1