diff mbox series

[ovs-dev] binding.c: update ld->peers when lsp type updated

Message ID 20220803142344.1950403-1-mheib@redhat.com
State Not Applicable
Headers show
Series [ovs-dev] binding.c: update ld->peers when lsp type updated | expand

Checks

Context Check Description
ovsrobot/apply-robot fail apply and check: fail
ovsrobot/intel-ovs-compilation fail test: fail

Commit Message

Mohammad Heib Aug. 3, 2022, 2:23 p.m. UTC 
The local_datapath->peer_ports list contains peers pointers
to lsp<-->lrp ports that are supposed to be router end ports,
those pointers are added and deleted to the  local_datapath->peer_ports
when logical switch port of type router are added or deleted from the database.

The deletion and creation of those ports are handled very well when the LSP type
is a router, but if in any case, the user has changed the LSP type from router
port to any other LSP type the ld->peer_ports will keep pointing to this port
and if it was deleted it will keep pointing to invalid memory regions and that
could lead to invalid memory access in the ovn-controller.

To solve the above issue this patch will update the local_dataoath->peer_ports
whenever a lport is updated.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2077078
Co-authored-by: Xavier Simonart <xsimonar@redhat.com>
Signed-off-by: Mohammad Heib <mheib@redhat.com>
Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
---
 controller/binding.c | 33 +++++++++++++++++++++++++++++++++
 tests/ovn.at         | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

Comments

0-day Robot Aug. 3, 2022, 2:38 p.m. UTC | #1 
Bleep bloop.  Greetings Mohammad Heib, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
error: sha1 information is lacking or useless (controller/binding.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 binding.c: update ld->peers when lsp type updated
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Patch skipped due to previous failure.

Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/controller/binding.c b/controller/binding.c
index 19b28369f..3bd8f2ada 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -505,6 +505,38 @@  remove_related_lport(const struct sbrec_port_binding *pb,
     }
 }
 
+/*
+ * Update local_datapath peers when port type changed
+ * and remove irrelevant ports from this list.
+ */
+static void
+update_ld_peers(const struct sbrec_port_binding *pb,
+                 struct hmap *local_datapaths)
+{
+    struct local_datapath *ld = get_local_datapath(
+                                local_datapaths, pb->datapath->tunnel_key);
+    if (!ld) {
+        return;
+    }
+
+    /*
+     * This will handle cases where the pb type was explicitly
+     * changed from router type to any other port type and will
+     * remove it from the ld peers list.
+     */
+    enum en_lport_type type = get_lport_type(pb);
+    int num_peers = ld->n_peer_ports;
+    if (type != LP_PATCH) {
+        remove_local_datapath_peer_port(pb, ld, local_datapaths);
+        if (num_peers != ld->n_peer_ports) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+            VLOG_DBG_RL(&rl,
+                        "removing lport %s from the ld peers list",
+                        pb->logical_port);
+        }
+    }
+}
+
 static void
 delete_active_pb_ras_pd(const struct sbrec_port_binding *pb,
                         struct shash *ras_pd_map)
@@ -2612,6 +2644,7 @@  delete_done:
             continue;
         }
 
+        update_ld_peers(pb, b_ctx_out->local_datapaths);
         update_active_pb_ras_pd(pb, b_ctx_out->local_active_ports_ipv6_pd,
                                 "ipv6_prefix_delegation");
 
diff --git a/tests/ovn.at b/tests/ovn.at
index 3ba6ced4e..54673e9ec 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -32349,3 +32349,36 @@  AT_CHECK([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:30") = 0])
 OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([router port type update and then remove])
+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 lrp-add ro0 lrp 00:00:00:00:00:1 aef0:0:0:0:0:0:0:1/64
+check ovn-nbctl set Logical_Router_Port lrp ipv6_ra_configs:send_periodic=true -- set Logical_Router_Port lrp ipv6_ra_configs:address_mode=slaac -- set Logical_Router_Port lrp ipv6_ra_configs:mtu=1280 -- set Logical_Router_Port lrp ipv6_ra_configs:max_interval=2 -- set Logical_Router_Port lrp ipv6_ra_configs:min_interval=1
+check ovn-nbctl lsp-set-type lsp localnet
+check ovn-nbctl --wait=hv sync
+check ovn-nbctl lsp-del lsp
+check ovn-nbctl lrp-del lrp
+check ovn-nbctl --wait=hv sync
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])