Handle ACL changes in port group incrementally. The logical switch
ACL changes are already handled, add similar method for port groups
which should prevent lflow recomputes. There is still lflow dependency
on ACL table that results in full lflow recompute. That will be solved
in future commit.
Signed-off-by: Ales Musil <amu...@redhat.com>
---
v2: Rebase on top of latest main.
---
northd/en-northd.c | 24 ++++++++++++
northd/en-northd.h | 2 +
northd/inc-proc-northd.c | 2 +
northd/northd.c | 56 +++++++++++++++++++++++++++
northd/northd.h | 3 ++
tests/ovn-northd.at | 81 +++++++++++++++++++++++++++++++++++-----
6 files changed, 159 insertions(+), 9 deletions(-)
diff --git a/northd/en-northd.c b/northd/en-northd.c
index 3359d8d0e..0f1425ec7 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -78,6 +78,8 @@ northd_get_input_data(struct engine_node *node,
EN_OVSDB_GET(engine_get_input("NB_mirror", node));
input_data->nbrec_mirror_rule_table =
EN_OVSDB_GET(engine_get_input("NB_mirror_rule", node));
+ input_data->nbrec_port_gorup_table =
+ EN_OVSDB_GET(engine_get_input("NB_port_group", node));
input_data->sbrec_datapath_binding_table =
EN_OVSDB_GET(engine_get_input("SB_datapath_binding", node));
@@ -238,6 +240,28 @@ northd_global_config_handler(struct engine_node *node,
void *data OVS_UNUSED)
return EN_HANDLED_UNCHANGED;
}
+enum engine_input_handler_result
+northd_nb_port_group_handler(struct engine_node *node, void *data)
+{
+ struct northd_data *nd = data;
+
+ struct northd_input input_data;
+ northd_get_input_data(node, &input_data);
+
+ /* This handler cares only about ACLs, the port group itself has separate
+ * node. */
+ if (!northd_handle_pgs_acl_changes(&input_data, nd)) {
+ return EN_UNHANDLED;
+ }
+
+ if (northd_has_tracked_data(&nd->trk_data)) {
+ return EN_HANDLED_UPDATED;
+ }
+
+ return EN_HANDLED_UNCHANGED;
+}
+
+
enum engine_input_handler_result
route_policies_northd_change_handler(struct engine_node *node,
void *data OVS_UNUSED)
diff --git a/northd/en-northd.h b/northd/en-northd.h
index b19b73270..4f744a6c5 100644
--- a/northd/en-northd.h
+++ b/northd/en-northd.h
@@ -26,6 +26,8 @@ northd_sb_port_binding_handler(struct engine_node *, void
*data);
enum engine_input_handler_result northd_lb_data_handler(struct engine_node *,
void *data);
enum engine_input_handler_result
+northd_nb_port_group_handler(struct engine_node *node, void *data);
+enum engine_input_handler_result
northd_sb_fdb_change_handler(struct engine_node *node, void *data);
void *en_routes_init(struct engine_node *node OVS_UNUSED,
struct engine_arg *arg OVS_UNUSED);
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index 5905462ec..be2f36e33 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -246,6 +246,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
engine_add_input(&en_northd, &en_nb_logical_router,
northd_nb_logical_router_handler);
engine_add_input(&en_northd, &en_lb_data, northd_lb_data_handler);
+ engine_add_input(&en_northd, &en_nb_port_group,
+ northd_nb_port_group_handler);
engine_add_input(&en_lr_nat, &en_northd, lr_nat_northd_handler);
diff --git a/northd/northd.c b/northd/northd.c
index 7b05147b4..2e8e7e63f 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -5192,6 +5192,62 @@ fail:
return false;
}
+static bool
+is_pg_acls_changed(const struct nbrec_port_group *npg) {
+
+ return (nbrec_port_group_is_updated(npg, NBREC_PORT_GROUP_COL_ACLS)
+ || is_acls_seqno_changed(npg->acls, npg->n_acls));
+}
+
+bool
+northd_handle_pgs_acl_changes(const struct northd_input *ni,
+ struct northd_data *nd)
+{
+ const struct nbrec_port_group *nb_pg;
+ struct northd_tracked_data *trk_data = &nd->trk_data;
+
+ NBREC_PORT_GROUP_TABLE_FOR_EACH_TRACKED (nb_pg,
+ ni->nbrec_port_gorup_table) {
+ if (!is_pg_acls_changed(nb_pg)) {
+ continue;
+ }
+
+ for (size_t i = 0; i < nb_pg->n_ports; i++) {
+ const char *port_name = nb_pg->ports[i]->name;
+ const struct ovn_datapath *od =
+ northd_get_datapath_for_port(&nd->ls_ports, port_name);
+
+ if (!od) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+ VLOG_ERR_RL(&rl, "lport %s in port group %s not found.",
+ port_name, nb_pg->name);
+ goto fail;
+ }
+
+ if (!od->nbs) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+ VLOG_WARN_RL(&rl, "lport %s in port group %s has no lswitch.",
+ nb_pg->ports[i]->name,
+ nb_pg->name);
+ goto fail;
+ }
+
+ hmapx_add(&trk_data->ls_with_changed_acls,
+ CONST_CAST(struct ovn_datapath *, od));
+ }
+ }
+
+ if (!hmapx_is_empty(&trk_data->ls_with_changed_acls)) {
+ trk_data->type |= NORTHD_TRACKED_LS_ACLS;
+ }
+
+ return true;
+
+fail:
+ destroy_northd_data_tracked_changes(nd);
+ return false;
+}
+
/* Returns true if the logical router has changes which can be
* incrementally handled.
* Presently supports i-p for the below changes:
diff --git a/northd/northd.h b/northd/northd.h
index 5a698458f..55155f7be 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -38,6 +38,7 @@ struct northd_input {
*nbrec_chassis_template_var_table;
const struct nbrec_mirror_table *nbrec_mirror_table;
const struct nbrec_mirror_rule_table *nbrec_mirror_rule_table;
+ const struct nbrec_port_group_table *nbrec_port_gorup_table;
/* Southbound table references */
const struct sbrec_datapath_binding_table *sbrec_datapath_binding_table;
@@ -829,6 +830,8 @@ bool northd_handle_ls_changes(struct ovsdb_idl_txn *,
struct northd_data *);
bool northd_handle_lr_changes(const struct northd_input *,
struct northd_data *);
+bool northd_handle_pgs_acl_changes(const struct northd_input *ni,
+ struct northd_data *nd);
void destroy_northd_data_tracked_changes(struct northd_data *);
void northd_destroy(struct northd_data *data);
void northd_init(struct northd_data *data);
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 39f27772b..6d0f8f86f 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -10095,7 +10095,7 @@ dnl The northd node should not recompute.
AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd],
[0], [dnl
Node: northd
- recompute: 0
-- compute: 0
+- compute: 1
- cancel: 0
])
dnl The port_group node recomputes every time a NB port group is added/deleted.
@@ -10132,7 +10132,7 @@ dnl The northd node should not recompute.
AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd],
[0], [dnl
Node: northd
- recompute: 0
-- compute: 0
+- compute: 1
- cancel: 0
])
dnl The port_group node recomputes also every time a port from a new switch
@@ -10168,7 +10168,7 @@ dnl The northd node should not recompute.
AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd],
[0], [dnl
Node: northd
- recompute: 0
-- compute: 0
+- compute: 1
- cancel: 0
])
dnl The port_group node recomputes also every time a port from a new switch
@@ -10205,7 +10205,7 @@ dnl The northd node should not recompute.
AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd],
[0], [dnl
Node: northd
- recompute: 0
-- compute: 0
+- compute: 1
- cancel: 0
])
dnl We did not change the set of switches a pg is applied to, there should be
@@ -10243,7 +10243,7 @@ dnl though, therefore "compute: 1".
AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd],
[0], [dnl
Node: northd
- recompute: 0
-- compute: 0
+- compute: 1
- cancel: 0
])
dnl We did not change the set of switches a pg is applied to, there should be
@@ -10279,7 +10279,7 @@ dnl The northd node should not recompute.
AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd],
[0], [dnl
Node: northd
- recompute: 0
-- compute: 0
+- compute: 1
- cancel: 0
])
dnl We changed the set of switches a pg is applied to, there should be
@@ -10316,7 +10316,7 @@ dnl The northd node should not recompute.
AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd],
[0], [dnl
Node: northd
- recompute: 0
-- compute: 0
+- compute: 1
- cancel: 0
])
dnl We changed the set of switches a pg is applied to, there should be
@@ -10353,7 +10353,7 @@ dnl The northd node should not recompute.
AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd],
[0], [dnl
Node: northd
- recompute: 0
-- compute: 0
+- compute: 1
- cancel: 0
])
dnl We changed the set of switches a pg is applied to, there should be a
@@ -10392,7 +10392,7 @@ dnl The northd node should not recompute,.
AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats northd],
[0], [dnl
Node: northd
- recompute: 0
-- compute: 0
+- compute: 1
- cancel: 0
])
dnl We changed the set of switches a pg is applied to, there should be a
@@ -17296,6 +17296,9 @@ AT_SETUP([ACL incremental processing])
ovn_start
check ovn-nbctl ls-add ls
+check ovn-nbctl lsp-add ls lsp0
+check ovn-nbctl pg-add pg lsp0
+
check ovn-nbctl meter-add meter1 drop 10 kbps
check ovn-nbctl meter-add meter2 drop 20 kbps
@@ -17364,5 +17367,65 @@ check_engine_stats sync_meters recompute nocompute
CHECK_NO_CHANGE_AFTER_RECOMPUTE
check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+AS_BOX([ACLs attached to LS])
+check ovn-nbctl --wait=sb acl-add ls from-lport 100 tcp drop
+acl_id=$(fetch_column nb:Acl _uuid match=tcp action=drop)
+check_engine_stats northd norecompute compute
+check_engine_stats lflow recompute nocompute
+check_engine_stats sync_meters norecompute compute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
+check ovn-nbctl --wait=sb acl-add ls from-lport 101 ip4 drop
+check_engine_stats northd norecompute compute
+check_engine_stats lflow recompute nocompute
+check_engine_stats sync_meters norecompute compute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
+check ovn-nbctl --wait=sb set acl $acl_id match=udp
+check_engine_stats northd norecompute compute
+check_engine_stats lflow recompute nocompute
+check_engine_stats sync_meters norecompute compute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
+check ovn-nbctl --wait=sb acl-del ls
+check_engine_stats northd norecompute compute
+check_engine_stats lflow recompute nocompute
+check_engine_stats sync_meters norecompute compute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
+AS_BOX([ACLs attached to PG])
+check ovn-nbctl --wait=sb acl-add pg from-lport 100 tcp drop
+acl_id=$(fetch_column nb:Acl _uuid match=tcp action=drop)
+check_engine_stats northd norecompute compute
+check_engine_stats lflow recompute nocompute
+check_engine_stats sync_meters norecompute compute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
+check ovn-nbctl --wait=sb acl-add pg from-lport 101 ip4 drop
+check_engine_stats northd norecompute compute
+check_engine_stats lflow recompute nocompute
+check_engine_stats sync_meters norecompute compute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
+check ovn-nbctl --wait=sb set acl $acl_id match=udp
+check_engine_stats northd norecompute compute
+check_engine_stats lflow recompute nocompute
+check_engine_stats sync_meters norecompute compute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
+check ovn-nbctl --wait=sb acl-del pg
+check_engine_stats northd norecompute compute
+check_engine_stats lflow recompute nocompute
+check_engine_stats sync_meters norecompute compute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+
AT_CLEANUP
])
--
2.49.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
