Hi Ales! thanks for the answer! there should be only one patch, it's my mistake when sending, sorry=) I'll be grateful if you add the missing things in the tests when merging
On 16.05.2025 13:12, Ales Musil wrote: On Thu, May 15, 2025 at 3:31 PM Alexandra Rukomoinikova <arukomoinikova@k2.cloud><mailto: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><mailto: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<http://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<http://ovn-northd.at> b/tests/ovn-northd.at<http://ovn-northd.at> index 69b75fe9d..945539c59 100644 --- a/tests/ovn-northd.at<http://ovn-northd.at> +++ b/tests/ovn-northd.at<http://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<mailto:d...@openvswitch.org> https://mail.openvswitch.org/mailman/listinfo/ovs-dev Thanks, Ales -- regards, Alexandra. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
