On 12/6/21 17:57, Eelco Chaudron wrote:
Currently, if a flow reply results in a message which exceeds
the maximum reply size, it will assert OVS. This would happen
when OVN uses OpenFlow15 to add large flows, and they get read
using OpenFlow10 with ovs-ofctl.
This patch prevents this and adds a test case to make sure the
code behaves as expected.
Signed-off-by: Eelco Chaudron <[email protected]>
---
lib/ofp-flow.c | 12 +++++++++++-
tests/ovs-ofctl.at | 19 +++++++++++++++++++
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/lib/ofp-flow.c b/lib/ofp-flow.c
index ff0396845..796699dba 100644
--- a/lib/ofp-flow.c
+++ b/lib/ofp-flow.c
@@ -1254,7 +1254,17 @@ ofputil_append_flow_stats_reply(const struct
ofputil_flow_stats *fs,
OVS_NOT_REACHED();
}
- ofpmp_postappend(replies, start_ofs);
+ if ((reply->size - start_ofs) >
+ (UINT16_MAX - ((char *) reply->msg - (char *) reply->header))) {
+ /* When this happens, the reply will not fit in a single OFP message,
+ * and we should not append it to the queue. We will log a warning
+ * and continue with the next flow stat entry. */
+ reply->size = start_ofs;
+ VLOG_WARN_RL(&rl, "Flow exceeded the maximum flow statistics reply "
+ "size and was excluded from the response set");
+ } else {
+ ofpmp_postappend(replies, start_ofs);
+ }
fs_->match.flow.tunnel.metadata.tab = orig_tun_table;
}
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index 586e55806..9772094fc 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -3252,3 +3252,22 @@ dnl because we need ovs-vswitchd to have the controller
config before starting
dnl the controller to 'snoop' the OpenFlow messages from beginning
OVS_VSWITCHD_STOP(["/connection failed (No such file or directory)/d"])
AT_CLEANUP
+
+
+AT_SETUP([ovs-ofctl show-flows - Oversized flow])
+OVS_VSWITCHD_START
+
+printf "add
priority=90,icmp,metadata=0x3,nw_dst=11.0.0.1,icmp_type=8,icmp_code=0,reg15=0x8005,metadata=0x1,actions="
> flow.txt
+for i in `seq 0 1021`; do printf
"set_field:0x399->reg13,set_field:0x$i->reg15,resubmit(,39),"; done >> flow.txt
+echo "resubmit(,39)" >> flow.txt
+
+AT_CHECK([ovs-ofctl -O OpenFlow15 add-flows br0 flow.txt])
+
+AT_CHECK([ovs-ofctl -O OpenFlow10 dump-flows br0 | ofctl_strip | sed
'/NXST_FLOW/d' | sort], [0], [])
+OVS_WAIT_UNTIL([grep -q "ofp_flow|WARN|Flow exceeded the maximum flow statistics
reply size and was excluded from the response set" ovs-vswitchd.log])
+
+dnl cat flow.txt > expout
+dnl AT_CHECK([ovs-ofctl -O OpenFlow15 dump-flows br0 | ofctl_strip | sed
'/OFPST_FLOW/d' | sort], [0], [expout])
+
+OVS_VSWITCHD_STOP(["/|WARN|/d"])
+AT_CLEANUP
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Reviewed and manually tested this patch. Verified excessively big flows are
skipped while the rest are sent properly. Although it's not ideal to skip a
flow, I don't see any better way to respond to this corner-case.
Acked-by: Adrian Moreno <[email protected]>
--
Adrián Moreno
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
