On Thu, May 15, 2025 at 3:31 PM Alexandra Rukomoinikova
<arukomoinikova@k2.cloud> wrote:
> Full recomputer triggers with any updates of fixed mirrors to ports.
> This is only necessary for lport mirror, as long as there is no
> incremental processing.
>
> Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud>
> ---
>
Hi Alexandra,
There are a few minor things that I've taken care of during merge.
Also the header states there are supposed to be 2 patches, but
only one arrived to the ML. If the second one should be reviewed
please post it again.
northd/northd.c | 27 ++++++++++++++++-----
> northd/northd.h | 3 +++
> tests/ovn-northd.at | 57 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 81 insertions(+), 6 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 7b05147b4..afc674600 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -1288,6 +1288,7 @@ ovn_port_create(struct hmap *ports, const char *key,
> ovn_port_set_nb(op, nbsp, nbrp);
> op->primary_port = op->cr_port = NULL;
> hmap_insert(ports, &op->key_node, hash_string(op->key, 0));
> + op->has_attached_lport_mirror = false;
>
> op->lflow_ref = lflow_ref_create();
> op->stateful_lflow_ref = lflow_ref_create();
> @@ -2212,6 +2213,7 @@ create_mirror_port(struct ovn_port *op, struct hmap
> *ports,
> mp->mirror_target_port = target_port;
> mp->od = op->od;
>
> + op->has_attached_lport_mirror = true;
> clear:
> free(mp_name);
> }
> @@ -4729,6 +4731,13 @@ lsp_can_be_inc_processed(const struct
> nbrec_logical_switch_port *nbsp)
> }
> }
>
> + /* Attaching lport mirror is not supported for now. */
> + for (size_t i = 0; i < nbsp->n_mirror_rules; i++) {
> + if (!strcmp("lport", nbsp->mirror_rules[i]->type)) {
> + return false;
> + }
> + }
> +
> return true;
> }
>
> @@ -4945,12 +4954,17 @@ is_lsp_mirror_target_port(
> }
>
> static bool
> -lsp_handle_mirror_rules_changes(const struct nbrec_logical_switch_port
> *nbsp)
> +lsp_handle_mirror_rules_changes(const struct ovn_port *op)
> {
> - if (nbrec_logical_switch_port_is_updated(nbsp,
> + if (!op->has_attached_lport_mirror) {
> + return true;
> + }
> +
> + if (nbrec_logical_switch_port_is_updated(op->nbsp,
> NBREC_LOGICAL_SWITCH_PORT_COL_MIRROR_RULES)) {
> return false;
> }
> +
> return true;
> }
>
> @@ -5024,7 +5038,7 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> * by this change. Fallback to recompute. */
> goto fail;
> }
> - if (!lsp_handle_mirror_rules_changes(new_nbsp) ||
> + if (!lsp_handle_mirror_rules_changes(op) ||
>
> is_lsp_mirror_target_port(ni->nbrec_mirror_by_type_and_sink,
> op)) {
> /* Fallback to recompute. */
> @@ -5078,10 +5092,11 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> sbrec_port_binding_delete(op->sb);
> delete_fdb_entries(ni->sbrec_fdb_by_dp_and_port,
> od->tunnel_key,
> op->tunnel_key);
> - if
> (is_lsp_mirror_target_port(ni->nbrec_mirror_by_type_and_sink,
> + if (op->has_attached_lport_mirror ||
> +
> is_lsp_mirror_target_port(ni->nbrec_mirror_by_type_and_sink,
> op)) {
> - /* This port was used as target mirror port, fallback
> - * to recompute. */
> + /* This port was used as target/source mirror port,
> + * fallback to recompute. */
> goto fail;
> }
> }
> diff --git a/northd/northd.h b/northd/northd.h
> index 5a698458f..13b80686d 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -689,6 +689,9 @@ struct ovn_port {
> * a parent port. */
> struct ovn_port *mirror_target_port;
>
> + /* Set to true if this port is attached to lport mirror. */
> + bool has_attached_lport_mirror;
> +
> bool has_unknown; /* If the addresses have 'unknown' defined. */
>
> /* The port's peer:
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 69b75fe9d..945539c59 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -17290,3 +17290,60 @@ AT_CHECK([cat trace | grep output], [0], [dnl
>
> AT_CLEANUP
> ])
> +
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([Lport mirror full recompute.])
> +AT_KEYWORDS([mirror])
> +ovn_start
> +
> +check ovn-nbctl ls-add ls1
> +check ovn-nbctl lsp-add ls1 lport1
> +check ovn-nbctl lsp-add ls1 lport2
> +
> +check ovn-nbctl mirror-add mirror_lport lport to-lport lport2
> +check ovn-nbctl mirror-add mirror_local local to-lport lport2
> +check ovn-nbctl mirror-add mirror_gre gre 1 to-lport 172.16.0.100
> +check ovn-nbctl mirror-add mirror_erspan erspan 3 to-lport 172.16.0.100
Missing ovn-nbctl --wait=sb sync call.
+
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb lsp-attach-mirror lport1 mirror_lport
> +check_engine_stats northd recompute compute
> +check_engine_stats lflow recompute nocompute
>
We should check if recompute doesn't change things:
CHECK_NO_CHANGE_AFTER_RECOMPUTE
I have added it after each block.
+
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb lsp-detach-mirror lport1 mirror_lport
> +check_engine_stats northd recompute compute
> +check_engine_stats lflow recompute nocompute
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb lsp-attach-mirror lport1 mirror_local
> +check_engine_stats northd norecompute compute
> +check_engine_stats lflow norecompute compute
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb lsp-detach-mirror lport1 mirror_local
> +check_engine_stats northd norecompute compute
> +check_engine_stats lflow norecompute compute
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb lsp-attach-mirror lport1 mirror_gre
> +check_engine_stats northd norecompute compute
> +check_engine_stats lflow norecompute compute
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb lsp-detach-mirror lport1 mirror_gre
> +check_engine_stats northd norecompute compute
> +check_engine_stats lflow norecompute compute
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb lsp-attach-mirror lport1 mirror_erspan
> +check_engine_stats northd norecompute compute
> +check_engine_stats lflow norecompute compute
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb lsp-detach-mirror lport1 mirror_erspan
> +check_engine_stats northd norecompute compute
> +check_engine_stats lflow norecompute compute
> +
> +AT_CLEANUP
> +])
> --
> 2.48.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Thanks,
Ales
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
