|
 |
bbaaef |
From 6c0f9dbf5d22fe7fdbe9bf2a05d2bed00881cff8 Mon Sep 17 00:00:00 2001
|
|
|
bbaaef |
From: Dumitru Ceara <dceara@redhat.com>
|
|
|
bbaaef |
Date: Tue, 31 Mar 2020 13:47:04 +0200
|
|
|
bbaaef |
Subject: [PATCH] ovn-controller: Fix potential segfault with "virtual"
|
|
|
bbaaef |
port bindings.
|
|
|
bbaaef |
|
|
|
bbaaef |
Even though ovn-controller tries to set port_binding->chassis to NULL
|
|
|
bbaaef |
every time port_binding->virtual_parent is set to NULL for bindings of
|
|
|
bbaaef |
type="virtual", there's no way to enforce that an operator doesn't
|
|
|
bbaaef |
manually clear the "virtual_parent" column in the Southbound database.
|
|
|
bbaaef |
|
|
|
bbaaef |
In such scenario ovn-controller would crash because of trying to
|
|
|
bbaaef |
dereference the NULL port_binding->virtual_parent column.
|
|
|
bbaaef |
|
|
|
bbaaef |
Add an extra check and release "virtual" port bindings that have
|
|
|
bbaaef |
"virtual_parent" NULL.
|
|
|
bbaaef |
|
|
|
bbaaef |
Reported-at: https://bugzilla.redhat.com/1818844
|
|
|
bbaaef |
CC: Numan Siddique <nusiddiq@redhat.com>
|
|
|
bbaaef |
Fixes: 054f4c85c413 ("Add a new logical switch port type - 'virtual'")
|
|
|
bbaaef |
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
|
|
|
bbaaef |
Signed-off-by: Numan Siddique <numans@ovn.org>
|
|
|
bbaaef |
(cherry picked from upstream commit 5b3e9879be2b6c9b07ed5c9e073f1c24080a49f7)
|
|
|
bbaaef |
|
|
|
bbaaef |
Change-Id: Icced69c2871d43316ec06f71f78024939c863225
|
|
|
bbaaef |
---
|
|
|
bbaaef |
ovn/controller/binding.c | 26 +++++++++++++++-----------
|
|
|
bbaaef |
tests/ovn.at | 18 ++++++++++++++++++
|
|
|
bbaaef |
2 files changed, 33 insertions(+), 11 deletions(-)
|
|
|
bbaaef |
|
|
|
bbaaef |
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
|
|
|
bbaaef |
index 1ae9151..bf7b5ff 100644
|
|
|
bbaaef |
--- a/ovn/controller/binding.c
|
|
|
bbaaef |
+++ b/ovn/controller/binding.c
|
|
|
bbaaef |
@@ -614,22 +614,26 @@ consider_local_virtual_port(struct ovsdb_idl_index *sbrec_port_binding_by_name,
|
|
|
bbaaef |
const struct sbrec_chassis *chassis_rec,
|
|
|
bbaaef |
const struct sbrec_port_binding *binding_rec)
|
|
|
bbaaef |
{
|
|
|
bbaaef |
+ if (binding_rec->virtual_parent) {
|
|
|
bbaaef |
+ const struct sbrec_port_binding *parent =
|
|
|
bbaaef |
+ lport_lookup_by_name(sbrec_port_binding_by_name,
|
|
|
bbaaef |
+ binding_rec->virtual_parent);
|
|
|
bbaaef |
+ if (parent && parent->chassis == chassis_rec) {
|
|
|
bbaaef |
+ return;
|
|
|
bbaaef |
+ }
|
|
|
bbaaef |
+ }
|
|
|
bbaaef |
+
|
|
|
bbaaef |
/* pinctrl module takes care of binding the ports of type 'virtual'.
|
|
|
bbaaef |
* Release such ports if their virtual parents are no longer claimed by
|
|
|
bbaaef |
* this chassis.
|
|
|
bbaaef |
*/
|
|
|
bbaaef |
- const struct sbrec_port_binding *parent =
|
|
|
bbaaef |
- lport_lookup_by_name(sbrec_port_binding_by_name,
|
|
|
bbaaef |
- binding_rec->virtual_parent);
|
|
|
bbaaef |
- if (!parent || parent->chassis != chassis_rec) {
|
|
|
bbaaef |
- VLOG_INFO("Releasing lport %s from this chassis.",
|
|
|
bbaaef |
- binding_rec->logical_port);
|
|
|
bbaaef |
- if (binding_rec->encap) {
|
|
|
bbaaef |
- sbrec_port_binding_set_encap(binding_rec, NULL);
|
|
|
bbaaef |
- }
|
|
|
bbaaef |
- sbrec_port_binding_set_chassis(binding_rec, NULL);
|
|
|
bbaaef |
- sbrec_port_binding_set_virtual_parent(binding_rec, NULL);
|
|
|
bbaaef |
+ VLOG_INFO("Releasing lport %s from this chassis.",
|
|
|
bbaaef |
+ binding_rec->logical_port);
|
|
|
bbaaef |
+ if (binding_rec->encap) {
|
|
|
bbaaef |
+ sbrec_port_binding_set_encap(binding_rec, NULL);
|
|
|
bbaaef |
}
|
|
|
bbaaef |
+ sbrec_port_binding_set_chassis(binding_rec, NULL);
|
|
|
bbaaef |
+ sbrec_port_binding_set_virtual_parent(binding_rec, NULL);
|
|
|
bbaaef |
}
|
|
|
bbaaef |
|
|
|
bbaaef |
static void
|
|
|
bbaaef |
diff --git a/tests/ovn.at b/tests/ovn.at
|
|
|
bbaaef |
index 0cd0985..94e5ecd 100644
|
|
|
bbaaef |
--- a/tests/ovn.at
|
|
|
bbaaef |
+++ b/tests/ovn.at
|
|
|
bbaaef |
@@ -14755,6 +14755,24 @@ AT_CHECK([cat lflows.txt], [0], [dnl
|
|
|
bbaaef |
table=11(lr_in_arp_resolve ), priority=100 , match=(outport == "lr0-sw0" && reg0 == 10.0.0.10), action=(eth.dst = 50:54:00:00:00:03; next;)
|
|
|
bbaaef |
])
|
|
|
bbaaef |
|
|
|
bbaaef |
+# Forcibly clear virtual_parent. ovn-controller should release the binding
|
|
|
bbaaef |
+# gracefully.
|
|
|
bbaaef |
+pb_uuid=$(ovn-sbctl --bare --columns _uuid find port_binding logical_port=sw0-vir)
|
|
|
bbaaef |
+ovn-sbctl clear port_binding $pb_uuid virtual_parent
|
|
|
bbaaef |
+
|
|
|
bbaaef |
+OVS_WAIT_UNTIL([test x$(ovn-sbctl --bare --columns chassis find port_binding \
|
|
|
bbaaef |
+logical_port=sw0-vir) = x])
|
|
|
bbaaef |
+
|
|
|
bbaaef |
+# From sw0-p0 resend GARP for 10.0.0.10. hv1 should reclaim sw0-vir
|
|
|
bbaaef |
+# and sw0-p1 should be its virtual_parent.
|
|
|
bbaaef |
+send_garp 1 1 $eth_src $eth_dst $spa $tpa
|
|
|
bbaaef |
+
|
|
|
bbaaef |
+OVS_WAIT_UNTIL([test x$(ovn-sbctl --bare --columns chassis find port_binding \
|
|
|
bbaaef |
+logical_port=sw0-vir) = x$hv1_ch_uuid], [0], [])
|
|
|
bbaaef |
+
|
|
|
bbaaef |
+AT_CHECK([test x$(ovn-sbctl --bare --columns virtual_parent find port_binding \
|
|
|
bbaaef |
+logical_port=sw0-vir) = xsw0-p1])
|
|
|
bbaaef |
+
|
|
|
bbaaef |
# From sw0-p3 send GARP for 10.0.0.10. hv1 should claim sw0-vir
|
|
|
bbaaef |
# and sw0-p3 should be its virtual_parent.
|
|
|
bbaaef |
eth_src=505400000005
|
|
|
bbaaef |
--
|
|
|
bbaaef |
1.8.3.1
|
|
|
bbaaef |
|