dpif-netdev: Fix crash in dpif_netdev_execute().

dp_netdev_get_pmd() is allowed to return NULL (even if we call it with
NON_PMD_CORE_ID) for different reasons:

* Since we use RCU to protect pmd threads, it is possible that
  ovs_refcount_try_ref_rcu() has failed.
* During reconfiguration we destroy every thread.

This commit makes sure that we always handle the case when
dp_netdev_get_pmd() returns NULL without crashing (the change in
dpif_netdev_run() doesn't fix anything, because everything is happening
in the main thread, but it's better to honor the interface in case we
change our threading model).

This actually fixes a pretty serious crash that happens if
dpif_netdev_execute() is called from a non pmd thread while
reconfiguration is happening.  It can be triggered by enabling bfd
(because it's handled by the monitor thread, which is a non pmd thread)
on an interface and changing something that requires datapath
reconfiguration (n_rxq, pmd-cpu-mask, mtu).

A testcase that reproduces the race condition is included.

This is a possible backtrace of the segfault:

 #0  0x000000000060c7f1 in dp_execute_cb (aux_=0x7f1dd2d2a320,
 packets_=0x7f1dd2d2a370, a=0x7f1dd2d2a658, may_steal=false) at
 ../lib/dpif-netdev.c:4357
 #1  0x00000000006448b2 in odp_execute_actions (dp=0x7f1dd2d2a320,
 batch=0x7f1dd2d2a370, steal=false, actions=0x7f1dd2d2a658,
 actions_len=8,
     dp_execute_action=0x60c7a5 <dp_execute_cb>) at
 ../lib/odp-execute.c:538
 #2  0x000000000060d00c in dp_netdev_execute_actions (pmd=0x0,
 packets=0x7f1dd2d2a370, may_steal=false, flow=0x7f1dd2d2ae70,
 actions=0x7f1dd2d2a658, actions_len=8,
     now=44965873) at ../lib/dpif-netdev.c:4577
 #3  0x000000000060834a in dpif_netdev_execute (dpif=0x2b67b70,
 execute=0x7f1dd2d2a578) at ../lib/dpif-netdev.c:2624
 #4  0x0000000000608441 in dpif_netdev_operate (dpif=0x2b67b70,
 ops=0x7f1dd2d2a5c8, n_ops=1) at ../lib/dpif-netdev.c:2654
 #5  0x0000000000610a30 in dpif_operate (dpif=0x2b67b70,
 ops=0x7f1dd2d2a5c8, n_ops=1) at ../lib/dpif.c:1268
 #6  0x000000000061098c in dpif_execute (dpif=0x2b67b70,
 execute=0x7f1dd2d2aa50) at ../lib/dpif.c:1233
 #7  0x00000000005b9008 in ofproto_dpif_execute_actions__
 (ofproto=0x2b69360, version=18446744073709551614, flow=0x7f1dd2d2ae70,
 rule=0x0, ofpacts=0x7f1dd2d2b100,
     ofpacts_len=16, indentation=0, depth=0, resubmits=0,
 packet=0x7f1dd2d2b5c0) at ../ofproto/ofproto-dpif.c:3806
 #8  0x00000000005b907a in ofproto_dpif_execute_actions
 (ofproto=0x2b69360, version=18446744073709551614, flow=0x7f1dd2d2ae70,
 rule=0x0, ofpacts=0x7f1dd2d2b100,
     ofpacts_len=16, packet=0x7f1dd2d2b5c0) at
 ../ofproto/ofproto-dpif.c:3823
 #9  0x00000000005dea9b in xlate_send_packet (ofport=0x2b98380,
 oam=false, packet=0x7f1dd2d2b5c0) at
 ../ofproto/ofproto-dpif-xlate.c:5792
 #10 0x00000000005bab12 in ofproto_dpif_send_packet (ofport=0x2b98380,
 oam=false, packet=0x7f1dd2d2b5c0) at ../ofproto/ofproto-dpif.c:4628
 #11 0x00000000005c3fc8 in monitor_mport_run (mport=0x2b8cd00,
 packet=0x7f1dd2d2b5c0) at ../ofproto/ofproto-dpif-monitor.c:287
 #12 0x00000000005c3d9b in monitor_run () at
 ../ofproto/ofproto-dpif-monitor.c:227
 #13 0x00000000005c3cab in monitor_main (args=0x0) at
 ../ofproto/ofproto-dpif-monitor.c:189
 #14 0x00000000006a183a in ovsthread_wrapper (aux_=0x2b8afd0) at
 ../lib/ovs-thread.c:342
 #15 0x00007f1dd75eb444 in start_thread (arg=0x7f1dd2d2c700) at
 pthread_create.c:333
 #16 0x00007f1dd6e1d20d in clone () at
 ../sysdeps/unix/sysv/linux/x86_64/clone.S:109

Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
Acked-by: Ben Pfaff <blp@ovn.org>

