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.
+         */
         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

Reply via email to