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. Acked-by: Han Zhou <[email protected]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
