Hi Mark, yes, you are right, this change needs to be handled in both incremental and recompute cases i submitted v2 <https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/> with recompute case which will prevent the binding from creating a local_binding for the port in iface_id if it is a container port and will throw a warning message.
On Tue, Apr 5, 2022 at 10:42 PM Mark Michelson <[email protected]> wrote: > 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
