On 12/26/2020 11:51 AM, wangyunjian wrote:
From: Lvmengfan <[email protected]>
When upcall_receive or process_upcall returns error in recv_upcalls, the
n_upcalls count would not increased. If this situation continues, the
loop may failed to exit, then the handler block occurs. Add a n_errors
to count the errors and jump out of the loop, cloud avoid block
typo
happening.
Fixes: cc377352d164 ("ofproto: Reorganize in preparation for direct dpdk
upcalls.")
Signed-off-by: Lvmengfan <[email protected]>
Could you please add a unit-test for this?
---
ofproto/ofproto-dpif-upcall.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 27924fa..6b32b18 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -776,9 +776,10 @@ recv_upcalls(struct handler *handler)
struct dpif_upcall dupcalls[UPCALL_MAX_BATCH];
struct upcall upcalls[UPCALL_MAX_BATCH];
struct flow flows[UPCALL_MAX_BATCH];
- size_t n_upcalls, i;
+ size_t n_upcalls, i, n_errors;
n_upcalls = 0;
+ n_errors = 0;
while (n_upcalls < UPCALL_MAX_BATCH) {
Maybe simpler instead of the below free_dupcall hunk, to do like:
while (n_upcalls + n_errors < UPCALL_MAX_BATCH) {
The downside is that in case of errors we will not fill the entire
UPCALL_MAX_BATCH, but simpler.
struct ofpbuf *recv_buf = &recv_bufs[n_upcalls];
struct dpif_upcall *dupcall = &dupcalls[n_upcalls];
@@ -810,6 +811,7 @@ recv_upcalls(struct handler *handler)
dupcall->type, dupcall->userdata, flow, mru,
&dupcall->ufid, PMD_ID_NULL);
if (error) {
+ n_errors++;
if (error == ENODEV) {
/* Received packet on datapath port for which we couldn't
* associate an ofproto. This can happen if a port is removed
@@ -837,6 +839,7 @@ recv_upcalls(struct handler *handler)
error = process_upcall(udpif, upcall,
&upcall->odp_actions, &upcall->wc);
if (error) {
+ n_errors++;
goto cleanup;
}
@@ -848,6 +851,13 @@ cleanup:
free_dupcall:
dp_packet_uninit(&dupcall->packet);
ofpbuf_uninit(recv_buf);
+ if (n_errors >= UPCALL_MAX_BATCH * 2) {
Why UPCALL_MAX_BATCH * 2 ? Please add a comment to explain, also the below.
+ if (n_upcalls == 0) {
+ return 1;
+ } else {
+ break;
+ }
+ }
}
if (n_upcalls) {
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
