Re: [ovs-dev] [PATCH V3 08/19] netdev-offload-dpdk: Protect UFID map by mutex

[贺鹏]

Got it.
You're correct.

Eli Britstein <el...@mellanox.com> 于2019年12月9日周一 下午3:21写道：
>
>
> On 12/9/2019 9:15 AM, 贺鹏 wrote:
> > Hi,
> >
> >
> > cmap support multiple readers and one writer.
> > since all write operations are performed in the offload thread,
> > why need a mutex here? Maybe I miss some patches.
> Collecting statistics using netdev_flow_get in patch 10/20 is another
> thread. There, it gets the rte_flow, but if not protected it might get
> destroyed by the offload thread before it uses it.
> >
> >
> >
> > Eli Britstein <el...@mellanox.com> 于2019年12月8日周日 下午9:23写道：
> >
> >> Flow deletion and dumping for statistics collection are called from
> >> different threads. As a pre-step towards collecting HW statistics,
> >> protect the UFID map by mutex to make it thread safe.
> >>
> >> Signed-off-by: Eli Britstein <el...@mellanox.com>
> >> ---
> >>   lib/netdev-offload-dpdk.c | 7 ++++++-
> >>   1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> >> index b2ec05cec..5568400b6 100644
> >> --- a/lib/netdev-offload-dpdk.c
> >> +++ b/lib/netdev-offload-dpdk.c
> >> @@ -51,6 +51,7 @@ static struct vlog_rate_limit error_rl = 
> >> VLOG_RATE_LIMIT_INIT(100, 5);
> >>    * A mapping from ufid to dpdk rte_flow.
> >>    */
> >>   static struct cmap ufid_to_rte_flow = CMAP_INITIALIZER;
> >> +static struct ovs_mutex ufid_map_mutex = OVS_MUTEX_INITIALIZER;
> >>
> >>   struct ufid_to_rte_flow_data {
> >>       struct cmap_node node;
> >> @@ -630,8 +631,11 @@ netdev_offload_dpdk_destroy_flow(struct netdev 
> >> *netdev,
> >>                                    struct rte_flow *rte_flow)
> >>   {
> >>       struct rte_flow_error error;
> >> -    int ret = netdev_dpdk_rte_flow_destroy(netdev, rte_flow, &error);
> >> +    int ret;
> >> +
> >> +    ovs_mutex_lock(&ufid_map_mutex);
> >>
> >> +    ret = netdev_dpdk_rte_flow_destroy(netdev, rte_flow, &error);
> >>       if (ret == 0) {
> >>           ufid_to_rte_flow_disassociate(ufid);
> >>           VLOG_DBG("%s: removed rte flow %p associated with ufid " 
> >> UUID_FMT "\n",
> >> @@ -642,6 +646,7 @@ netdev_offload_dpdk_destroy_flow(struct netdev *netdev,
> >>                    netdev_get_name(netdev), error.message, error.type);
> >>       }
> >>
> >> +    ovs_mutex_unlock(&ufid_map_mutex);
> >>       return ret;
> >>   }
> >>
> >> --
> >> 2.14.5
> >>
> >> _______________________________________________
> >> dev mailing list
> >> d...@openvswitch.org
> >> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-dev&amp;data=02%7C01%7Celibr%40mellanox.com%7C9af2f119de544768711408d77c779a66%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637114725449215533&amp;sdata=VyrLUh6%2BP0ne9dXUSDjME31NBj0tHTV%2FEciNZY%2Baugo%3D&amp;reserved=0
> >
> >
> > --
> > hepeng
> > ICT



-- 
hepeng
Bytedance
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev