| Message ID | 20221127070437.126785-3-hepeng.0320@bytedance.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,ovs-dev,v6,1/3] ofproto-dpif-upcall: fix push_dp_ops | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
Bleep bloop. Greetings Peng He, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Author Peng He <xnhp0320@gmail.com> needs to sign off. WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Peng He <hepeng.0320@bytedance.com> Lines checked: 51, Warnings: 1, Errors: 1 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
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 + */ netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL); if (OVS_LIKELY(!netdev_flow)) { netdev_flow = dp_netdev_flow_add(pmd, &match, &ufid,
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 <hepeng.0320@bytedance.com> --- lib/dpif-netdev.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)