On 3 Oct 2022, at 6:02, 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 > introduced by manually calling dpctl/add-flow, there are no other > threads which would insert datapath flows. > > Signed-off-by: Peng He <[email protected]> > --- > lib/dpif-netdev.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index b316e59ef..680321600 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -8297,12 +8297,10 @@ 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); > + /* We need to lookup the flow in case someone uses dpctl/add-flow > + * to add a datapath flow and this flow also cover this packet. > + */ I actually was looking for code paths that could hit this case, but was not able to quickly identify anything but this one. However are we sure there is no such odd kernel tunnel-like interface that could take this path? However, if this happens it’s still being done in the same thread that handles the dpctl/add-flow flow. See the discussion in https://mail.openvswitch.org/pipermail/ovs-dev/2022-October/398619.html. //Eelco > netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL); > if (OVS_UNLIKELY(netdev_flow)) { > struct dp_netdev_actions *old_act = > -- > 2.25.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
