From: Alejandro Pacheco <[email protected]>
handle_flow_monitor_request() keeps every monitor that a request adds
in a local 'monitors' array, to collect their initial rules once the
whole request is processed, or to destroy them if a later entry of the
request fails. OpenFlow 1.4 added the MODIFY and DELETE commands,
which destroy an existing monitor looked up by id. When that monitor
was added by an earlier entry of the same request, it stays in the
array after ofmonitor_destroy() freed it, and the array is then used to
collect initial rules or destroyed again on the error path.
Remove a monitor from the array before destroying it for MODIFY and
DELETE.
Fixes: c3e64047d1cc ("ofp-monitor: Support flow monitoring for OpenFlow 1.3,
1.4+.")
Assisted-by: claude-opus-5-5, Claude Code
Signed-off-by: Alejandro Pacheco <[email protected]>
---
ofproto/ofproto.c | 18 ++++++++++++++++++
tests/ofproto.at | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 57 insertions(+)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 02f82c519..8b636abef 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -6702,6 +6702,22 @@ flow_monitor_delete(struct ofconn *ofconn, uint32_t id)
return error;
}
+/* Removes 'm' from the 'n_monitors' elements of 'monitors', if it is there,
+ * so that it can be destroyed while the request is still being processed. */
+static void
+flow_monitor_forget(struct ofmonitor **monitors, size_t *n_monitors,
+ const struct ofmonitor *m)
+{
+ for (size_t i = 0; i < *n_monitors; i++) {
+ if (monitors[i] == m) {
+ memmove(&monitors[i], &monitors[i + 1],
+ (*n_monitors - i - 1) * sizeof *monitors);
+ (*n_monitors)--;
+ return;
+ }
+ }
+}
+
static enum ofperr
handle_flow_monitor_request(struct ofconn *ofconn, const struct ovs_list *msgs)
OVS_EXCLUDED(ofproto_mutex)
@@ -6756,6 +6772,7 @@ handle_flow_monitor_request(struct ofconn *ofconn, const
struct ovs_list *msgs)
error = OFPERR_OFPMOFC_UNKNOWN_MONITOR;
goto error;
}
+ flow_monitor_forget(monitors, &n_monitors, m);
ofmonitor_destroy(m);
error = ofmonitor_create(&request, ofconn, &m);
@@ -6775,6 +6792,7 @@ handle_flow_monitor_request(struct ofconn *ofconn, const
struct ovs_list *msgs)
error = OFPERR_OFPMOFC_UNKNOWN_MONITOR;
goto error;
}
+ flow_monitor_forget(monitors, &n_monitors, m);
ofmonitor_destroy(m);
break;
}
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 220d96691..1277ba302 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -5057,6 +5057,45 @@ OVS_APP_EXIT_AND_WAIT([ovs-ofctl])
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([ofproto - OpenFlow14 flow monitor modify and delete in one request])
+AT_KEYWORDS([monitor])
+OVS_VSWITCHD_START
+
+ovs-ofctl -O OpenFlow14 monitor br0 --detach --no-chdir --pidfile >monitor.log
2>&1
+AT_CAPTURE_FILE([monitor.log])
+
+# Add monitor 1 and modify it in the same request.
+ovs-appctl -t ovs-ofctl ofctl/send
0512004000000002001000000000000000000001ffffffffffffffff000fff00000100040000000000000001ffffffffffffffff0003ff010001000400000000
+# Add monitor 2 and delete it in the same request.
+ovs-appctl -t ovs-ofctl ofctl/send
0512004000000003001000000000000000000002ffffffffffffffff000fff00000100040000000000000002ffffffffffffffff0000ff020001000400000000
+ovs-appctl -t ovs-ofctl ofctl/barrier
+
+AT_CHECK([sed 's/ (xid=0x[[1-9a-fA-F]][[0-9a-fA-F]]*)//' monitor.log], [0],
[dnl
+send: OFPST_FLOW_MONITOR request (OF1.4):
+ id=1 flags=initial,add,delete,modify
+ id=1 flags=initial,add
+OFPST_FLOW_MONITOR reply (OF1.4):
+send: OFPST_FLOW_MONITOR request (OF1.4):
+ id=2 flags=initial,add,delete,modify
+ id=2 command=delete
+OFPST_FLOW_MONITOR reply (OF1.4):
+OFPT_BARRIER_REPLY (OF1.4):
+])
+
+# Monitor 1 reports additions.
+ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log
+ovs-ofctl add-flow br0 udp,actions=normal
+ovs-appctl -t ovs-ofctl ofctl/barrier
+AT_CHECK([sed 's/ (xid=0x[[1-9a-fA-F]][[0-9a-fA-F]]*)//' monitor.log], [0],
[dnl
+OFPST_FLOW_MONITOR reply (OF1.4) (xid=0x0):
+ event=ADDED table=0 cookie=0 udp
+OFPT_BARRIER_REPLY (OF1.4):
+])
+
+OVS_APP_EXIT_AND_WAIT([ovs-ofctl])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([ofproto - event filtering (OpenFlow 1.3)])
AT_KEYWORDS([monitor])
OVS_VSWITCHD_START
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
