Hi Eelco,
On 1/5/22 10:48, 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]>
---
v2:
- Fixed test to actually verify the output.
- Used ofpbuf_oversized() instead of manual calculation.
lib/ofp-flow.c | 11 ++++++++++-
tests/ovs-ofctl.at | 19 +++++++++++++++++++
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/lib/ofp-flow.c b/lib/ofp-flow.c
index ff0396845..97b342c03 100644
--- a/lib/ofp-flow.c
+++ b/lib/ofp-flow.c
@@ -1254,7 +1254,16 @@ ofputil_append_flow_stats_reply(const struct
ofputil_flow_stats *fs,
OVS_NOT_REACHED();
}
- ofpmp_postappend(replies, start_ofs);
+ if (ofpbuf_oversized(reply)) {
+ /* 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..090b5f0ad 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 "
priority=90,icmp,reg15=0x8005,metadata=0x1,nw_dst=11.0.0.1,icmp_type=8,icmp_code=0
actions=" > flow.txt
+for i in `seq 1 1022`; do printf
"set_field:0x399->reg13,set_field:0x$i->reg15,resubmit(,39),"; done >> flow.txt
+echo "resubmit(,39)" >> flow.txt
Minor comment, but maybe using printf everywhere would be more
consistent.
Other than that, it looks good to me:
Reviewed-by: Maxime Coquelin <[email protected]>
Thanks,
Maxime
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
