On 11/22/21 09:07, Vladislav Odintsov wrote:
> Thanks for the patch Ilya!
> 
> I’ve tested this with my setup in OVN similar to described in message by link 
> at "Reported-at".
> ovs-vswitchd with this patch applied seems working fine. It processed flows 
> and though with high CPU load during ~3-5 seconds after start, it went to 
> normal CPU load about 2-4%.
> I’ve tested this with both: OVS v2.13.5 and master branch.
> 
> Tested-by: Vladislav Odintsov <odiv...@gmail.com <mailto:odiv...@gmail.com>>


Thanks!  Applied and backported down to 2.13.

Best regards, Ilya Maximets.

> 
> Regards,
> Vladislav Odintsov
> 
>> On 20 Nov 2021, at 02:07, Ilya Maximets <i.maxim...@ovn.org 
>> <mailto:i.maxim...@ovn.org>> wrote:
>>
>> 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 'valgrid --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 <odiv...@gmail.com 
>> <mailto:odiv...@gmail.com>>
>> Reported-at: 
>> https://mail.openvswitch.org/pipermail/ovs-dev/2021-November/389503.html 
>> <https://mail.openvswitch.org/pipermail/ovs-dev/2021-November/389503.html>
>> Signed-off-by: Ilya Maximets <i.maxim...@ovn.org <mailto:i.maxim...@ovn.org>>
>> ---
>> ofproto/ofproto-provider.h |  1 +
>> ofproto/ofproto.c          | 47 ++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 48 insertions(+)
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to