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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
