| Message ID | 20181112020838.17495-1-chrism@mellanox.com |
|---|---|
| State | Accepted |
| Delegated to: | Simon Horman |
| Headers | show |
| Series | [ovs-dev,ovs-dev,v2] netdev-tc-offloads: Delete ufid tc mapping in the right place | expand |
On Mon, Nov 12, 2018 at 11:08:38AM +0900, 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. > > * V2: > - Add a wrapper function del_filter_and_ufid_mapping() > > 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, I have applied this to master and branch-2.10. It looks like the fix is also relevant to branch-2.9 and branch-2.8 but does not apply cleanly there. Could you consider posting a backport to those branches? Thanks!
> -----Original Message----- > From: Simon Horman [mailto:simon.horman@netronome.com] > Sent: Tuesday, November 13, 2018 9:29 PM > To: Chris Mi <chrism@mellanox.com> > Cc: ovs-dev@openvswitch.org > Subject: Re: [ovs-dev v2] netdev-tc-offloads: Delete ufid tc mapping in the > right place > > On Mon, Nov 12, 2018 at 11:08:38AM +0900, 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. > > > > * V2: > > - Add a wrapper function del_filter_and_ufid_mapping() > > > > 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, > > I have applied this to master and branch-2.10. > > It looks like the fix is also relevant to branch-2.9 and branch-2.8 but does not > apply cleanly there. Could you consider posting a backport to those branches? > > Thanks! Hi Simon, Thanks for applying it. I posted a fix just now, branch-2.8 and 2.9 can use the same fix. But I'm not sure if I used the right email format. Thanks, Chris
diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c index 6f1fbe667..606a4f4db 100644 --- a/lib/netdev-tc-offloads.c +++ b/lib/netdev-tc-offloads.c @@ -163,8 +163,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, + uint32_t block_id, const ovs_u128 *ufid) +{ + int err; + + err = tc_del_filter(ifindex, prio, handle, block_id); + 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) @@ -173,8 +185,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; @@ -1302,7 +1312,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, block_id); + del_filter_and_ufid_mapping(ifindex, prio, handle, block_id, ufid); } if (!prio) { @@ -1413,8 +1423,7 @@ netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED, } } - error = tc_del_filter(ifindex, prio, handle, block_id); - del_ufid_tc_mapping(ufid); + error = del_filter_and_ufid_mapping(ifindex, prio, handle, block_id, ufid); netdev_close(dev);
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. * V2: - Add a wrapper function del_filter_and_ufid_mapping() 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(-)