Message ID | 20200923171032.1354591-1-numans@ovn.org |
---|---|
State | Superseded |
Headers | show |
Series | [ovs-dev] controller: binding: Ignore changes to OVS interfaces which doesn't belong to int bridge. | expand |
On Wed, Sep 23, 2020 at 10:10 AM <numans@ovn.org> wrote: > > From: Numan Siddique <numans@ovn.org> > > Consider the below commands. > > ovn-nbctl ls-add sw0 > ovn-nbctl lsp-add sw0 sw0-p1 > > ovs-vsctl add-br br-temp > ovs-vsctl add port br-temp t1 -- set interface t1 external_ids:iface-id=sw0-p1 > > When ovn-controller handles the OVS db changes incrementally, it binds the lport > sw0-p1, which is wrong as t1 is not in the integration bridge - br-int. > > If a recompute is triggered, ovn-controller releases the lport sw0-p1. > > This patch fixes this issue. > > Fixes: 354bdba51abf ("ovn-controller: I-P for SB port binding and OVS interface in runtime_data.") > CC: Han Zhou <hzhou@ovn.org> > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > controller/binding.c | 21 +++++++++++++++++++++ > tests/ovn.at | 43 +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 64 insertions(+) > > diff --git a/controller/binding.c b/controller/binding.c > index 99e9fae301..bf4aa70c5d 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -1799,6 +1799,21 @@ is_iface_vif(const struct ovsrec_interface *iface_rec) > return true; > } > > +static bool > +is_iface_in_int_bridge(const struct ovsrec_interface *iface, > + const struct ovsrec_bridge *br_int) > +{ > + for (size_t i = 0; i < br_int->n_ports; i++) { > + const struct ovsrec_port *p = br_int->ports[i]; > + for (size_t j = 0; j < p->n_interfaces; j++) { > + if (!strcmp(iface->name, p->interfaces[j]->name)) { > + return true; > + } > + } > + } > + return false; > +} > + Hi Numan, would it be better to build a hmap first so that the check is faster (O(n) instead of O(n^2))? There can be a big number of ports in a large scale environment considering that all tunnel ports are on the br-int bridge. Or, we can create an OVSDB IDL index for the interface->name column, which would be O(nlogn), but the engine node interface would need to be updated with the new index added. > /* Returns true if the ovs interface changes were handled successfully, > * false otherwise. > */ > @@ -1904,6 +1919,12 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, > continue; > } > > + /* If the changed interface doesn't belong to the integration bridge, > + * ignore it. */ > + if (!is_iface_in_int_bridge(iface_rec, b_ctx_in->br_int)) { > + continue; > + } > + > const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id"); > int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; > if (iface_id && ofport > 0) { > diff --git a/tests/ovn.at b/tests/ovn.at > index 59e59f43f8..b6c8622ba4 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -22256,3 +22256,46 @@ grep conjunction | wc -l`]) > > OVN_CLEANUP([hv1]) > AT_CLEANUP > + > +AT_SETUP([ovn -- I-P of OVS interface changes which belong to non integration bridge]) > +ovn_start > + > +net_add n1 > +sim_add hv1 > +as hv1 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.10 > + > +ovn-nbctl ls-add sw0 > +ovn-nbctl lsp-add sw0 sw0-p1 > +ovn-nbctl lsp-add sw0 sw0-p2 > + > +as hv1 ovs-vsctl \ > + -- add-port br-int vif1 \ > + -- set Interface vif1 external_ids:iface-id=sw0-p1 > + > +# Wait for port to be bound. > +OVS_WAIT_UNTIL([test $(ovn-sbctl --columns _uuid --bare list chassis hv1 | wc -l) -eq 1]) > +ch=$(ovn-sbctl --columns _uuid --bare list chassis hv1) > +OVS_WAIT_UNTIL([test $(ovn-sbctl --columns chassis --bare list port_binding sw0-p1 | grep $ch -c) -eq 1]) > + > +as hv1 ovs-vsctl add-br br-temp > +as hv1 ovs-vsctl \ > + -- add-port br-temp t1 \ > + -- set Interface t1 external_ids:iface-id=sw0-p2 > + > +ovn-nbctl --wait=hv sync > + > +# hv1 ovn-controller should not bind sw0-p2. > +AT_CHECK([test $(ovn-sbctl --columns chassis --bare list port_binding sw0-p2 | grep $ch -c) -eq 0]) > + > +# Trigger recompute and sw0-p2 should not be claimed. > +as hv1 ovn-appctl -t ovn-controller recompute > +ovn-nbctl --wait=hv sync > + > +AT_CHECK([test $(ovn-sbctl --columns chassis --bare list port_binding sw0-p2 | grep $ch -c) -eq 0]) > + > +ovn-sbctl --columns chassis --bare list port_binding sw0-p2 > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > -- > 2.26.2 >
On Thu, Sep 24, 2020 at 5:55 AM Han Zhou <hzhou@ovn.org> wrote: > On Wed, Sep 23, 2020 at 10:10 AM <numans@ovn.org> wrote: > > > > From: Numan Siddique <numans@ovn.org> > > > > Consider the below commands. > > > > ovn-nbctl ls-add sw0 > > ovn-nbctl lsp-add sw0 sw0-p1 > > > > ovs-vsctl add-br br-temp > > ovs-vsctl add port br-temp t1 -- set interface t1 > external_ids:iface-id=sw0-p1 > > > > When ovn-controller handles the OVS db changes incrementally, it binds > the lport > > sw0-p1, which is wrong as t1 is not in the integration bridge - br-int. > > > > If a recompute is triggered, ovn-controller releases the lport sw0-p1. > > > > This patch fixes this issue. > > > > Fixes: 354bdba51abf ("ovn-controller: I-P for SB port binding and OVS > interface in runtime_data.") > > CC: Han Zhou <hzhou@ovn.org> > > Signed-off-by: Numan Siddique <numans@ovn.org> > > --- > > controller/binding.c | 21 +++++++++++++++++++++ > > tests/ovn.at | 43 +++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 64 insertions(+) > > > > diff --git a/controller/binding.c b/controller/binding.c > > index 99e9fae301..bf4aa70c5d 100644 > > --- a/controller/binding.c > > +++ b/controller/binding.c > > @@ -1799,6 +1799,21 @@ is_iface_vif(const struct ovsrec_interface > *iface_rec) > > return true; > > } > > > > +static bool > > +is_iface_in_int_bridge(const struct ovsrec_interface *iface, > > + const struct ovsrec_bridge *br_int) > > +{ > > + for (size_t i = 0; i < br_int->n_ports; i++) { > > + const struct ovsrec_port *p = br_int->ports[i]; > > + for (size_t j = 0; j < p->n_interfaces; j++) { > > + if (!strcmp(iface->name, p->interfaces[j]->name)) { > > + return true; > > + } > > + } > > + } > > + return false; > > +} > > + > > Hi Numan, would it be better to build a hmap first so that the check is > faster (O(n) instead of O(n^2))? There can be a big number of ports in a > large scale environment considering that all tunnel ports are on the br-int > bridge. Or, we can create an OVSDB IDL index for the interface->name > column, which would be O(nlogn), but the engine node interface would need > to be updated with the new index added. > > Hi Han, Thanks for the review. I agree with you. But I have a couple of questions on how to do it. An OVS port can have multiple OVS interfaces. But generally I have seen only one interface associated with an OVS port. If we assume that, the complexity of the function is_iface_in_int_bridge() would be O(n) right ? In order to improve this, we need to find the OVS port in the br-int using the OVS interface name. Given that an OVS port can have multiple interfaces, can we assume that we can find the OVS port in the br-int using the OVS interface name ? If you have any top of the head ideas on how to maintain the hmap please let me know. No worries otherwise. I will think about it. Thanks Numan > /* Returns true if the ovs interface changes were handled successfully, > > * false otherwise. > > */ > > @@ -1904,6 +1919,12 @@ binding_handle_ovs_interface_changes(struct > binding_ctx_in *b_ctx_in, > > continue; > > } > > > > + /* If the changed interface doesn't belong to the integration > bridge, > > + * ignore it. */ > > + if (!is_iface_in_int_bridge(iface_rec, b_ctx_in->br_int)) { > > + continue; > > + } > > + > > const char *iface_id = smap_get(&iface_rec->external_ids, > "iface-id"); > > int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; > > if (iface_id && ofport > 0) { > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 59e59f43f8..b6c8622ba4 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -22256,3 +22256,46 @@ grep conjunction | wc -l`]) > > > > OVN_CLEANUP([hv1]) > > AT_CLEANUP > > + > > +AT_SETUP([ovn -- I-P of OVS interface changes which belong to non > integration bridge]) > > +ovn_start > > + > > +net_add n1 > > +sim_add hv1 > > +as hv1 > > +ovs-vsctl add-br br-phys > > +ovn_attach n1 br-phys 192.168.0.10 > > + > > +ovn-nbctl ls-add sw0 > > +ovn-nbctl lsp-add sw0 sw0-p1 > > +ovn-nbctl lsp-add sw0 sw0-p2 > > + > > +as hv1 ovs-vsctl \ > > + -- add-port br-int vif1 \ > > + -- set Interface vif1 external_ids:iface-id=sw0-p1 > > + > > +# Wait for port to be bound. > > +OVS_WAIT_UNTIL([test $(ovn-sbctl --columns _uuid --bare list chassis hv1 > | wc -l) -eq 1]) > > +ch=$(ovn-sbctl --columns _uuid --bare list chassis hv1) > > +OVS_WAIT_UNTIL([test $(ovn-sbctl --columns chassis --bare list > port_binding sw0-p1 | grep $ch -c) -eq 1]) > > + > > +as hv1 ovs-vsctl add-br br-temp > > +as hv1 ovs-vsctl \ > > + -- add-port br-temp t1 \ > > + -- set Interface t1 external_ids:iface-id=sw0-p2 > > + > > +ovn-nbctl --wait=hv sync > > + > > +# hv1 ovn-controller should not bind sw0-p2. > > +AT_CHECK([test $(ovn-sbctl --columns chassis --bare list port_binding > sw0-p2 | grep $ch -c) -eq 0]) > > + > > +# Trigger recompute and sw0-p2 should not be claimed. > > +as hv1 ovn-appctl -t ovn-controller recompute > > +ovn-nbctl --wait=hv sync > > + > > +AT_CHECK([test $(ovn-sbctl --columns chassis --bare list port_binding > sw0-p2 | grep $ch -c) -eq 0]) > > + > > +ovn-sbctl --columns chassis --bare list port_binding sw0-p2 > > + > > +OVN_CLEANUP([hv1]) > > +AT_CLEANUP > > -- > > 2.26.2 > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
On Wed, Sep 23, 2020 at 5:40 PM Numan Siddique <numans@ovn.org> wrote: > > > On Thu, Sep 24, 2020 at 5:55 AM Han Zhou <hzhou@ovn.org> wrote: > >> On Wed, Sep 23, 2020 at 10:10 AM <numans@ovn.org> wrote: >> > >> > From: Numan Siddique <numans@ovn.org> >> > >> > Consider the below commands. >> > >> > ovn-nbctl ls-add sw0 >> > ovn-nbctl lsp-add sw0 sw0-p1 >> > >> > ovs-vsctl add-br br-temp >> > ovs-vsctl add port br-temp t1 -- set interface t1 >> external_ids:iface-id=sw0-p1 >> > >> > When ovn-controller handles the OVS db changes incrementally, it binds >> the lport >> > sw0-p1, which is wrong as t1 is not in the integration bridge - br-int. >> > >> > If a recompute is triggered, ovn-controller releases the lport sw0-p1. >> > >> > This patch fixes this issue. >> > >> > Fixes: 354bdba51abf ("ovn-controller: I-P for SB port binding and OVS >> interface in runtime_data.") >> > CC: Han Zhou <hzhou@ovn.org> >> > Signed-off-by: Numan Siddique <numans@ovn.org> >> > --- >> > controller/binding.c | 21 +++++++++++++++++++++ >> > tests/ovn.at | 43 +++++++++++++++++++++++++++++++++++++++++++ >> > 2 files changed, 64 insertions(+) >> > >> > diff --git a/controller/binding.c b/controller/binding.c >> > index 99e9fae301..bf4aa70c5d 100644 >> > --- a/controller/binding.c >> > +++ b/controller/binding.c >> > @@ -1799,6 +1799,21 @@ is_iface_vif(const struct ovsrec_interface >> *iface_rec) >> > return true; >> > } >> > >> > +static bool >> > +is_iface_in_int_bridge(const struct ovsrec_interface *iface, >> > + const struct ovsrec_bridge *br_int) >> > +{ >> > + for (size_t i = 0; i < br_int->n_ports; i++) { >> > + const struct ovsrec_port *p = br_int->ports[i]; >> > + for (size_t j = 0; j < p->n_interfaces; j++) { >> > + if (!strcmp(iface->name, p->interfaces[j]->name)) { >> > + return true; >> > + } >> > + } >> > + } >> > + return false; >> > +} >> > + >> >> Hi Numan, would it be better to build a hmap first so that the check is >> faster (O(n) instead of O(n^2))? There can be a big number of ports in a >> large scale environment considering that all tunnel ports are on the >> br-int >> bridge. Or, we can create an OVSDB IDL index for the interface->name >> column, which would be O(nlogn), but the engine node interface would need >> to be updated with the new index added. >> >> > Hi Han, > > Thanks for the review. I agree with you. But I have a couple of questions > on how to do it. An OVS port > can have multiple OVS interfaces. But generally I have seen only one > interface associated with an OVS port. > If we assume that, the complexity of the function is_iface_in_int_bridge() > would be O(n) right ? > > In order to improve this, we need to find the OVS port in the br-int using > the OVS interface name. > Given that an OVS port can have multiple interfaces, can we assume that we > can find the OVS port > in the br-int using the OVS interface name ? > > If you have any top of the head ideas on how to maintain the hmap please > let me know. No worries > otherwise. I will think about it. > > Thanks > Numan > Sorry I think I was fooled by the nested loop without looking at it more closely. Yes I agree with you in OVN use case it is 1-1 mapping between ports and interfaces. Using index would change this from O(n) to O(log(n)). But I am ok if you think that's not quite necessary here. So: Acked-by: Han Zhou <hzhou@ovn.org> > > /* Returns true if the ovs interface changes were handled successfully, >> > * false otherwise. >> > */ >> > @@ -1904,6 +1919,12 @@ binding_handle_ovs_interface_changes(struct >> binding_ctx_in *b_ctx_in, >> > continue; >> > } >> > >> > + /* If the changed interface doesn't belong to the integration >> bridge, >> > + * ignore it. */ >> > + if (!is_iface_in_int_bridge(iface_rec, b_ctx_in->br_int)) { >> > + continue; >> > + } >> > + >> > const char *iface_id = smap_get(&iface_rec->external_ids, >> "iface-id"); >> > int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; >> > if (iface_id && ofport > 0) { >> > diff --git a/tests/ovn.at b/tests/ovn.at >> > index 59e59f43f8..b6c8622ba4 100644 >> > --- a/tests/ovn.at >> > +++ b/tests/ovn.at >> > @@ -22256,3 +22256,46 @@ grep conjunction | wc -l`]) >> > >> > OVN_CLEANUP([hv1]) >> > AT_CLEANUP >> > + >> > +AT_SETUP([ovn -- I-P of OVS interface changes which belong to non >> integration bridge]) >> > +ovn_start >> > + >> > +net_add n1 >> > +sim_add hv1 >> > +as hv1 >> > +ovs-vsctl add-br br-phys >> > +ovn_attach n1 br-phys 192.168.0.10 >> > + >> > +ovn-nbctl ls-add sw0 >> > +ovn-nbctl lsp-add sw0 sw0-p1 >> > +ovn-nbctl lsp-add sw0 sw0-p2 >> > + >> > +as hv1 ovs-vsctl \ >> > + -- add-port br-int vif1 \ >> > + -- set Interface vif1 external_ids:iface-id=sw0-p1 >> > + >> > +# Wait for port to be bound. >> > +OVS_WAIT_UNTIL([test $(ovn-sbctl --columns _uuid --bare list chassis >> hv1 >> | wc -l) -eq 1]) >> > +ch=$(ovn-sbctl --columns _uuid --bare list chassis hv1) >> > +OVS_WAIT_UNTIL([test $(ovn-sbctl --columns chassis --bare list >> port_binding sw0-p1 | grep $ch -c) -eq 1]) >> > + >> > +as hv1 ovs-vsctl add-br br-temp >> > +as hv1 ovs-vsctl \ >> > + -- add-port br-temp t1 \ >> > + -- set Interface t1 external_ids:iface-id=sw0-p2 >> > + >> > +ovn-nbctl --wait=hv sync >> > + >> > +# hv1 ovn-controller should not bind sw0-p2. >> > +AT_CHECK([test $(ovn-sbctl --columns chassis --bare list port_binding >> sw0-p2 | grep $ch -c) -eq 0]) >> > + >> > +# Trigger recompute and sw0-p2 should not be claimed. >> > +as hv1 ovn-appctl -t ovn-controller recompute >> > +ovn-nbctl --wait=hv sync >> > + >> > +AT_CHECK([test $(ovn-sbctl --columns chassis --bare list port_binding >> sw0-p2 | grep $ch -c) -eq 0]) >> > + >> > +ovn-sbctl --columns chassis --bare list port_binding sw0-p2 >> > + >> > +OVN_CLEANUP([hv1]) >> > +AT_CLEANUP >> > -- >> > 2.26.2 >> > >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >>
On Thu, Sep 24, 2020 at 6:19 AM Han Zhou <hzhou@ovn.org> wrote: > On Wed, Sep 23, 2020 at 5:40 PM Numan Siddique <numans@ovn.org> wrote: > > > > > > > On Thu, Sep 24, 2020 at 5:55 AM Han Zhou <hzhou@ovn.org> wrote: > > > >> On Wed, Sep 23, 2020 at 10:10 AM <numans@ovn.org> wrote: > >> > > >> > From: Numan Siddique <numans@ovn.org> > >> > > >> > Consider the below commands. > >> > > >> > ovn-nbctl ls-add sw0 > >> > ovn-nbctl lsp-add sw0 sw0-p1 > >> > > >> > ovs-vsctl add-br br-temp > >> > ovs-vsctl add port br-temp t1 -- set interface t1 > >> external_ids:iface-id=sw0-p1 > >> > > >> > When ovn-controller handles the OVS db changes incrementally, it binds > >> the lport > >> > sw0-p1, which is wrong as t1 is not in the integration bridge - > br-int. > >> > > >> > If a recompute is triggered, ovn-controller releases the lport sw0-p1. > >> > > >> > This patch fixes this issue. > >> > > >> > Fixes: 354bdba51abf ("ovn-controller: I-P for SB port binding and OVS > >> interface in runtime_data.") > >> > CC: Han Zhou <hzhou@ovn.org> > >> > Signed-off-by: Numan Siddique <numans@ovn.org> > >> > --- > >> > controller/binding.c | 21 +++++++++++++++++++++ > >> > tests/ovn.at | 43 > +++++++++++++++++++++++++++++++++++++++++++ > >> > 2 files changed, 64 insertions(+) > >> > > >> > diff --git a/controller/binding.c b/controller/binding.c > >> > index 99e9fae301..bf4aa70c5d 100644 > >> > --- a/controller/binding.c > >> > +++ b/controller/binding.c > >> > @@ -1799,6 +1799,21 @@ is_iface_vif(const struct ovsrec_interface > >> *iface_rec) > >> > return true; > >> > } > >> > > >> > +static bool > >> > +is_iface_in_int_bridge(const struct ovsrec_interface *iface, > >> > + const struct ovsrec_bridge *br_int) > >> > +{ > >> > + for (size_t i = 0; i < br_int->n_ports; i++) { > >> > + const struct ovsrec_port *p = br_int->ports[i]; > >> > + for (size_t j = 0; j < p->n_interfaces; j++) { > >> > + if (!strcmp(iface->name, p->interfaces[j]->name)) { > >> > + return true; > >> > + } > >> > + } > >> > + } > >> > + return false; > >> > +} > >> > + > >> > >> Hi Numan, would it be better to build a hmap first so that the check is > >> faster (O(n) instead of O(n^2))? There can be a big number of ports in a > >> large scale environment considering that all tunnel ports are on the > >> br-int > >> bridge. Or, we can create an OVSDB IDL index for the interface->name > >> column, which would be O(nlogn), but the engine node interface would > need > >> to be updated with the new index added. > >> > >> > > Hi Han, > > > > Thanks for the review. I agree with you. But I have a couple of questions > > on how to do it. An OVS port > > can have multiple OVS interfaces. But generally I have seen only one > > interface associated with an OVS port. > > If we assume that, the complexity of the function > is_iface_in_int_bridge() > > would be O(n) right ? > > > > In order to improve this, we need to find the OVS port in the br-int > using > > the OVS interface name. > > Given that an OVS port can have multiple interfaces, can we assume that > we > > can find the OVS port > > in the br-int using the OVS interface name ? > > > > If you have any top of the head ideas on how to maintain the hmap please > > let me know. No worries > > otherwise. I will think about it. > > > > Thanks > > Numan > > > > Sorry I think I was fooled by the nested loop without looking at it more > closely. Yes I agree with you in OVN use case it is 1-1 mapping between > ports and interfaces. > Using index would change this from O(n) to O(log(n)). But I am ok if you > think that's not quite necessary here. So: > > Acked-by: Han Zhou <hzhou@ovn.org> > Thanks for the Ack. I think it's not quite necessary here. I submitted v2 by moving the call to is_iface_in_int_bridge() only if external_ids:iface-id is set and ofport > 0. Request to take a look - https://patchwork.ozlabs.org/project/ovn/patch/20200924014435.1503008-1-numans@ovn.org/ and see if it is fine. I have added your Acked-by too. Thanks Numan > > > > /* Returns true if the ovs interface changes were handled > successfully, > >> > * false otherwise. > >> > */ > >> > @@ -1904,6 +1919,12 @@ binding_handle_ovs_interface_changes(struct > >> binding_ctx_in *b_ctx_in, > >> > continue; > >> > } > >> > > >> > + /* If the changed interface doesn't belong to the integration > >> bridge, > >> > + * ignore it. */ > >> > + if (!is_iface_in_int_bridge(iface_rec, b_ctx_in->br_int)) { > >> > + continue; > >> > + } > >> > + > >> > const char *iface_id = smap_get(&iface_rec->external_ids, > >> "iface-id"); > >> > int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : > 0; > >> > if (iface_id && ofport > 0) { > >> > diff --git a/tests/ovn.at b/tests/ovn.at > >> > index 59e59f43f8..b6c8622ba4 100644 > >> > --- a/tests/ovn.at > >> > +++ b/tests/ovn.at > >> > @@ -22256,3 +22256,46 @@ grep conjunction | wc -l`]) > >> > > >> > OVN_CLEANUP([hv1]) > >> > AT_CLEANUP > >> > + > >> > +AT_SETUP([ovn -- I-P of OVS interface changes which belong to non > >> integration bridge]) > >> > +ovn_start > >> > + > >> > +net_add n1 > >> > +sim_add hv1 > >> > +as hv1 > >> > +ovs-vsctl add-br br-phys > >> > +ovn_attach n1 br-phys 192.168.0.10 > >> > + > >> > +ovn-nbctl ls-add sw0 > >> > +ovn-nbctl lsp-add sw0 sw0-p1 > >> > +ovn-nbctl lsp-add sw0 sw0-p2 > >> > + > >> > +as hv1 ovs-vsctl \ > >> > + -- add-port br-int vif1 \ > >> > + -- set Interface vif1 external_ids:iface-id=sw0-p1 > >> > + > >> > +# Wait for port to be bound. > >> > +OVS_WAIT_UNTIL([test $(ovn-sbctl --columns _uuid --bare list chassis > >> hv1 > >> | wc -l) -eq 1]) > >> > +ch=$(ovn-sbctl --columns _uuid --bare list chassis hv1) > >> > +OVS_WAIT_UNTIL([test $(ovn-sbctl --columns chassis --bare list > >> port_binding sw0-p1 | grep $ch -c) -eq 1]) > >> > + > >> > +as hv1 ovs-vsctl add-br br-temp > >> > +as hv1 ovs-vsctl \ > >> > + -- add-port br-temp t1 \ > >> > + -- set Interface t1 external_ids:iface-id=sw0-p2 > >> > + > >> > +ovn-nbctl --wait=hv sync > >> > + > >> > +# hv1 ovn-controller should not bind sw0-p2. > >> > +AT_CHECK([test $(ovn-sbctl --columns chassis --bare list port_binding > >> sw0-p2 | grep $ch -c) -eq 0]) > >> > + > >> > +# Trigger recompute and sw0-p2 should not be claimed. > >> > +as hv1 ovn-appctl -t ovn-controller recompute > >> > +ovn-nbctl --wait=hv sync > >> > + > >> > +AT_CHECK([test $(ovn-sbctl --columns chassis --bare list port_binding > >> sw0-p2 | grep $ch -c) -eq 0]) > >> > + > >> > +ovn-sbctl --columns chassis --bare list port_binding sw0-p2 > >> > + > >> > +OVN_CLEANUP([hv1]) > >> > +AT_CLEANUP > >> > -- > >> > 2.26.2 > >> > > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >> > >> > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
diff --git a/controller/binding.c b/controller/binding.c index 99e9fae301..bf4aa70c5d 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -1799,6 +1799,21 @@ is_iface_vif(const struct ovsrec_interface *iface_rec) return true; } +static bool +is_iface_in_int_bridge(const struct ovsrec_interface *iface, + const struct ovsrec_bridge *br_int) +{ + for (size_t i = 0; i < br_int->n_ports; i++) { + const struct ovsrec_port *p = br_int->ports[i]; + for (size_t j = 0; j < p->n_interfaces; j++) { + if (!strcmp(iface->name, p->interfaces[j]->name)) { + return true; + } + } + } + return false; +} + /* Returns true if the ovs interface changes were handled successfully, * false otherwise. */ @@ -1904,6 +1919,12 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, continue; } + /* If the changed interface doesn't belong to the integration bridge, + * ignore it. */ + if (!is_iface_in_int_bridge(iface_rec, b_ctx_in->br_int)) { + continue; + } + const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id"); int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; if (iface_id && ofport > 0) { diff --git a/tests/ovn.at b/tests/ovn.at index 59e59f43f8..b6c8622ba4 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -22256,3 +22256,46 @@ grep conjunction | wc -l`]) OVN_CLEANUP([hv1]) AT_CLEANUP + +AT_SETUP([ovn -- I-P of OVS interface changes which belong to non integration bridge]) +ovn_start + +net_add n1 +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.10 + +ovn-nbctl ls-add sw0 +ovn-nbctl lsp-add sw0 sw0-p1 +ovn-nbctl lsp-add sw0 sw0-p2 + +as hv1 ovs-vsctl \ + -- add-port br-int vif1 \ + -- set Interface vif1 external_ids:iface-id=sw0-p1 + +# Wait for port to be bound. +OVS_WAIT_UNTIL([test $(ovn-sbctl --columns _uuid --bare list chassis hv1 | wc -l) -eq 1]) +ch=$(ovn-sbctl --columns _uuid --bare list chassis hv1) +OVS_WAIT_UNTIL([test $(ovn-sbctl --columns chassis --bare list port_binding sw0-p1 | grep $ch -c) -eq 1]) + +as hv1 ovs-vsctl add-br br-temp +as hv1 ovs-vsctl \ + -- add-port br-temp t1 \ + -- set Interface t1 external_ids:iface-id=sw0-p2 + +ovn-nbctl --wait=hv sync + +# hv1 ovn-controller should not bind sw0-p2. +AT_CHECK([test $(ovn-sbctl --columns chassis --bare list port_binding sw0-p2 | grep $ch -c) -eq 0]) + +# Trigger recompute and sw0-p2 should not be claimed. +as hv1 ovn-appctl -t ovn-controller recompute +ovn-nbctl --wait=hv sync + +AT_CHECK([test $(ovn-sbctl --columns chassis --bare list port_binding sw0-p2 | grep $ch -c) -eq 0]) + +ovn-sbctl --columns chassis --bare list port_binding sw0-p2 + +OVN_CLEANUP([hv1]) +AT_CLEANUP