Hi I have sent a new version with your re-write.
Eelco Chaudron <[email protected]> 于2022年12月8日周四 19:08写道： > > > On 27 Nov 2022, at 8:28, 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 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. > > > > Signed-off-by: Peng He <[email protected]> > > --- > > 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 a45b46014..597faa047 100644 > > --- a/lib/dpif-netdev.c > > +++ b/lib/dpif-netdev.c > > @@ -8297,12 +8297,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 that race could happen: > > + * 1) manually add megaflow through dpctl/add-flow > > + * 2) handler installs a megaflow with pmd-id == PMD_ID_NULL > > + */ > > Maybe just a little re-write so it’s clear what you mean with race. > > /* 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 [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
