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]> Reviewed-by: Maxime Coquelin <[email protected]> --- v4: - Use only printf in unit test, instead of mixed echo and printf - Use ofpbuf_headersize() v3: - Remove ofpbuf_oversized(), as we need to know if our message is larger than 64k (minus common header), i.e., not including the already existing ones. 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..3bc744f78 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 ((reply->size - start_ofs) > (UINT16_MAX - ofpbuf_headersize(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..267711bfa 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 +printf "resubmit(,39)\n" >> 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]) + +cat flow.txt > expout +AT_CHECK([ovs-ofctl -O OpenFlow15 dump-flows br0 | ofctl_strip | sed '/OFPST_FLOW/d' | sort], [0], [expout]) + +OVS_VSWITCHD_STOP(["/Flow exceeded the maximum flow statistics reply size and was excluded from the response set/d"]) +AT_CLEANUP _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
