Abhiram, Thanks for confirming the code should already handle bulk updates; perhaps then I just misunderstand how it works. Can we get test cases added for these bulk update scenarios? This should be achievable eg. as in:
check as hv1 ovn-appctl -t ovn-controller debug/pause # modify mirror relationships check as hv1 ovn-appctl -t ovn-controller debug/resume # check that mirrors are properly set in ovsdb There are several test cases in the tree that already emulate bulk / delayed updates using this technique, for your reference. Thanks again, Ihar On Mon, Oct 3, 2022 at 3:13 PM Abhiram R N <a...@redhat.com> wrote: > > Hi Ihar, > > Thanks for your review. > Please see below for replies inline. > > On Mon, Oct 3, 2022 at 11:28 PM Ihar Hrachyshka <ihrac...@redhat.com> wrote: >> >> NEWS file should be updated. See more comments inline below. > > Sure. Will update the NEWS file in the next patch. >> >> >> On Wed, Sep 28, 2022 at 9:08 AM Abhiram R N <abhira...@gmail.com> wrote: >> > >> > Added changes in ovn-nbctl, ovn-sbctl, northd and in ovn-controller. >> > While Mirror creation just creates the mirror, the lsp-attach-mirror >> > triggers the sequence to create Mirror in OVS DB on compute node. >> > OVS already supports Port Mirroring. >> > >> > Note: This is targeted to mirror to destinations anywhere outside the >> > cluster where the analyser resides and it need not be an OVN node. >> > >> > Example commands are as below: >> > >> > Mirror creation >> > ovn-nbctl mirror-add mirror1 gre 0 from-lport 10.10.10.2 >> > >> > Attach a logical port to the mirror. >> > ovn-nbctl lsp-attach-mirror sw0-port1 mirror1 >> > >> > Detach a source from Mirror >> > ovn-nbctl lsp-detach-mirror sw0-port1 mirror1 >> > >> > Mirror deletion >> > ovn-nbctl mirror-del mirror1 >> > >> > Co-authored-by: Veda Barrenkala <vedabarrenk...@gmail.com> >> > Signed-off-by: Veda Barrenkala <vedabarrenk...@gmail.com> >> > Signed-off-by: Abhiram R N <abhira...@gmail.com> >> > --- >> > V6 --> V7: Addressed review comments from V6 by Numan >> > i) Did suggested changes in mirror.c and ovn-controller.c >> > ii) Handled the use-case and added to ovn.at test >> > iii) Indentation correction in ovn-nbctl.c >> > >> > Files modified (V6 --> v7): >> > Code --> mirror.c, ovn-controller.c, ovn-nbctl.c >> > Test --> ovn.at >> > >> > controller/automake.mk | 4 +- >> > controller/mirror.c | 537 ++++++++++++++++++++++++++++++++++++ >> > controller/mirror.h | 53 ++++ >> > controller/ovn-controller.c | 266 ++++++++++++++++-- >> > northd/en-northd.c | 4 + >> > northd/inc-proc-northd.c | 4 + >> > northd/northd.c | 172 ++++++++++++ >> > northd/northd.h | 2 + >> > ovn-nb.ovsschema | 31 ++- >> > ovn-nb.xml | 63 +++++ >> > ovn-sb.ovsschema | 26 +- >> > ovn-sb.xml | 50 ++++ >> > tests/ovn-nbctl.at | 116 ++++++++ >> > tests/ovn-northd.at | 102 +++++++ >> > tests/ovn.at | 231 ++++++++++++++++ >> > utilities/ovn-nbctl.c | 357 ++++++++++++++++++++++++ >> > utilities/ovn-sbctl.c | 4 + >> > 17 files changed, 1994 insertions(+), 28 deletions(-) >> > create mode 100644 controller/mirror.c >> > create mode 100644 controller/mirror.h >> > >> > diff --git a/controller/automake.mk b/controller/automake.mk >> > index c2ab1bbe6..334672b4d 100644 >> > --- a/controller/automake.mk >> > +++ b/controller/automake.mk >> > @@ -41,7 +41,9 @@ controller_ovn_controller_SOURCES = \ >> > controller/ovsport.h \ >> > controller/ovsport.c \ >> > controller/vif-plug.h \ >> > - controller/vif-plug.c >> > + controller/vif-plug.c \ >> > + controller/mirror.h \ >> > + controller/mirror.c >> > >> > controller_ovn_controller_LDADD = lib/libovn.la >> > $(OVS_LIBDIR)/libopenvswitch.la >> > man_MANS += controller/ovn-controller.8 >> > diff --git a/controller/mirror.c b/controller/mirror.c >> > new file mode 100644 >> > index 000000000..37f8b2e9f >> > --- /dev/null >> > +++ b/controller/mirror.c >> > @@ -0,0 +1,537 @@ >> > +/* Copyright (c) 2022 Red Hat, Inc. >> > + * >> > + * Licensed under the Apache License, Version 2.0 (the "License"); >> > + * you may not use this file except in compliance with the License. >> > + * You may obtain a copy of the License at: >> > + * >> > + * http://www.apache.org/licenses/LICENSE-2.0 >> > + * >> > + * Unless required by applicable law or agreed to in writing, software >> > + * distributed under the License is distributed on an "AS IS" BASIS, >> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >> > implied. >> > + * See the License for the specific language governing permissions and >> > + * limitations under the License. >> > + */ >> > + >> > +#include <config.h> >> > +#include <unistd.h> >> > + >> > +/* library headers */ >> > +#include "lib/sset.h" >> > +#include "lib/util.h" >> > + >> > +/* OVS includes. */ >> > +#include "lib/vswitch-idl.h" >> > +#include "openvswitch/vlog.h" >> > + >> > +/* OVN includes. */ >> > +#include "binding.h" >> > +#include "lib/ovn-sb-idl.h" >> > +#include "mirror.h" >> > + >> > +VLOG_DEFINE_THIS_MODULE(port_mirror); >> > + >> > +/* Static function declarations */ >> > + >> > +static const struct ovsrec_port * >> > +get_port_for_iface(const struct ovsrec_interface *iface, >> > + const struct ovsrec_bridge *br_int) >> > +{ >> > + for (size_t i = 0; i < br_int->n_ports; i++) { >> > + const struct ovsrec_port *p = br_int->ports[i]; >> > + for (size_t j = 0; j < p->n_interfaces; j++) { >> > + if (!strcmp(iface->name, p->interfaces[j]->name)) { >> > + return p; >> > + } >> > + } >> > + } >> > + return NULL; >> > +} >> > + >> > +static bool >> > +mirror_create(const struct sbrec_port_binding *pb, >> > + struct port_mirror_ctx *pm_ctx) >> > +{ >> > + const struct ovsrec_mirror *mirror = NULL; >> > + >> > + if (pb->n_up && !pb->up[0]) { >> > + return true; >> > + } >> > + >> > + if (pb->chassis != pm_ctx->chassis_rec) { >> > + return true; >> > + } >> > + >> > + if (!pm_ctx->ovs_idl_txn) { >> > + return false; >> > + } >> > + >> > + >> > + VLOG_INFO("Mirror rule(s) present for %s ", pb->logical_port); >> > + /* Loop through the mirror rules */ >> > + for (size_t i =0; i < pb->n_mirror_rules; i++) { >> > + /* check if the mirror already exists in OVS DB */ >> > + bool create_mirror = true; >> > + OVSREC_MIRROR_TABLE_FOR_EACH (mirror, pm_ctx->mirror_table) { >> > + if (!strcmp(pb->mirror_rules[i]->name, mirror->name)) { >> > + /* Mirror with same name already exists >> > + * No need to create mirror >> > + */ >> > + create_mirror = false; >> > + break; >> > + } >> > + } >> > + >> > + if (create_mirror) { >> > + >> > + struct smap options = SMAP_INITIALIZER(&options); >> > + char *port_name, *key; >> > + >> > + key = xasprintf("%ld",(long int)pb->mirror_rules[i]->index); >> > + smap_add(&options, "remote_ip", pb->mirror_rules[i]->sink); >> > + >> > + if (!strcmp(pb->mirror_rules[i]->type, "gre")) { >> > + /* Set the GRE key */ >> > + smap_add(&options, "key", key); >> > + >> > + } else if (!strcmp(pb->mirror_rules[i]->type, "erspan")) { >> > + /* Set the ERSPAN index */ >> > + smap_add(&options, "key", key); >> > + smap_add(&options, "erspan_idx", key); >> > + smap_add(&options, "erspan_ver","1"); >> > + >> > + } >> > + struct ovsrec_interface *iface = >> > + ovsrec_interface_insert(pm_ctx->ovs_idl_txn); >> > + port_name = xasprintf("ovn-%s", >> > + pb->mirror_rules[i]->name); >> > + >> > + ovsrec_interface_set_name(iface, port_name); >> > + ovsrec_interface_set_type(iface, pb->mirror_rules[i]->type); >> > + ovsrec_interface_set_options(iface, &options); >> > + >> > + struct ovsrec_port *port = >> > + ovsrec_port_insert(pm_ctx->ovs_idl_txn); >> > + ovsrec_port_set_name(port, port_name); >> > + ovsrec_port_set_interfaces(port, &iface, 1); >> > + >> > + ovsrec_bridge_update_ports_addvalue(pm_ctx->br_int, port); >> > + >> > + smap_destroy(&options); >> > + free(port_name); >> > + free(key); >> > + >> > + VLOG_INFO("Creating Mirror in OVS DB"); >> > + mirror = ovsrec_mirror_insert(pm_ctx->ovs_idl_txn); >> > + ovsrec_mirror_set_name(mirror,pb->mirror_rules[i]->name); >> > + ovsrec_mirror_update_output_port_addvalue(mirror, port); >> > + ovsrec_bridge_update_mirrors_addvalue(pm_ctx->br_int, >> > + mirror); >> > + } >> > + >> > + struct local_binding *lbinding = local_binding_find( >> > + pm_ctx->local_bindings, pb->logical_port); >> > + const struct ovsrec_port *p = >> > + get_port_for_iface(lbinding->iface, pm_ctx->br_int); >> > + if (p) { >> > + if (!strcmp(pb->mirror_rules[i]->filter,"from-lport")) { >> > + ovsrec_mirror_update_select_src_port_addvalue(mirror, p); >> > + } else if (!strcmp(pb->mirror_rules[i]->filter,"to-lport")) { >> > + ovsrec_mirror_update_select_dst_port_addvalue(mirror, p); >> > + } else { >> > + ovsrec_mirror_update_select_src_port_addvalue(mirror, p); >> > + ovsrec_mirror_update_select_dst_port_addvalue(mirror, p); >> > + } >> > + } >> > + } >> > + return true; >> > +} >> > + >> > +static void >> > +check_and_update_mirror_table(const struct sbrec_mirror *sb_mirror, >> > + struct ovsrec_mirror *ovs_mirror) >> > +{ >> > + char *filter; >> > + if ((ovs_mirror->n_select_dst_port) >> > + && (ovs_mirror->n_select_src_port)) { >> > + filter = "both"; >> > + } else if (ovs_mirror->n_select_dst_port) { >> > + filter = "to-lport"; >> > + } else { >> > + filter = "from-lport"; >> > + } >> > + >> > + if (strcmp(sb_mirror->filter, filter)) { >> > + if (!strcmp(sb_mirror->filter,"from-lport") >> > + && !strcmp(filter,"both")) { >> > + for (size_t i = 0; i < ovs_mirror->n_select_dst_port; i++) { >> > + ovsrec_mirror_update_select_dst_port_delvalue(ovs_mirror, >> > + >> > ovs_mirror->select_dst_port[i]); >> > + } >> > + } else if (!strcmp(sb_mirror->filter,"to-lport") >> > + && !strcmp(filter,"both")) { >> > + for (size_t i = 0; i < ovs_mirror->n_select_src_port; i++) { >> > + ovsrec_mirror_update_select_src_port_delvalue(ovs_mirror, >> > + >> > ovs_mirror->select_src_port[i]); >> > + } >> > + } else if (!strcmp(sb_mirror->filter,"both") >> > + && !strcmp(filter,"from-lport")) { >> > + for (size_t i = 0; i < ovs_mirror->n_select_src_port; i++) { >> > + ovsrec_mirror_update_select_dst_port_addvalue(ovs_mirror, >> > + >> > ovs_mirror->select_src_port[i]); >> > + } >> > + } else if (!strcmp(sb_mirror->filter,"both") >> > + && !strcmp(filter,"to-lport")) { >> > + for (size_t i = 0; i < ovs_mirror->n_select_dst_port; i++) { >> > + ovsrec_mirror_update_select_src_port_addvalue(ovs_mirror, >> > + >> > ovs_mirror->select_dst_port[i]); >> > + } >> > + } else if (!strcmp(sb_mirror->filter,"to-lport") >> > + && !strcmp(filter,"from-lport")) { >> > + for (size_t i = 0; i < ovs_mirror->n_select_src_port; i++) { >> > + ovsrec_mirror_update_select_dst_port_addvalue(ovs_mirror, >> > + >> > ovs_mirror->select_src_port[i]); >> > + ovsrec_mirror_update_select_src_port_delvalue(ovs_mirror, >> > + >> > ovs_mirror->select_src_port[i]); >> > + } >> > + } else if (!strcmp(sb_mirror->filter,"from-lport") >> > + && !strcmp(filter,"to-lport")) { >> > + for (size_t i = 0; i < ovs_mirror->n_select_dst_port; i++) { >> > + ovsrec_mirror_update_select_src_port_addvalue(ovs_mirror, >> > + >> > ovs_mirror->select_dst_port[i]); >> > + ovsrec_mirror_update_select_dst_port_delvalue(ovs_mirror, >> > + >> > ovs_mirror->select_dst_port[i]); >> > + } >> > + } >> > + } >> > +} >> > + >> > +static void >> > +check_and_update_interface_table(const struct sbrec_mirror *sb_mirror, >> > + struct ovsrec_mirror *ovs_mirror) >> > +{ >> > + struct smap options = SMAP_INITIALIZER(&options); >> > + char *key, *type; >> > + struct smap *opts = &ovs_mirror->output_port->interfaces[0]->options; >> > + struct ovsrec_interface *iface = >> > + ovs_mirror->output_port->interfaces[0]; >> > + >> >> nit: first calculate iface, then do: opts = iface->options > > Sure. >> >> >> > + const char *erspan_ver = smap_get(opts, "erspan_ver"); >> > + if (erspan_ver) { >> > + type = "erspan"; >> > + } else { >> > + type = "gre"; >> > + } >> > + if (strcmp(type, sb_mirror->type)) { >> > + ovsrec_interface_set_type(iface, sb_mirror->type); >> > + } >> > + >> > + key = xasprintf("%ld",(long int)sb_mirror->index); >> >> Why do we need to force convert to long int? Wouldn't "%d" formatter >> work the same? > > I guess it should. Will change it and check once though >> >> >> > + smap_add(&options, "remote_ip", sb_mirror->sink); >> > + >> > + if (!strcmp(sb_mirror->type, "gre")) { >> > + /* Set the GRE key */ >> > + smap_add(&options, "key", key); >> >> nit: This is common to all types, so maybe move it outside the if >> block and remove the duplicate line below. That said... > > Sure. >> >> >> > + >> > + } else if (!strcmp(sb_mirror->type, "erspan")) { >> > + /* Set the ERSPAN index */ >> > + smap_add(&options, "key", key); >> >> ... is it really needed to set "key" here? Doesn't "erspan_idx" >> override it for erspan tunnels? (It's more of a question, not a >> statement. I really don't know.) > > Yeah it's needed from what I remember. > Although yeah I agree they are having erspan_idx. > Will recheck it and keep it outside if so as suggested above. >> >> >> > + smap_add(&options, "erspan_idx", key); >> > + smap_add(&options, "erspan_ver","1"); >> > + >> > + } >> >> nit: you could combine the if-else-if block with the 'if (erspan_ver)' >> block above. > > Sure. >> >> >> > + >> > + ovsrec_interface_set_options(iface, &options); >> > + smap_destroy(&options); >> > + free(key); >> > + >> > +} >> > + >> > +static void >> > +mirror_update(const struct sbrec_mirror *sb_mirror, >> > + struct ovsrec_mirror *ovs_mirror) >> > +{ >> > + check_and_update_interface_table(sb_mirror, ovs_mirror); >> > + >> > + check_and_update_mirror_table(sb_mirror, ovs_mirror); >> > +} >> > + >> > +static bool >> > +mirror_delete(const struct sbrec_port_binding *pb, >> > + struct port_mirror_ctx *pm_ctx, >> > + struct shash *pb_mirror_map, >> > + bool delete_all) >> > +{ >> > + >> > + if (!pm_ctx->ovs_idl_txn) { >> > + return false; >> > + } >> > + >> > + struct sset pb_mirrors = SSET_INITIALIZER(&pb_mirrors); >> > + >> > + if (!delete_all) { >> > + for (size_t i = 0; i < pb->n_mirror_rules ; i++) { >> > + sset_add(&pb_mirrors, pb->mirror_rules[i]->name); >> > + } >> > + } >> > + >> > + if (delete_all && (shash_is_empty(pb_mirror_map)) && >> > pb->n_mirror_rules) { >> > + for (size_t i = 0; i < pb->n_mirror_rules ; i++) { >> > + >> > + struct ovsrec_mirror *ovs_mirror = NULL; >> > + ovs_mirror = shash_find_data(pm_ctx->ovs_mirrors, >> > + pb->mirror_rules[i]->name); >> > + if (ovs_mirror) { >> > + ovsrec_bridge_update_ports_delvalue(pm_ctx->br_int, >> > + ovs_mirror->output_port); >> > + ovsrec_bridge_update_mirrors_delvalue(pm_ctx->br_int, >> > + ovs_mirror); >> > + ovsrec_port_delete(ovs_mirror->output_port); >> > + ovsrec_mirror_delete(ovs_mirror); >> > + } >> > + } >> > + } >> > + >> > + struct shash_node *mirror_node; >> > + SHASH_FOR_EACH (mirror_node, pb_mirror_map) { >> > + struct ovsrec_mirror *ovs_mirror = mirror_node->data; >> > + if (!sset_find(&pb_mirrors, ovs_mirror->name)) { >> > + /* Find if the mirror has other sources i*/ >> > + if ((ovs_mirror->n_select_dst_port > 1) || >> > + (ovs_mirror->n_select_src_port > 1)) { >> > + /* More than 1 source then just >> > + * update the mirror table >> > + */ >> > + bool done = false; >> > + for (size_t i = 0; ((i < ovs_mirror->n_select_dst_port) >> > + && (done == false)); >> > i++) { >> > + const struct ovsrec_port *port_rec = >> > + >> > ovs_mirror->select_dst_port[i]; >> > + for (size_t j = 0; j < port_rec->n_interfaces; j++) { >> > + const struct ovsrec_interface *iface_rec; >> > + >> > + iface_rec = port_rec->interfaces[j]; >> > + const char *iface_id = >> > + >> > smap_get(&iface_rec->external_ids, >> > + >> > "iface-id"); >> > + if (!strcmp(iface_id,pb->logical_port)) { >> > + ovsrec_mirror_update_select_dst_port_delvalue( >> > + ovs_mirror, >> > port_rec); >> > + done = true; >> > + break; >> > + } >> > + } >> > + } >> > + done = false; >> > + for (size_t i = 0; ((i < ovs_mirror->n_select_src_port) >> > + && (done == false)); >> > i++) { >> > + const struct ovsrec_port *port_rec = >> > + >> > ovs_mirror->select_src_port[i]; >> > + for (size_t j = 0; j < port_rec->n_interfaces; j++) { >> > + const struct ovsrec_interface *iface_rec; >> > + >> > + iface_rec = port_rec->interfaces[j]; >> > + const char *iface_id = >> > + >> > smap_get(&iface_rec->external_ids, >> > + >> > "iface-id"); >> > + if (!strcmp(iface_id,pb->logical_port)) { >> > + ovsrec_mirror_update_select_src_port_delvalue( >> > + ovs_mirror, >> > port_rec); >> > + done = true; >> > + break; >> > + } >> > + } >> > + } >> > + } else { >> > + /* >> > + * If only 1 source delete the output port >> > + * and then delete the mirror completely >> > + */ >> > + VLOG_INFO("Only 1 source for the mirror. Hence delete >> > it"); >> > + ovsrec_bridge_update_ports_delvalue(pm_ctx->br_int, >> > + >> > ovs_mirror->output_port); >> > + ovsrec_bridge_update_mirrors_delvalue(pm_ctx->br_int, >> > + ovs_mirror); >> > + ovsrec_port_delete(ovs_mirror->output_port); >> > + ovsrec_mirror_delete(ovs_mirror); >> > + } >> > + } >> > + } >> > + >> > + const char *used_node, *used_next; >> > + SSET_FOR_EACH_SAFE (used_node, used_next, &pb_mirrors) { >> > + sset_delete(&pb_mirrors, SSET_NODE_FROM_NAME(used_node)); >> > + } >> > + sset_destroy(&pb_mirrors); >> > + >> > + return true; >> > +} >> > + >> > +static void >> > +find_port_specific_mirrors (const struct sbrec_port_binding *pb, >> > + struct port_mirror_ctx *pm_ctx, >> > + struct shash *pb_mirror_map) >> > +{ >> > + const struct ovsrec_mirror *mirror = NULL; >> > + >> > + OVSREC_MIRROR_TABLE_FOR_EACH (mirror, pm_ctx->mirror_table) { >> > + for (size_t i = 0; i < mirror->n_select_dst_port; i++) { >> > + const struct ovsrec_port *port_rec = >> > mirror->select_dst_port[i]; >> > + for (size_t j = 0; j < port_rec->n_interfaces; j++) { >> > + const struct ovsrec_interface *iface_rec; >> > + iface_rec = port_rec->interfaces[j]; >> > + const char *logical_port = >> > + smap_get(&iface_rec->external_ids, "iface-id"); >> > + if (!strcmp(logical_port, pb->logical_port)) { >> > + shash_add_once(pb_mirror_map, mirror->name, mirror); >> > + } >> > + } >> > + } >> > + for (size_t i = 0; i < mirror->n_select_src_port; i++) { >> > + const struct ovsrec_port *port_rec = >> > mirror->select_src_port[i]; >> > + for (size_t j = 0; j < port_rec->n_interfaces; j++) { >> > + const struct ovsrec_interface *iface_rec; >> > + iface_rec = port_rec->interfaces[j]; >> > + const char *logical_port = >> > + smap_get(&iface_rec->external_ids, "iface-id"); >> > + if (!strcmp(logical_port, pb->logical_port)) { >> > + shash_add_once(pb_mirror_map, mirror->name, mirror); >> > + } >> > + } >> > + } >> > + } >> > +} >> > + >> > +void >> > +mirror_register_ovs_idl(struct ovsdb_idl *ovs_idl) >> > +{ >> > + ovsdb_idl_add_column(ovs_idl, &ovsrec_bridge_col_mirrors); >> > + >> > + ovsdb_idl_add_table(ovs_idl, &ovsrec_table_mirror); >> > + ovsdb_idl_add_column(ovs_idl, &ovsrec_mirror_col_name); >> > + ovsdb_idl_add_column(ovs_idl, &ovsrec_mirror_col_output_port); >> > + ovsdb_idl_add_column(ovs_idl, &ovsrec_mirror_col_select_dst_port); >> > + ovsdb_idl_add_column(ovs_idl, &ovsrec_mirror_col_select_src_port); >> > +} >> > + >> > + >> > +void >> > +ovn_port_mirror_init(struct shash *ovs_mirrors) >> > +{ >> > + shash_init(ovs_mirrors); >> > +} >> > + >> > +void >> > +ovn_port_mirror_run(struct port_mirror_ctx *pm_ctx) >> > +{ >> > + const struct sbrec_port_binding *pb; >> > + SBREC_PORT_BINDING_TABLE_FOR_EACH (pb, >> > + pm_ctx->port_binding_table) { >> > + ovn_port_mirror_handle_lport(pb, false, pm_ctx); >> > + } >> > +} >> > + >> > +bool >> > +ovn_port_mirror_handle_lport(const struct sbrec_port_binding *pb, bool >> > removed, >> > + struct port_mirror_ctx *pm_ctx) >> > +{ >> > + bool ret = true; >> > + struct local_binding *lbinding = local_binding_find( >> > + pm_ctx->local_bindings, pb->logical_port); >> > + >> > + if (strcmp(pb->type, "") && (!lbinding)) { >> > + return ret; >> > + } >> > + >> > + struct shash port_ovs_mirrors = SHASH_INITIALIZER(&port_ovs_mirrors); >> > + >> > + /* Need to find if mirror needs update */ >> > + find_port_specific_mirrors(pb, pm_ctx, &port_ovs_mirrors); >> > + if (!removed) { >> > + if ((pb->n_mirror_rules == 0) >> > + && (shash_is_empty(&port_ovs_mirrors))) { >> > + /* No mirror update */ >> > + } else if (pb->n_mirror_rules == shash_count(&port_ovs_mirrors)) { >> > + /* Though number of mirror rules are same, >> > + * need to verify the contents >> > + */ >> > + for (size_t i = 0; i < pb->n_mirror_rules; i++) { >> > + if (!shash_find(&port_ovs_mirrors, >> > + pb->mirror_rules[i]->name)) { >> > + /* Mis match in OVN SB DB and OVS DB >> > + * Delete and Create mirror(s) with proper sources >> > + */ >> > + ret = mirror_delete(pb, pm_ctx, >> > + &port_ovs_mirrors, false); >> > + if (ret) { >> > + ret = mirror_create(pb, pm_ctx); >> > + } >> > + break; >> >> This break suggests that if *two* mirror entries changed, then you >> will handle one of the changes here only. This seems incorrect. I >> think we had a similar discussion before around the possibility of >> bulk updates to mirror bindings where multiple mirror relationships >> can be included in the same transaction handled by the engine. The >> engine handler should be ready to handle bulk updates. I believe there >> are some debug commands available in appctl to emulate bulk updates in >> test cases ("pause"). You could use them to test the scenarios in test >> suite. > > Already mirrror_create and mirror_delete takes care of bulk updates. > I thought about it and since we are already looping through the mirror_rules > inside of the above 2 functions it gets handled. > Infact, I have tested the same with 2 mirror updates. :) >> >> >> > + } >> > + } >> > + } else { >> > + /* Update Mirror */ >> > + if (pb->n_mirror_rules > shash_count(&port_ovs_mirrors)) { >> > + /* create mirror, >> > + * if mirror already exists only update selection >> > + */ >> > + ret = mirror_create(pb, pm_ctx); >> > + } else { >> > + /* delete mirror, >> > + * if mirror has other sources only update selection >> > + */ >> > + ret = mirror_delete(pb, pm_ctx, &port_ovs_mirrors, false); >> > + } >> > + } >> > + } else { >> > + ret = mirror_delete(pb, pm_ctx, &port_ovs_mirrors, true); >> > + } >> > + >> > + struct shash_node *ovs_mirror_node, *ovs_mirror_next; >> > + SHASH_FOR_EACH_SAFE (ovs_mirror_node, ovs_mirror_next, >> > + &port_ovs_mirrors) { >> > + shash_delete(&port_ovs_mirrors, ovs_mirror_node); >> > + } >> > + shash_destroy(&port_ovs_mirrors); >> > + >> > + return ret; >> > +} >> > + >> > +bool >> > +ovn_port_mirror_handle_update(struct port_mirror_ctx *pm_ctx) >> > +{ >> > + const struct sbrec_mirror *mirror = NULL; >> > + struct ovsrec_mirror *ovs_mirror = NULL; >> > + >> > + SBREC_MIRROR_TABLE_FOR_EACH_TRACKED (mirror, pm_ctx->sb_mirror_table) >> > { >> > + /* For each tracked mirror entry check if OVS entry is there*/ >> > + ovs_mirror = shash_find_data(pm_ctx->ovs_mirrors, mirror->name); >> > + if (ovs_mirror) { >> > + if (sbrec_mirror_is_deleted(mirror)) { >> > + /* Need to delete the mirror in OVS */ >> > + VLOG_INFO("Delete mirror and remove port"); >> > + ovsrec_bridge_update_ports_delvalue(pm_ctx->br_int, >> > + >> > ovs_mirror->output_port); >> > + ovsrec_bridge_update_mirrors_delvalue(pm_ctx->br_int, >> > + ovs_mirror); >> > + ovsrec_port_delete(ovs_mirror->output_port); >> > + ovsrec_mirror_delete(ovs_mirror); >> > + } else { >> > + mirror_update(mirror, ovs_mirror); >> > + } >> > + } >> > + } >> > + >> > + return true; >> > +} >> > + >> > +void >> > +ovn_port_mirror_destroy(struct shash *ovs_mirrors) >> > +{ >> > + struct shash_node *ovs_mirror_node, *ovs_mirror_next; >> > + SHASH_FOR_EACH_SAFE (ovs_mirror_node, ovs_mirror_next, >> > + ovs_mirrors) { >> > + shash_delete(ovs_mirrors, ovs_mirror_node); >> > + } >> > + shash_destroy(ovs_mirrors); >> > +} >> > diff --git a/controller/mirror.h b/controller/mirror.h >> > new file mode 100644 >> > index 000000000..85b964f55 >> > --- /dev/null >> > +++ b/controller/mirror.h >> > @@ -0,0 +1,53 @@ >> > +/* Copyright (c) 2022 Red Hat, Inc. >> > + * >> > + * Licensed under the Apache License, Version 2.0 (the "License"); >> > + * you may not use this file except in compliance with the License. >> > + * You may obtain a copy of the License at: >> > + * >> > + * http://www.apache.org/licenses/LICENSE-2.0 >> > + * >> > + * Unless required by applicable law or agreed to in writing, software >> > + * distributed under the License is distributed on an "AS IS" BASIS, >> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >> > implied. >> > + * See the License for the specific language governing permissions and >> > + * limitations under the License. >> > + */ >> > + >> > +#ifndef OVN_MIRROR_H >> > +#define OVN_MIRROR_H 1 >> > + >> > +struct ovsdb_idl_txn; >> > +struct ovsrec_port_table; >> > +struct ovsrec_bridge; >> > +struct ovsrec_bridge_table; >> > +struct ovsrec_open_vswitch_table; >> > +struct sbrec_chassis; >> > +struct ovsrec_interface_table; >> > +struct ovsrec_mirror_table; >> > +struct sbrec_mirror_table; >> > +struct sbrec_port_binding_table; >> > + >> > +struct port_mirror_ctx { >> > + struct shash *ovs_mirrors; >> > + struct ovsdb_idl_txn *ovs_idl_txn; >> > + const struct ovsrec_port_table *port_table; >> > + const struct ovsrec_bridge *br_int; >> > + const struct sbrec_chassis *chassis_rec; >> > + const struct ovsrec_bridge_table *bridge_table; >> > + const struct ovsrec_open_vswitch_table *ovs_table; >> > + const struct ovsrec_interface_table *iface_table; >> > + const struct ovsrec_mirror_table *mirror_table; >> > + const struct sbrec_mirror_table *sb_mirror_table; >> > + const struct sbrec_port_binding_table *port_binding_table; >> > + struct shash *local_bindings; >> > +}; >> > + >> > +void mirror_register_ovs_idl(struct ovsdb_idl *); >> > +void ovn_port_mirror_init(struct shash *); >> > +void ovn_port_mirror_destroy(struct shash *); >> > +void ovn_port_mirror_run(struct port_mirror_ctx *pm_ctx); >> > +bool ovn_port_mirror_handle_lport(const struct sbrec_port_binding *pb, >> > + bool removed, >> > + struct port_mirror_ctx *pm_ctx); >> > +bool ovn_port_mirror_handle_update(struct port_mirror_ctx *pm_ctx); >> > +#endif >> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c >> > index bc2c5ab63..43e71ea3e 100644 >> > --- a/controller/ovn-controller.c >> > +++ b/controller/ovn-controller.c >> > @@ -77,6 +77,7 @@ >> > #include "stopwatch.h" >> > #include "lib/inc-proc-eng.h" >> > #include "hmapx.h" >> > +#include "mirror.h" >> > >> > VLOG_DEFINE_THIS_MODULE(main); >> > >> > @@ -969,6 +970,7 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl) >> > ovsdb_idl_track_add_column(ovs_idl, &ovsrec_port_col_name); >> > ovsdb_idl_track_add_column(ovs_idl, &ovsrec_port_col_interfaces); >> > ovsdb_idl_track_add_column(ovs_idl, &ovsrec_port_col_external_ids); >> > + mirror_register_ovs_idl(ovs_idl); >> > } >> > >> > #define SB_NODES \ >> > @@ -989,6 +991,7 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl) >> > SB_NODE(load_balancer, "load_balancer") \ >> > SB_NODE(fdb, "fdb") \ >> > SB_NODE(meter, "meter") \ >> > + SB_NODE(mirror, "mirror") \ >> > SB_NODE(static_mac_binding, "static_mac_binding") >> > >> > enum sb_engine_node { >> > @@ -1006,7 +1009,8 @@ enum sb_engine_node { >> > OVS_NODE(bridge, "bridge") \ >> > OVS_NODE(port, "port") \ >> > OVS_NODE(interface, "interface") \ >> > - OVS_NODE(qos, "qos") >> > + OVS_NODE(qos, "qos") \ >> > + OVS_NODE(mirror, "mirror") >> > >> > enum ovs_engine_node { >> > #define OVS_NODE(NAME, NAME_STR) OVS_##NAME, >> > @@ -2386,6 +2390,203 @@ load_balancers_by_dp_cleanup(struct hmap *lbs) >> > free(lbs); >> > } >> > >> > +/* Mirror Engine */ >> > +struct ed_type_port_mirror { >> > + struct shash ovs_mirrors; >> > +}; >> > + >> > +static void * >> > +en_port_mirror_init(struct engine_node *node OVS_UNUSED, >> > + struct engine_arg *arg OVS_UNUSED) >> > +{ >> > + struct ed_type_port_mirror *port_mirror = xzalloc(sizeof >> > *port_mirror); >> > + ovn_port_mirror_init(&port_mirror->ovs_mirrors); >> > + return port_mirror; >> > +} >> > + >> > +static void >> > +en_port_mirror_cleanup(void *data) >> > +{ >> > + struct ed_type_port_mirror *port_mirror = data; >> > + ovn_port_mirror_destroy(&port_mirror->ovs_mirrors); >> > +} >> > + >> > +static void >> > +init_port_mirror_ctx(struct engine_node *node, >> > + struct ed_type_runtime_data *rt_data, >> > + struct ed_type_port_mirror *port_mirror_data, >> > + struct port_mirror_ctx *pm_ctx) >> > +{ >> > + struct ovsrec_open_vswitch_table *ovs_table = >> > + (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET( >> > + engine_get_input("OVS_open_vswitch", node)); >> > + struct ovsrec_bridge_table *bridge_table = >> > + (struct ovsrec_bridge_table *)EN_OVSDB_GET( >> > + engine_get_input("OVS_bridge", node)); >> > + const char *chassis_id = get_ovs_chassis_id(ovs_table); >> > + const struct ovsrec_bridge *br_int = get_br_int(bridge_table, >> > ovs_table); >> > + >> > + ovs_assert(br_int && chassis_id); >> > + const struct sbrec_chassis *chassis = NULL; >> > + struct ovsdb_idl_index *sbrec_chassis_by_name = >> > + engine_ovsdb_node_get_index( >> > + engine_get_input("SB_chassis", node), >> > + "name"); >> > + >> > + if (chassis_id) { >> > + chassis = chassis_lookup_by_name(sbrec_chassis_by_name, >> > chassis_id); >> > + } >> > + ovs_assert(chassis); >> > + >> > + struct ovsrec_port_table *port_table = >> > + (struct ovsrec_port_table *)EN_OVSDB_GET( >> > + engine_get_input("OVS_port", node)); >> > + >> > + struct ed_type_ovs_interface_shadow *iface_shadow = >> > + engine_get_input_data("ovs_interface_shadow", node); >> > + >> > + struct ovsrec_mirror_table *mirror_table = >> > + (struct ovsrec_mirror_table *)EN_OVSDB_GET( >> > + engine_get_input("OVS_mirror", node)); >> > + >> > + struct sbrec_port_binding_table *pb_table = >> > + (struct sbrec_port_binding_table *)EN_OVSDB_GET( >> > + engine_get_input("SB_port_binding", node)); >> > + >> > + struct sbrec_mirror_table *sb_mirror_table = >> > + (struct sbrec_mirror_table *)EN_OVSDB_GET( >> > + engine_get_input("SB_mirror", node)); >> > + >> > + struct shash_node *ovs_mirror_node, *ovs_mirror_next; >> > + SHASH_FOR_EACH_SAFE (ovs_mirror_node, ovs_mirror_next, >> > + &port_mirror_data->ovs_mirrors) { >> > + shash_delete(&port_mirror_data->ovs_mirrors, ovs_mirror_node); >> > + } >> > + >> > + const struct ovsrec_mirror *ovsmirror = NULL; >> > + OVSREC_MIRROR_TABLE_FOR_EACH (ovsmirror, mirror_table) { >> > + shash_add(&port_mirror_data->ovs_mirrors, ovsmirror->name, >> > ovsmirror); >> > + } >> > + >> > + pm_ctx->ovs_idl_txn = engine_get_context()->ovs_idl_txn; >> > + pm_ctx->port_table = port_table; >> > + pm_ctx->iface_table = iface_shadow->iface_table; >> > + pm_ctx->mirror_table = mirror_table; >> > + pm_ctx->port_binding_table = pb_table; >> > + pm_ctx->sb_mirror_table = sb_mirror_table; >> > + pm_ctx->br_int = br_int; >> > + pm_ctx->chassis_rec = chassis; >> > + pm_ctx->bridge_table = bridge_table; >> > + pm_ctx->ovs_table = ovs_table; >> > + pm_ctx->ovs_mirrors = &port_mirror_data->ovs_mirrors; >> > + pm_ctx->local_bindings = &rt_data->lbinding_data.bindings; >> > +} >> > + >> > +static void >> > +en_port_mirror_run(struct engine_node *node, void *data) >> > +{ >> > + struct port_mirror_ctx pm_ctx; >> > + struct ed_type_port_mirror *port_mirror_data = data; >> > + struct ed_type_runtime_data *rt_data = >> > + engine_get_input_data("runtime_data", node); >> > + >> > + init_port_mirror_ctx(node, rt_data, port_mirror_data, &pm_ctx); >> > + >> > + ovn_port_mirror_run(&pm_ctx); >> > + engine_set_node_state(node, EN_UPDATED); >> > +} >> > + >> > +static bool >> > +port_mirror_runtime_data_handler(struct engine_node *node, void *data) >> > +{ >> > + struct ed_type_runtime_data *rt_data = >> > + engine_get_input_data("runtime_data", node); >> > + >> > + /* There is no tracked data. Fall back to full recompute of >> > port_mirror */ >> > + if (!rt_data->tracked) { >> > + return false; >> > + } >> > + >> > + struct hmap *tracked_dp_bindings = &rt_data->tracked_dp_bindings; >> > + if (hmap_is_empty(tracked_dp_bindings)) { >> > + return true; >> > + } >> > + >> > + struct port_mirror_ctx pm_ctx; >> > + struct ed_type_port_mirror *port_mirror_data = data; >> > + init_port_mirror_ctx(node, rt_data, port_mirror_data, &pm_ctx); >> > + >> > + struct tracked_datapath *tdp; >> > + HMAP_FOR_EACH (tdp, node, tracked_dp_bindings) { >> > + if (tdp->tracked_type != TRACKED_RESOURCE_UPDATED) { >> > + /* Fall back to full recompute when a local datapath >> > + * is added or deleted. */ >> > + return false; >> > + } >> > + >> > + struct shash_node *shash_node; >> > + SHASH_FOR_EACH (shash_node, &tdp->lports) { >> > + struct tracked_lport *lport = shash_node->data; >> > + bool removed = >> > + lport->tracked_type == TRACKED_RESOURCE_REMOVED ? true: >> > false; >> > + if (!ovn_port_mirror_handle_lport(lport->pb, removed, >> > &pm_ctx)) { >> > + return false; >> > + } >> > + } >> > + } >> > + >> > + engine_set_node_state(node, EN_UPDATED); >> > + return true; >> > +} >> > + >> > +static bool >> > +port_mirror_port_binding_handler(struct engine_node *node, void *data) >> > +{ >> > + struct port_mirror_ctx pm_ctx; >> > + struct ed_type_port_mirror *port_mirror_data = data; >> > + struct ed_type_runtime_data *rt_data = >> > + engine_get_input_data("runtime_data", node); >> > + >> > + init_port_mirror_ctx(node, rt_data, port_mirror_data, &pm_ctx); >> > + >> > + /* handle port binding updates (i.,e when the mirror column >> > + * of port_binding is updated) >> > + */ >> > + const struct sbrec_port_binding *pb; >> > + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, >> > + pm_ctx.port_binding_table) >> > { >> > + bool removed = sbrec_port_binding_is_deleted(pb); >> > + if (!ovn_port_mirror_handle_lport(pb, removed, &pm_ctx)) { >> > + return false; >> > + } >> > + } >> > + >> > + engine_set_node_state(node, EN_UPDATED); >> > + return true; >> > + >> > +} >> > + >> > +static bool >> > +port_mirror_sb_mirror_handler(struct engine_node *node, void *data) >> > +{ >> > + struct port_mirror_ctx pm_ctx; >> > + struct ed_type_port_mirror *port_mirror_data = data; >> > + struct ed_type_runtime_data *rt_data = >> > + engine_get_input_data("runtime_data", node); >> > + >> > + init_port_mirror_ctx(node, rt_data, port_mirror_data, &pm_ctx); >> > + >> > + /* handle sb mirror updates >> > + */ >> > + if (!ovn_port_mirror_handle_update(&pm_ctx)) { >> > + return false; >> > + } >> > + >> > + engine_set_node_state(node, EN_UPDATED); >> > + return true; >> > + >> > +} >> > + >> > /* Engine node which is used to handle the Non VIF data like >> > * - OVS patch ports >> > * - Tunnel ports and the related chassis information. >> > @@ -3645,6 +3846,7 @@ main(int argc, char *argv[]) >> > ENGINE_NODE_WITH_CLEAR_TRACK_DATA(addr_sets, "addr_sets"); >> > ENGINE_NODE_WITH_CLEAR_TRACK_DATA(port_groups, "port_groups"); >> > ENGINE_NODE(northd_options, "northd_options"); >> > + ENGINE_NODE(port_mirror, "port_mirror"); >> > >> > #define SB_NODE(NAME, NAME_STR) ENGINE_NODE_SB(NAME, NAME_STR); >> > SB_NODES >> > @@ -3801,6 +4003,22 @@ main(int argc, char *argv[]) >> > engine_add_input(&en_flow_output, &en_pflow_output, >> > flow_output_pflow_output_handler); >> > >> > + engine_add_input(&en_port_mirror, &en_ovs_open_vswitch, NULL); >> > + engine_add_input(&en_port_mirror, &en_ovs_bridge, NULL); >> > + engine_add_input(&en_port_mirror, &en_ovs_mirror, NULL); >> > + engine_add_input(&en_port_mirror, &en_sb_chassis, NULL); >> > + engine_add_input(&en_port_mirror, &en_ovs_port, engine_noop_handler); >> > + engine_add_input(&en_port_mirror, &en_ovs_interface_shadow, >> > + engine_noop_handler); >> > + engine_add_input(&en_flow_output, &en_port_mirror, >> > + engine_noop_handler); >> > + engine_add_input(&en_port_mirror, &en_runtime_data, >> > + port_mirror_runtime_data_handler); >> > + engine_add_input(&en_port_mirror, &en_sb_mirror, >> > + port_mirror_sb_mirror_handler); >> > + engine_add_input(&en_port_mirror, &en_sb_port_binding, >> > + port_mirror_port_binding_handler); >> > + >> > struct engine_arg engine_arg = { >> > .sb_idl = ovnsb_idl_loop.idl, >> > .ovs_idl = ovs_idl_loop.idl, >> > @@ -4070,34 +4288,36 @@ main(int argc, char *argv[]) >> > >> > stopwatch_start(CONTROLLER_LOOP_STOPWATCH_NAME, >> > time_msec()); >> > - if (ovnsb_idl_txn) { >> > - if (ofctrl_has_backlog()) { >> > - /* When there are in-flight messages pending >> > to >> > - * ovs-vswitchd, we should hold on >> > recomputing so >> > - * that the previous flow installations won't >> > be >> > - * delayed. However, we still want to try if >> > - * recompute is not needed and we can quickly >> > - * incrementally process the new changes, to >> > avoid >> > - * unnecessarily forced recomputes later on. >> > This >> > - * is because the OVSDB change tracker cannot >> > - * preserve tracked changes across >> > iterations. If >> > - * change tracking is improved, we can simply >> > skip >> > - * this round of engine_run and continue >> > processing >> > - * acculated changes incrementally later when >> > - * ofctrl_has_backlog() returns false. */ >> > - engine_run(false); >> > - } else { >> > - engine_run(true); >> > - } >> > - } else { >> > - /* Even if there's no SB DB transaction available, >> > + >> > + bool allow_engine_recompute = true; >> > + >> > + if (!ovnsb_idl_txn || !ovs_idl_txn || >> > + >> > ofctrl_has_backlog()) { >> > + /* When there are in-flight messages pending to >> > + * ovs-vswitchd, we should hold on recomputing so >> > + * that the previous flow installations won't be >> > + * delayed. However, we still want to try if >> > + * recompute is not needed and we can quickly >> > + * incrementally process the new changes, to avoid >> > + * unnecessarily forced recomputes later on. This >> > + * is because the OVSDB change tracker cannot >> > + * preserve tracked changes across iterations. If >> > + * change tracking is improved, we can simply skip >> > + * this round of engine_run and continue >> > processing >> > + * acculated changes incrementally later when >> > + * ofctrl_has_backlog() returns false. */ >> > + >> > + /* Even if there's no SB/OVS DB transaction >> > available, >> > * try to run the engine so that we can handle any >> > * incremental changes that don't require a >> > recompute. >> > * If a recompute is required, the engine will >> > abort, >> > * triggerring a full run in the next iteration. >> > */ >> > - engine_run(false); >> > + allow_engine_recompute = false; >> > } >> > + >> > + engine_run(allow_engine_recompute); >> > + >> > stopwatch_stop(CONTROLLER_LOOP_STOPWATCH_NAME, >> > time_msec()); >> > if (engine_has_updated()) { >> > diff --git a/northd/en-northd.c b/northd/en-northd.c >> > index 7fe83db64..608220b1f 100644 >> > --- a/northd/en-northd.c >> > +++ b/northd/en-northd.c >> > @@ -80,6 +80,8 @@ void en_northd_run(struct engine_node *node, void *data) >> > EN_OVSDB_GET(engine_get_input("NB_acl", node)); >> > input_data.nbrec_static_mac_binding_table = >> > EN_OVSDB_GET(engine_get_input("NB_static_mac_binding", node)); >> > + input_data.nbrec_mirror_table = >> > + EN_OVSDB_GET(engine_get_input("NB_mirror", node)); >> > >> > input_data.sbrec_sb_global_table = >> > EN_OVSDB_GET(engine_get_input("SB_sb_global", node)); >> > @@ -113,6 +115,8 @@ void en_northd_run(struct engine_node *node, void >> > *data) >> > EN_OVSDB_GET(engine_get_input("SB_chassis_private", node)); >> > input_data.sbrec_static_mac_binding_table = >> > EN_OVSDB_GET(engine_get_input("SB_static_mac_binding", node)); >> > + input_data.sbrec_mirror_table = >> > + EN_OVSDB_GET(engine_get_input("SB_mirror", node)); >> > >> > northd_run(&input_data, data, >> > eng_ctx->ovnnb_idl_txn, >> > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c >> > index 54e0ad3b0..ac27a730e 100644 >> > --- a/northd/inc-proc-northd.c >> > +++ b/northd/inc-proc-northd.c >> > @@ -50,6 +50,7 @@ VLOG_DEFINE_THIS_MODULE(inc_proc_northd); >> > NB_NODE(acl, "acl") \ >> > NB_NODE(logical_router, "logical_router") \ >> > NB_NODE(qos, "qos") \ >> > + NB_NODE(mirror, "mirror") \ >> > NB_NODE(meter, "meter") \ >> > NB_NODE(meter_band, "meter_band") \ >> > NB_NODE(logical_router_port, "logical_router_port") \ >> > @@ -92,6 +93,7 @@ VLOG_DEFINE_THIS_MODULE(inc_proc_northd); >> > SB_NODE(logical_flow, "logical_flow") \ >> > SB_NODE(logical_dp_group, "logical_DP_group") \ >> > SB_NODE(multicast_group, "multicast_group") \ >> > + SB_NODE(mirror, "mirror") \ >> > SB_NODE(meter, "meter") \ >> > SB_NODE(meter_band, "meter_band") \ >> > SB_NODE(datapath_binding, "datapath_binding") \ >> > @@ -172,6 +174,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, >> > engine_add_input(&en_northd, &en_nb_acl, NULL); >> > engine_add_input(&en_northd, &en_nb_logical_router, NULL); >> > engine_add_input(&en_northd, &en_nb_qos, NULL); >> > + engine_add_input(&en_northd, &en_nb_mirror, NULL); >> > engine_add_input(&en_northd, &en_nb_meter, NULL); >> > engine_add_input(&en_northd, &en_nb_meter_band, NULL); >> > engine_add_input(&en_northd, &en_nb_logical_router_port, NULL); >> > @@ -194,6 +197,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, >> > engine_add_input(&en_northd, &en_sb_address_set, NULL); >> > engine_add_input(&en_northd, &en_sb_port_group, NULL); >> > engine_add_input(&en_northd, &en_sb_logical_dp_group, NULL); >> > + engine_add_input(&en_northd, &en_sb_mirror, NULL); >> > engine_add_input(&en_northd, &en_sb_meter, NULL); >> > engine_add_input(&en_northd, &en_sb_meter_band, NULL); >> > engine_add_input(&en_northd, &en_sb_datapath_binding, NULL); >> > diff --git a/northd/northd.c b/northd/northd.c >> > index 84440a47f..5880ddc69 100644 >> > --- a/northd/northd.c >> > +++ b/northd/northd.c >> > @@ -3220,6 +3220,89 @@ ovn_port_update_sbrec_chassis( >> > free(requested_chassis_sb); >> > } >> > >> > +static void >> > +do_sb_mirror_addition(struct northd_input *input_data, >> > + const struct ovn_port *op) >> > +{ >> > + for (size_t i = 0; i < op->nbsp->n_mirror_rules; i++) { >> > + const struct sbrec_mirror *sb_mirror; >> > + SBREC_MIRROR_TABLE_FOR_EACH (sb_mirror, >> > + input_data->sbrec_mirror_table) { >> > + if (!strcmp(sb_mirror->name, >> > + op->nbsp->mirror_rules[i]->name)) { >> > + /* Add the value to SB */ >> > + sbrec_port_binding_update_mirror_rules_addvalue(op->sb, >> > + >> > sb_mirror); >> > + } >> > + } >> > + } >> > +} >> > + >> > +static void >> > +sbrec_port_binding_update_mirror_rules(struct northd_input *input_data, >> > + const struct ovn_port *op) >> > +{ >> > + size_t i = 0; >> > + if (op->sb->n_mirror_rules > op->nbsp->n_mirror_rules) { >> > + /* Needs deletion in SB */ >> > + struct shash nb_mirror_rules = >> > SHASH_INITIALIZER(&nb_mirror_rules); >> > + for (i = 0; i < op->nbsp->n_mirror_rules; i++) { >> > + shash_add(&nb_mirror_rules, >> > + op->nbsp->mirror_rules[i]->name, >> > + op->nbsp->mirror_rules[i]); >> > + } >> > + >> > + for (i = 0; i < op->sb->n_mirror_rules; i++) { >> > + if (!shash_find(&nb_mirror_rules, >> > + op->sb->mirror_rules[i]->name)) { >> > + >> > sbrec_port_binding_update_mirror_rules_delvalue(op->sb, >> > + op->sb->mirror_rules[i]); >> > + } >> > + } >> >> There should also be the case of "present in nb but not in sb" here. >> Remember, a single transaction may include any number of modifications >> to the list. Both deletions and additions. So I don't think you should >> even have separate branches (<, >, ==), the same code may handle it >> consistently. (I think the current == branch could handle it already.) > > > Yes, "present in nb but not in sb" is already there. See the else if block > below > with " } else if (op->sb->n_mirror_rules < op->nbsp->n_mirror_rules) {" > > Yes, a single transaction swapping the mirrors which you told is handled in > == case. > > Further, I feel it might not be too efficient to keep it common. > Because with checking ( > or <) we are doing high level and taking action > In case it hits the == we do more work to find out more if something needs to > be done or not. > So, I feel let's keep these 3 separate. >> >> >> > + >> > + struct shash_node *node, *next; >> > + SHASH_FOR_EACH_SAFE (node, next, &nb_mirror_rules) { >> > + shash_delete(&nb_mirror_rules, node); >> > + } >> > + shash_destroy(&nb_mirror_rules); >> > + >> > + } else if (op->sb->n_mirror_rules < op->nbsp->n_mirror_rules) { >> > + /* Needs addition in SB */ >> > + do_sb_mirror_addition(input_data, op); >> > + } else if (op->sb->n_mirror_rules == op->nbsp->n_mirror_rules) { >> > + /* >> > + * Check if its the same mirrors on both SB and NB DBs >> > + * If not update accordingly. >> > + */ >> > + bool needs_sb_addition = false; >> > + struct shash nb_mirror_rules = >> > SHASH_INITIALIZER(&nb_mirror_rules); >> > + for (i = 0; i < op->nbsp->n_mirror_rules; i++) { >> > + shash_add(&nb_mirror_rules, >> > + op->nbsp->mirror_rules[i]->name, >> > + op->nbsp->mirror_rules[i]); >> > + } >> > + >> > + for (i = 0; i < op->sb->n_mirror_rules; i++) { >> > + if (!shash_find(&nb_mirror_rules, >> > + op->sb->mirror_rules[i]->name)) { >> > + >> > sbrec_port_binding_update_mirror_rules_delvalue(op->sb, >> > + op->sb->mirror_rules[i]); >> > + needs_sb_addition = true; >> > + } >> > + } >> > + >> > + struct shash_node *node, *next; >> > + SHASH_FOR_EACH_SAFE (node, next, &nb_mirror_rules) { >> > + shash_delete(&nb_mirror_rules, node); >> > + } >> > + shash_destroy(&nb_mirror_rules); >> > + >> > + if (needs_sb_addition) { >> > + do_sb_mirror_addition(input_data, op); >> > + } >> > + } >> > +} >> > + >> > static void >> > ovn_port_update_sbrec(struct northd_input *input_data, >> > struct ovsdb_idl_txn *ovnsb_txn, >> > @@ -3579,6 +3662,15 @@ ovn_port_update_sbrec(struct northd_input >> > *input_data, >> > } >> > sbrec_port_binding_set_external_ids(op->sb, &ids); >> > smap_destroy(&ids); >> > + >> > + if (!op->nbsp->n_mirror_rules) { >> > + /* Nothing is set. Clear mirror_rules from pb. */ >> > + sbrec_port_binding_set_mirror_rules(op->sb, NULL, 0); >> > + } else { >> > + /* Check if SB DB update needed */ >> > + sbrec_port_binding_update_mirror_rules(input_data, op); >> > + } >> > + >> > } >> > if (op->tunnel_key != op->sb->tunnel_key) { >> > sbrec_port_binding_set_tunnel_key(op->sb, op->tunnel_key); >> > @@ -14956,6 +15048,85 @@ sync_meters(struct northd_input *input_data, >> > shash_destroy(&sb_meters); >> > } >> > >> > +static bool >> > +mirror_needs_update(const struct nbrec_mirror *nb_mirror, >> > + const struct sbrec_mirror *sb_mirror) >> > +{ >> > + >> > + if (nb_mirror->index != sb_mirror->index) { >> > + return true; >> > + } else if (strcmp(nb_mirror->sink, sb_mirror->sink)) { >> > + return true; >> > + } else if (strcmp(nb_mirror->type, sb_mirror->type)) { >> > + return true; >> > + } else if (strcmp(nb_mirror->filter, sb_mirror->filter)) { >> > + return true; >> > + } >> > + >> > + return false; >> > +} >> > + >> > +static void >> > +sync_mirrors_iterate_nb_mirror(struct ovsdb_idl_txn *ovnsb_txn, >> > + const char *mirror_name, >> > + const struct nbrec_mirror *nb_mirror, >> > + struct shash *sb_mirrors, >> > + struct sset *used_sb_mirrors) >> > +{ >> > + const struct sbrec_mirror *sb_mirror; >> > + bool new_sb_mirror = false; >> > + >> > + sb_mirror = shash_find_data(sb_mirrors, mirror_name); >> > + if (!sb_mirror) { >> > + sb_mirror = sbrec_mirror_insert(ovnsb_txn); >> > + sbrec_mirror_set_name(sb_mirror, mirror_name); >> > + shash_add(sb_mirrors, sb_mirror->name, sb_mirror); >> > + new_sb_mirror = true; >> > + } >> > + sset_add(used_sb_mirrors, mirror_name); >> > + >> > + if ((new_sb_mirror) || mirror_needs_update(nb_mirror, sb_mirror)) { >> > + sbrec_mirror_set_filter(sb_mirror,nb_mirror->filter); >> > + sbrec_mirror_set_index(sb_mirror, nb_mirror->index); >> > + sbrec_mirror_set_sink(sb_mirror, nb_mirror->sink); >> > + sbrec_mirror_set_type(sb_mirror, nb_mirror->type); >> > + } >> > +} >> > + >> > +static void >> > +sync_mirrors(struct northd_input *input_data, >> > + struct ovsdb_idl_txn *ovnsb_txn) >> > +{ >> > + struct shash sb_mirrors = SHASH_INITIALIZER(&sb_mirrors); >> > + struct sset used_sb_mirrors = SSET_INITIALIZER(&used_sb_mirrors); >> > + >> > + const struct sbrec_mirror *sb_mirror; >> > + SBREC_MIRROR_TABLE_FOR_EACH (sb_mirror, >> > input_data->sbrec_mirror_table) { >> > + shash_add(&sb_mirrors, sb_mirror->name, sb_mirror); >> > + } >> > + >> > + const struct nbrec_mirror *nb_mirror; >> > + NBREC_MIRROR_TABLE_FOR_EACH (nb_mirror, >> > input_data->nbrec_mirror_table) { >> > + sync_mirrors_iterate_nb_mirror(ovnsb_txn, nb_mirror->name, >> > nb_mirror, >> > + &sb_mirrors, &used_sb_mirrors); >> > + } >> > + >> > + const char *used_mirror; >> > + const char *used_mirror_next; >> > + SSET_FOR_EACH_SAFE (used_mirror, used_mirror_next, &used_sb_mirrors) { >> > + shash_find_and_delete(&sb_mirrors, used_mirror); >> > + sset_delete(&used_sb_mirrors, SSET_NODE_FROM_NAME(used_mirror)); >> > + } >> > + sset_destroy(&used_sb_mirrors); >> > + >> > + struct shash_node *node, *next; >> > + SHASH_FOR_EACH_SAFE (node, next, &sb_mirrors) { >> > + sbrec_mirror_delete(node->data); >> > + shash_delete(&sb_mirrors, node); >> > + } >> > + shash_destroy(&sb_mirrors); >> > +} >> > + >> > /* >> > * struct 'dns_info' is used to sync the DNS records between OVN >> > Northbound db >> > * and Southbound db. >> > @@ -15583,6 +15754,7 @@ ovnnb_db_run(struct northd_input *input_data, >> > sync_address_sets(input_data, ovnsb_txn, &data->datapaths); >> > sync_port_groups(input_data, ovnsb_txn, &data->port_groups); >> > sync_meters(input_data, ovnsb_txn, &data->meter_groups); >> > + sync_mirrors(input_data, ovnsb_txn); >> > sync_dns_entries(input_data, ovnsb_txn, &data->datapaths); >> > cleanup_stale_fdb_entries(input_data, &data->datapaths); >> > stopwatch_stop(CLEAR_LFLOWS_CTX_STOPWATCH_NAME, time_msec()); >> > diff --git a/northd/northd.h b/northd/northd.h >> > index aa9a3ae6e..f5ed45e20 100644 >> > --- a/northd/northd.h >> > +++ b/northd/northd.h >> > @@ -36,6 +36,7 @@ struct northd_input { >> > const struct nbrec_acl_table *nbrec_acl_table; >> > const struct nbrec_static_mac_binding_table >> > *nbrec_static_mac_binding_table; >> > + const struct nbrec_mirror_table *nbrec_mirror_table; >> > >> > /* Southbound table references */ >> > const struct sbrec_sb_global_table *sbrec_sb_global_table; >> > @@ -55,6 +56,7 @@ struct northd_input { >> > const struct sbrec_chassis_private_table *sbrec_chassis_private_table; >> > const struct sbrec_static_mac_binding_table >> > *sbrec_static_mac_binding_table; >> > + const struct sbrec_mirror_table *sbrec_mirror_table; >> > >> > /* Indexes */ >> > struct ovsdb_idl_index *sbrec_chassis_by_name; >> > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema >> > index 174364c8b..01de55222 100644 >> > --- a/ovn-nb.ovsschema >> > +++ b/ovn-nb.ovsschema >> > @@ -1,7 +1,7 @@ >> > { >> > "name": "OVN_Northbound", >> > - "version": "6.3.0", >> > - "cksum": "4042813038 31869", >> > + "version": "6.4.0", >> > + "cksum": "589874483 33352", >> > "tables": { >> > "NB_Global": { >> > "columns": { >> > @@ -132,6 +132,11 @@ >> > "refType": "weak"}, >> > "min": 0, >> > "max": 1}}, >> > + "mirror_rules": {"type": {"key": {"type": "uuid", >> > + "refTable": "Mirror", >> > + "refType": "weak"}, >> > + "min": 0, >> > + "max": "unlimited"}}, >> > "ha_chassis_group": { >> > "type": {"key": {"type": "uuid", >> > "refTable": "HA_Chassis_Group", >> > @@ -301,6 +306,28 @@ >> > "type": {"key": "string", "value": "string", >> > "min": 0, "max": "unlimited"}}}, >> > "isRoot": false}, >> > + "Mirror": { >> > + "columns": { >> > + "name": {"type": "string"}, >> > + "filter": {"type": {"key": {"type": "string", >> > + "enum": ["set", ["from-lport", >> > + "to-lport", >> > + "both"]]}}}, >> > + "sink":{"type": "string"}, >> > + "type": {"type": {"key": {"type": "string", >> > + "enum": ["set", ["gre", >> > + >> > "erspan"]]}}}, >> > + "index": {"type": "integer"}, >> > + "src": {"type": {"key": {"type": "uuid", >> > + "refTable": >> > "Logical_Switch_Port", >> > + "refType": "weak"}, >> > + "min": 0, >> > + "max": "unlimited"}}, >> > + "external_ids": { >> > + "type": {"key": "string", "value": "string", >> > + "min": 0, "max": "unlimited"}}}, >> > + "indexes": [["name"]], >> > + "isRoot": true}, >> > "Meter": { >> > "columns": { >> > "name": {"type": "string"}, >> > diff --git a/ovn-nb.xml b/ovn-nb.xml >> > index f41e9d7c0..8e42c6bec 100644 >> > --- a/ovn-nb.xml >> > +++ b/ovn-nb.xml >> > @@ -1554,6 +1554,11 @@ >> > </column> >> > </group> >> > >> > + <column name="mirror_rules"> >> > + Mirror rules that apply to logical switch port which is the >> > source. >> > + Please see the <ref table="Mirror"/> table. >> > + </column> >> > + >> > <column name="ha_chassis_group"> >> > References a row in the OVN Northbound database's >> > <ref table="HA_Chassis_Group" db="OVN_Northbound"/> table. >> > @@ -2491,6 +2496,64 @@ >> > </column> >> > </table> >> > >> > + <table name="Mirror" title="Mirror Entry"> >> > + <p> >> > + Each row in this table represents one Mirror that can be used for >> > + port mirroring. These Mirrors are referenced by the >> > + <ref column="mirror_rules" table="Logical_Switch_Port"/> column in >> > + the <ref table="Logical_Switch_Port"/> table. >> > + </p> >> > + >> > + <column name="name"> >> > + <p> >> > + Represents the name of the mirror. >> > + </p> >> > + </column> >> > + >> > + <column name="filter"> >> > + <p> >> > + The value of this field represents selection criteria of the >> > mirror. >> > + Supported values for filter to-lport / from-lport / both >> > + to-lport - to mirror packets coming into logical port >> > + from-lport - to mirror packets going out of logical port >> > + both - to mirror packets coming into and going out of logical >> > port. >> > + </p> >> > + </column> >> > + >> > + <column name="sink"> >> > + <p> >> > + The value of this field represents the destination/sink of the >> > mirror. >> > + The value it takes is an IP address of the sink port. >> > + </p> >> > + </column> >> > + >> > + <column name="type"> >> > + <p> >> > + The value of this field represents the type of the tunnel used for >> > + sending the mirrored packets. Supported Tunnel types GRE and >> > ERSPAN >> >> I think we should document the exact type names to use here. ("gre" >> and "erspan") These are currently not clear from the text (one could >> think that CAPS names should used.) > > Sure, Will document it clearly. >> >> >> > + </p> >> > + </column> >> > + >> > + <column name="index"> >> > + <p> >> > + The value of this field represents the tunnel ID. Depending on the >> > + tunnel type configured, GRE key value if type GRE and erspan_idx >> > value >> > + if ERSPAN >> > + </p> >> > + </column> >> > + >> > + <column name="src"> >> > + <p> >> > + The value of this field represents a list of source ports for the >> > + mirror. Please see the <ref table="Logical_Switch_Port"/> table. >> > + </p> >> > + </column> >> > + >> > + <column name="external_ids"> >> > + See <em>External IDs</em> at the beginning of this document. >> > + </column> >> > + </table> >> > + >> > <table name="Meter" title="Meter entry"> >> > <p> >> > Each row in this table represents a meter that can be used for QoS >> > or >> > diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema >> > index 576ebbdeb..b83134416 100644 >> > --- a/ovn-sb.ovsschema >> > +++ b/ovn-sb.ovsschema >> > @@ -1,7 +1,7 @@ >> > { >> > "name": "OVN_Southbound", >> > - "version": "20.25.0", >> > - "cksum": "53184112 28845", >> > + "version": "20.26.0", >> > + "cksum": "2344151793 30004", >> > "tables": { >> > "SB_Global": { >> > "columns": { >> > @@ -142,6 +142,23 @@ >> > "indexes": [["datapath", "tunnel_key"], >> > ["datapath", "name"]], >> > "isRoot": true}, >> > + "Mirror": { >> > + "columns": { >> > + "name": {"type": "string"}, >> > + "filter": {"type": {"key": {"type": "string", >> > + "enum": ["set", >> > + ["from-lport", >> > + >> > "to-lport","both"]]}}}, >> > + "sink":{"type": "string"}, >> > + "type": {"type": {"key": {"type": "string", >> > + "enum": ["set", >> > + ["gre", >> > "erspan"]]}}}, >> > + "index": {"type": "integer"}, >> > + "external_ids": { >> > + "type": {"key": "string", "value": "string", >> > + "min": 0, "max": "unlimited"}}}, >> > + "indexes": [["name"]], >> > + "isRoot": true}, >> > "Meter": { >> > "columns": { >> > "name": {"type": "string"}, >> > @@ -230,6 +247,11 @@ >> > "refTable": "Encap", >> > "refType": "weak"}, >> > "min": 0, "max": "unlimited"}}, >> > + "mirror_rules": {"type": {"key": {"type": "uuid", >> > + "refTable": "Mirror", >> > + "refType": "weak"}, >> > + "min": 0, >> > + "max": "unlimited"}}, >> > "mac": {"type": {"key": "string", >> > "min": 0, >> > "max": "unlimited"}}, >> > diff --git a/ovn-sb.xml b/ovn-sb.xml >> > index 315d60853..05c0db6b4 100644 >> > --- a/ovn-sb.xml >> > +++ b/ovn-sb.xml >> > @@ -2742,6 +2742,51 @@ tcp.flags = RST; >> > </column> >> > </table> >> > >> > + <table name="Mirror" title="Mirror Entry"> >> > + <p> >> > + Each row in this table represents one Mirror that can be used for >> > + port mirroring. These Mirrors are referenced by the >> > + <ref column="mirror_rules" table="Port_Binding"/> column in >> > + the <ref table="Port_Binding"/> table. >> > + </p> >> > + >> > + <column name="name"> >> > + <p> >> > + Represents the name of the mirror. >> > + </p> >> > + </column> >> > + >> > + <column name="filter"> >> > + <p> >> > + The value of this field represents selection criteria of the >> > mirror. >> > + </p> >> > + </column> >> > + >> > + <column name="sink"> >> > + <p> >> > + The value of this field represents the destination/sink of the >> > mirror. >> > + </p> >> > + </column> >> > + >> > + <column name="type"> >> > + <p> >> > + The value of this field represents the type of the tunnel used for >> > + sending the mirrored packets >> > + </p> >> > + </column> >> > + >> > + <column name="index"> >> > + <p> >> > + The value of this field represents the key/idx depending on the >> > + tunnel type configured >> > + </p> >> > + </column> >> > + >> > + <column name="external_ids"> >> > + See <em>External IDs</em> at the beginning of this document. >> > + </column> >> > + </table> >> > + >> > <table name="Meter" title="Meter entry"> >> > <p> >> > Each row in this table represents a meter that can be used for QoS >> > or >> > @@ -3244,6 +3289,11 @@ tcp.flags = RST; >> > </column> >> > </group> >> > >> > + <column name="mirror_rules"> >> > + Mirror rules that apply to the port binding. >> > + Please see the <ref table="Mirror"/> table. >> > + </column> >> > + >> > <group title="Patch Options"> >> > <p> >> > These options apply to logical ports with <ref column="type"/> of >> > diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at >> > index 6a5d52a5d..a9c0853f5 100644 >> > --- a/tests/ovn-nbctl.at >> > +++ b/tests/ovn-nbctl.at >> > @@ -435,6 +435,122 @@ AT_CHECK([ovn-nbctl meter-list], [0], [dnl >> > >> > dnl --------------------------------------------------------------------- >> > >> > +OVN_NBCTL_TEST([ovn_nbctl_mirrors], [mirrors], [ >> > +AT_CHECK([ovn-nbctl mirror-add mirror1 gre 0 from-lport 10.10.10.1]) >> > +AT_CHECK([ovn-nbctl mirror-add mirror2 erspan 1 both 10.10.10.2]) >> > +AT_CHECK([ovn-nbctl mirror-add mirror3 gre 2 to-lport 10.10.10.3]) >> > +AT_CHECK([ovn-nbctl ls-add sw0]) >> > +AT_CHECK([ovn-nbctl lsp-add sw0 sw0-port1]) >> > +AT_CHECK([ovn-nbctl lsp-add sw0 sw0-port2]) >> > +AT_CHECK([ovn-nbctl lsp-add sw0 sw0-port3]) >> > + >> > +dnl Add duplicate mirror name >> > +AT_CHECK([ovn-nbctl mirror-add mirror1 gre 0 from-lport 10.10.10.5], [1], >> > [], [stderr]) >> > +AT_CHECK([grep 'already exists' stderr], [0], [ignore]) >> > + >> > +dnl Attach invalid source port to mirror >> > +AT_CHECK([ovn-nbctl lsp-attach-mirror sw0-port4 mirror3], [1], [], >> > [stderr]) >> > +AT_CHECK([grep 'port name not found' stderr], [0], [ignore]) >> > + >> > +dnl Attach source port to mirror >> > +AT_CHECK([ovn-nbctl lsp-attach-mirror sw0-port1 mirror3]) >> > + >> > +dnl Attach one more source port to mirror >> > +AT_CHECK([ovn-nbctl lsp-attach-mirror sw0-port3 mirror3]) >> > + >> > +dnl Verify if multiple ports are attached to the same mirror properly >> > +AT_CHECK([ovn-nbctl mirror-list], [0], [dnl >> > +mirror1: >> > + Type : gre >> > + Sink : 10.10.10.1 >> > + Filter : from-lport >> > + Index/Key: 0 >> > + Sources : None attached >> > +mirror2: >> > + Type : erspan >> > + Sink : 10.10.10.2 >> > + Filter : both >> > + Index/Key: 1 >> > + Sources : None attached >> > +mirror3: >> > + Type : gre >> > + Sink : 10.10.10.3 >> > + Filter : to-lport >> > + Index/Key: 2 >> > + Sources : sw0-port1 sw0-port3 >> > +]) >> > + >> > +dnl Detach one source port from mirror >> > +AT_CHECK([ovn-nbctl lsp-detach-mirror sw0-port3 mirror3]) >> > + >> > +dnl Verify if detach source port from mirror happens properly >> > +AT_CHECK([ovn-nbctl mirror-list], [0], [dnl >> > +mirror1: >> > + Type : gre >> > + Sink : 10.10.10.1 >> > + Filter : from-lport >> > + Index/Key: 0 >> > + Sources : None attached >> > +mirror2: >> > + Type : erspan >> > + Sink : 10.10.10.2 >> > + Filter : both >> > + Index/Key: 1 >> > + Sources : None attached >> > +mirror3: >> > + Type : gre >> > + Sink : 10.10.10.3 >> > + Filter : to-lport >> > + Index/Key: 2 >> > + Sources : sw0-port1 >> > +]) >> > + >> > +dnl Delete a single mirror which has source attached. >> > +AT_CHECK([ovn-nbctl mirror-del mirror3]) >> > + >> > +dnl Check if the detach happened from source properly >> > +AT_CHECK([ovn-nbctl get Logical_Switch_Port sw0-port1 mirror_rules | cut >> > -b 3], [0], [dnl >> > + >> > +]) >> > + >> > +dnl Check if the mirror deleted properly >> > +AT_CHECK([ovn-nbctl mirror-list], [0], [dnl >> > +mirror1: >> > + Type : gre >> > + Sink : 10.10.10.1 >> > + Filter : from-lport >> > + Index/Key: 0 >> > + Sources : None attached >> > +mirror2: >> > + Type : erspan >> > + Sink : 10.10.10.2 >> > + Filter : both >> > + Index/Key: 1 >> > + Sources : None attached >> > +]) >> > + >> > +dnl Delete another mirror >> > +AT_CHECK([ovn-nbctl mirror-del mirror2]) >> > + >> > +dnl Update the Sink address >> > +AT_CHECK([ovn-nbctl set mirror . sink=192.168.1.13]) >> > + >> > +AT_CHECK([ovn-nbctl mirror-list], [0], [dnl >> > +mirror1: >> > + Type : gre >> > + Sink : 192.168.1.13 >> > + Filter : from-lport >> > + Index/Key: 0 >> > + Sources : None attached >> > +]) >> > + >> > +dnl Delete all mirrors >> > +AT_CHECK([ovn-nbctl mirror-del]) >> > +AT_CHECK([ovn-nbctl mirror-list], [0], [dnl >> > +])]) >> > + >> > +dnl --------------------------------------------------------------------- >> > + >> > OVN_NBCTL_TEST([ovn_nbctl_nats], [NATs], [ >> > AT_CHECK([ovn-nbctl lr-add lr0]) >> > AT_CHECK([ovn-nbctl lr-nat-add lr0 snatt 30.0.0.2 192.168.1.2], [1], [], >> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >> > index 093e01c6d..fd8d98cbf 100644 >> > --- a/tests/ovn-northd.at >> > +++ b/tests/ovn-northd.at >> > @@ -2319,6 +2319,108 @@ check_meter_by_name NOT meter_me__${acl1} >> > meter_me__${acl2} >> > AT_CLEANUP >> > ]) >> > >> > +OVN_FOR_EACH_NORTHD([ >> > +AT_SETUP([Check NB-SB mirrors sync]) >> > +AT_KEYWORDS([mirrors]) >> > +ovn_start >> > + >> > +check ovn-nbctl --wait=sb mirror-add mirror1 erspan 0 both 10.10.10.2 >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . Sink], [0], [dnl >> > +"10.10.10.2" >> > +]) >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . type], [0], [dnl >> > +erspan >> > +]) >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . index], [0], [dnl >> > +0 >> > +]) >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . filter], [0], [dnl >> > +both >> > +]) >> > + >> > +check ovn-nbctl --wait=sb \ >> > + -- set mirror . sink=192.168.1.13 >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . Sink], [0], [dnl >> > +"192.168.1.13" >> > +]) >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . type], [0], [dnl >> > +erspan >> > +]) >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . index], [0], [dnl >> > +0 >> > +]) >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . filter], [0], [dnl >> > +both >> > +]) >> > + >> > +check ovn-nbctl --wait=sb \ >> > + -- set mirror . type=gre >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . type], [0], [dnl >> > +gre >> > +]) >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . Sink], [0], [dnl >> > +"192.168.1.13" >> > +]) >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . index], [0], [dnl >> > +0 >> > +]) >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . filter], [0], [dnl >> > +both >> > +]) >> > + >> > +check ovn-nbctl --wait=sb \ >> > + -- set mirror . index=12 >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . index], [0], [dnl >> > +12 >> > +]) >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . type], [0], [dnl >> > +gre >> > +]) >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . Sink], [0], [dnl >> > +"192.168.1.13" >> > +]) >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . filter], [0], [dnl >> > +both >> > +]) >> > + >> > +check ovn-nbctl --wait=sb \ >> > + -- set mirror . filter=to-lport >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . filter], [0], [dnl >> > +to-lport >> > +]) >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . index], [0], [dnl >> > +12 >> > +]) >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . type], [0], [dnl >> > +gre >> > +]) >> > + >> > +AT_CHECK([ovn-sbctl get Mirror . Sink], [0], [dnl >> > +"192.168.1.13" >> > +]) >> > + >> > +AT_CLEANUP >> > +]) >> > + >> > OVN_FOR_EACH_NORTHD([ >> > AT_SETUP([ACL skip hints for stateless config]) >> > AT_KEYWORDS([acl]) >> > diff --git a/tests/ovn.at b/tests/ovn.at >> > index 07f72dc31..42f28ab1b 100644 >> > --- a/tests/ovn.at >> > +++ b/tests/ovn.at >> > @@ -16121,6 +16121,237 @@ OVN_CLEANUP([hv1], [hv2]) >> > AT_CLEANUP >> > ]) >> > >> > +OVN_FOR_EACH_NORTHD([ >> > +AT_SETUP([Mirror]) >> > +AT_KEYWORDS([Mirror]) >> > +ovn_start >> > + >> > +# Logical network: >> > +# One LR - R1 has switch ls1 (191.168.1.0/24) connected to it, >> > +# and has switch ls2 (172.16.1.0/24) connected to it. >> > + >> > +ovn-nbctl lr-add R1 >> > + >> > +ovn-nbctl ls-add ls1 >> > +ovn-nbctl ls-add ls2 >> > + >> > +# Connect ls1 to R1 >> > +ovn-nbctl lrp-add R1 ls1 00:00:00:01:02:f1 192.168.1.1/24 >> > +ovn-nbctl lsp-add ls1 rp-ls1 -- set Logical_Switch_Port rp-ls1 \ >> > + type=router options:router-port=ls1 addresses=\"00:00:00:01:02:f1\" >> > + >> > +# Connect ls2 to R1 >> > +ovn-nbctl lrp-add R1 ls2 00:00:00:01:02:f2 172.16.1.1/24 >> > +ovn-nbctl lsp-add ls2 rp-ls2 -- set Logical_Switch_Port rp-ls2 \ >> > + type=router options:router-port=ls2 addresses=\"00:00:00:01:02:f2\" >> > + >> > +# Create logical port ls1-lp1 in ls1 >> > +ovn-nbctl lsp-add ls1 ls1-lp1 \ >> > +-- lsp-set-addresses ls1-lp1 "00:00:00:01:02:03 192.168.1.2" >> > + >> > +# Create logical port ls2-lp1 in ls2 >> > +ovn-nbctl lsp-add ls2 ls2-lp1 \ >> > +-- lsp-set-addresses ls2-lp1 "00:00:00:01:02:04 172.16.1.2" >> > + >> > +ovn-nbctl lsp-add ls1 ln-public >> > +ovn-nbctl lsp-set-type ln-public localnet >> > +ovn-nbctl lsp-set-addresses ln-public unknown >> > +ovn-nbctl lsp-set-options ln-public network_name=public >> > + >> > +# Create one hypervisor and create OVS ports corresponding to logical >> > ports. >> > +net_add n1 >> > + >> > +sim_add hv1 >> > +as hv1 >> > +ovs-vsctl add-br br-phys -- set bridge br-phys >> > other-config:hwaddr=\"00:00:00:01:02:00\" >> > +ovn_attach n1 br-phys 192.168.1.11 >> > + >> > +ovs-vsctl -- add-port br-int vif1 -- \ >> > + set interface vif1 external-ids:iface-id=ls1-lp1 \ >> > + options:tx_pcap=hv1/vif1-tx.pcap \ >> > + options:rxq_pcap=hv1/vif1-rx.pcap \ >> > + ofport-request=1 >> > + >> > +ovs-vsctl -- add-port br-int vif2 -- \ >> > + set interface vif2 external-ids:iface-id=ls2-lp1 \ >> > + options:tx_pcap=hv1/vif2-tx.pcap \ >> > + options:rxq_pcap=hv1/vif2-rx.pcap \ >> > + ofport-request=1 >> > + >> > +ovs-vsctl set open . external-ids:ovn-bridge-mappings=public:br-phys >> > + >> > +# Allow some time for ovn-northd and ovn-controller to catch up. >> > +wait_for_ports_up >> > +check ovn-nbctl --wait=hv sync >> > +ovn-nbctl dump-flows > sbflows >> > +AT_CAPTURE_FILE([sbflows]) >> > + >> > +for i in 1 2; do >> > + : > vif$i.expected >> > +done >> > + >> > +net_add n2 >> > + >> > +sim_add hv2 >> > +as hv2 >> > +ovs-vsctl add-br br-phys -- set bridge br-phys >> > other-config:hwaddr=\"00:00:00:02:02:00\" >> > +ovn_attach n2 br-phys 192.168.1.12 >> > + >> > +OVN_POPULATE_ARP >> > + >> > +as hv1 >> > + >> > +# test_ipv4_icmp_request INPORT ETH_SRC ETH_DST IPV4_SRC IPV4_DST >> > IP_CHKSUM ICMP_CHKSUM [EXP_IP_CHKSUM EXP_ICMP_CHKSUM] ENCAP_TYPE FILTER >> > +# >> > +# Causes a packet to be received on INPORT. The packet is an ICMPv4 >> > +# request with ETH_SRC, ETH_DST, IPV4_SRC, IPV4_DST, IP_CHSUM and >> > +# ICMP_CHKSUM as specified. If EXP_IP_CHKSUM and EXP_ICMP_CHKSUM are >> > +# provided, then it should be the ip and icmp checksums of the packet >> > +# responded; otherwise, no reply is expected. >> > +# In the absence of an ip checksum calculation helpers, this relies >> > +# on the caller to provide the checksums for the ip and icmp headers. >> > +# XXX This should be more systematic. >> > +# >> > +# INPORT is an lport number, e.g. 11 for vif11. >> > +# ETH_SRC and ETH_DST are each 12 hex digits. >> > +# IPV4_SRC and IPV4_DST are each 8 hex digits. >> > +# IP_CHSUM and ICMP_CHKSUM are each 4 hex digits. >> > +# EXP_IP_CHSUM and EXP_ICMP_CHKSUM are each 4 hex digits. >> > +# ENCAP_TYPE - gre or erspan >> > +# FILTER - Mirror Filter - to-lport / from-lport >> > +test_ipv4_icmp_request() { >> > + local inport=$1 eth_src=$2 eth_dst=$3 ipv4_src=$4 ipv4_dst=$5 >> > ip_chksum=$6 icmp_chksum=$7 >> > + local exp_ip_chksum=$8 exp_icmp_chksum=$9 mirror_encap_type=${10} >> > mirror_filter=${11} >> > + shift; shift; shift; shift; shift; shift; shift >> > + shift; shift; shift; shift; >> > + >> > + # Use ttl to exercise section 4.2.2.9 of RFC1812 >> > + local ip_ttl=02 >> > + local icmp_id=5fbf >> > + local icmp_seq=0001 >> > + local icmp_data=$(seq 1 56 | xargs printf "%02x") >> > + local icmp_type_code_request=0800 >> > + local >> > icmp_payload=${icmp_type_code_request}${icmp_chksum}${icmp_id}${icmp_seq}${icmp_data} >> > + local >> > packet=${eth_dst}${eth_src}08004500005400004000${ip_ttl}01${ip_chksum}${ipv4_src}${ipv4_dst}${icmp_payload} >> > + >> > + as hv1 ovs-appctl netdev-dummy/receive vif$inport $packet >> > + >> > + # Expect to receive the reply, if any. In same port where packet was >> > sent. >> > + # Note: src and dst fields are expected to be reversed. >> > + local icmp_type_code_response=0000 >> > + local reply_icmp_ttl=fe >> > + local >> > reply_icmp_payload=${icmp_type_code_response}${exp_icmp_chksum}${icmp_id}${icmp_seq}${icmp_data} >> > + local >> > reply=${eth_src}${eth_dst}08004500005400004000${reply_icmp_ttl}01${exp_ip_chksum}${ipv4_dst}${ipv4_src}${reply_icmp_payload} >> > + echo $reply >> vif$inport.expected >> > + local remote_mac=000000020200 >> > + local local_mac=000000010200 >> > + if test ${mirror_encap_type} = "gre" ; then >> > + local >> > ipv4_gre=4500007e00004000402fb6e9c0a8010bc0a8010c2000655800000000 >> > + if test ${mirror_filter} = "to-lport" ; then >> > + local mirror=${remote_mac}${local_mac}0800${ipv4_gre}${reply} >> > + elif test ${mirror_filter} = "from-lport" ; then >> > + local mirror=${remote_mac}${local_mac}0800${ipv4_gre}${packet} >> > + fi >> > + elif test ${mirror_encap_type} = "erspan" ; then >> > + local ipv4_erspan=4500008600004000402fb6e1c0a8010bc0a8010c >> > + local erspan_seq0=100088be000000001000000000000000 >> > + local erspan_seq1=100088be000000011000000000000000 >> > + if test ${mirror_filter} = "to-lport" ; then >> > + local >> > mirror=${remote_mac}${local_mac}0800${ipv4_erspan}${erspan_seq0}${reply} >> > + elif test ${mirror_filter} = "from-lport" ; then >> > + local >> > mirror=${remote_mac}${local_mac}0800${ipv4_erspan}${erspan_seq1}${packet} >> > + fi >> > + fi >> > + echo $mirror >> br-phys_n1.expected >> > + >> > +} >> > + >> > +# Set IPs >> > +rtr_l2_ip=$(ip_to_hex 172 16 1 1) >> > +l1_ip=$(ip_to_hex 192 168 1 2) >> > + >> > +check ovn-nbctl mirror-add mirror0 gre 0 to-lport 192.168.1.12 >> > +check ovn-nbctl lsp-attach-mirror ls1-lp1 mirror0 >> > + >> > +# Send ping packet and check for mirrored packet of the reply >> > +test_ipv4_icmp_request 1 000000010203 0000000102f1 $l1_ip $rtr_l2_ip 0000 >> > 8510 03ff 8d10 "gre" "to-lport" >> > + >> > +OVN_CHECK_PACKETS_CONTAIN([hv1/br-phys_n1-tx.pcap], [br-phys_n1.expected]) >> > +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected]) >> > + >> > +as hv1 reset_pcap_file vif1 hv1/vif1 >> > +as hv1 reset_pcap_file br-phys_n1 hv1/br-phys_n1 >> > +rm -f br-phys_n1.expected >> > +rm -f vif1.expected >> > + >> > +check ovn-nbctl set mirror . type=erspan >> > + >> > +# Send ping packet and check for mirrored packet of the reply >> > +test_ipv4_icmp_request 1 000000010203 0000000102f1 $l1_ip $rtr_l2_ip 0000 >> > 8510 03ff 8d10 "erspan" "to-lport" >> > + >> > +OVN_CHECK_PACKETS_CONTAIN([hv1/br-phys_n1-tx.pcap], [br-phys_n1.expected]) >> > +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected]) >> > + >> > +as hv1 reset_pcap_file vif1 hv1/vif1 >> > +as hv1 reset_pcap_file br-phys_n1 hv1/br-phys_n1 >> > +rm -f br-phys_n1.expected >> > +rm -f vif1.expected >> > + >> > +check ovn-nbctl set mirror . filter=from-lport >> > + >> > +# Send ping packet and check for mirrored packet of the request >> > +test_ipv4_icmp_request 1 000000010203 0000000102f1 $l1_ip $rtr_l2_ip 0000 >> > 8510 03ff 8d10 "erspan" "from-lport" >> > + >> > +OVN_CHECK_PACKETS_CONTAIN([hv1/br-phys_n1-tx.pcap], [br-phys_n1.expected]) >> > +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected]) >> > + >> > +as hv1 reset_pcap_file vif1 hv1/vif1 >> > +as hv1 reset_pcap_file br-phys_n1 hv1/br-phys_n1 >> > +rm -f br-phys_n1.expected >> > +rm -f vif1.expected >> > + >> > +check ovn-nbctl set mirror . type=gre >> > + >> > +# Send ping packet and check for mirrored packet of the request >> > +test_ipv4_icmp_request 1 000000010203 0000000102f1 $l1_ip $rtr_l2_ip 0000 >> > 8510 03ff 8d10 "gre" "from-lport" >> > + >> > +OVN_CHECK_PACKETS_CONTAIN([hv1/br-phys_n1-tx.pcap], [br-phys_n1.expected]) >> > +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected]) >> > + >> > +echo "---------OVN NB Mirror-----" >> > +ovn-nbctl mirror-list >> > + >> > +echo "---------OVS Mirror----" >> > +ovs-vsctl list Mirror >> > + >> > +echo "-----------------------" >> > + >> > +echo "Verifying Mirror deletion in OVS" >> > +# Set vif1 iface-id such that OVN releases port binding >> > +check ovs-vsctl set interface vif1 external_ids:iface-id=foo >> > +check ovn-nbctl --wait=hv sync >> > + >> > +AT_CHECK([as hv1 ovs-vsctl list Mirror], [0], [dnl >> > +]) >> > + >> > +# Set vif1 iface-id back to ls1-lp1 >> > +check ovs-vsctl set interface vif1 external_ids:iface-id=ls1-lp1 >> > +check ovn-nbctl --wait=hv sync >> > + >> > +AT_CHECK([as hv1 ovs-vsctl get Mirror mirror0 name], [0], [dnl >> > +mirror0 >> > +]) >> > + >> > +# Delete vif1 so that OVN releases port binding >> > +check ovs-vsctl del-port br-int vif1 >> > +check ovn-nbctl --wait=hv sync >> > + >> > +AT_CHECK([as hv1 ovs-vsctl list Mirror], [0], [dnl >> > +]) >> > + >> > +OVN_CLEANUP([hv1], [hv2]) >> > +AT_CLEANUP >> > +]) >> > >> > OVN_FOR_EACH_NORTHD([ >> > AT_SETUP([Port Groups]) >> > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c >> > index 811468dc6..7d593b726 100644 >> > --- a/utilities/ovn-nbctl.c >> > +++ b/utilities/ovn-nbctl.c >> > @@ -271,6 +271,19 @@ QoS commands:\n\ >> > remove QoS rules from SWITCH\n\ >> > qos-list SWITCH print QoS rules for SWITCH\n\ >> > \n\ >> > +Mirror commands:\n\ >> > + mirror-add NAME TYPE INDEX FILTER IP\n\ >> > + add a mirror with given name\n\ >> > + specify TYPE 'gre' or 'erspan'\n\ >> > + specify the tunnel INDEX value\n\ >> > + (indicates key if GRE\n\ >> > + erpsan_idx if ERSPAN)\n\ >> > + specify FILTER for mirroring selection\n\ >> > + 'to-lport' / 'from-lport' / 'both'\n\ >> > + specify Sink / Destination i.e. Remote IP\n\ >> > + mirror-del [NAME] remove mirrors\n\ >> > + mirror-list print mirrors\n\ >> > +\n\ >> > Meter commands:\n\ >> > [--fair]\n\ >> > meter-add NAME ACTION RATE UNIT [BURST]\n\ >> > @@ -311,6 +324,8 @@ Logical switch port commands:\n\ >> > set dhcpv6 options for PORT\n\ >> > lsp-get-dhcpv6-options PORT get the dhcpv6 options for PORT\n\ >> > lsp-get-ls PORT get the logical switch which the port belongs >> > to\n\ >> > + lsp-attach-mirror PORT MIRROR attach source PORT to MIRROR\n\ >> > + lsp-detach-mirror PORT MIRROR detach source PORT from MIRROR\n\ >> > \n\ >> > Forwarding group commands:\n\ >> > [--liveness]\n\ >> > @@ -1685,6 +1700,130 @@ nbctl_pre_lsp_type(struct ctl_context *ctx) >> > ovsdb_idl_add_column(ctx->idl, &nbrec_logical_switch_port_col_type); >> > } >> > >> > +static void >> > +nbctl_pre_lsp_mirror(struct ctl_context *ctx) >> > +{ >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_logical_switch_port_col_name); >> > + ovsdb_idl_add_column(ctx->idl, >> > + &nbrec_logical_switch_port_col_mirror_rules); >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_mirror_col_name); >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_mirror_col_src); >> > +} >> > + >> > +static int >> > +mirror_cmp(const void *mirror1_, const void *mirror2_) >> > +{ >> > + const struct nbrec_mirror *const *mirror_1 = mirror1_; >> > + const struct nbrec_mirror *const *mirror_2 = mirror2_; >> > + >> > + const struct nbrec_mirror *mirror1 = *mirror_1; >> > + const struct nbrec_mirror *mirror2 = *mirror_2; >> > + >> > + return strcmp(mirror1->name,mirror2->name); >> > +} >> > + >> > +static char * OVS_WARN_UNUSED_RESULT >> > +mirror_by_name_or_uuid(struct ctl_context *ctx, const char *id, >> > + bool must_exist, >> > + const struct nbrec_mirror **mirror_p) >> > +{ >> > + const struct nbrec_mirror *mirror = NULL; >> > + *mirror_p = NULL; >> > + >> > + struct uuid mirror_uuid; >> > + bool is_uuid = uuid_from_string(&mirror_uuid, id); >> > + if (is_uuid) { >> > + mirror = nbrec_mirror_get_for_uuid(ctx->idl, &mirror_uuid); >> > + } >> > + >> > + if (!mirror) { >> > + NBREC_MIRROR_FOR_EACH (mirror, ctx->idl) { >> > + if (!strcmp(mirror->name, id)) { >> > + break; >> > + } >> > + } >> > + } >> > + >> > + if (!mirror && must_exist) { >> > + return xasprintf("%s: mirror %s not found", >> > + id, is_uuid ? "UUID" : "name"); >> > + } >> > + >> > + *mirror_p = mirror; >> > + return NULL; >> > +} >> > + >> > +static void >> > +nbctl_lsp_attach_mirror(struct ctl_context *ctx) >> > +{ >> > + const char *port = ctx->argv[1]; >> > + const char *mirror_name = ctx->argv[2]; >> > + const struct nbrec_logical_switch_port *lsp = NULL; >> > + const struct nbrec_mirror *mirror; >> > + >> > + char *error; >> > + >> > + error = lsp_by_name_or_uuid(ctx, port, true, &lsp); >> > + if (error) { >> > + ctx->error = error; >> > + return; >> > + } >> > + >> > + >> > + /*check if a mirror rule actually exists on that name or not*/ >> > + error = mirror_by_name_or_uuid(ctx, mirror_name, true, &mirror); >> > + if (error) { >> > + ctx->error = error; >> > + return; >> > + } >> > + >> > + /* Check if same mirror rule already exists for the lsp */ >> > + for (size_t i = 0; i < lsp->n_mirror_rules; i++) { >> > + if (!mirror_cmp(&lsp->mirror_rules[i], &mirror)) { >> > + bool may_exist = shash_find(&ctx->options, "--may-exist") != >> > NULL; >> > + if (!may_exist) { >> > + ctl_error(ctx, "Same mirror already existed on the lsp >> > %s.", >> > + ctx->argv[1]); >> > + return; >> > + } >> > + return; >> > + } >> > + } >> > + >> > + nbrec_logical_switch_port_update_mirror_rules_addvalue(lsp, mirror); >> > + nbrec_mirror_update_src_addvalue(mirror,lsp); >> > + >> > +} >> > + >> > +static void >> > +nbctl_lsp_detach_mirror(struct ctl_context *ctx) >> > +{ >> > + const char *port = ctx->argv[1]; >> > + const char *mirror_name = ctx->argv[2]; >> > + const struct nbrec_logical_switch_port *lsp = NULL; >> > + const struct nbrec_mirror *mirror; >> > + >> > + char *error; >> > + >> > + error = lsp_by_name_or_uuid(ctx, port, true, &lsp); >> > + if (error) { >> > + ctx->error = error; >> > + return; >> > + } >> > + >> > + >> > + /*check if a mirror rule actually exists on that name or not*/ >> > + error = mirror_by_name_or_uuid(ctx, mirror_name, true, &mirror); >> > + if (error) { >> > + ctx->error = error; >> > + return; >> > + } >> > + >> > + nbrec_logical_switch_port_update_mirror_rules_delvalue(lsp, mirror); >> > + nbrec_mirror_update_src_delvalue(mirror,lsp); >> > + >> > +} >> > + >> > static void >> > nbctl_lsp_set_type(struct ctl_context *ctx) >> > { >> > @@ -7241,6 +7380,211 @@ cmd_ha_ch_grp_set_chassis_prio(struct ctl_context >> > *ctx) >> > nbrec_ha_chassis_set_priority(ha_chassis, priority); >> > } >> > >> > +static char * OVS_WARN_UNUSED_RESULT >> > +parse_filter(const char *arg, const char **selection_p) >> > +{ >> > + /* Validate selection. Only require the first letter. */ >> > + if (arg[0] == 't') { >> > + *selection_p = "to-lport"; >> > + } else if (arg[0] == 'f') { >> > + *selection_p = "from-lport"; >> > + } else if (arg[0] == 'b') { >> > + *selection_p = "both"; >> > + } else { >> > + *selection_p = NULL; >> > + return xasprintf("%s: selection must be \"to-lport\" or " >> > + "\"from-lport\" or \"both\" ", arg); >> > + } >> > + return NULL; >> > +} >> > + >> > +static char * OVS_WARN_UNUSED_RESULT >> > +parse_type(const char *arg, const char **type_p) >> > +{ >> > + /* Validate type. Only require the first letter. */ >> > + if (arg[0] == 'g') { >> > + *type_p = "gre"; >> > + } else if (arg[0] == 'e') { >> > + *type_p = "erspan"; >> > + } else { >> > + *type_p = NULL; >> > + return xasprintf("%s: type must be \"gre\" or " >> > + "\"erspan\"", arg); >> > + } >> > + return NULL; >> > +} >> > + >> > +static void >> > +nbctl_pre_mirror_add(struct ctl_context *ctx) >> > +{ >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_mirror_col_name); >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_mirror_col_filter); >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_mirror_col_index); >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_mirror_col_sink); >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_mirror_col_type); >> > +} >> > + >> > +static void >> > +nbctl_mirror_add(struct ctl_context *ctx) >> > +{ >> > + const char *filter = NULL; >> > + const char *sink_ip = NULL; >> > + const char *type = NULL; >> > + const char *name = NULL; >> > + char *new_sink_ip = NULL; >> > + int64_t index; >> > + char *error = NULL; >> > + const struct nbrec_mirror *mirror_check = NULL; >> > + >> > + /* Mirror Name */ >> > + name = ctx->argv[1]; >> > + NBREC_MIRROR_FOR_EACH (mirror_check, ctx->idl) { >> > + if (!strcmp(mirror_check->name, name)) { >> > + ctl_error(ctx, "Mirror with %s name already exists.", >> > + name); >> > + return; >> > + } >> > + } >> > + >> > + /* Tunnel Type - GRE/ERSPAN */ >> > + error = parse_type(ctx->argv[2], &type); >> > + if (error) { >> > + ctx->error = error; >> > + return; >> > + } >> > + >> > + /* tunnel index / GRE key / ERSPAN idx */ >> > + index = atoi(ctx->argv[3]); >> > + >> > + /* Filter for mirroring */ >> > + error = parse_filter(ctx->argv[4], &filter); >> > + if (error) { >> > + ctx->error = error; >> > + return; >> > + } >> > + >> > + /* Destination / Sink details */ >> > + sink_ip = ctx->argv[5]; >> > + >> > + /* check if it is a valid ip */ >> > + new_sink_ip = normalize_ipv4_addr_str(sink_ip); >> > + if (!new_sink_ip) { >> > + new_sink_ip = normalize_ipv6_addr_str(sink_ip); >> > + } >> > + >> > + if (new_sink_ip) { >> > + free(new_sink_ip); >> > + } else { >> > + ctl_error(ctx, "Invalid sink ip: %s", sink_ip); >> > + return; >> > + } >> > + >> > + /* Create the mirror. */ >> > + struct nbrec_mirror *mirror = nbrec_mirror_insert(ctx->txn); >> > + nbrec_mirror_set_name(mirror, name); >> > + nbrec_mirror_set_index(mirror, index); >> > + nbrec_mirror_set_filter(mirror, filter); >> > + nbrec_mirror_set_type(mirror, type); >> > + nbrec_mirror_set_sink(mirror, sink_ip); >> > + >> > +} >> > + >> > +static void >> > +nbctl_pre_mirror_del(struct ctl_context *ctx) >> > +{ >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_mirror_col_name); >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_mirror_col_src); >> > +} >> > + >> > +static void >> > +nbctl_mirror_del(struct ctl_context *ctx) >> > +{ >> > + const struct nbrec_mirror *mirror, *next; >> > + >> > + /* If a name is not specified, delete all mirrors. */ >> > + if (ctx->argc == 1) { >> > + NBREC_MIRROR_FOR_EACH_SAFE (mirror, next, ctx->idl) { >> > + nbrec_mirror_delete(mirror); >> > + } >> > + return; >> > + } >> > + >> > + /* Remove the matching mirror. */ >> > + NBREC_MIRROR_FOR_EACH (mirror, ctx->idl) { >> > + if (strcmp(ctx->argv[1], mirror->name)) { >> > + continue; >> > + } >> > + nbrec_mirror_delete(mirror); >> > + return; >> > + } >> > +} >> > + >> > +static void >> > +nbctl_pre_mirror_list(struct ctl_context *ctx) >> > +{ >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_logical_switch_port_col_name); >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_mirror_col_name); >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_mirror_col_filter); >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_mirror_col_index); >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_mirror_col_sink); >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_mirror_col_type); >> > + ovsdb_idl_add_column(ctx->idl, &nbrec_mirror_col_src); >> > +} >> > + >> > +static void >> > +nbctl_mirror_list(struct ctl_context *ctx) >> > +{ >> > + >> > + const struct nbrec_mirror **mirrors = NULL; >> > + const struct nbrec_mirror *mirror; >> > + size_t n_capacity = 0; >> > + size_t n_mirrors = 0; >> > + >> > + NBREC_MIRROR_FOR_EACH (mirror, ctx->idl) { >> > + if (n_mirrors == n_capacity) { >> > + mirrors = x2nrealloc(mirrors, &n_capacity, sizeof *mirrors); >> > + } >> > + >> > + mirrors[n_mirrors] = mirror; >> > + n_mirrors++; >> > + } >> > + >> > + if (n_mirrors) { >> > + qsort(mirrors, n_mirrors, sizeof *mirrors, mirror_cmp); >> > + } >> > + >> > + for (size_t i = 0; i < n_mirrors; i++) { >> > + mirror = mirrors[i]; >> > + ds_put_format(&ctx->output, "%s:\n", mirror->name); >> > + /* print all the values */ >> > + ds_put_format(&ctx->output, " Type : %s\n", mirror->type); >> > + ds_put_format(&ctx->output, " Sink : %s\n", mirror->sink); >> > + ds_put_format(&ctx->output, " Filter : %s\n", mirror->filter); >> > + ds_put_format(&ctx->output, " Index/Key: %ld\n", >> > + (long int)mirror->index); >> > + ds_put_cstr(&ctx->output, " Sources :"); >> > + if (mirror->n_src > 0) { >> > + struct svec srcs; >> > + const char *src; >> > + size_t j; >> > + svec_init(&srcs); >> > + for (j = 0; j < mirror->n_src; j++) { >> > + svec_add(&srcs, mirror->src[j]->name); >> > + } >> > + svec_sort(&srcs); >> > + SVEC_FOR_EACH (j, src, &srcs) { >> > + ds_put_format(&ctx->output, " %s", src); >> > + } >> > + svec_destroy(&srcs); >> > + } else { >> > + ds_put_cstr(&ctx->output, " None attached"); >> > + } >> > + ds_put_cstr(&ctx->output, "\n"); >> > + } >> > + >> > + free(mirrors); >> > +} >> > + >> > static const struct ctl_table_class tables[NBREC_N_TABLES] = { >> > [NBREC_TABLE_DHCP_OPTIONS].row_ids >> > = {{&nbrec_logical_switch_port_col_name, NULL, >> > @@ -7334,6 +7678,15 @@ static const struct ctl_command_syntax >> > nbctl_commands[] = { >> > { "qos-list", 1, 1, "SWITCH", nbctl_pre_qos_list, nbctl_qos_list, >> > NULL, "", RO }, >> > >> > + /* mirror commands. */ >> > + { "mirror-add", 5, 5, >> > + "NAME TYPE INDEX FILTER IP", >> > + nbctl_pre_mirror_add, nbctl_mirror_add, NULL, "--may-exist", RW }, >> > + { "mirror-del", 0, 1, "[NAME]", >> > + nbctl_pre_mirror_del, nbctl_mirror_del, NULL, "", RW }, >> > + { "mirror-list", 0, 0, "", nbctl_pre_mirror_list, nbctl_mirror_list, >> > + NULL, "", RO }, >> > + >> > /* meter commands. */ >> > { "meter-add", 4, 5, "NAME ACTION RATE UNIT [BURST]", >> > nbctl_pre_meter_add, >> > nbctl_meter_add, NULL, "--fair,--may-exist", RW }, >> > @@ -7388,6 +7741,10 @@ static const struct ctl_command_syntax >> > nbctl_commands[] = { >> >> > nbctl_lsp_get_dhcpv6_options, NULL, "", RO }, >> > { "lsp-get-ls", 1, 1, "PORT", nbctl_pre_lsp_get_ls, nbctl_lsp_get_ls, >> > NULL, "", RO }, >> > + { "lsp-attach-mirror", 2, 2, "PORT MIRROR", nbctl_pre_lsp_mirror, >> > + nbctl_lsp_attach_mirror, NULL, "", RW }, >> > + { "lsp-detach-mirror", 2, 2, "PORT MIRROR", nbctl_pre_lsp_mirror, >> > + nbctl_lsp_detach_mirror, NULL, "", RW }, >> > >> > /* forwarding group commands. */ >> > { "fwd-group-add", 4, INT_MAX, "SWITCH GROUP VIP VMAC PORT...", >> > diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c >> > index e35556d34..1aa6cc26f 100644 >> > --- a/utilities/ovn-sbctl.c >> > +++ b/utilities/ovn-sbctl.c >> > @@ -307,6 +307,7 @@ pre_get_info(struct ctl_context *ctx) >> > ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_chassis); >> > ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_datapath); >> > ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_up); >> > + ovsdb_idl_add_column(ctx->idl, &sbrec_port_binding_col_mirror_rules); >> > >> > ovsdb_idl_add_column(ctx->idl, >> > &sbrec_logical_flow_col_logical_datapath); >> > ovsdb_idl_add_column(ctx->idl, >> > &sbrec_logical_flow_col_logical_dp_group); >> > @@ -1434,6 +1435,9 @@ static const struct ctl_table_class >> > tables[SBREC_N_TABLES] = { >> > [SBREC_TABLE_HA_CHASSIS].row_ids[0] >> > = {&sbrec_ha_chassis_col_chassis, NULL, NULL}, >> > >> > + [SBREC_TABLE_MIRROR].row_ids[0] >> > + = {&sbrec_mirror_col_name, NULL, NULL}, >> > + >> > [SBREC_TABLE_METER].row_ids[0] >> > = {&sbrec_meter_col_name, NULL, NULL}, >> > >> > -- >> > 2.27.0 >> > >> > Thanks & Regards, > Abhiram R N _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
