Great, thanks!
Eelco Chaudron <echau...@redhat.com> 于2023年5月11日周四 15:02写道：
>
>
> On 4 May 2023, at 13:29, Peng He wrote:
>
> > The following comments (brought in at 0de8783a9):
> >
> > /* XXX: There's a race window where a flow covering this packet
> > * could have already been installed since we last did the flow
> > * lookup before upcall. This could be solved by moving the
> > * mutex lock outside the loop, but that's an awful long time
> > * to be locking revalidators out of making flow modifications. */
> >
> > is out-dated. Back at commit 0de8783a9, the classifier is per-datapath,
> > multiple PMDs share a same classifier. Since now we have changed into
> > per-PMD classifier, the lookup code only prevents from the race
> > results from megaflows installed by another non-pmd threads, through
> > either manually calling dpctl/add-flow or handler thread installing
> > megaflow with pmd-id == PMD_ID_NULL, there are no other threads which
> > would insert datapath flows.
>
> Thanks this looks good to me. Will try to review patch 1/2 next week.
>
> Acked-by: Eelco Chaudron <echau...@redhat.com>
>
> > Signed-off-by: Peng He <hepeng.0...@bytedance.com>
> > ---
> > lib/dpif-netdev.c | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 70b953ae6..d92ec01fb 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -8403,12 +8403,11 @@ handle_packet_upcall(struct dp_netdev_pmd_thread
> *pmd,
> > if (OVS_LIKELY(error != ENOSPC)) {
> > struct dp_netdev_flow *netdev_flow;
> >
> > - /* XXX: There's a race window where a flow covering this packet
> > - * could have already been installed since we last did the flow
> > - * lookup before upcall. This could be solved by moving the
> > - * mutex lock outside the loop, but that's an awful long time
> > - * to be locking revalidators out of making flow modifications.
> */
> > ovs_mutex_lock(&pmd->flow_mutex);
> > + /* Two scenarios exist where a flow could have been added while
> > + * processing the upcall:
> > + * 1) a flow was manually added through dpctl/add-flow.
> > + * 2) a handler installed a flow with pmd-id == PMD_ID_NULL. */
> > netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL);
> > if (OVS_LIKELY(!netdev_flow)) {
> > netdev_flow = dp_netdev_flow_add(pmd, &match, &ufid,
> > --
> > 2.25.1
>
>
--
hepeng
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
