On 6/9/22 21:39, Mark Michelson wrote:
> I'm in agreement with Han and Numan that this is a good idea.
> 

Thanks, Han, Mark, Numan!

> One thing that may be worth considering (maybe as a follow-up) is a
> re-think of parallelization when dp groups are always enabled. With dp
> groups disabled, the lflows can be divided into equal parts, processed
> in parallel, and then combined together by the main thread. With dp
> groups enabled, each thread is working on the same table of lflows,
> necessitating locking.
> 
> As far as I can remember, the most recent tests run with parallelization
> also had dp groups enabled, and we saw an improvement in northd loop
> times. So there's no reason to do something so drastic as to remove
> parallelization altogether. I'm curious if the permanent existence of dp
> groups might allow for us to view parallelization differently and
> potentially improve it from its current design to reduce contention. No,
> I don't have any specific ideas of my own at the moment :)
> 

Maybe have each worker thread build its own output lflows table and then
run the "group-into-datapath-groups" stage twice, once in the worker
thread and once in the main thread, when aggregating results?

But, like you said, this should probably be a follow up.  I'll add a
TODO in the formal patch.

Thanks,
Dumitru

> On 6/8/22 11:09, Numan Siddique wrote:
>> On Wed, Jun 8, 2022 at 1:19 AM Han Zhou <hz...@ovn.org> wrote:
>>>
>>> On Tue, Jun 7, 2022 at 8:20 AM Dumitru Ceara <dce...@redhat.com> 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/20220531093318.2270409-1-mh...@redhat.com/
>>>
>>>>
>>>> Signed-off-by: Dumitru Ceara <dce...@redhat.com>
>>>
>>> Thanks Dumitru! I haven't reviewed the patch in detail, but the idea
>>> sounds
>>> good to me.
>>>
>>
>> +1 from me to move this patch from RFC to a formal one.
>>
>> Numan
>>
>>> Han
>>>
>>>> ---
>>>>   NEWS                |  1 +
>>>>   northd/northd.c     | 77 +++++++++++--------------------------
>>>>   ovn-nb.xml          | 19 +++-------
>>>>   tests/ovn-macros.at | 23 ++----------
>>>>   tests/ovn-northd.at | 92
>>>> ++++-----------------------------------------
>>>>   tests/ovs-macros.at |  4 +-
>>>>   6 files changed, 41 insertions(+), 175 deletions(-)
>>>>
>>>> diff --git a/NEWS b/NEWS
>>>> index e015ae8e7..20b4c5d91 100644
>>>> --- a/NEWS
>>>> +++ b/NEWS
>>>> @@ -4,6 +4,7 @@ Post v22.06.0
>>>>       "ovn-encap-df_default" to enable or disable tunnel DF flag.
>>>>     - Add option "localnet_learn_fdb" to LSP that will allow localnet
>>>>       ports to learn MAC addresses and store them in FDB table.
>>>> +  - Removed possibility of disabling logical datapath groups.
>>>>
>>>>   OVN v22.06.0 - XX XXX XXXX
>>>>   --------------------------
>>>> diff --git a/northd/northd.c b/northd/northd.c
>>>> index 0207f6ce1..a97a321cd 100644
>>>> --- a/northd/northd.c
>>>> +++ b/northd/northd.c
>>>> @@ -4813,10 +4813,6 @@ ovn_lflow_equal(const struct ovn_lflow *a, const
>>> struct ovn_datapath *od,
>>>>               && nullable_string_is_equal(a->ctrl_meter, ctrl_meter));
>>>>   }
>>>>
>>>> -/* If this option is 'true' northd will combine logical flows that
>>> differ by
>>>> - * logical datapath only by creating a datapath group. */
>>>> -static bool use_logical_dp_groups = false;
>>>> -
>>>>   enum {
>>>>       STATE_NULL,               /* parallelization is off */
>>>>       STATE_INIT_HASH_SIZES,    /* parallelization is on; hashes sizing
>>> needed */
>>>> @@ -4841,8 +4837,7 @@ ovn_lflow_init(struct ovn_lflow *lflow, struct
>>> ovn_datapath *od,
>>>>       lflow->ctrl_meter = ctrl_meter;
>>>>       lflow->dpg = NULL;
>>>>       lflow->where = where;
>>>> -    if ((parallelization_state != STATE_NULL)
>>>> -        && use_logical_dp_groups) {
>>>> +    if (parallelization_state != STATE_NULL) {
>>>>           ovs_mutex_init(&lflow->odg_lock);
>>>>       }
>>>>   }
>>>> @@ -4852,7 +4847,7 @@ ovn_dp_group_add_with_reference(struct ovn_lflow
>>> *lflow_ref,
>>>>                                   struct ovn_datapath *od)
>>>>                                   OVS_NO_THREAD_SAFETY_ANALYSIS
>>>>   {
>>>> -    if (!use_logical_dp_groups || !lflow_ref) {
>>>> +    if (!lflow_ref) {
>>>>           return false;
>>>>       }
>>>>
>>>> @@ -4931,13 +4926,11 @@ do_ovn_lflow_add(struct hmap *lflow_map, struct
>>> ovn_datapath *od,
>>>>       struct ovn_lflow *old_lflow;
>>>>       struct ovn_lflow *lflow;
>>>>
>>>> -    if (use_logical_dp_groups) {
>>>> -        old_lflow = ovn_lflow_find(lflow_map, NULL, stage, priority,
>>> match,
>>>> -                                   actions, ctrl_meter, hash);
>>>> -        if (old_lflow) {
>>>> -            ovn_dp_group_add_with_reference(old_lflow, od);
>>>> -            return old_lflow;
>>>> -        }
>>>> +    old_lflow = ovn_lflow_find(lflow_map, NULL, stage, priority,
>>>> match,
>>>> +                               actions, ctrl_meter, hash);
>>>> +    if (old_lflow) {
>>>> +        ovn_dp_group_add_with_reference(old_lflow, od);
>>>> +        return old_lflow;
>>>>       }
>>>>
>>>>       lflow = xmalloc(sizeof *lflow);
>>>> @@ -4993,8 +4986,7 @@ ovn_lflow_add_at_with_hash(struct hmap
>>>> *lflow_map,
>>> struct ovn_datapath *od,
>>>>       struct ovn_lflow *lflow;
>>>>
>>>>       ovs_assert(ovn_stage_to_datapath_type(stage) ==
>>> ovn_datapath_get_type(od));
>>>> -    if (use_logical_dp_groups
>>>> -        && (parallelization_state == STATE_USE_PARALLELIZATION)) {
>>>> +    if (parallelization_state == STATE_USE_PARALLELIZATION) {
>>>>           lflow = do_ovn_lflow_add_pd(lflow_map, od, hash, stage,
>>>> priority,
>>>>                                       match, actions, io_port,
>>>> stage_hint,
>>> where,
>>>>                                       ctrl_meter);
>>>> @@ -5080,8 +5072,7 @@ static void
>>>>   ovn_lflow_destroy(struct hmap *lflows, struct ovn_lflow *lflow)
>>>>   {
>>>>       if (lflow) {
>>>> -        if ((parallelization_state != STATE_NULL)
>>>> -            && use_logical_dp_groups) {
>>>> +        if (parallelization_state != STATE_NULL) {
>>>>               ovs_mutex_destroy(&lflow->odg_lock);
>>>>           }
>>>>           if (lflows) {
>>>> @@ -13883,24 +13874,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;
>>>>
>>>>           /* Set up "work chunks" for each thread to work on. */
>>>>
>>>>           for (index = 0; index < build_lflows_pool->size; index++) {
>>>> -            if (use_logical_dp_groups) {
>>>> -                /* if dp_groups are in use we lock a shared lflows
>>>> hash
>>>> -                 * on a per-bucket level instead of merging hash
>>>> frags */
>>>> -                lsiv[index].lflows = lflows;
>>>> -            } else {
>>>> -                fast_hmap_init(&lflow_segs[index], lflows->mask);
>>>> -                lsiv[index].lflows = &lflow_segs[index];
>>>> -            }
>>>> -
>>>> +            /* dp_groups are in use so we lock a shared lflows hash
>>>> +             * on a per-bucket level.
>>>> +             */
>>>> +            lsiv[index].lflows = lflows;
>>>>               lsiv[index].datapaths = datapaths;
>>>>               lsiv[index].ports = ports;
>>>>               lsiv[index].port_groups = port_groups;
>>>> @@ -13919,13 +13901,8 @@ build_lswitch_and_lrouter_flows(const struct
>>> hmap *datapaths,
>>>>           }
>>>>
>>>>           /* Run thread pool. */
>>>> -        if (use_logical_dp_groups) {
>>>> -            run_pool_callback(build_lflows_pool, NULL, NULL,
>>>> -                              noop_callback);
>>>> -            fix_flow_map_size(lflows, lsiv, build_lflows_pool->size);
>>>> -        } else {
>>>> -            run_pool_hash(build_lflows_pool, lflows, lflow_segs);
>>>> -        }
>>>> +        run_pool_callback(build_lflows_pool, NULL, NULL,
>>>> noop_callback);
>>>> +        fix_flow_map_size(lflows, lsiv, build_lflows_pool->size);
>>>>
>>>>           for (index = 0; index < build_lflows_pool->size; index++) {
>>>>               ds_destroy(&lsiv[index].match);
>>>> @@ -14077,21 +14054,15 @@ void run_update_worker_pool(int n_threads)
>>>>               lflow_hash_lock_destroy();
>>>>               parallelization_state = STATE_NULL;
>>>>           } else if (parallelization_state !=
>>>> STATE_USE_PARALLELIZATION) {
>>>> -            if (use_logical_dp_groups) {
>>>> -                lflow_hash_lock_init();
>>>> -                parallelization_state = STATE_INIT_HASH_SIZES;
>>>> -            } else {
>>>> -                parallelization_state = STATE_USE_PARALLELIZATION;
>>>> -            }
>>>> +            lflow_hash_lock_init();
>>>> +            parallelization_state = STATE_INIT_HASH_SIZES;
>>>>           }
>>>>       }
>>>>   }
>>>>
>>>> -static void worker_pool_init_for_ldp(void)
>>>> +static void init_worker_pool(void)
>>>>   {
>>>> -    /* If parallelization is enabled, make sure locks are initialized
>>>> -     * when ldp are used.
>>>> -     */
>>>> +    /* If parallelization is enabled, make sure locks are
>>>> initialized. */
>>>>       if (parallelization_state != STATE_NULL) {
>>>>           lflow_hash_lock_init();
>>>>           parallelization_state = STATE_INIT_HASH_SIZES;
>>>> @@ -15361,13 +15332,6 @@ ovnnb_db_run(struct northd_input *input_data,
>>>>           nbrec_nb_global_set_options(nb, &options);
>>>>       }
>>>>
>>>> -    bool old_use_ldp = use_logical_dp_groups;
>>>> -    use_logical_dp_groups = smap_get_bool(&nb->options,
>>>> -                                          "use_logical_dp_groups",
>>>> true);
>>>> -    if (use_logical_dp_groups && !old_use_ldp) {
>>>> -        worker_pool_init_for_ldp();
>>>> -    }
>>>> -
>>>>       use_ct_inv_match = smap_get_bool(&nb->options,
>>>>                                        "use_ct_inv_match", true);
>>>>
>>>> @@ -15666,6 +15630,7 @@ void northd_run(struct northd_input
>>>> *input_data,
>>>>                   struct ovsdb_idl_txn *ovnsb_txn)
>>>>   {
>>>>       stopwatch_start(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec());
>>>> +    init_worker_pool();
>>>>       ovnnb_db_run(input_data, data, ovnnb_txn, ovnsb_txn,
>>>>                    input_data->sbrec_chassis_by_name,
>>>>                    input_data->sbrec_chassis_by_hostname);
>>>> diff --git a/ovn-nb.xml b/ovn-nb.xml
>>>> index 618a48b97..dcc75ea73 100644
>>>> --- a/ovn-nb.xml
>>>> +++ b/ovn-nb.xml
>>>> @@ -196,21 +196,14 @@
>>>>
>>>>         <column name="options" key="use_logical_dp_groups">
>>>>           <p>
>>>> -          If set to <code>true</code>, <code>ovn-northd</code> will
>>> combine
>>>> -          logical flows that differs only by logical datapath into a
>>> single
>>>> -          logical flow with logical datapath group attached.
>>>> +          Note: This option is deprecated, the only behavior is to
>>>> always
>>>> +          combine logical flows by datapath groups.  Changing the
>>>> value
>>> or
>>>> +          removing this option all toghether will have no effect.
>>>>           </p>
>>>>           <p>
>>>> -          While this should significantly reduce number of logical
>>>> flows
>>> stored
>>>> -          in Southbound database this could also increase processing
>>> complexity
>>>> -          on the <code>ovn-controller</code> side, e.g.,
>>>> -          <code>ovn-controller</code> will re-consider logical flow
>>>> for
>>> all
>>>> -          logical datapaths in a group.  If the option set to
>>>> -          <code>false</code>, there will be separate logical flow per
>>> logical
>>>> -          datapath and only this flow will be re-considered.
>>>> -        </p>
>>>> -        <p>
>>>> -          The default value is <code>false</code>.
>>>> +          <code>ovn-northd</code> combines logical flows that differs
>>>> +          only by logical datapath into a single logical flow with
>>>> +          logical datapath group attached.
>>>>           </p>
>>>>         </column>
>>>>         <column name="options" key="use_parallel_build">
>>>> diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at
>>>> index c6f0f6251..89f432f16 100644
>>>> --- a/tests/ovn-macros.at
>>>> +++ b/tests/ovn-macros.at
>>>> @@ -246,12 +246,6 @@ ovn_start () {
>>>>                  --ic-nb-db=unix:"$ovs_base"/ovn-ic-nb/ovn-ic-nb.sock \
>>>>                  --ic-sb-db=unix:"$ovs_base"/ovn-ic-sb/ovn-ic-sb.sock
>>>>       fi
>>>> -
>>>> -    if test X$NORTHD_USE_DP_GROUPS = Xyes; then
>>>> -        ovn-nbctl set NB_Global . options:use_logical_dp_groups=true
>>>> -    else
>>>> -        ovn-nbctl set NB_Global . options:use_logical_dp_groups=false
>>>> -    fi
>>>>   }
>>>>
>>>>   # Interconnection networks.
>>>> @@ -749,21 +743,12 @@ m4_define([OVN_POPULATE_ARP],
>>> [AT_CHECK(ovn_populate_arp__, [0], [ignore])])
>>>>   # datapath groups.
>>>>   m4_define([OVN_FOR_EACH_NORTHD],
>>>>     [m4_foreach([NORTHD_TYPE], [ovn-northd, ovn-northd-ddlog],
>>>> -     [m4_foreach([NORTHD_USE_DP_GROUPS], [yes, no],
>>>> -       [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1
>>>> -])])])])
>>>> -
>>>> -# Some tests aren't prepared for dp groups to be enabled.
>>>> -m4_define([OVN_FOR_EACH_NORTHD_WITHOUT_DP_GROUPS],
>>>> -  [m4_foreach([NORTHD_TYPE], [ovn-northd, ovn-northd-ddlog],
>>>> -     [m4_foreach([NORTHD_USE_DP_GROUPS], [no],
>>>> -       [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1
>>>> -])])])])
>>>> +     [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1
>>>> +])])])
>>>>
>>>>   # Some tests aren't prepared for ddlog to be enabled.
>>>>   m4_define([OVN_FOR_EACH_NORTHD_WITHOUT_DDLOG],
>>>>     [m4_foreach([NORTHD_TYPE], [ovn-northd],
>>>> -     [m4_foreach([NORTHD_USE_DP_GROUPS], [yes, no],
>>>> -       [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1
>>>> -])])])])
>>>> +     [m4_foreach([NORTHD_USE_PARALLELIZATION], [yes, no], [$1
>>>> +])])])
>>>>
>>>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
>>>> index a94a7d441..e6a2e9d83 100644
>>>> --- a/tests/ovn-northd.at
>>>> +++ b/tests/ovn-northd.at
>>>> @@ -755,7 +755,7 @@ wait_row_count Datapath_Binding 2
>>>>   AT_CLEANUP
>>>>   ])
>>>>
>>>> -OVN_FOR_EACH_NORTHD_WITHOUT_DP_GROUPS([
>>>> +OVN_FOR_EACH_NORTHD([
>>>>   AT_SETUP([northbound database reconnection])
>>>>
>>>>   ovn_start --backup-northd=none
>>>> @@ -765,7 +765,7 @@ ovn_start --backup-northd=none
>>>>   check_row_count Datapath_Binding 0
>>>>   check ovn-nbctl --wait=sb ls-add sw0
>>>>   check_row_count Datapath_Binding 1
>>>> -lf=$(count_rows Logical_Flow)
>>>> +dp1=$(fetch_column Datapath_Binding _uuid external_ids:name=sw0)
>>>>
>>>>   # Make nbdb ovsdb-server drop connection from ovn-northd.
>>>>   conn=$(as ovn-nb ovs-appctl -t ovsdb-server
>>> ovsdb-server/list-remotes|grep ^punix)
>>>> @@ -777,7 +777,6 @@ check as ovn-nb ovs-appctl -t ovsdb-server
>>> ovsdb-server/add-remote "$conn2"
>>>>   check ovn-nbctl --db="${conn2#p}" ls-add sw1
>>>>   sleep 5
>>>>   check_row_count Datapath_Binding 1
>>>> -check_row_count Logical_Flow $lf
>>>>
>>>>   # Now re-enable the nbdb connection and observe ovn-northd catch up.
>>>>   #
>>>> @@ -786,12 +785,13 @@ check_row_count Logical_Flow $lf
>>>>   # differently on reconnection.
>>>>   check as ovn-nb ovs-appctl -t ovsdb-server ovsdb-server/add-remote
>>> "$conn"
>>>>   wait_row_count Datapath_Binding 2
>>>> -wait_row_count Logical_Flow $(expr 2 \* $lf)
>>>> +dp2=$(fetch_column Datapath_Binding _uuid external_ids:name=sw1)
>>>> +wait_column "$dp1 $dp2" Logical_DP_Group datapaths
>>>>
>>>>   AT_CLEANUP
>>>>   ])
>>>>
>>>> -OVN_FOR_EACH_NORTHD_WITHOUT_DP_GROUPS([
>>>> +OVN_FOR_EACH_NORTHD([
>>>>   AT_SETUP([southbound database reconnection])
>>>>
>>>>   ovn_start --backup-northd=none
>>>> @@ -801,7 +801,7 @@ ovn_start --backup-northd=none
>>>>   check_row_count Datapath_Binding 0
>>>>   check ovn-nbctl --wait=sb ls-add sw0
>>>>   check_row_count Datapath_Binding 1
>>>> -lf=$(count_rows Logical_Flow)
>>>> +dp1=$(fetch_column Datapath_Binding _uuid external_ids:name=sw0)
>>>>
>>>>   # Make sbdb ovsdb-server drop connection from ovn-northd.
>>>>   conn=$(as ovn-sb ovs-appctl -t ovsdb-server
>>> ovsdb-server/list-remotes|grep ^punix)
>>>> @@ -813,7 +813,6 @@ check as ovn-sb ovs-appctl -t ovsdb-server
>>> ovsdb-server/add-remote "$conn2"
>>>>   check ovn-nbctl ls-add sw1
>>>>   sleep 5
>>>>   OVN_SB_DB=${conn2#p} check_row_count Datapath_Binding 1
>>>> -OVN_SB_DB=${conn2#p} check_row_count Logical_Flow $lf
>>>>
>>>>   # Now re-enable the sbdb connection and observe ovn-northd catch up.
>>>>   #
>>>> @@ -822,7 +821,8 @@ OVN_SB_DB=${conn2#p} check_row_count
>>>> Logical_Flow $lf
>>>>   # differently on reconnection.
>>>>   check as ovn-sb ovs-appctl -t ovsdb-server ovsdb-server/add-remote
>>> "$conn"
>>>>   wait_row_count Datapath_Binding 2
>>>> -wait_row_count Logical_Flow $(expr 2 \* $lf)
>>>> +dp2=$(fetch_column Datapath_Binding _uuid external_ids:name=sw1)
>>>> +wait_column "$dp1 $dp2" Logical_DP_Group datapaths
>>>>
>>>>   AT_CLEANUP
>>>>   ])
>>>> @@ -2648,82 +2648,6 @@ AT_CHECK([ovn-sbctl lflow-list sw0 | grep
>>> ls_in_hairpin | sort | sed 's/table=..
>>>>   AT_CLEANUP
>>>>   ])
>>>>
>>>> -OVN_FOR_EACH_NORTHD([
>>>> -AT_SETUP([logical gatapath groups])
>>>> -AT_KEYWORDS([use_logical_dp_groups])
>>>> -ovn_start
>>>> -
>>>> -dnl Disabling datapath groups.
>>>> -ovn-nbctl --wait=sb set NB_Global .
>>>> options:use_logical_dp_groups=false
>>>> -
>>>> -ovn-nbctl ls-add sw1
>>>> -ovn-nbctl ls-add sw2
>>>> -ovn-nbctl lsp-add sw1 swp1
>>>> -ovn-nbctl lsp-add sw2 swp2
>>>> -ovn-nbctl --wait=sb sync
>>>> -
>>>> -sw1_sb_uuid=$(fetch_column datapath_binding _uuid
>>>> external_ids:name=sw1)
>>>> -sw2_sb_uuid=$(fetch_column datapath_binding _uuid
>>>> external_ids:name=sw2)
>>>> -
>>>> -dnl Check that we have no datapath groups.
>>>> -check_row_count Logical_DP_Group 0
>>>> -
>>>> -dnl Number of logical flows that depends on logical switch or
>>>> multicast
>>> group.
>>>> -dnl These will not be combined.
>>>> -n_flows_specific=$(ovn-sbctl -f table find Logical_Flow | grep -cE
>>>> 'swp')
>>>> -echo "Number of specific flows: "${n_flows_specific}
>>>> -
>>>> -dnl Both logical switches configured identically, so there should
>>>> be same
>>>> -dnl number of logical flows per logical switch/logical datapath.
>>>> -n_flows=$(count_rows Logical_Flow)
>>>> -echo "Total number of flows with datapath groups disabled: "${n_flows}
>>>> -n_flows_half=$((${n_flows} / 2))
>>>> -check_row_count Logical_Flow ${n_flows_half}
>>> logical_datapath=${sw1_sb_uuid}
>>>> -check_row_count Logical_Flow ${n_flows_half}
>>> logical_datapath=${sw2_sb_uuid}
>>>> -
>>>> -dnl Enabling datapath groups.
>>>> -ovn-nbctl --wait=sb set NB_Global . options:use_logical_dp_groups=true
>>>> -
>>>> -dnl Check that one datapath group created.
>>>> -check_row_count Logical_DP_Group 1
>>>> -dp_group_uuid=$(fetch_column logical_dp_group _uuid)
>>>> -
>>>> -dnl Check that datapath group contains both datapaths.
>>>> -check_column "${sw1_sb_uuid} ${sw2_sb_uuid}" Logical_DP_Group
>>>> datapaths
>>>> -
>>>> -dnl Calculating number of flows that should be combined for a datapath
>>> group.
>>>> -n_flows=$(count_rows Logical_Flow)
>>>> -echo "Total number of flows with datapath groups enabled: "${n_flows}
>>>> -n_flows_common=$((${n_flows} - ${n_flows_specific}))
>>>> -
>>>> -check_row_count Logical_Flow ${n_flows_common}
>>> logical_dp_group=${dp_group_uuid}
>>>> -check_row_count Logical_Flow ${n_flows_common} logical_datapath=[[]]
>>>> -check_row_count Logical_Flow ${n_flows_common} \
>>>> -    logical_dp_group=${dp_group_uuid} logical_datapath=[[]]
>>>> -
>>>> -dnl Adding 8 more logical switches and ports.
>>>> -for i in $(seq 3 10); do
>>>> -    ovn-nbctl ls-add sw${i}
>>>> -    ovn-nbctl lsp-add sw${i} swp${i}
>>>> -done
>>>> -ovn-nbctl --wait=sb sync
>>>> -
>>>> -dnl Number of logical flows should be increased only due to specific
>>> flows.
>>>> -expected_n_flows=$((${n_flows_common} + 5 * ${n_flows_specific}))
>>>> -echo "Total number of flows with 10 logical switches should be: " \
>>>> -     ${expected_n_flows}
>>>> -check_row_count Logical_Flow ${expected_n_flows}
>>>> -
>>>> -dnl Should be still only one datapath group.
>>>> -check_row_count Logical_DP_Group 1
>>>> -dp_group_uuid=$(fetch_column logical_dp_group _uuid)
>>>> -
>>>> -dnl Number of common flows should be the same.
>>>> -check_row_count Logical_Flow ${n_flows_common}
>>> logical_dp_group=${dp_group_uuid}
>>>> -
>>>> -AT_CLEANUP
>>>> -])
>>>> -
>>>>   OVN_FOR_EACH_NORTHD([
>>>>   AT_SETUP([Router policies - ECMP reroute])
>>>>   AT_KEYWORDS([router policies ecmp reroute])
>>>> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
>>>> index 09c05cf16..5d325559c 100644
>>>> --- a/tests/ovs-macros.at
>>>> +++ b/tests/ovs-macros.at
>>>> @@ -10,11 +10,9 @@ dnl   set it as a shell variable as well.
>>>>   dnl - Skip the test if it's for ovn-northd-ddlog but it didn't get
>>>> built.
>>>>   m4_rename([AT_SETUP], [OVS_AT_SETUP])
>>>>   m4_define([AT_SETUP],
>>>> -  [OVS_AT_SETUP($@[]m4_ifdef([NORTHD_TYPE], [ --
>>> NORTHD_TYPE])[]m4_ifdef([NORTHD_USE_DP_GROUPS], [ --
>>> dp-groups=NORTHD_USE_DP_GROUPS])[]m4_ifdef([NORTHD_USE_PARALLELIZATION],
>>> [
>>> -- parallelization=NORTHD_USE_PARALLELIZATION]))
>>>> +  [OVS_AT_SETUP($@[]m4_ifdef([NORTHD_TYPE], [ --
>>> NORTHD_TYPE])[]m4_ifdef([NORTHD_USE_PARALLELIZATION], [ --
>>> parallelization=NORTHD_USE_PARALLELIZATION]))
>>>>   m4_ifdef([NORTHD_TYPE], [[NORTHD_TYPE]=NORTHD_TYPE
>>>>   ])dnl
>>>> -m4_ifdef([NORTHD_USE_DP_GROUPS],
>>> [[NORTHD_USE_DP_GROUPS]=NORTHD_USE_DP_GROUPS
>>>> -])dnl
>>>>   m4_ifdef([NORTHD_USE_PARALLELIZATION],
>>> [[NORTHD_USE_PARALLELIZATION]=NORTHD_USE_PARALLELIZATION
>>>>   ])dnl
>>>>   m4_ifdef([NORTHD_DUMMY_NUMA], [[NORTHD_DUMMY_NUMA]=NORTHD_DUMMY_NUMA
>>>> -- 
>>>> 2.31.1
>>>>
>>> _______________________________________________
>>> dev mailing list
>>> d...@openvswitch.org
>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>
>> _______________________________________________
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
> 

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to