1 files changed, 20 insertions, 13 deletions

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 6e09e44eb..83a309788 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2602,6 +2602,9 @@ dpif_netdev_execute(struct dpif *dpif, struct dpif_execute *execute)
     pmd = ovsthread_getspecific(dp->per_pmd_key);
     if (!pmd) {
         pmd = dp_netdev_get_pmd(dp, NON_PMD_CORE_ID);
+        if (!pmd) {
+            return EBUSY;
+        }
     }
 
     /* If the current thread is non-pmd thread, acquires
@@ -2967,25 +2970,28 @@ dpif_netdev_run(struct dpif *dpif)
 {
     struct dp_netdev_port *port;
     struct dp_netdev *dp = get_dp_netdev(dpif);
-    struct dp_netdev_pmd_thread *non_pmd = dp_netdev_get_pmd(dp,
-                                                             NON_PMD_CORE_ID);
+    struct dp_netdev_pmd_thread *non_pmd;
     uint64_t new_tnl_seq;
 
     ovs_mutex_lock(&dp->port_mutex);
-    ovs_mutex_lock(&dp->non_pmd_mutex);
-    HMAP_FOR_EACH (port, node, &dp->ports) {
-        if (!netdev_is_pmd(port->netdev)) {
-            int i;
+    non_pmd = dp_netdev_get_pmd(dp, NON_PMD_CORE_ID);
+    if (non_pmd) {
+        ovs_mutex_lock(&dp->non_pmd_mutex);
+        HMAP_FOR_EACH (port, node, &dp->ports) {
+            if (!netdev_is_pmd(port->netdev)) {
+                int i;
 
-            for (i = 0; i < port->n_rxq; i++) {
-                dp_netdev_process_rxq_port(non_pmd, port, port->rxqs[i].rxq);
+                for (i = 0; i < port->n_rxq; i++) {
+                    dp_netdev_process_rxq_port(non_pmd, port,
+                                               port->rxqs[i].rxq);
+                }
             }
         }
-    }
-    dpif_netdev_xps_revalidate_pmd(non_pmd, time_msec(), false);
-    ovs_mutex_unlock(&dp->non_pmd_mutex);
+        dpif_netdev_xps_revalidate_pmd(non_pmd, time_msec(), false);
+        ovs_mutex_unlock(&dp->non_pmd_mutex);
 
-    dp_netdev_pmd_unref(non_pmd);
+        dp_netdev_pmd_unref(non_pmd);
+    }
 
     if (dp_netdev_is_reconf_required(dp) || ports_require_restart(dp)) {
         reconfigure_pmd_threads(dp);
@@ -3189,7 +3195,8 @@ dp_netdev_pmd_reload_done(struct dp_netdev_pmd_thread *pmd)
 }
 
 /* Finds and refs the dp_netdev_pmd_thread on core 'core_id'.  Returns
- * the pointer if succeeds, otherwise, NULL.
+ * the pointer if succeeds, otherwise, NULL (it can return NULL even if
+ * 'core_id' is NON_PMD_CORE_ID).
  *
  * Caller must unrefs the returned reference.  */
 static struct dp_netdev_pmd_thread *