[ovs-discuss] ofproto: update mtu when port is getting removed as well

[AZ]

When we're adding the port into ovs bridge, it's mtu is updated to the minimal
mtu of the included port. But when the port is getting removed, no such update
is performed, which leads to bug. For example, when the port with minimal mtu
is removed, bridge's mtu must adapt to new value, but it won't happen.
How to reproduce the problem:

# ovs-vsctl add-br testing
# ip link add name gretap11 type gretap local 10.0.0.1 remote 10.0.0.100
# ip link add name gretap12 type gretap local 10.0.0.1 remote 10.0.0.200
# ip link set dev gretap12 mtu 1600
# ovs-vsctl add-port testing gretap11
# ovs-vsctl add-port testing gretap12
# ip a sh testing
16: testing: <BROADCAST,MULTICAST> mtu 1462 qdisc noop state DOWN
group default qlen 1
    link/ether 7a:42:95:00:96:40 brd ff:ff:ff:ff:ff:ff

# ovs-vsctl del-port gretap11
# ip a sh testing
16: testing: <BROADCAST,MULTICAST> mtu 1462 qdisc noop state DOWN
group default qlen 1
    link/ether 7a:42:95:00:96:40 brd ff:ff:ff:ff:ff:ff

### as we can see here, 'testing' bridge mtu is stuck, while it must
adapt to new '1600' value,
### cause there is only one port 'gretap12' left, and it's mtu is '1600':

# ip a sh gretap12
19: gretap12 at NONE: <BROADCAST,MULTICAST> mtu 1600 qdisc noop master
ovs-system state DOWN group default qlen 1000
    link/ether b2:c6:1d:9f:be:0d brd ff:ff:ff:ff:ff:ff

My commit fixes this problem - mtu update is performed on port removal as well.

Signed-off-by: wisd0me <ak47izatool at gmail.com>
---
 ofproto/ofproto.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 89e75d5..9e03616 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -219,6 +219,7 @@ static void learned_cookies_flush(struct ofproto
*, struct ovs_list *dead_cookie
 /* ofport. */
 static void ofport_destroy__(struct ofport *) OVS_EXCLUDED(ofproto_mutex);
 static void ofport_destroy(struct ofport *, bool del);
+static inline bool ofport_is_internal(const struct ofport *);

 static int update_port(struct ofproto *, const char *devname);
 static int init_ports(struct ofproto *);
@@ -320,6 +321,7 @@ static uint64_t pick_datapath_id(const struct ofproto *);
 static uint64_t pick_fallback_dpid(void);
 static void ofproto_destroy__(struct ofproto *);
 static void update_mtu(struct ofproto *, struct ofport *);
+static void update_mtu_ofproto(struct ofproto *);
 static void meter_delete(struct ofproto *, uint32_t first, uint32_t last);
 static void meter_insert_rule(struct rule *);

@@ -2390,9 +2392,14 @@ error:
 static void
 ofport_remove(struct ofport *ofport)
 {
+    struct ofproto *p = ofport->ofproto;
+    bool is_internal = ofport_is_internal(ofport);
+
     connmgr_send_port_status(ofport->ofproto->connmgr, NULL, &ofport->pp,
                              OFPPR_DELETE);
     ofport_destroy(ofport, true);
+    if (!is_internal)
+        update_mtu_ofproto(p);
 }

 /* If 'ofproto' contains an ofport named 'name', removes it from 'ofproto' and
@@ -2673,6 +2680,12 @@ init_ports(struct ofproto *p)
     return 0;
 }

+static inline bool
+ofport_is_internal(const struct ofport *port)
+{
+    return strcmp(netdev_get_type(port->netdev), "internal") == 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
@@ -2687,7 +2700,7 @@ find_min_mtu(struct ofproto *p)

         /* Skip any internal ports, since that's what we're trying to
          * set. */
-        if (!strcmp(netdev_get_type(netdev), "internal")) {
+        if (ofport_is_internal(ofport)) {
             continue;
         }

@@ -2707,15 +2720,14 @@ find_min_mtu(struct ofproto *p)
 static void
 update_mtu(struct ofproto *p, struct ofport *port)
 {
-    struct ofport *ofport;
     struct netdev *netdev = port->netdev;
-    int dev_mtu, old_min;
+    int dev_mtu;

     if (netdev_get_mtu(netdev, &dev_mtu)) {
         port->mtu = 0;
         return;
     }
-    if (!strcmp(netdev_get_type(port->netdev), "internal")) {
+    if (ofport_is_internal(port)) {
         if (dev_mtu > p->min_mtu) {
            if (!netdev_set_mtu(port->netdev, p->min_mtu)) {
                dev_mtu = p->min_mtu;
@@ -2725,9 +2737,18 @@ update_mtu(struct ofproto *p, struct ofport *port)
         return;
     }

-    /* For non-internal port find new min mtu. */
-    old_min = p->min_mtu;
     port->mtu = dev_mtu;
+    /* For non-internal port find new min mtu. */
+    update_mtu_ofproto(p);
+}
+
+static void
+update_mtu_ofproto(struct ofproto *p)
+{
+    /* For non-internal port find new min mtu. */
+    struct ofport *ofport;
+    int old_min = p->min_mtu;
+
     p->min_mtu = find_min_mtu(p);
     if (p->min_mtu == old_min) {
         return;
@@ -2736,7 +2757,7 @@ update_mtu(struct ofproto *p, struct ofport *port)
     HMAP_FOR_EACH (ofport, hmap_node, &p->ports) {
         struct netdev *netdev = ofport->netdev;

-        if (!strcmp(netdev_get_type(netdev), "internal")) {
+        if (ofport_is_internal(ofport)) {
             if (!netdev_set_mtu(netdev, p->min_mtu)) {
                 ofport->mtu = p->min_mtu;
             }
-- 
2.8.0