The PMD reference taken is not actually used, it is only needed to get
the dp_netdev linked. Additionally, the taking of the PMD reference
does not protect against the disappearance of the dp_netdev,
so it is misleading.

The dp reference is protected by the way the ports are being deleted
during datapath deletion. No further offload request should be found
past a flush, so it is safe to keep this reference in the offload item.

Signed-off-by: Gaetan Rivet <[email protected]>
---
 lib/dpif-netdev.c | 50 +++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index bb03cf137..4d886092b 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -348,7 +348,6 @@ enum {
 };
 
 struct dp_offload_flow_item {
-    struct dp_netdev_pmd_thread *pmd;
     struct dp_netdev_flow *flow;
     int op;
     struct match match;
@@ -358,7 +357,6 @@ struct dp_offload_flow_item {
 };
 
 struct dp_offload_flush_item {
-    struct dp_netdev *dp;
     struct netdev *netdev;
     struct ovs_barrier *barrier;
 };
@@ -372,6 +370,7 @@ struct dp_offload_thread_item {
     struct mpsc_queue_node node;
     enum dp_offload_type type;
     long long int timestamp;
+    struct dp_netdev *dp;
     union dp_offload_thread_data data[0];
 };
 
@@ -2559,10 +2558,10 @@ flow_mark_has_no_ref(uint32_t mark)
 }
 
 static int
