diff options
author | Ilya Maximets <i.maximets@ovn.org> | 2021-11-20 00:07:38 +0100 |
---|---|---|
committer | Ilya Maximets <i.maximets@ovn.org> | 2021-11-30 13:43:09 +0100 |
commit | a05883b897a1ad6321c0f445ee4065da73b3f215 (patch) | |
tree | f28565a820f203d5c92652d8eddcd9ac7c893177 /ofproto/ofproto-provider.h | |
parent | 79953a57ea204f1bfb387019c20f1b0490cbb2ea (diff) | |
download | openvswitch-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.h | 1 |
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. */ |