lport mirrors by design don't get added to the ovn_mirrors hashmap
so they don't get recreated in OVS later. But no exception was made
for them when processing the lookup hashmap. So if there are other
mirrors on the ports bound to the controller, the code just crashes
with an assertion.

Fixes: 6ccb350705cc ("controller: Added support for port mirroring in OVN 
overlay.")
Signed-off-by: Alexandra Rukomoinikova <[email protected]>
---
 controller/mirror.c | 15 ++++++++++++---
 tests/ovn.at        | 26 ++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/controller/mirror.c b/controller/mirror.c
index 62b4ac98a..15b5d58e0 100644
--- a/controller/mirror.c
+++ b/controller/mirror.c
@@ -75,6 +75,12 @@ char *get_mirror_tunnel_type(const struct sbrec_mirror *);
 static void build_ovs_mirror_ports(const struct ovsrec_bridge *,
                                    struct shash *ovs_mirror_ports);
 
+static inline bool
+is_lport_mirror(const struct sbrec_mirror *sb_mirror)
+{
+    return !strcmp(sb_mirror->type, "lport") ? true : false;
+}
+
 void
 mirror_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 {
@@ -117,7 +123,7 @@ mirror_run(struct ovsdb_idl_txn *ovs_idl_txn,
     const struct sbrec_mirror *sb_mirror;
     SBREC_MIRROR_TABLE_FOR_EACH (sb_mirror, sb_mirror_table) {
         /* We don't need to add mirror to ovs if it is lport mirror. */
-        if (!strcmp(sb_mirror->type, "lport")) {
+        if (is_lport_mirror(sb_mirror)) {
             continue;
         }
         struct ovn_mirror *m = ovn_mirror_create(sb_mirror->name);
@@ -161,6 +167,9 @@ mirror_run(struct ovsdb_idl_txn *ovs_idl_txn,
         }
 
         for (size_t i = 0; i < pb->n_mirror_rules; i++) {
+            if (is_lport_mirror(pb->mirror_rules[i])) {
+                continue;
+            }
             struct ovn_mirror *m = ovn_mirror_find(&ovn_mirrors,
                                                    pb->mirror_rules[i]->name);
             ovs_assert(m);
@@ -171,10 +180,10 @@ mirror_run(struct ovsdb_idl_txn *ovs_idl_txn,
     /* Iterate through the built 'ovn_mirrors' and
      * sync with the local ovsdb i.e.
      * create/update or delete the ovsrec mirror(s). */
-     SHASH_FOR_EACH (node, &ovn_mirrors) {
+    SHASH_FOR_EACH (node, &ovn_mirrors) {
         struct ovn_mirror *m = node->data;
         sync_ovn_mirror(m, ovs_idl_txn, br_int, &ovs_local_mirror_ports);
-     }
+    }
 
     SHASH_FOR_EACH_SAFE (node, &ovn_mirrors) {
         ovn_mirror_delete(node->data);
diff --git a/tests/ovn.at b/tests/ovn.at
index 2593dcbe9..13163af10 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -19197,6 +19197,32 @@ OVN_CLEANUP([hv1],[hv2],[hv3])
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([Mirror - multiple mirror types attachment])
+ovn_start
+
+net_add n1
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.11
+ovs-vsctl add-port br-int lport1 -- set interface lport1 
external_ids:iface-id=lport1
+
+check ovn-nbctl ls-add ls1 -- lsp-add ls1 lport1 -- lsp-add ls1 lport2 -- \
+                mirror-add mirror0 gre 1 to-lport 172.16.0.100 -- \
+                mirror-add mirror1 erspan 2 to-lport 172.16.0.100 -- \
+                mirror-add mirror2 lport both lport2
+
+check ovn-nbctl lsp-attach-mirror lport1 mirror0 -- \
+                lsp-attach-mirror lport1 mirror1 -- \
+                lsp-attach-mirror lport1 mirror2
+
+check ovn-nbctl --wait=hv sync
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([Port Groups])
 AT_KEYWORDS([ovnpg])
-- 
2.48.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to