> -----Original Message----- > From: Simon Horman [mailto:[email protected]] > Sent: Friday, November 9, 2018 6:31 PM > To: Chris Mi <[email protected]> > Cc: [email protected] > Subject: Re: [ovs-dev] netdev-tc-offloads: Delete ufid tc mapping in the right > place > > On Thu, Nov 08, 2018 at 05:12:50AM -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 <[email protected]> > > Reviewed-by: Roi Dayan <[email protected]> > > --- > > lib/netdev-tc-offloads.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c index > > 6f1fbe6..27a40bf 100644 > > --- a/lib/netdev-tc-offloads.c > > +++ b/lib/netdev-tc-offloads.c > > @@ -163,8 +163,7 @@ 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. */ > > +/* 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 > > +172,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; > > @@ -1303,6 +1300,7 @@ netdev_tc_flow_put(struct netdev *netdev, > struct match *match, > > if (handle && prio) { > > VLOG_DBG_RL(&rl, "updating old handle: %d prio: %d", handle, prio); > > tc_del_filter(ifindex, prio, handle, block_id); > > + del_ufid_tc_mapping(ufid); > > } > > > > Hi Chris, > > Above the filter and then ufid mapping is deleted while in > netdev_tc_flow_del the order is reversed. > > I wonder if we could make things more consistent using a wrapper as it > seems that in netdev-tc-offloads.c these two operations always occur > together. Hi Simon,
Thanks for your suggestion. Above comment has been addressed in v2. Please help review again. Thanks, Chris _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
