netdev, dpif: fix the crash/assert on port delete

a crash is seen in "netdev_ports_remove" when an interface is deleted and added
back in the system and when the interface is part of a bridge configuration.
e.g. steps:
  create a tap0 interface using "ip tuntap add.."
  add the tap0 interface to br0 using "ovs-vsctl add-port.."
  delete the tap0 interface from system using "ip tuntap del.."
  add the tap0 interface back in system using "ip tuntap add.."
                       (this changes the ifindex of the interface)
  delete tap0 from br0 using "ovs-vsctl del-port.."

In the function "netdev_ports_insert", two hmap entries were created for
mapping "portnum -> netdev" and "ifindex -> portnum".
When the interface is deleted from the system, the "netdev_ports_remove"
function is not getting called and the old ifindex entry is not getting
cleaned up from the "ifindex_to_port" hmap.

As part of the fix, added function "dpif_port_remove" which will call
"netdev_ports_remove" in the path where the interface deletion from the system
is detected.
Also, in "netdev_ports_remove", added the code where the "ifindex_to_port_data"
(ifindex -> portnum map node) is getting freed when the ifindex is not
available any more. (as the interface is already deleted.)

VMware-BZ: #1975788
Signed-off-by: Ashish Varma <ashishvarma.ovs@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>

1 files changed, 11 insertions, 5 deletions

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 0e86d041e..b14b339a9 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -414,7 +414,7 @@ type_run(const char *type)
         }
 
         SIMAP_FOR_EACH (node, &tmp_backers) {
-            dpif_port_del(backer->dpif, u32_to_odp(node->data));
+            dpif_port_del(backer->dpif, u32_to_odp(node->data), false);
         }
         simap_destroy(&tmp_backers);
 
@@ -571,7 +571,7 @@ process_dpif_port_change(struct dpif_backer *backer, const char *devname)
     } else if (!ofproto) {
         /* 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);
+        dpif_port_del(backer->dpif, port.port_no, false);
     } else {
         struct ofport_dpif *ofport;
 
@@ -762,7 +762,7 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
     dpif_port_dump_done(&port_dump);
 
     LIST_FOR_EACH_POP (garbage, list_node, &garbage_list) {
-        dpif_port_del(backer->dpif, garbage->odp_port);
+        dpif_port_del(backer->dpif, garbage->odp_port, false);
         free(garbage);
     }
 
@@ -1921,7 +1921,13 @@ port_destruct(struct ofport *port_, bool del)
          * assumes that removal of attached ports will happen as part of
          * destruction. */
         if (!port->is_tunnel) {
-            dpif_port_del(ofproto->backer->dpif, port->odp_port);
+            dpif_port_del(ofproto->backer->dpif, port->odp_port, false);
+        }
+    } else if (del) {
+        /* The underlying device is already deleted (e.g. tunctl -d).
+         * Calling dpif_port_remove to do local cleanup for the netdev */
+        if (!port->is_tunnel) {
+            dpif_port_del(ofproto->backer->dpif, port->odp_port, true);
         }
     }
 
@@ -3698,7 +3704,7 @@ port_del(struct ofproto *ofproto_, ofp_port_t ofp_port)
                          netdev_get_name(ofport->up.netdev));
     ofproto->backer->need_revalidate = REV_RECONFIGURE;
     if (!ofport->is_tunnel && !netdev_vport_is_patch(ofport->up.netdev)) {
-        error = dpif_port_del(ofproto->backer->dpif, ofport->odp_port);
+        error = dpif_port_del(ofproto->backer->dpif, ofport->odp_port, false);
         if (!error) {
             /* The caller is going to close ofport->up.netdev.  If this is a
              * bonded port, then the bond is using that netdev, so remove it