On 3/27/23 19:43, Han Zhou wrote: > On Mon, Mar 27, 2023 at 4:04 AM Dumitru Ceara <[email protected]> wrote: >> >> Commits 53febfbc3776 ("northd: Split switch and router datapaths.") and >> b2f09ac55041 ("northd: Split switch ports and router ports.") made it >> such that for most router/switch specific functions we never process >> records that are not applicable to the function type. Remove the >> redundant checks and replace them with asserts as it's a bug to call the >> functions with the wrong types of ports/datapaths. >> >> Signed-off-by: Dumitru Ceara <[email protected]> >> --- >> CC: Han Zhou <[email protected]> >> --- >> northd/en-sync-sb.c | 4 +--- >> northd/mac-binding-aging.c | 4 +++- >> northd/northd.c | 32 +++++++++----------------------- >> 3 files changed, 13 insertions(+), 27 deletions(-) >> >> diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c >> index 1e4e73ebcd..758f00bfd5 100644 >> --- a/northd/en-sync-sb.c >> +++ b/northd/en-sync-sb.c >> @@ -273,9 +273,7 @@ sync_addr_sets(struct ovsdb_idl_txn *ovnsb_txn, >> /* Sync router load balancer VIP generated address sets. */ >> struct ovn_datapath *od; >> HMAP_FOR_EACH (od, key_node, &lr_datapaths->datapaths) { >> - if (!od->nbr) { >> - continue; >> - } >> + ovs_assert(od->nbr); >> >> if (sset_count(&od->lb_ips->ips_v4_reachable)) { >> char *ipv4_addrs_name = > lr_lb_address_set_name(od->tunnel_key, >> diff --git a/northd/mac-binding-aging.c b/northd/mac-binding-aging.c >> index 6db00c1660..725c52f903 100644 >> --- a/northd/mac-binding-aging.c >> +++ b/northd/mac-binding-aging.c >> @@ -110,7 +110,9 @@ en_mac_binding_aging_run(struct engine_node *node, > void *data OVS_UNUSED) >> >> struct ovn_datapath *od; >> HMAP_FOR_EACH (od, key_node, &northd_data->lr_datapaths.datapaths) { >> - if (od->sb && od->nbr) { >> + ovs_assert(od->nbr); >> + >> + if (od->sb) { >> mac_binding_aging_run_for_datapath(od->sb, od->nbr, >> > sbrec_mac_binding_by_datapath, >> now, &next_expire_msec, >> diff --git a/northd/northd.c b/northd/northd.c >> index 339286be34..5afb0dc6a0 100644 >> --- a/northd/northd.c >> +++ b/northd/northd.c >> @@ -654,9 +654,7 @@ snat_ip_add(struct ovn_datapath *od, const char *ip, > struct ovn_nat *nat_entry) >> static void >> init_nat_entries(struct ovn_datapath *od) >> { >> - if (!od->nbr) { >> - return; >> - } >> + ovs_assert(od->nbr); >> >> shash_init(&od->snat_ips); >> if (get_force_snat_ip(od, "dnat", &od->dnat_force_snat_addrs)) { >> @@ -755,9 +753,7 @@ destroy_nat_entries(struct ovn_datapath *od) >> static void >> init_router_external_ips(struct ovn_datapath *od) >> { >> - if (!od->nbr) { >> - return; >> - } >> + ovs_assert(od->nbr); >> >> sset_init(&od->external_ips); >> for (size_t i = 0; i < od->nbr->n_nat; i++) { >> @@ -841,10 +837,6 @@ destroy_lb_for_datapath(struct ovn_datapath *od) >> { >> ovn_lb_ip_set_destroy(od->lb_ips); >> od->lb_ips = NULL; >> - >> - if (!od->nbs && !od->nbr) { >> - return; >> - } >> } >> >> /* A group of logical router datapaths which are connected - either >> @@ -1072,9 +1064,7 @@ init_mcast_info_for_switch_datapath(struct > ovn_datapath *od) >> static void >> init_mcast_flow_count(struct ovn_datapath *od) >> { >> - if (od->nbr) { >> - return; >> - } >> + ovs_assert(od->nbs); >> >> struct mcast_switch_info *mcast_sw_info = &od->mcast_info.sw; >> atomic_init(&mcast_sw_info->active_v4_flows, 0); >> @@ -3198,9 +3188,7 @@ ovn_update_ipv6_prefix(struct hmap *lr_ports) >> { >> const struct ovn_port *op; >> HMAP_FOR_EACH (op, key_node, lr_ports) { >> - if (!op->nbrp) { >> - continue; >> - } >> + ovs_assert(op->nbrp); >> >> if (!smap_get_bool(&op->nbrp->options, "prefix", false)) { >> continue; >> @@ -4092,9 +4080,7 @@ build_lbs(const struct nbrec_load_balancer_table > *nbrec_load_balancer_table, >> } >> >> HMAP_FOR_EACH (od, key_node, &lr_datapaths->datapaths) { >> - if (!od->nbr) { >> - continue; >> - } >> + ovs_assert(od->nbr); >> >> /* Checking load balancer groups first, starting from the > largest one, >> * to more efficiently copy IP sets. */ >> @@ -4247,9 +4233,7 @@ build_lrouter_lbs_check(const struct ovn_datapaths > *lr_datapaths) >> struct ovn_datapath *od; >> >> HMAP_FOR_EACH (od, key_node, &lr_datapaths->datapaths) { >> - if (!od->nbr) { >> - continue; >> - } >> + ovs_assert(od->nbr); >> >> if (od->has_lb_vip && od->n_l3dgw_ports > 1 >> && !smap_get(&od->nbr->options, "chassis")) { >> @@ -6707,7 +6691,9 @@ ovn_update_ipv6_options(struct hmap *lr_ports) >> { >> struct ovn_port *op; >> HMAP_FOR_EACH (op, key_node, lr_ports) { >> - if (!op->nbrp || op->nbrp->peer || !op->peer) { >> + ovs_assert(op->nbrp); >> + >> + if (op->nbrp->peer || !op->peer) { >> continue; >> } >> >> -- >> 2.31.1 >> > > Thanks Dumitru for the followup. Sorry for missing these places in my > earlier patches.
No problem! > > Acked-by: Han Zhou <[email protected]> > Thanks, I applied this to the main branch. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
