[ovs-dev,branch-2.8/2.9,backport] netdev-tc-offloads: Delete ufid tc mapping in the right place

Message ID 1542159364-91647-1-git-send-email-chrism@mellanox.com
State New
Headers show
Series
  • [ovs-dev,branch-2.8/2.9,backport] netdev-tc-offloads: Delete ufid tc mapping in the right place
Related show

Commit Message

Chris Mi Nov. 14, 2018, 1:36 a.m.
Currently, the ufid tc mapping is deleted in add_ufid_tc_mapping().
But if tc_replace_flower() failed, the old ufid tc mapping will not
be deleted. If another thread adds the same tc mapping successfully,
then there will be multiple mappings for the same ifindex, handle
and prio.

Fixes: 9116730db ("netdev-tc-offloads: Add ufid to tc/netdev map")
Signed-off-by: Chris Mi <chrism@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
 lib/netdev-tc-offloads.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

Comments

Simon Horman Nov. 16, 2018, 3:40 p.m. | #1
On Tue, Nov 13, 2018 at 08:36:04PM -0500, Chris Mi wrote:
> Currently, the ufid tc mapping is deleted in add_ufid_tc_mapping().
> But if tc_replace_flower() failed, the old ufid tc mapping will not
> be deleted. If another thread adds the same tc mapping successfully,
> then there will be multiple mappings for the same ifindex, handle
> and prio.
> 
> Fixes: 9116730db ("netdev-tc-offloads: Add ufid to tc/netdev map")
> Signed-off-by: Chris Mi <chrism@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>

Thanks Chris,

applied to branch-2.8 and branch-2.9.

Patch

diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index 1ca7d45..ea5ad59 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -161,8 +161,20 @@  del_ufid_tc_mapping(const ovs_u128 *ufid)
     ovs_mutex_unlock(&ufid_lock);
 }
 
-/* Add ufid entry to ufid_tc hashmap.
- * If entry exists already it will be replaced. */
+/* Wrapper function to delete filter and ufid tc mapping */
+static int
+del_filter_and_ufid_mapping(int ifindex, int prio, int handle,
+                            const ovs_u128 *ufid)
+{
+    int err;
+
+    err = tc_del_filter(ifindex, prio, handle);
+    del_ufid_tc_mapping(ufid);
+
+    return err;
+}
+
+/* Add ufid entry to ufid_tc hashmap. */
 static void
 add_ufid_tc_mapping(const ovs_u128 *ufid, int prio, int handle,
                     struct netdev *netdev, int ifindex)
@@ -171,8 +183,6 @@  add_ufid_tc_mapping(const ovs_u128 *ufid, int prio, int handle,
     size_t tc_hash = hash_int(hash_int(prio, handle), ifindex);
     struct ufid_tc_data *new_data = xzalloc(sizeof *new_data);
 
-    del_ufid_tc_mapping(ufid);
-
     new_data->ufid = *ufid;
     new_data->prio = prio;
     new_data->handle = handle;
@@ -1033,7 +1043,7 @@  netdev_tc_flow_put(struct netdev *netdev, struct match *match,
     handle = get_ufid_tc_mapping(ufid, &prio, NULL);
     if (handle && prio) {
         VLOG_DBG_RL(&rl, "updating old handle: %d prio: %d", handle, prio);
-        tc_del_filter(ifindex, prio, handle);
+        del_filter_and_ufid_mapping(ifindex, prio, handle, ufid);
     }
 
     if (!prio) {
@@ -1138,8 +1148,7 @@  netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
         }
     }
 
-    error = tc_del_filter(ifindex, prio, handle);
-    del_ufid_tc_mapping(ufid);
+    error = del_filter_and_ufid_mapping(ifindex, prio, handle, ufid);
 
     netdev_close(dev);