diff mbox series

[ovs-dev,branch-2.12,4/4] ovn-controller: Skip vport bindings done through OVS external_ids:iface-id.

Message ID 20200407110408.7801.39822.stgit@dceara.remote.csb
State New
Headers show
Series OVN: Backport virtual port related fixes. | expand

Commit Message

Dumitru Ceara April 7, 2020, 11:04 a.m. UTC
Port bindings of type "virtual" should not have an associated OVS port
in the integration bridge. If this is the case, it's a misconfig and
ovn-controller should ignore it.

If such a situation is detected, ovn-controller will also log a warning
message to inform the user about the wrong configuration.

Reported-at: https://bugzilla.redhat.com/1818844
CC: Numan Siddique <nusiddiq@redhat.com>
Fixes: 054f4c85c413 ("Add a new logical switch port type - 'virtual'")
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>

(cherry picked from OVN commit 1cadfb515124e6bf394c4e22a459ed7722c7a0d5)
---
 ovn/controller/binding.c |   12 ++++++++++++
 tests/ovn.at             |   20 ++++++++++++++++++++
 2 files changed, 32 insertions(+)

Comments

0-day Robot April 7, 2020, 1:13 p.m. UTC | #1
Bleep bloop.  Greetings Dumitru Ceara, 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.


checkpatch:
WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Numan Siddique <numans@ovn.org>
Lines checked: 89, Warnings: 1, Errors: 0


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/ovn/controller/binding.c b/ovn/controller/binding.c
index 3f411f0..32018cd 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -435,6 +435,18 @@  is_our_chassis(const struct sbrec_chassis *chassis_rec,
     const struct ovsrec_interface *iface_rec
         = shash_find_data(lport_to_iface, binding_rec->logical_port);
 
+    /* Ports of type "virtual" should never be explicitly bound to an OVS
+     * port in the integration bridge. If that's the case, ignore the binding
+     * and log a warning.
+     */
+    if (iface_rec && !strcmp(binding_rec->type, "virtual")) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+        VLOG_WARN_RL(&rl,
+                     "Virtual port %s should not be bound to OVS port %s",
+                     binding_rec->logical_port, iface_rec->name);
+        return false;
+    }
+
     bool our_chassis = false;
     if (iface_rec
         || (binding_rec->parent_port && binding_rec->parent_port[0] &&
diff --git a/tests/ovn.at b/tests/ovn.at
index fc76927..0162029 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -14477,6 +14477,11 @@  ovs-vsctl -- add-port br-int hv1-vif2 -- \
     options:tx_pcap=hv1/vif2-tx.pcap \
     options:rxq_pcap=hv1/vif2-rx.pcap \
     ofport-request=2
+ovs-vsctl -- add-port br-int hv1-vif3 -- \
+    set interface hv1-vif3 \
+    options:tx_pcap=hv1/vif3-tx.pcap \
+    options:rxq_pcap=hv1/vif3-rx.pcap \
+    ofport-request=3
 
 sim_add hv2
 as hv2
@@ -14570,6 +14575,21 @@  logical_port=sw0-vir) = x], [0], [])
 AT_CHECK([test x$(ovn-sbctl --bare --columns virtual_parent find port_binding \
 logical_port=sw0-vir) = x])
 
+# Try to bind sw0-vir directly to an OVS port. This should be ignored by
+# ovn-controller.
+as hv1
+ovs-vsctl set interface hv1-vif3 external-ids:iface-id=sw0-vir
+
+AT_CHECK([test x$(ovn-sbctl --bare --columns chassis find port_binding \
+logical_port=sw0-vir) = x], [0], [])
+
+# Cleanup hv1-vif3.
+as hv1
+ovs-vsctl del-port hv1-vif3
+
+AT_CHECK([test x$(ovn-sbctl --bare --columns chassis find port_binding \
+logical_port=sw0-vir) = x], [0], [])
+
 # From sw0-p0 send GARP for 10.0.0.10. hv1 should claim sw0-vir
 # and sw0-p1 should be its virtual_parent.
 eth_src=505400000003