diff mbox series

[ovs-dev,branch-2.12] dpif-netlink: avoid netlink modify flow put op failed after tc modify flow put op failed.

Message ID 1584773661-6886-1-git-send-email-wenxu@ucloud.cn
State Accepted
Commit 757888c7af5c30d3e11c0afdf348eb366004fe1d
Headers show
Series [ovs-dev,branch-2.12] dpif-netlink: avoid netlink modify flow put op failed after tc modify flow put op failed. | expand

Commit Message

wenxu March 21, 2020, 6:54 a.m. UTC 
From: wenxu <wenxu@ucloud.cn>

The tc modify flow put always delete the original flow first and
then add the new flow. If the modfiy flow put operation failed,
the flow put operation will change from modify to create if success
to delete the original flow in tc (which will be always failed with
ENOENT, the flow is already be deleted before add the new flow in tc).
Finally, the modify flow put will failed to add in kernel datapath.

Signed-off-by: wenxu <wenxu@ucloud.cn>
---
 lib/dpif-netlink.c      | 7 ++++++-
 lib/netdev-offload-tc.c | 7 +++++--
 lib/netdev-offload.h    | 3 +++
 3 files changed, 14 insertions(+), 3 deletions(-)

Comments

Simon Horman March 25, 2020, 5:41 p.m. UTC | #1 
On Sat, Mar 21, 2020 at 02:54:21PM +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
> 
> The tc modify flow put always delete the original flow first and
> then add the new flow. If the modfiy flow put operation failed,
> the flow put operation will change from modify to create if success
> to delete the original flow in tc (which will be always failed with
> ENOENT, the flow is already be deleted before add the new flow in tc).
> Finally, the modify flow put will failed to add in kernel datapath.
> 
> Signed-off-by: wenxu <wenxu@ucloud.cn>

Thanks, applied to branch 2.12.
diff mbox series

Patch

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 7bc71d6..7e088c0 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -2038,6 +2038,7 @@  parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put)
     info.dpif_class = dpif_class;
     info.tp_dst_port = dst_port;
     info.tunnel_csum_on = csum_on;
+    info.tc_modify_flow_deleted = false;
     err = netdev_flow_put(dev, &match,
                           CONST_CAST(struct nlattr *, put->actions),
                           put->actions_len,
@@ -2088,7 +2089,11 @@  parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put)
 out:
     if (err && err != EEXIST && (put->flags & DPIF_FP_MODIFY)) {
         /* Modified rule can't be offloaded, try and delete from HW */
-        int del_err = netdev_flow_del(dev, put->ufid, put->stats);
+        int del_err = 0;
+
+        if (!info.tc_modify_flow_deleted) {
+            del_err = netdev_flow_del(dev, put->ufid, put->stats);
+        }
 
         if (!del_err) {
             /* Delete from hw success, so old flow was offloaded.
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 4cc044b..c95de1e 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -1359,9 +1359,12 @@  netdev_tc_flow_put(struct netdev *netdev, struct match *match,
     block_id = get_block_id_from_netdev(netdev);
     handle = get_ufid_tc_mapping(ufid, &prio, NULL);
     if (handle && prio) {
+        bool flow_deleted;
+
         VLOG_DBG_RL(&rl, "updating old handle: %d prio: %d", handle, prio);
-        del_filter_and_ufid_mapping(ifindex, prio, handle, block_id, ufid,
-                                    hook);
+        flow_deleted = !del_filter_and_ufid_mapping(ifindex, prio, handle,
+                                                    block_id, ufid, hook);
+        info->tc_modify_flow_deleted = flow_deleted;
     }
 
     if (!prio) {
diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h
index 97a5006..34721ef 100644
--- a/lib/netdev-offload.h
+++ b/lib/netdev-offload.h
@@ -71,6 +71,9 @@  struct offload_info {
      * it will be in the pkt meta data.
      */
     uint32_t flow_mark;
+
+    bool tc_modify_flow_deleted; /* Indicate the tc modify flow put success
+                                  * to delete the original flow. */
 };
 
 int netdev_flow_flush(struct netdev *);