diff mbox series

[ovs-dev,V3,3/4] dpif-netdev: Fix offloads of modified flows

Message ID 20210726081455.10632-4-elibr@nvidia.com
State Accepted
Headers show
Series dpif-netdev offload transitions | expand

Commit Message

Eli Britstein July 26, 2021, 8:14 a.m. UTC 
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@nvidia.com>
Reviewed-by: Gaetan Rivet <gaetanr@nvidia.com>
---
 lib/dpif-netdev.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)
diff mbox series

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 5fe06b996..6a7bed840 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2714,10 +2714,9 @@  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, int op)
 {
     struct dp_flow_offload_item *offload;
-    int op;
 
     if (!netdev_is_flow_api_enabled()) {
         return;
@@ -2730,11 +2729,6 @@  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;
-    }
     offload = dp_netdev_alloc_flow_offload(pmd, flow, op);
     offload->match = *match;
     offload->actions = xmalloc(actions_len);
@@ -3586,7 +3580,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, DP_NETDEV_FLOW_OFFLOAD_OP_ADD);
 
     if (OVS_UNLIKELY(!VLOG_DROP_DBG((&upcall_rl)))) {
         struct ds ds = DS_EMPTY_INITIALIZER;
@@ -3673,7 +3667,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,
+                                  DP_NETDEV_FLOW_OFFLOAD_OP_MOD);
 
             if (stats) {
                 get_dpif_flow_status(pmd->dp, netdev_flow, stats, NULL);