[ovs-dev] [PATCH V2 2/3] dpif-netdev: Fix offloads of modified flows

Eli Britstein elibr at nvidia.com
Mon Jul 12 15:07:09 UTC 2021


Association of a mark to a flow is done as part of its offload handling,
in the offloading thread. However, the PMD thread specifies whether an
offload request is an "add" or "modify" by the association of a mark to
the flow.
This is exposed to a race condition. A flow might be created with
actions that cannot be fully offloaded, for example flooding (before MAC
learning), and later modified to have actions that can be fully
offloaded. If the two requests are queued before the offload thread
handling, they are both marked as "add". When the offload thread handles
them, the first request is partially offloaded, and the second one is
ignored as the flow is already considered as offloaded.

Fix it by specifying add/modify of an offload request by the actual flow
state change, without relying on the mark.

Fixes: 3c7330ebf036 ("netdev-offload-dpdk: Support offload of output action.")
Signed-off-by: Eli Britstein <elibr at nvidia.com>
Reviewed-by: Gaetan Rivet <gaetanr at nvidia.com>
---
 lib/dpif-netdev.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 21b0e025d..9b2b8d6d9 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2451,7 +2451,8 @@ static void
 queue_netdev_flow_put(struct dp_netdev_pmd_thread *pmd,
                       struct dp_netdev_flow *flow, struct match *match,
                       const struct nlattr *actions, size_t actions_len,
-                      odp_port_t orig_in_port)
+                      odp_port_t orig_in_port,
+                      const struct dp_netdev_actions *old_actions)
 {
     struct dp_flow_offload_item *offload;
     int op;
@@ -2467,11 +2468,9 @@ queue_netdev_flow_put(struct dp_netdev_pmd_thread *pmd,
         ovsthread_once_done(&offload_thread_once);
     }
 
-    if (flow->mark != INVALID_FLOW_MARK) {
-        op = DP_NETDEV_FLOW_OFFLOAD_OP_MOD;
-    } else {
-        op = DP_NETDEV_FLOW_OFFLOAD_OP_ADD;
-    }
+    op = old_actions
+        ? DP_NETDEV_FLOW_OFFLOAD_OP_MOD
+        : DP_NETDEV_FLOW_OFFLOAD_OP_ADD;
     offload = dp_netdev_alloc_flow_offload(pmd, flow, op);
     offload->match = *match;
     offload->actions = xmalloc(actions_len);
@@ -3323,7 +3322,7 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd,
                 dp_netdev_flow_hash(&flow->ufid));
 
     queue_netdev_flow_put(pmd, flow, match, actions, actions_len,
-                          orig_in_port);
+                          orig_in_port, NULL);
 
     if (OVS_UNLIKELY(!VLOG_DROP_DBG((&upcall_rl)))) {
         struct ds ds = DS_EMPTY_INITIALIZER;
@@ -3410,7 +3409,8 @@ flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,
             ovsrcu_set(&netdev_flow->actions, new_actions);
 
             queue_netdev_flow_put(pmd, netdev_flow, match,
-                                  put->actions, put->actions_len, ODPP_NONE);
+                                  put->actions, put->actions_len, ODPP_NONE,
+                                  old_actions);
 
             if (stats) {
                 get_dpif_flow_status(pmd->dp, netdev_flow, stats, NULL);
-- 
2.28.0.2311.g225365fb51



More information about the dev mailing list