[...]
>
> Can be replaced with "dynamic_bitmap_scan()".
ack, I will fix it in v7.
>
> + if (index < vector_len(&datapaths->dps)) {
> > + /* We can reuse stale vector entries. */
> > + vector_get(&datapaths->dps, index, struct ovn_datapath *) = od;
> > + } else {
> > + vector_insert(&datapaths->dps, index, &od);
> > + }
> > + bitmap_set1(datapaths->dps_index_map.map, index);
> > + datapaths->dps_index_map.n_elems++;
> >
>
> Can be replaced with "dynamic_bitmap_set1()".
ack, I will fix it in v7.
>
> + od->datapaths = datapaths;
> > + od->index = index;
> > +}
> > +
> > /* Initializes 'ls_datapaths' to contain a "struct ovn_datapath" for every
> > * logical switch, and initializes 'lr_datapaths' to contain a
> > * "struct ovn_datapath" for every logical router.
> > @@ -3990,6 +4015,19 @@ build_ports(struct ovsdb_idl_txn *ovnsb_txn,
> > sset_destroy(&active_ha_chassis_grps);
> > }
> >
> > +static void
> > +destroy_tracked_dps(struct tracked_dps *trk_dps)
> > +{
> > + hmapx_clear(&trk_dps->crupdated);
> > +
> > + struct hmapx_node *n;
> > + HMAPX_FOR_EACH_SAFE (n, &trk_dps->deleted) {
> > + ovn_datapath_destroy(n->data);
> > + hmapx_delete(&trk_dps->deleted, n);
> > + }
> > + hmapx_clear(&trk_dps->deleted);
> > +}
> > +
> > static void
> > destroy_tracked_ovn_ports(struct tracked_ovn_ports *trk_ovn_ports)
> > {
> > @@ -4032,6 +4070,7 @@ destroy_northd_data_tracked_changes(struct
> > northd_data *nd)
> > hmapx_clear(&trk_changes->ls_with_changed_lbs);
> > hmapx_clear(&trk_changes->ls_with_changed_acls);
> > hmapx_clear(&trk_changes->ls_with_changed_ipam);
> > + destroy_tracked_dps(&trk_changes->trk_switches);
> > trk_changes->type = NORTHD_TRACKED_NONE;
> > }
> >
> > @@ -4040,6 +4079,8 @@ init_northd_tracked_data(struct northd_data *nd)
> > {
> > struct northd_tracked_data *trk_data = &nd->trk_data;
> > trk_data->type = NORTHD_TRACKED_NONE;
> > + hmapx_init(&trk_data->trk_switches.crupdated);
> > + hmapx_init(&trk_data->trk_switches.deleted);
> > hmapx_init(&trk_data->trk_lsps.created);
> > hmapx_init(&trk_data->trk_lsps.updated);
> > hmapx_init(&trk_data->trk_lsps.deleted);
> > @@ -4056,7 +4097,12 @@ destroy_northd_tracked_data(struct northd_data *nd)
> > {
> > struct northd_tracked_data *trk_data = &nd->trk_data;
> > trk_data->type = NORTHD_TRACKED_NONE;
> > + hmapx_destroy(&trk_data->trk_switches.crupdated);
> > hmapx_destroy(&trk_data->trk_lsps.created);
> > + struct hmapx_node *n;
> > + HMAPX_FOR_EACH (n, &trk_data->trk_switches.deleted) {
> > + free(n->data);
> > + }
> > hmapx_destroy(&trk_data->trk_lsps.updated);
> > hmapx_destroy(&trk_data->trk_lsps.deleted);
> > hmapx_destroy(&trk_data->trk_lbs.crupdated);
> > @@ -4352,13 +4398,15 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> > *ovnsb_idl_txn,
> > struct ovn_datapath *od,
> > struct tracked_ovn_ports *trk_lsps)
> > {
> > - bool ls_ports_changed = false;
> > + bool ls_deleted = nbrec_logical_switch_is_deleted(changed_ls);
> > + bool ls_ports_changed = ls_deleted;
> > if (!nbrec_logical_switch_is_updated(changed_ls,
> > NBREC_LOGICAL_SWITCH_COL_PORTS))
> > {
> >
> > for (size_t i = 0; i < changed_ls->n_ports; i++) {
> > if (nbrec_logical_switch_port_row_get_seqno(
> > - changed_ls->ports[i], OVSDB_IDL_CHANGE_MODIFY) > 0) {
> > + changed_ls->ports[i], OVSDB_IDL_CHANGE_MODIFY) > 0 ||
> > + !ovn_port_find_in_datapath(od, changed_ls->ports[i])) {
> > ls_ports_changed = true;
> > break;
> > }
> > @@ -4450,7 +4498,7 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> > *ovnsb_idl_txn,
> > } else if (!strcmp(op->nbsp->type, "virtual")) {
> > ovs_list_push_back(&existing_virtual_ports, &op->list);
> > }
> > - op->visited = true;
> > + op->visited = !ls_deleted;
> > }
> >
> > /* Check for deleted ports */
> > @@ -4572,19 +4620,55 @@ northd_handle_ls_changes(struct ovsdb_idl_txn
> > *ovnsb_idl_txn,
> > const struct northd_input *ni,
> > struct northd_data *nd)
> > {
> > - const struct nbrec_logical_switch *changed_ls;
> > struct northd_tracked_data *trk_data = &nd->trk_data;
> > + nd->trk_data.type = NORTHD_TRACKED_NONE;
> >
> > - if (!hmapx_is_empty(&ni->synced_lses->new) ||
> > - !hmapx_is_empty(&ni->synced_lses->deleted) ||
> > + if (hmapx_is_empty(&ni->synced_lses->new) &&
> > + hmapx_is_empty(&ni->synced_lses->deleted) &&
> > hmapx_is_empty(&ni->synced_lses->updated)) {
> > goto fail;
> > }
> >
> > struct hmapx_node *node;
> > + HMAPX_FOR_EACH (node, &ni->synced_lses->new) {
> > + const struct ovn_synced_logical_switch *synced = node->data;
> > + const struct nbrec_logical_switch *new_ls = synced->nb;
> > +
> > + /* If a logical switch is created with the below columns set,
> > + * then we can't handle this yet. Goto fail. */
> > + if (new_ls->copp || new_ls->n_dns_records ||
> > + new_ls->n_forwarding_groups || new_ls->n_qos_rules) {
> > + goto fail;
> > + }
> > +
> > + struct ovn_datapath *od = ovn_datapath_create(
> > + &nd->ls_datapaths.datapaths, &new_ls->header_.uuid, new_ls,
> > + NULL, synced->sdp);
> > +
> > + ods_assign_array_index(&nd->ls_datapaths, od);
> > + init_ipam_info_for_datapath(od);
> > + init_mcast_info_for_datapath(od);
> > +
> > + /* Create SB:IP_Multicast for the logical switch. */
> > + const struct sbrec_ip_multicast *ip_mcast =
> > + sbrec_ip_multicast_insert(ovnsb_idl_txn);
> > + store_mcast_info_for_switch_datapath(ip_mcast, od);
> > +
> > + if (!ls_handle_lsp_changes(ovnsb_idl_txn, new_ls,
> > + ni, nd, od, &trk_data->trk_lsps)) {
> > + goto fail;
> > + }
> > +
> > + if (new_ls->n_acls) {
> > + hmapx_add(&trk_data->ls_with_changed_acls, od);
> > + }
> > + hmapx_add(&trk_data->trk_switches.crupdated, od);
> > + }
> > +
> > HMAPX_FOR_EACH (node, &ni->synced_lses->updated) {
> > const struct ovn_synced_logical_switch *synced = node->data;
> > - changed_ls = synced->nb;
> > + const struct nbrec_logical_switch *changed_ls = synced->nb;
> > +
> > struct ovn_datapath *od = ovn_datapath_find_(
> > &nd->ls_datapaths.datapaths,
> > &changed_ls->header_.uuid);
> > @@ -4618,6 +4702,55 @@ northd_handle_ls_changes(struct ovsdb_idl_txn
> > *ovnsb_idl_txn,
> > }
> > }
> >
> > + HMAPX_FOR_EACH (node, &ni->synced_lses->deleted) {
> > + const struct ovn_synced_logical_switch *synced = node->data;
> > + const struct nbrec_logical_switch *deleted_ls = synced->nb;
> > +
> > + struct ovn_datapath *od = ovn_datapath_find_(
> > + &nd->ls_datapaths.datapaths,
> > + &deleted_ls->header_.uuid);
> > + if (!od) {
> > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> > + VLOG_WARN_RL(&rl, "Internal error: a tracked deleted LS
> > doesn't "
> > + "exist in ls_datapaths: "UUID_FMT,
> > + UUID_ARGS(&deleted_ls->header_.uuid));
> > + goto fail;
> > + }
> > +
> > + if (deleted_ls->copp || deleted_ls->n_dns_records ||
> > + deleted_ls->n_forwarding_groups || deleted_ls->n_qos_rules ||
> > + deleted_ls->n_load_balancer ||
> > deleted_ls->n_load_balancer_group) {
> > + goto fail;
> > + }
> > +
> > + if (!ls_handle_lsp_changes(ovnsb_idl_txn, deleted_ls,
> > + ni, nd, od, &trk_data->trk_lsps)) {
> > + goto fail;
> > + }
> > +
> > + hmap_remove(&nd->ls_datapaths.datapaths, &od->key_node);
> > + vector_get(&nd->ls_datapaths.dps, od->index,
> > + struct ovn_datapath *) = NULL;
> > + bitmap_set0(nd->ls_datapaths.dps_index_map.map, od->index);
> > + nd->ls_datapaths.dps_index_map.n_elems--;
> >
>
> Can be replaced with "dynamic_bitmap_set0()".
ack, I will fix it in v7.
Regards,
Lorenzo
>
>
> > +
> > + const struct sbrec_ip_multicast *ip_mcast =
> > + ip_mcast_lookup(ni->sbrec_ip_mcast_by_dp, od->sdp->sb_dp);
> > + if (ip_mcast) {
> > + sbrec_ip_multicast_delete(ip_mcast);
> > + }
> > +
> > + if (is_ls_acls_changed(deleted_ls)) {
> > + hmapx_add(&trk_data->ls_with_changed_acls, od);
> > + }
> > + hmapx_add(&trk_data->trk_switches.deleted, od);
> > + }
> > +
> > + if (!hmapx_is_empty(&trk_data->trk_switches.crupdated) ||
> > + !hmapx_is_empty(&trk_data->trk_switches.deleted)) {
> > + trk_data->type |= NORTHD_TRACKED_SWITCHES;
> > + }
> > +
> > if (!hmapx_is_empty(&trk_data->trk_lsps.created)
> > || !hmapx_is_empty(&trk_data->trk_lsps.updated)
> > || !hmapx_is_empty(&trk_data->trk_lsps.deleted)) {
> > @@ -18937,8 +19070,8 @@ static void
> > ovn_datapaths_destroy(struct ovn_datapaths *datapaths)
> > {
> > struct ovn_datapath *dp;
> > - HMAP_FOR_EACH_SAFE (dp, key_node, &datapaths->datapaths) {
> > - ovn_datapath_destroy(&datapaths->datapaths, dp);
> > + HMAP_FOR_EACH_POP (dp, key_node, &datapaths->datapaths) {
> > + ovn_datapath_destroy(dp);
> > }
> > hmap_destroy(&datapaths->datapaths);
> >
> > diff --git a/northd/northd.h b/northd/northd.h
> > index bb12ec781..677e87d18 100644
> > --- a/northd/northd.h
> > +++ b/northd/northd.h
> > @@ -22,6 +22,7 @@
> > #include "lib/sset.h"
> > #include "northd/en-port-group.h"
> > #include "northd/ipam.h"
> > +#include "northd/lb.h"
> > #include "openvswitch/hmap.h"
> > #include "simap.h"
> > #include "ovs-thread.h"
> > @@ -100,7 +101,7 @@ struct ovn_datapaths {
> > static inline size_t
> > ods_size(const struct ovn_datapaths *datapaths)
> > {
> > - return hmap_count(&datapaths->datapaths);
> > + return vector_len(&datapaths->dps);
> > }
> >
> > struct ovn_datapath *
> > @@ -115,6 +116,13 @@ enum redirected_routing_protcol_flag_type {
> > REDIRECT_BFD = (1 << 1),
> > };
> >
> > +struct tracked_dps {
> > + /* Tracked created or updated datapaths. */
> > + struct hmapx crupdated;
> > + /* Tracked deleted datapaths. */
> > + struct hmapx deleted;
> > +};
> > +
> > struct tracked_ovn_ports {
> > /* tracked created ports.
> > * hmapx node data is 'struct ovn_port *' */
> > @@ -146,6 +154,7 @@ enum northd_tracked_data_type {
> > NORTHD_TRACKED_LR_NATS = (1 << 2),
> > NORTHD_TRACKED_LS_LBS = (1 << 3),
> > NORTHD_TRACKED_LS_ACLS = (1 << 4),
> > + NORTHD_TRACKED_SWITCHES = (1 << 5),
> > };
> >
> > /* Track what's changed in the northd engine node.
> > @@ -154,6 +163,7 @@ enum northd_tracked_data_type {
> > struct northd_tracked_data {
> > /* Indicates the type of data tracked. One or all of
> > NORTHD_TRACKED_*. */
> > enum northd_tracked_data_type type;
> > + struct tracked_dps trk_switches;
> > struct tracked_ovn_ports trk_lsps;
> > struct tracked_lbs trk_lbs;
> >
> > @@ -997,6 +1007,13 @@ northd_has_ls_acls_in_tracked_data(struct
> > northd_tracked_data *trk_nd_changes)
> > return trk_nd_changes->type & NORTHD_TRACKED_LS_ACLS;
> > }
> >
> > +static inline bool
> > +northd_has_lswitches_in_tracked_data(
> > + struct northd_tracked_data *trk_nd_changes)
> > +{
> > + return trk_nd_changes->type & NORTHD_TRACKED_SWITCHES;
> > +}
> > +
> > /* Returns 'true' if the IPv4 'addr' is on the same subnet with one of the
> > * IPs configured on the router port.
> > */
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 11bbb211d..9e0e80418 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -14544,6 +14544,67 @@ AT_CHECK([grep "lr_in_dnat" lr1flows |
> > ovn_strip_lflows | grep "30.0.0.1"], [0],
> > AT_CLEANUP
> > ])
> >
> > +OVN_FOR_EACH_NORTHD_NO_HV([
> > +AT_SETUP([Logical switch incremental processing])
> > +
> > +ovn_start
> > +
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +check ovn-nbctl --wait=sb ls-add sw0
> > +check_engine_stats northd norecompute compute
> > +check_engine_stats ls_stateful recompute nocompute
> > +check_engine_stats lflow recompute nocompute
> > +
> > +# For the below engine nodes, en_northd is input. So check
> > +# their engine status.
> > +check_engine_stats lr_stateful norecompute compute
> > +check_engine_stats route_policies norecompute compute
> > +check_engine_stats routes norecompute compute
> > +check_engine_stats bfd_sync norecompute compute
> > +check_engine_stats sync_to_sb_lb norecompute compute
> > +check_engine_stats sync_to_sb_pb norecompute compute
> > +
> > +CHECK_NO_CHANGE_AFTER_RECOMPUTE((1))
> > +
> > +# Update the logical switch.
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +check ovn-nbctl --wait=sb set logical_switch sw0 other_config:foo=bar
> > +
> > +check_engine_stats northd recompute nocompute
> > +check_engine_stats ls_stateful recompute nocompute
> > +check_engine_stats lflow recompute nocompute
> > +
> > +# For the below engine nodes, en_northd is input. So check
> > +# their engine status.
> > +check_engine_stats lr_stateful recompute nocompute
> > +check_engine_stats route_policies recompute nocompute
> > +check_engine_stats routes recompute nocompute
> > +check_engine_stats bfd_sync recompute nocompute
> > +check_engine_stats sync_to_sb_lb recompute nocompute
> > +check_engine_stats sync_to_sb_pb recompute nocompute
> > +
> > +# Create a logical port
> > +check ovn-nbctl --wait=sb lsp-add sw0 lsp0
> > +
> > +# Delete the logical switch
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +check ovn-nbctl --wait=sb ls-del sw0
> > +check_engine_stats northd norecompute compute
> > +check_engine_stats ls_stateful recompute nocompute
> > +check_engine_stats lflow recompute nocompute
> > +
> > +# For the below engine nodes, en_northd is input. So check
> > +# their engine status.
> > +check_engine_stats lr_stateful norecompute compute
> > +check_engine_stats route_policies norecompute compute
> > +check_engine_stats routes norecompute compute
> > +check_engine_stats bfd_sync norecompute compute
> > +check_engine_stats sync_to_sb_lb norecompute compute
> > +check_engine_stats sync_to_sb_pb norecompute compute
> > +
> > +AT_CLEANUP
> > +])
> > +
> > AT_SETUP([RBAC -- Recover builtin role and permissions])
> > ovn_start
> >
> > --
> > 2.50.1
> >
> >
> Thanks,
> Ales
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