-mark_to_flow_disassociate(struct dp_netdev_pmd_thread *pmd,
+mark_to_flow_disassociate(struct dp_netdev *dp,
                           struct dp_netdev_flow *flow)
 {
-    const char *dpif_type_str = dpif_normalize_type(pmd->dp->class->type);
+    const char *dpif_type_str = dpif_normalize_type(dp->class->type);
     struct cmap_node *mark_node = CONST_CAST(struct cmap_node *,
                                              &flow->mark_node);
     unsigned int tid = netdev_offload_thread_id();
@@ -2591,9 +2590,9 @@ mark_to_flow_disassociate(struct dp_netdev_pmd_thread 
*pmd,
         if (port) {
             /* Taking a global 'port_rwlock' to fulfill thread safety
              * restrictions regarding netdev port mapping. */
-            ovs_rwlock_rdlock(&pmd->dp->port_rwlock);
+            ovs_rwlock_rdlock(&dp->port_rwlock);
             ret = netdev_flow_del(port, &flow->mega_ufid, NULL);
-            ovs_rwlock_unlock(&pmd->dp->port_rwlock);
+            ovs_rwlock_unlock(&dp->port_rwlock);
             netdev_close(port);
         }
 
@@ -2635,7 +2634,7 @@ mark_to_flow_find(const struct dp_netdev_pmd_thread *pmd,
 }
 
 static struct dp_offload_thread_item *
-dp_netdev_alloc_flow_offload(struct dp_netdev_pmd_thread *pmd,
+dp_netdev_alloc_flow_offload(struct dp_netdev *dp,
                              struct dp_netdev_flow *flow,
                              int op)
 {
@@ -2646,13 +2645,12 @@ dp_netdev_alloc_flow_offload(struct 
dp_netdev_pmd_thread *pmd,
     flow_offload = &item->data->flow;
 
     item->type = DP_OFFLOAD_FLOW;
+    item->dp = dp;
 
-    flow_offload->pmd = pmd;
     flow_offload->flow = flow;
     flow_offload->op = op;
 
     dp_netdev_flow_ref(flow);
-    dp_netdev_pmd_try_ref(pmd);
 
     return item;
 }
@@ -2671,7 +2669,6 @@ dp_netdev_free_flow_offload(struct dp_offload_thread_item 
*offload)
 {
     struct dp_offload_flow_item *flow_offload = &offload->data->flow;
 
-    dp_netdev_pmd_unref(flow_offload->pmd);
     dp_netdev_flow_unref(flow_offload->flow);
     ovsrcu_postpone(dp_netdev_free_flow_offload__, offload);
 }
@@ -2714,9 +2711,9 @@ dp_netdev_offload_flow_enqueue(struct 
dp_offload_thread_item *item)
 }
 
 static int
-dp_netdev_flow_offload_del(struct dp_offload_flow_item *offload)
+dp_netdev_flow_offload_del(struct dp_offload_thread_item *item)
 {
-    return mark_to_flow_disassociate(offload->pmd, offload->flow);
+    return mark_to_flow_disassociate(item->dp, item->data->flow.flow);
 }
 
 /*
@@ -2731,12 +2728,13 @@ dp_netdev_flow_offload_del(struct dp_offload_flow_item 
*offload)
  * valid, thus only item 2 needed.
  */
 static int
-dp_netdev_flow_offload_put(struct dp_offload_flow_item *offload)
+dp_netdev_flow_offload_put(struct dp_offload_thread_item *item)
 {
-    struct dp_netdev_pmd_thread *pmd = offload->pmd;
+    struct dp_offload_flow_item *offload = &item->data->flow;
+    struct dp_netdev *dp = item->dp;
     struct dp_netdev_flow *flow = offload->flow;
     odp_port_t in_port = flow->flow.in_port.odp_port;
-    const char *dpif_type_str = dpif_normalize_type(pmd->dp->class->type);
+    const char *dpif_type_str = dpif_normalize_type(dp->class->type);
     bool modification = offload->op == DP_NETDEV_FLOW_OFFLOAD_OP_MOD
                         && flow->mark != INVALID_FLOW_MARK;
     struct offload_info info;
@@ -2782,12 +2780,12 @@ dp_netdev_flow_offload_put(struct dp_offload_flow_item 
*offload)
 
     /* Taking a global 'port_rwlock' to fulfill thread safety
      * restrictions regarding the netdev port mapping. */
-    ovs_rwlock_rdlock(&pmd->dp->port_rwlock);
+    ovs_rwlock_rdlock(&dp->port_rwlock);
     ret = netdev_flow_put(port, &offload->match,
                           CONST_CAST(struct nlattr *, offload->actions),
                           offload->actions_len, &flow->mega_ufid, &info,
                           NULL);
-    ovs_rwlock_unlock(&pmd->dp->port_rwlock);
+    ovs_rwlock_unlock(&dp->port_rwlock);
     netdev_close(port);
 
     if (ret) {
@@ -2804,7 +2802,7 @@ err_free:
     if (!modification) {
         flow_mark_free(mark);
     } else {
-        mark_to_flow_disassociate(pmd, flow);
+        mark_to_flow_disassociate(item->dp, flow);
     }
     return -1;
 }
@@ -2819,15 +2817,15 @@ dp_offload_flow(struct dp_offload_thread_item *item)
     switch (flow_offload->op) {
     case DP_NETDEV_FLOW_OFFLOAD_OP_ADD:
         op = "add";
-        ret = dp_netdev_flow_offload_put(flow_offload);
+        ret = dp_netdev_flow_offload_put(item);
         break;
     case DP_NETDEV_FLOW_OFFLOAD_OP_MOD:
         op = "modify";
-        ret = dp_netdev_flow_offload_put(flow_offload);
+        ret = dp_netdev_flow_offload_put(item);
         break;
     case DP_NETDEV_FLOW_OFFLOAD_OP_DEL:
         op = "delete";
-        ret = dp_netdev_flow_offload_del(flow_offload);
+        ret = dp_netdev_flow_offload_del(item);
         break;
     default:
         OVS_NOT_REACHED();
@@ -2843,9 +2841,9 @@ dp_offload_flush(struct dp_offload_thread_item *item)
 {
     struct dp_offload_flush_item *flush = &item->data->flush;
 
-    ovs_rwlock_rdlock(&flush->dp->port_rwlock);
+    ovs_rwlock_rdlock(&item->dp->port_rwlock);
     netdev_flow_flush(flush->netdev);
-    ovs_rwlock_unlock(&flush->dp->port_rwlock);
+    ovs_rwlock_unlock(&item->dp->port_rwlock);
 
     ovs_barrier_block(flush->barrier);
 
@@ -2931,7 +2929,7 @@ queue_netdev_flow_del(struct dp_netdev_pmd_thread *pmd,
         return;
     }
 
-    offload = dp_netdev_alloc_flow_offload(pmd, flow,
+    offload = dp_netdev_alloc_flow_offload(pmd->dp, flow,
                                            DP_NETDEV_FLOW_OFFLOAD_OP_DEL);
     offload->timestamp = pmd->ctx.now;
     dp_netdev_offload_flow_enqueue(offload);
@@ -3015,7 +3013,7 @@ queue_netdev_flow_put(struct dp_netdev_pmd_thread *pmd,
         return;
     }
 
-    item = dp_netdev_alloc_flow_offload(pmd, flow, op);
+    item = dp_netdev_alloc_flow_offload(pmd->dp, flow, op);
     flow_offload = &item->data->flow;
     flow_offload->match = *match;
     flow_offload->actions = xmalloc(actions_len);
@@ -3062,10 +3060,10 @@ dp_netdev_offload_flush_enqueue(struct dp_netdev *dp,
 
         item = xmalloc(sizeof *item + sizeof *flush);
         item->type = DP_OFFLOAD_FLUSH;
+        item->dp = dp;
         item->timestamp = now_us;
 
         flush = &item->data->flush;
-        flush->dp = dp;
         flush->netdev = netdev;
         flush->barrier = barrier;
 
-- 
2.31.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to