ofproto-dpif: Tolerate spontaneous changes in datapath port numbers.

This can happen on ESX.

Also adds a test to make sure this works.

Bug #17634.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Tested-by: Guolin Yang <gyang@vmware.com>

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 8763e5c..4890398 100644 (file)
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -52,6 +52,7 @@
 #include "shash.h"
 #include "sset.h"
 #include "timeval.h"
+#include "unixctl.h"
 #include "util.h"
 #include "vlog.h"
 
@@ -1327,6 +1328,41 @@ const struct dpif_class dpif_netdev_class = {
     dpif_netdev_recv_purge,
 };
 
+static void
+dpif_dummy_change_port_number(struct unixctl_conn *conn, int argc OVS_UNUSED,
+                              const char *argv[], void *aux OVS_UNUSED)
+{
+    struct dp_netdev_port *port;
+    struct dp_netdev *dp;
+    int port_no;
+
+    dp = shash_find_data(&dp_netdevs, argv[1]);
+    if (!dp || !dpif_netdev_class_is_dummy(dp->class)) {
+        unixctl_command_reply_error(conn, "unknown datapath or not a dummy");
+        return;
+    }
+
+    if (get_port_by_name(dp, argv[2], &port)) {
+        unixctl_command_reply_error(conn, "unknown port");
+        return;
+    }
+
+    port_no = atoi(argv[3]);
+    if (port_no <= 0 || port_no >= MAX_PORTS) {
+        unixctl_command_reply_error(conn, "bad port number");
+        return;
+    }
+    if (dp->ports[port_no]) {
+        unixctl_command_reply_error(conn, "port number already in use");
+        return;
+    }
+    dp->ports[odp_to_u32(port->port_no)] = NULL;
+    dp->ports[port_no] = port;
+    port->port_no = u32_to_odp(port_no);
+    dp->serial++;
+    unixctl_command_reply(conn, NULL);
+}
+
 static void
 dpif_dummy_register__(const char *type)
 {
@@ -1356,4 +1392,8 @@ dpif_dummy_register(bool override)
     }
 
     dpif_dummy_register__("dummy");
+
+    unixctl_command_register("dpif-dummy/change-port-number",
+                             "DP PORT NEW-NUMBER",
+                             3, 3, dpif_dummy_change_port_number, NULL);
 }

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index a02d0ab..ca0a317 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -950,6 +950,24 @@ process_dpif_port_change(struct dpif_backer *backer, const char *devname)
         /* The port was added, but we don't know with which
          * ofproto we should associate it.  Delete it. */
         dpif_port_del(backer->dpif, port.port_no);
+    } else {
+        struct ofport_dpif *ofport;
+
+        ofport = ofport_dpif_cast(shash_find_data(
+                                      &ofproto->up.port_by_name, devname));
+        if (ofport
+            && ofport->odp_port != port.port_no
+            && !odp_port_to_ofport(backer, port.port_no))
+        {
+            /* 'ofport''s datapath port number has changed from
+             * 'ofport->odp_port' to 'port.port_no'.  Update our internal data
+             * structures to match. */
+            hmap_remove(&backer->odp_to_ofport_map, &ofport->odp_port_node);
+            ofport->odp_port = port.port_no;
+            hmap_insert(&backer->odp_to_ofport_map, &ofport->odp_port_node,
+                        hash_odp_port(port.port_no));
+            backer->need_revalidate = REV_RECONFIGURE;
+        }
     }
     dpif_port_destroy(&port);
 }

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index c3c1c64..2728a28 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -2632,3 +2632,24 @@ skb_priority=0,icmp,in_port=1,nw_src=10.0.0.4,nw_dst=10.0.0.3,nw_tos=0,nw_ecn=0,
 ])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([ofproto-dpif - datapath port number change])
+OVS_VSWITCHD_START([set Bridge br0 fail-mode=standalone])
+ADD_OF_PORTS([br0], 1)
+
+# Trace a flow that should output to p1.
+AT_CHECK([ovs-appctl ofproto/trace br0 in_port=LOCAL,dl_src=10:20:30:40:50:60],
+  [0], [stdout])
+AT_CHECK([tail -1 stdout], [0], [Datapath actions: 1
+])
+
+# Change p1's port number to 5.
+AT_CHECK([ovs-appctl dpif-dummy/change-port-number ovs-dummy p1 5])
+
+# Trace a flow that should output to p1 in its new location.
+AT_CHECK([ovs-appctl ofproto/trace br0 in_port=LOCAL,dl_src=10:20:30:40:50:60],
+  [0], [stdout])
+AT_CHECK([tail -1 stdout], [0], [Datapath actions: 5
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP