summaryrefslogtreecommitdiff
path: root/ofproto/ofproto-provider.h
diff options
context:
space:
mode:
authorIlya Maximets <i.maximets@ovn.org>2021-11-20 00:07:38 +0100
committerIlya Maximets <i.maximets@ovn.org>2021-11-30 13:43:09 +0100
commita05883b897a1ad6321c0f445ee4065da73b3f215 (patch)
treef28565a820f203d5c92652d8eddcd9ac7c893177 /ofproto/ofproto-provider.h
parent79953a57ea204f1bfb387019c20f1b0490cbb2ea (diff)
downloadopenvswitch-a05883b897a1ad6321c0f445ee4065da73b3f215.tar.gz
ofproto: Fix resource usage explosion while processing bundled FLOW_MOD.
While processing a bundle, OVS will add all new and modified rules to classifiers. Classifiers are using RCU-protected pvector to store subtables. Addition of a new subtable or removal of the old one leads to re-allocation and memory copy of the pvector array. Old version of that array is given to RCU thread to free it later. The problem is that bundle is processed under the mutex without entering the quiescent state. Therefore, memory can not be freed until the whole bundle is processed. So, if a few thousands of flows added to the same table in a bundle, pvector will be re-allocated while adding each of them. So, we'll have a few thousands of copies of the same array waiting to be freed. In case of OVN deployments, there could be hundreds of thousands of flows in the same table leading to a fast consumption of a huge amount of memory and wasting a lot of CPU cycles on allocations and copies. Below snippet of the 'valgrind --tool=massif' output shows ovs-vswitchd consuming 3.5GB of RAM while processing a bundle with 65K FLOW_MODs in the OVN deployment. 3.4GB of that memory are copies of the same pvector. ------------------------------------------------------------------- n time(i) total(B) useful-heap(B) extra-heap(B) ------------------------------------------------------------------- 64 109,907,465,404 3,559,987,568 3,546,879,748 13,107,820 99.63% (3,546,879,748B) (heap allocation functions) malloc/new/new[] ->97.61% (3,474,750,333B) xmalloc__ (util.c:137) |->97.61% (3,474,750,333B) xmalloc (util.c:172) | ->96.38% (3,431,068,352B) pvector_impl_dup (pvector.c:48) || ->96.38% (3,431,067,840B) pvector_insert (pvector.c:138) || |->96.38% (3,431,067,840B) classifier_replace (classifier.c:664) || | ->96.38% (3,431,067,840B) classifier_insert (classifier.c:695) || | ->96.38% (3,431,067,840B) replace_rule_start (ofproto.c:5563) || | ->96.38% (3,431,067,840B) add_flow_start (ofproto.c:5179) || | ->96.38% (3,431,067,840B) ofproto_flow_mod_start (ofproto.c:8017) || | ->96.38% (3,431,067,744B) do_bundle_commit (ofproto.c:8168) || | |->96.38% (3,431,067,744B) handle_bundle_control (ofproto.c:8309) || | | ->96.38% (3,431,067,744B) handle_single_part_openflow (ofproto.c:8593) || | | ->96.38% (3,431,067,744B) handle_openflow (ofproto.c:8674) || | | ->96.38% (3,431,067,744B) ofconn_run (connmgr.c:1329) || | | ->96.38% (3,431,067,744B) connmgr_run (connmgr.c:356) || | | ->96.38% (3,431,067,744B) ofproto_run (ofproto.c:1879) || | | ->96.38% (3,431,067,744B) bridge_run__ (bridge.c:3251) || | | ->96.38% (3,431,067,744B) bridge_run (bridge.c:3310) || | | ->96.38% (3,431,067,744B) main (ovs-vswitchd.c:127) Fixing that by postponing the publishing of classifier updates, so each flow modification can work with the same version of pvector. Unfortunately, bundled PACKET_OUT messages requires all previous changes to be published before processing, otherwise the packet will use wrong version of OF tables. Publishing all changes before processing PACKET_OUT messages to avoid this issue. Hopefully, mixup of a big number of FLOW_MOD and PACKET_OUT messages is not a common usecase. Reported-by: Vladislav Odintsov <odivlad@gmail.com> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2021-November/389503.html Tested-by: Vladislav Odintsov <odivlad@gmail.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Diffstat (limited to 'ofproto/ofproto-provider.h')
-rw-r--r--ofproto/ofproto-provider.h1
1 files changed, 1 insertions, 0 deletions
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 57c7d17cb..a934a98f2 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1962,6 +1962,7 @@ struct ofproto_flow_mod {
bool modify_may_add_flow;
bool modify_keep_counts;
enum nx_flow_update_event event;
+ uint8_t table_id;
/* These are only used during commit execution.
* ofproto_flow_mod_uninit() does NOT clean these up. */