netdev-offload: Set 'miss_api_supported' to be under netdev.

Cited commit introduced a flag in dpif-netdev level, to optimize
performance and avoid hw_miss_packet_recover() for devices with no such
support.
However, there is a race condition between traffic processing and
assigning a 'flow_api' object to the netdev. In such case, EOPNOTSUPP is
returned by netdev_hw_miss_packet_recover() in netdev-offload.c layer
because 'flow_api' is not yet initialized. As a result, the flag is
falsely disabled, and subsequent packets won't be recovered, though they
should.

In order to fix it, move the flag to be in netdev-offload layer, to
avoid that race.

Fixes: 6e50c1651869 ("dpif-netdev: Avoid hw_miss_packet_recover() for devices with no support.")
Signed-off-by: Eli Britstein <elibr@nvidia.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

4 files changed, 33 insertions, 16 deletions

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 3d9d8929f..24b64b168 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -431,7 +431,6 @@ struct dp_netdev_rxq {
     unsigned intrvl_idx;               /* Write index for 'cycles_intrvl'. */
     struct dp_netdev_pmd_thread *pmd;  /* pmd thread that polls this queue. */
     bool is_vhost;                     /* Is rxq of a vhost port. */
-    bool hw_miss_api_supported;        /* hw_miss_packet_recover() supported.*/
 
     /* Counters of cycles spent successfully polling and processing pkts. */
     atomic_ullong cycles[RXQ_N_CYCLES];
@@ -5431,7 +5430,6 @@ port_reconfigure(struct dp_netdev_port *port)
 
         port->rxqs[i].port = port;
         port->rxqs[i].is_vhost = !strncmp(port->type, "dpdkvhost", 9);
-        port->rxqs[i].hw_miss_api_supported = true;
 
         err = netdev_rxq_open(netdev, &port->rxqs[i].rx, i);
         if (err) {
@@ -8031,17 +8029,15 @@ dp_netdev_hw_flow(const struct dp_netdev_pmd_thread *pmd,
 #ifdef ALLOW_EXPERIMENTAL_API /* Packet restoration API required. */
     /* Restore the packet if HW processing was terminated before completion. */
     struct dp_netdev_rxq *rxq = pmd->ctx.last_rxq;
+    bool miss_api_supported;
 
-    if (rxq->hw_miss_api_supported) {
+    atomic_read_relaxed(&rxq->port->netdev->hw_info.miss_api_supported,
+                        &miss_api_supported);
+    if (miss_api_supported) {
         int err = netdev_hw_miss_packet_recover(rxq->port->netdev, packet);
-        if (err) {
-            if (err != EOPNOTSUPP) {
-                COVERAGE_INC(datapath_drop_hw_miss_recover);
-                return -1;
-            } else {
-                /* API unsupported by the port; avoid subsequent calls. */
-                rxq->hw_miss_api_supported = false;
-            }
+        if (err && err != EOPNOTSUPP) {
+            COVERAGE_INC(datapath_drop_hw_miss_recover);
+            return -1;
         }
     }
 #endif
diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
index fb108c0d5..eea8fadc0 100644
--- a/lib/netdev-offload.c
+++ b/lib/netdev-offload.c
@@ -182,6 +182,7 @@ netdev_assign_flow_api(struct netdev *netdev)
     CMAP_FOR_EACH (rfa, cmap_node, &netdev_flow_apis) {
         if (!rfa->flow_api->init_flow_api(netdev)) {
             ovs_refcount_ref(&rfa->refcnt);
+            atomic_store_relaxed(&netdev->hw_info.miss_api_supported, true);
             ovsrcu_set(&netdev->flow_api, rfa->flow_api);
             VLOG_INFO("%s: Assigned flow API '%s'.",
                       netdev_get_name(netdev), rfa->flow_api->type);
@@ -190,6 +191,7 @@ netdev_assign_flow_api(struct netdev *netdev)
         VLOG_DBG("%s: flow API '%s' is not suitable.",
                  netdev_get_name(netdev), rfa->flow_api->type);
     }
+    atomic_store_relaxed(&netdev->hw_info.miss_api_supported, false);
     VLOG_INFO("%s: No suitable flow API found.", netdev_get_name(netdev));
 
     return -1;
@@ -263,12 +265,28 @@ int
 netdev_hw_miss_packet_recover(struct netdev *netdev,
                               struct dp_packet *packet)
 {
-    const struct netdev_flow_api *flow_api =
-        ovsrcu_get(const struct netdev_flow_api *, &netdev->flow_api);
+    const struct netdev_flow_api *flow_api;
+    bool miss_api_supported;
+    int rv;
+
+    atomic_read_relaxed(&netdev->hw_info.miss_api_supported,
+                        &miss_api_supported);
+    if (!miss_api_supported) {
+        return EOPNOTSUPP;
+    }
+
+    flow_api = ovsrcu_get(const struct netdev_flow_api *, &netdev->flow_api);
+    if (!flow_api || !flow_api->hw_miss_packet_recover) {
+        return EOPNOTSUPP;
+    }
+
+    rv = flow_api->hw_miss_packet_recover(netdev, packet);
+    if (rv == EOPNOTSUPP) {
+        /* API unsupported by the port; avoid subsequent calls. */
+        atomic_store_relaxed(&netdev->hw_info.miss_api_supported, false);
+    }
 
-    return (flow_api && flow_api->hw_miss_packet_recover)
-            ? flow_api->hw_miss_packet_recover(netdev, packet)
-            : EOPNOTSUPP;
+    return rv;
 }
 
 int
diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h
index 93eb2df48..13ab06d11 100644
--- a/lib/netdev-offload.h
+++ b/lib/netdev-offload.h
@@ -20,6 +20,7 @@
 
 #include "openvswitch/netdev.h"
 #include "openvswitch/types.h"
+#include "ovs-atomic.h"
 #include "ovs-rcu.h"
 #include "ovs-thread.h"
 #include "packets.h"
@@ -45,6 +46,7 @@ struct ovs_action_push_tnl;
 /* Offload-capable (HW) netdev information */
 struct netdev_hw_info {
     bool oor;		/* Out of Offload Resources ? */
+    atomic_bool miss_api_supported;  /* hw_miss_packet_recover() supported.*/
     int offload_count;  /* Pending (non-offloaded) flow count */
     int pending_count;  /* Offloaded flow count */
     OVSRCU_TYPE(void *) offload_data; /* Offload metadata. */
diff --git a/lib/netdev.c b/lib/netdev.c
index ce0d4117a..c79778378 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -431,6 +431,7 @@ netdev_open(const char *name, const char *type, struct netdev **netdevp)
                     seq_read(netdev->reconfigure_seq);
                 ovsrcu_set(&netdev->flow_api, NULL);
                 netdev->hw_info.oor = false;
+                atomic_init(&netdev->hw_info.miss_api_supported, false);
                 netdev->node = shash_add(&netdev_shash, name, netdev);
 
                 /* By default enable one tx and rx queue per netdev. */