diff mbox series

[ovs-dev,v2] controller/binding: prevent claiming container lport to ovs bridge

Message ID 20220427090842.1842824-1-mheib@redhat.com
State Superseded
Headers show
Series [ovs-dev,v2] controller/binding: prevent claiming container lport to ovs bridge | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test fail github build: failed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Mohammad Heib April 27, 2022, 9:08 a.m. UTC 
currently ovn-controller allow users to claim lport of type container
to ovs bridge which is invalid use-case.

This patch will prevent such invalid use-cases by ignoring the claiming
requests for container lports and will throw a warning message to the
controller logs.

Signed-off-by: Mohammad Heib <mheib@redhat.com>
---
 controller/binding.c | 26 ++++++++++++++++++++++++++
 tests/ovn.at         | 11 +++++++++++
 2 files changed, 37 insertions(+)
diff mbox series

Patch

diff --git a/controller/binding.c b/controller/binding.c
index 7281b0485..19d0b3783 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -1497,6 +1497,21 @@  build_local_bindings(struct binding_ctx_in *b_ctx_in,
                 struct local_binding *lbinding =
                     local_binding_find(local_bindings, iface_id);
                 if (!lbinding) {
+                    const struct sbrec_port_binding *pb = NULL;
+                    pb = lport_lookup_by_name(
+                                        b_ctx_in->sbrec_port_binding_by_name,
+                                        iface_id);
+                    if (pb && (get_lport_type(pb) == LP_CONTAINER)) {
+                        static struct vlog_rate_limit rl =
+                                                  VLOG_RATE_LIMIT_INIT(1, 1);
+                        VLOG_WARN_RL(&rl,
+                                   "Can't claim lport %s of type container to "
+                                   "OVS bridge,\nplease remove the lport"
+                                   " parent_name before claiming it.",
+                                   pb->logical_port);
+                        continue;
+                    }
+
                     lbinding = local_binding_create(iface_id, iface_rec);
                     local_binding_add(local_bindings, lbinding);
                 } else {
@@ -2022,6 +2037,17 @@  binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in,
         int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0;
         if (iface_id && ofport > 0 &&
                 is_iface_in_int_bridge(iface_rec, b_ctx_in->br_int)) {
+            const struct sbrec_port_binding *pb = NULL;
+            pb = lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name,
+                                      iface_id);
+            if (pb && (get_lport_type(pb) == LP_CONTAINER)) {
+                static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+                VLOG_WARN_RL(&rl, "Can't claim lport %s of type container to "
+                             "OVS bridge,\nplease remove the lport parent_name"
+                             " before claiming it.", pb->logical_port);
+                continue;
+            }
+
             handled = consider_iface_claim(iface_rec, iface_id, b_ctx_in,
                                            b_ctx_out, qos_map_ptr);
             if (!handled) {
diff --git a/tests/ovn.at b/tests/ovn.at
index 34b6abfc0..bc59f3f13 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -28535,6 +28535,11 @@  check ovn-nbctl --wait=sb set logical_switch_port vm1 parent_name=vm-cont1
 
 wait_for_ports_up
 
+# Try to claim container port to ovs
+check ovn-nbctl set logical_switch_port vm-cont2 parent_name=vm2
+check as hv1 ovs-vsctl set Interface vm1 external_ids:iface-id=vm-cont2
+AT_CHECK([test 1 = `cat hv1/ovn-controller.log |grep -c "claim lport vm-cont2 of type container"`])
+
 # Delete vm1, vm-cont1 and vm-cont2 and recreate again.
 check ovn-nbctl lsp-del vm1
 check ovn-nbctl lsp-del vm-cont1
@@ -28546,6 +28551,12 @@  check ovn-nbctl lsp-add ls vm-cont1 vm1 1
 check ovn-nbctl lsp-add ls vm-cont2 vm1 2
 
 wait_for_ports_up
+check as hv1 ovn-appctl -t ovn-controller debug/pause
+# Try to claim container port to ovs with recompute
+check ovn-nbctl set logical_switch_port vm-cont2 parent_name=vm2
+check as hv1 ovs-vsctl set Interface vm1 external_ids:iface-id=vm-cont2
+check as hv1 ovn-appctl -t ovn-controller debug/resume
+AT_CHECK([test 1 = `cat hv1/ovn-controller.log |grep -c "claim lport vm-cont2 of type container"`])
 
 # Make vm1 as a child port of some non existent lport - foo. vm1, vm1-cont1 and
 # vm1-cont2 should be released.