On 6/27/22 22:49, Mark Michelson wrote:
> Hi Dumitru,
>
Hi Mark,
> I have a small note below, and with it fixed,
>
> Acked-by: Mark Michelson
Thanks!
>
> On 6/23/22 15:58, Dumitru Ceara wrote:
>> In large scale scenarios this option hugely reduces the size of the
>> Southbound database positively affecting end to end performance. In
>> such scenarios there's no real reason to ever disable datapath groups.
>>
>> In lower scale scenarios any potential overhead due to logical datapath
>> groups is, very likely, negligible.
>>
>> Aside from potential scalability concerns, the
>> NB.NB_Global.options:use_logical_dp_group knob was kept until now to
>> ensure that in case of a bug in the logical datapath groups code a CMS
>> may turn it off and fall back to the mode in which logical flows are not
>> grouped together. As far as I know, this has never happened until now.
>>
>> Moreover, datpath_groups are enabled by default since v21.09.0 (4 stable
>> releases ago), via 90daa7ce18dc ("northd: Enable logical dp groups by
>> default.").
>>
>> From a testing perspective removing this knob will halve the CI matrix.
>> This is desirable, especially in the context of more tests being added,
>> e.g.:
>> https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/
>>
>>
>> This commit also adds OVN_FOR_EACH_NORTHD to the "ovn-northd -- lr
>> multiple gw ports NAT" test case. That was previously skipped because
>> the duplicate logical flow would have been merged when running with
>> datapath groups enabled. Instead, change the expected output to not
>> include the duplicate.
>>
>> Signed-off-by: Dumitru Ceara <[email protected]>
>> ---
[...]
>> @@ -13930,24 +13921,15 @@ build_lswitch_and_lrouter_flows(const struct
>> hmap *datapaths,
>> int index;
>> lsiv = xcalloc(sizeof(*lsiv), build_lflows_pool->size);
>> - if (use_logical_dp_groups) {
>> - lflow_segs = NULL;
>> - } else {
>> - lflow_segs = xcalloc(sizeof(*lflow_segs),
>> build_lflows_pool->size);
>> - }
>> + lflow_segs = NULL;
>
> lflow_segs is unused now, so it can be removed.
>
Oops, thanks for pointing out, fixed in v2.
Thanks,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
