Hi Mohammad,

The change itself seems like a good idea. I have a question. This code is only written for the incremental case, not the recompute case. Is this because this situation can only arise in the incremental case?

On 3/31/22 07:01, Mohammad Heib wrote:
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 <[email protected]>
---
  controller/binding.c | 11 +++++++++++
  tests/ovn.at         |  5 +++++
  2 files changed, 16 insertions(+)

diff --git a/controller/binding.c b/controller/binding.c
index 4d62b0858..1051651f4 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -2012,6 +2012,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 prent_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 0c2fe9f97..d32240cf4 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -28232,6 +28232,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


_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to