Thank you for the contribution! Just wanted to inform you that the test appears to be flaky, and it would be great if you have a moment to check.
https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/2002406 -- Frode Nordahl On Tue, Dec 13, 2022 at 8:34 PM Abhiram R N <[email protected]> wrote: > > Added the required schema SB and related xml changes. > Changes which syncs the NB port mirrors with SB port mirrors. > Also syncs mirror_rules column in Logical_Switch_Port table > of NB DB with corresponding mirror_rules column in > Port_Binding table of SB DB. > Further test added to check the NB and SB sync > > Co-authored-by: Veda Barrenkala <[email protected]> > Signed-off-by: Veda Barrenkala <[email protected]> > Signed-off-by: Abhiram R N <[email protected]> > Acked-By: Ihar Hrachyshka <[email protected]> > --- > northd/en-northd.c | 4 ++ > northd/inc-proc-northd.c | 4 ++ > northd/northd.c | 133 +++++++++++++++++++++++++++++++++++++++ > northd/northd.h | 2 + > ovn-sb.ovsschema | 26 +++++++- > ovn-sb.xml | 50 +++++++++++++++ > tests/ovn-northd.at | 105 +++++++++++++++++++++++++++++++ > utilities/ovn-sbctl.c | 4 ++ > 8 files changed, 326 insertions(+), 2 deletions(-) > > diff --git a/northd/en-northd.c b/northd/en-northd.c > index 9360c68e9..09fe8976a 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_static_mac_binding", node)); > input_data.nbrec_chassis_template_var_table = > EN_OVSDB_GET(engine_get_input("NB_chassis_template_var", 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_static_mac_binding", node)); > input_data.sbrec_chassis_template_var_table = > EN_OVSDB_GET(engine_get_input("SB_chassis_template_var", 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 ff3620d62..363e384bd 100644 > --- a/northd/inc-proc-northd.c > +++ b/northd/inc-proc-northd.c > @@ -52,6 +52,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") \ > @@ -95,6 +96,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") \ > @@ -178,6 +180,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); > @@ -200,6 +203,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > engine_add_input(&en_northd, &en_sb_encap, 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 7c48bb3b4..6ada6eb68 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -3238,6 +3238,60 @@ ovn_port_update_sbrec_chassis( > free(requested_chassis_sb); > } > > +static void > +check_and_do_sb_mirror_deletion(const struct ovn_port *op) > +{ > + size_t i = 0; > + 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)) { > + /* Delete from SB since its not present in NB*/ > + sbrec_port_binding_update_mirror_rules_delvalue(op->sb, > + op->sb->mirror_rules[i]); > + } > + } > + > + 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); > +} > + > +static void > +check_and_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) > +{ > + check_and_do_sb_mirror_deletion(op); > + check_and_do_sb_mirror_addition(input_data, op); > +} > + > static void > ovn_port_update_sbrec(struct northd_input *input_data, > struct ovsdb_idl_txn *ovnsb_txn, > @@ -3597,6 +3651,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); > @@ -15377,6 +15440,75 @@ 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) > +{ > + 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; > + } > + > + 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); > + > + 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); > + shash_find_and_delete(&sb_mirrors, nb_mirror->name); > + } > + > + 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. > @@ -16047,6 +16179,7 @@ ovnnb_db_run(struct northd_input *input_data, > sync_lbs(input_data, ovnsb_txn, &data->datapaths, &data->lbs); > 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); > sync_template_vars(input_data, ovnsb_txn); > > diff --git a/northd/northd.h b/northd/northd.h > index 7942c0a34..ff8727cb7 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -37,6 +37,7 @@ struct northd_input { > *nbrec_static_mac_binding_table; > const struct nbrec_chassis_template_var_table > *nbrec_chassis_template_var_table; > + const struct nbrec_mirror_table *nbrec_mirror_table; > > /* Southbound table references */ > const struct sbrec_sb_global_table *sbrec_sb_global_table; > @@ -57,6 +58,7 @@ struct northd_input { > *sbrec_static_mac_binding_table; > const struct sbrec_chassis_template_var_table > *sbrec_chassis_template_var_table; > + const struct sbrec_mirror_table *sbrec_mirror_table; > > /* Indexes */ > struct ovsdb_idl_index *sbrec_chassis_by_name; > diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema > index 95c7c2d7e..79ba6841e 100644 > --- a/ovn-sb.ovsschema > +++ b/ovn-sb.ovsschema > @@ -1,7 +1,7 @@ > { > "name": "OVN_Southbound", > - "version": "20.26.0", > - "cksum": "3311869408 29176", > + "version": "20.27.0", > + "cksum": "4078371916 30328", > "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"]]}}}, > + "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 4f485b860..97d4c5c79 100644 > --- a/ovn-sb.xml > +++ b/ovn-sb.xml > @@ -2881,6 +2881,51 @@ tcp.flags = RST; > </column> > </table> > > + <table name="Mirror" title="Mirror Entry"> > + <p> > + Each row in this table represents a 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 > @@ -3383,6 +3428,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-northd.at b/tests/ovn-northd.at > index ca4263eac..edd98743f 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -2326,6 +2326,111 @@ check_meter_by_name NOT meter_me__${acl1} > meter_me__${acl2} > AT_CLEANUP > ]) > > +OVN_FOR_EACH_NORTHD_NO_HV([ > +AT_SETUP([Check NB-SB mirrors sync]) > +AT_KEYWORDS([mirrors]) > +ovn_start > + > +check ovn-nbctl ls-add sw0 > +check ovn-nbctl lsp-add sw0 sw0-port1 > +check ovn-nbctl lsp-add sw0 sw0-port2 > +check ovn-nbctl --wait=sb mirror-add mirror1 erspan 1 from-lport 10.10.10.2 > +check_column mirror1 Mirror name > +check_column 10.10.10.2 Mirror sink > +check_column erspan Mirror type > +check_column 1 Mirror index > +check_column from-lport Mirror filter > + > +check ovn-nbctl --wait=sb set mirror . sink=192.168.1.13 > + > +check_column 192.168.1.13 Mirror sink > +check_column erspan Mirror type > +check_column 1 Mirror index > +check_column from-lport Mirror filter > + > +check ovn-nbctl --wait=sb set mirror . type=gre > + > +check_column 192.168.1.13 Mirror sink > +check_column gre Mirror type > +check_column 1 Mirror index > +check_column from-lport Mirror filter > + > +check ovn-nbctl --wait=sb set mirror . index=12 > + > +check_column 192.168.1.13 Mirror sink > +check_column gre Mirror type > +check_column 12 Mirror index > +check_column from-lport Mirror filter > + > +check ovn-nbctl --wait=sb set mirror . filter=to-lport > + > +check_column 192.168.1.13 Mirror sink > +check_column gre Mirror type > +check_column 12 Mirror index > +check_column to-lport Mirror filter > + > +# Verify mirror attach > +check ovn-nbctl --wait=sb lsp-attach-mirror sw0-port1 mirror1 > + > +mirror1uuid=$(fetch_column sb:Mirror _uuid name=mirror1) > +check_column "$mirror1uuid" sb:Port_Binding mirror_rules > logical_port=sw0-port1 > + > +check ovn-nbctl --wait=sb mirror-add mirror2 gre 2 from-lport 10.10.10.2 > +check_row_count sb:Mirror 2 > + > +# Verify mirror detach (and another attach) > +check ovn-nbctl lsp-attach-mirror sw0-port1 mirror2 > +check ovn-nbctl lsp-detach-mirror sw0-port1 mirror1 > +check ovn-nbctl --wait=sb sync > + > +mirror2uuid=$(fetch_column sb:Mirror _uuid name=mirror2) > +check_column "$mirror2uuid" sb:Port_Binding mirror_rules > logical_port=sw0-port1 > + > +# Verify mirror-del (one by one) > +check ovn-nbctl --wait=sb mirror-del mirror2 > +check_row_count sb:Mirror 1 > +check ovn-nbctl --wait=sb mirror-del mirror1 > +check_row_count sb:Mirror 0 > +check_column "" sb:Port_Binding mirror_rules logical_port=sw0-port1 > + > +# Verify mirror-add > +check ovn-nbctl --wait=sb mirror-add mirror2 gre 2 to-lport 10.10.10.2 > +check_row_count sb:Mirror 1 > + > +check_column 10.10.10.2 Mirror sink > +check_column gre Mirror type > +check_column 2 Mirror index > +check_column to-lport Mirror filter > + > +# Verify same attached to multiple ports > +check ovn-nbctl --wait=sb lsp-attach-mirror sw0-port1 mirror2 > +check ovn-nbctl --wait=sb lsp-attach-mirror sw0-port2 mirror2 > + > +mirror2uuid=$(fetch_column sb:Mirror _uuid name=mirror2) > +check_column "$mirror2uuid" sb:Port_Binding mirror_rules > logical_port=sw0-port1 > +check_column "$mirror2uuid" sb:Port_Binding mirror_rules > logical_port=sw0-port2 > + > +# Verify same port attached to multiple mirrors > +check ovn-nbctl --wait=sb mirror-add mirror1 erspan 1 to-lport 10.10.10.2 > +check ovn-nbctl --wait=sb lsp-attach-mirror sw0-port1 mirror1 > +check_row_count sb:Mirror 2 > +check_row_count nb:Mirror 2 > + > +mirror1uuid=$(fetch_column sb:Mirror _uuid name=mirror1) > +check_column "$mirror2uuid $mirror1uuid" sb:Port_Binding mirror_rules > logical_port=sw0-port1 > + > +# Verify delete (bulk) > +check ovn-nbctl --wait=sb mirror-del > +check_row_count nb:Mirror 0 > +check_column "" nb:Logical_Switch_Port mirror_rules name=sw0-port1 > +check_column "" nb:Logical_Switch_Port mirror_rules name=sw0-port2 > +check_row_count sb:Mirror 0 > +check_column "" sb:Port_Binding mirror_rules logical_port=sw0-port1 > +check_column "" sb:Port_Binding mirror_rules logical_port=sw0-port2 > + > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD_NO_HV([ > AT_SETUP([ACL skip hints for stateless config]) > AT_KEYWORDS([acl]) > diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c > index 00b2f785a..ca6734ba3 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); > @@ -1431,6 +1432,9 @@ static const struct ctl_table_class > tables[SBREC_N_TABLES] = { > [SBREC_TABLE_HA_CHASSIS_GROUP].row_ids[0] > = {&sbrec_ha_chassis_group_col_name, 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.31.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
