[ovs-dev] [PATCH ovn] controller: fix potential segmentation violation when removing ports

[Xavier Simonart]

If a logical switch port is added and connected to a logical router
port (through options: router-port) before the router port is
created, then this might cause further issues such as segmentation
violation when the switch and router ports are deleted.

Signed-off-by: Xavier Simonart <xsimo...@redhat.com>
---
 controller/local_data.c | 30 +++++++++++++-----------------
 tests/ovn.at            | 30 ++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/controller/local_data.c b/controller/local_data.c
index 7f874fc19..c5a948702 100644
--- a/controller/local_data.c
+++ b/controller/local_data.c
@@ -194,17 +194,7 @@ add_local_datapath_peer_port(
         return;
     }
 
-    bool present = false;
-    for (size_t i = 0; i < ld->n_peer_ports; i++) {
-        if (ld->peer_ports[i].local == pb) {
-            present = true;
-            break;
-        }
-    }
-
-    if (!present) {
-        local_datapath_peer_port_add(ld, pb, peer);
-    }
+    local_datapath_peer_port_add(ld, pb, peer);
 
     struct local_datapath *peer_ld =
         get_local_datapath(local_datapaths,
@@ -218,12 +208,6 @@ add_local_datapath_peer_port(
         return;
     }
 
-    for (size_t i = 0; i < peer_ld->n_peer_ports; i++) {
-        if (peer_ld->peer_ports[i].local == peer) {
-            return;
-        }
-    }
-
     local_datapath_peer_port_add(peer_ld, peer, pb);
 }
 
@@ -597,6 +581,13 @@ add_local_datapath__(struct ovsdb_idl_index 
*sbrec_datapath_binding_by_key,
                                              tracked_datapaths);
                     }
                     local_datapath_peer_port_add(ld, pb, peer);
+                    struct local_datapath *peer_ld =
+                        get_local_datapath(local_datapaths,
+                                           peer->datapath->tunnel_key);
+
+                    if (peer_ld != NULL) {
+                        local_datapath_peer_port_add(peer_ld, peer, pb);
+                    }
                 }
             }
         }
@@ -622,6 +613,11 @@ local_datapath_peer_port_add(struct local_datapath *ld,
                              const struct sbrec_port_binding *local,
                              const struct sbrec_port_binding *remote)
 {
+    for (size_t i = 0; i < ld->n_peer_ports; i++) {
+        if (ld->peer_ports[i].local == local) {
+            return;
+        }
+    }
     ld->n_peer_ports++;
     if (ld->n_peer_ports > ld->n_allocated_peer_ports) {
         size_t old_n_ports = ld->n_allocated_peer_ports;
diff --git a/tests/ovn.at b/tests/ovn.at
index bba2c9c1d..ae0918d55 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -32580,3 +32580,33 @@ check ovn-nbctl --wait=hv sync
 OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([router port add then remove - lsp first])
+ovn_start
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+ovs-vsctl -- add-port br-int hv1-vif1 -- \
+    set interface hv1-vif1 external-ids:iface-id=sw0-p1 \
+    options:tx_pcap=hv1/vif1-tx.pcap \
+    options:rxq_pcap=hv1/vif1-rx.pcap \
+    ofport-request=1
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl lr-add ro0
+check ovn-nbctl lsp-add sw0 sw0-p1
+check ovn-nbctl lsp-add sw0 lsp
+check ovn-nbctl lsp-set-type lsp router
+check ovn-nbctl lsp-set-options lsp router-port=lrp
+check ovn-nbctl lsp-set-addresses lsp  00:00:00:00:00:1
+check ovn-nbctl --wait=hv lrp-add ro0 lrp 00:00:00:00:00:1 
aef0:0:0:0:0:0:0:1/64
+check ovn-nbctl --wait=hv lsp-del lsp
+check ovn-nbctl lrp-del lrp
+check ovn-nbctl --wait=hv sync
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])
-- 
2.31.1

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev