| Message ID | 20210903192748.1408062-4-frode.nordahl@canonical.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Introduce infrastructure for plugging providers | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Fri, Sep 3, 2021 at 3:28 PM Frode Nordahl <frode.nordahl@canonical.com> wrote: > > Improve the efficiency of the requested-chassis feature by using > the new Southbound Port_Binding:requested_chassis column instead > of each chassis performing option processing and string comparison. > > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> I've one comment which seems to be discussed in v3 of the first patch. Looks to me this patch would break the existing behavior (which the patch 2 addresses). Right now when CMS sets requested-chassis=foo for a logical port 'P1', then other ovn-controllers will not claim 'P1' if there is a VIF for this even if chassis 'foo' doesn't exist. With this patch series, other ovn-controller will claim 'P1' for the above scenario I mentioned. Is this going to be a problem ? Or are we going to break some existing deployments ? I'd suggest not breaking this scenario. If the port_binding->options:requested-chassis is set, then ovn-controller should not claim that port binding if the port_binding->requested_chassis is set. In other words ovn-controller should claim only if port_binding->requested_chassis == this_chassis when the port_binding->options:requested-chassis is set. Does this make sense ? Numan > --- > controller/binding.c | 29 ++++++++++++----------------- > controller/ovn-controller.c | 3 +++ > controller/physical.c | 7 ++----- > 3 files changed, 17 insertions(+), 22 deletions(-) > > diff --git a/controller/binding.c b/controller/binding.c > index 34935bb9c..938e1d81d 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -1051,11 +1051,10 @@ is_binding_lport_this_chassis(struct binding_lport *b_lport, > > static bool > can_bind_on_this_chassis(const struct sbrec_chassis *chassis_rec, > - const char *requested_chassis) > + const struct sbrec_port_binding *pb) > { > - return !requested_chassis || !requested_chassis[0] > - || !strcmp(requested_chassis, chassis_rec->name) > - || !strcmp(requested_chassis, chassis_rec->hostname); > + return !pb->requested_chassis > + || chassis_rec == pb->requested_chassis; > } > > /* Returns 'true' if the 'lbinding' has binding lports of type LP_CONTAINER, > @@ -1093,7 +1092,7 @@ release_binding_lport(const struct sbrec_chassis *chassis_rec, > > static bool > consider_vif_lport_(const struct sbrec_port_binding *pb, > - bool can_bind, const char *vif_chassis, > + bool can_bind, > struct binding_ctx_in *b_ctx_in, > struct binding_ctx_out *b_ctx_out, > struct binding_lport *b_lport, > @@ -1134,7 +1133,8 @@ consider_vif_lport_(const struct sbrec_port_binding *pb, > "requested-chassis %s", > pb->logical_port, > b_ctx_in->chassis_rec->name, > - vif_chassis); > + pb->requested_chassis ? > + pb->requested_chassis->name : "(none)"); > } > } > > @@ -1157,9 +1157,7 @@ consider_vif_lport(const struct sbrec_port_binding *pb, > struct local_binding *lbinding, > struct hmap *qos_map) > { > - const char *vif_chassis = smap_get(&pb->options, "requested-chassis"); > - bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, > - vif_chassis); > + bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, pb); > > if (!lbinding) { > lbinding = local_binding_find(&b_ctx_out->lbinding_data->bindings, > @@ -1189,8 +1187,8 @@ consider_vif_lport(const struct sbrec_port_binding *pb, > } > } > > - return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, > - b_ctx_out, b_lport, qos_map); > + return consider_vif_lport_(pb, can_bind, b_ctx_in, b_ctx_out, > + b_lport, qos_map); > } > > static bool > @@ -1274,12 +1272,9 @@ consider_container_lport(const struct sbrec_port_binding *pb, > } > > ovs_assert(parent_b_lport && parent_b_lport->pb); > - const char *vif_chassis = smap_get(&parent_b_lport->pb->options, > - "requested-chassis"); > - bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, > - vif_chassis); > + bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, pb); > > - return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, b_ctx_out, > + return consider_vif_lport_(pb, can_bind, b_ctx_in, b_ctx_out, > container_b_lport, qos_map); > } > > @@ -1328,7 +1323,7 @@ consider_virtual_lport(const struct sbrec_port_binding *pb, > } > } > > - if (!consider_vif_lport_(pb, true, NULL, b_ctx_in, b_ctx_out, > + if (!consider_vif_lport_(pb, true, b_ctx_in, b_ctx_out, > virtual_b_lport, qos_map)) { > return false; > } > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 0031a1035..7387a177b 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -232,6 +232,9 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, > sbrec_port_binding_add_clause_chassis(&pb, OVSDB_F_EQ, > &chassis->header_.uuid); > > + sbrec_port_binding_add_clause_requested_chassis( > + &pb, OVSDB_F_EQ, &chassis->header_.uuid); > + > /* Ensure that we find out about l2gateway and l3gateway ports that > * should be present on this chassis. Otherwise, we might never find > * out about those ports, if their datapaths don't otherwise have a VIF > diff --git a/controller/physical.c b/controller/physical.c > index 6f2c1cea0..2f6bd0e91 100644 > --- a/controller/physical.c > +++ b/controller/physical.c > @@ -1066,11 +1066,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, > } else { > ofport = local_binding_get_lport_ofport(local_bindings, > binding->logical_port); > - const char *requested_chassis = smap_get(&binding->options, > - "requested-chassis"); > - if (ofport && requested_chassis && requested_chassis[0] && > - strcmp(requested_chassis, chassis->name) && > - strcmp(requested_chassis, chassis->hostname)) { > + if (ofport && binding->requested_chassis > + && binding->requested_chassis != chassis) { > /* Even though there is an ofport for this port_binding, it is > * requested on a different chassis. So ignore this ofport. > */ > -- > 2.32.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Thu, Sep 16, 2021 at 6:12 PM Numan Siddique <numans@ovn.org> wrote: > > On Fri, Sep 3, 2021 at 3:28 PM Frode Nordahl > <frode.nordahl@canonical.com> wrote: > > > > Improve the efficiency of the requested-chassis feature by using > > the new Southbound Port_Binding:requested_chassis column instead > > of each chassis performing option processing and string comparison. > > > > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> > > I've one comment which seems to be discussed in v3 of the first patch. > > Looks to me this patch would break the existing behavior (which the > patch 2 addresses). > > Right now when CMS sets requested-chassis=foo for a logical port 'P1', > then other ovn-controllers > will not claim 'P1' if there is a VIF for this even if chassis 'foo' > doesn't exist. > > With this patch series, other ovn-controller will claim 'P1' for the > above scenario I mentioned. > > Is this going to be a problem ? Or are we going to break some existing > deployments ? I have to admit that I would find it unusual for a CMS to refer to non-existing chassis, but I do see that it could happen if the CMS knows about a hypervisor prior to OVN knowing about it, so I see what you mean. > I'd suggest not breaking this scenario. If the > port_binding->options:requested-chassis is set, > then ovn-controller should not claim that port binding if the > port_binding->requested_chassis is > set. In other words ovn-controller should claim only if > port_binding->requested_chassis == this_chassis > when the port_binding->options:requested-chassis is set. > > Does this make sense ? Yes. I guess it's not really possible to point at a non-existing chassis at all with this approach, and having a phantom chassis for this would be entering hacky territory? I can drop this patch, but I would still need the column for the ovn-controller to be made aware of ports it should consider prior to any interface existing locally. Would you be ok with keeping the name, or should I find a different name so that it does not conflict with the requested-chassis use case?
On Thu, Sep 16, 2021 at 1:33 PM Frode Nordahl <frode.nordahl@canonical.com> wrote: > > On Thu, Sep 16, 2021 at 6:12 PM Numan Siddique <numans@ovn.org> wrote: > > > > On Fri, Sep 3, 2021 at 3:28 PM Frode Nordahl > > <frode.nordahl@canonical.com> wrote: > > > > > > Improve the efficiency of the requested-chassis feature by using > > > the new Southbound Port_Binding:requested_chassis column instead > > > of each chassis performing option processing and string comparison. > > > > > > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> > > > > I've one comment which seems to be discussed in v3 of the first patch. > > > > Looks to me this patch would break the existing behavior (which the > > patch 2 addresses). > > > > Right now when CMS sets requested-chassis=foo for a logical port 'P1', > > then other ovn-controllers > > will not claim 'P1' if there is a VIF for this even if chassis 'foo' > > doesn't exist. > > > > With this patch series, other ovn-controller will claim 'P1' for the > > above scenario I mentioned. > > > > Is this going to be a problem ? Or are we going to break some existing > > deployments ? > > I have to admit that I would find it unusual for a CMS to refer to > non-existing chassis, but I do see that it could happen if the CMS > knows about a hypervisor prior to OVN knowing about it, so I see what > you mean. One usecase of this happening is when ovn-controller exits (without --restart) option. In this case, it deletes its own chassis record. > > > I'd suggest not breaking this scenario. If the > > port_binding->options:requested-chassis is set, > > then ovn-controller should not claim that port binding if the > > port_binding->requested_chassis is > > set. In other words ovn-controller should claim only if > > port_binding->requested_chassis == this_chassis > > when the port_binding->options:requested-chassis is set. > > > > Does this make sense ? > > Yes. I guess it's not really possible to point at a non-existing > chassis at all with this approach, and having a phantom chassis for > this would be entering hacky territory? I can drop this patch, but I > would still need the column for the ovn-controller to be made aware of > ports it should consider prior to any interface existing locally. I'm not suggesting to drop this patch but to also check if port_binding->options:requested-chassis is set or not. If CMS sets logical_port->options:requested-chassis, this will be copied over to port_binding->options and only then would ovn-northd set port_binding->requested_chassis if the chassis row exists in SB DB. Something like : if (smap_get(&port_binding->options, "requested-chassis") && !port_binding->requested_chassis) then don't claim the port binding if there is a VIF for this. If you do this way, then we would not need patch 2. Let me know if this is fine. > > Would you be ok with keeping the name, or should I find a different > name so that it does not conflict with the requested-chassis use case? I have no strong preference. If you can come up with a better name that would do too :) Thanks Numan > > -- > Frode Nordahl > > > Numan > > > > > > > > > --- > > > controller/binding.c | 29 ++++++++++++----------------- > > > controller/ovn-controller.c | 3 +++ > > > controller/physical.c | 7 ++----- > > > 3 files changed, 17 insertions(+), 22 deletions(-) > > > > > > diff --git a/controller/binding.c b/controller/binding.c > > > index 34935bb9c..938e1d81d 100644 > > > --- a/controller/binding.c > > > +++ b/controller/binding.c > > > @@ -1051,11 +1051,10 @@ is_binding_lport_this_chassis(struct binding_lport *b_lport, > > > > > > static bool > > > can_bind_on_this_chassis(const struct sbrec_chassis *chassis_rec, > > > - const char *requested_chassis) > > > + const struct sbrec_port_binding *pb) > > > { > > > - return !requested_chassis || !requested_chassis[0] > > > - || !strcmp(requested_chassis, chassis_rec->name) > > > - || !strcmp(requested_chassis, chassis_rec->hostname); > > > + return !pb->requested_chassis > > > + || chassis_rec == pb->requested_chassis; > > > } > > > > > > /* Returns 'true' if the 'lbinding' has binding lports of type LP_CONTAINER, > > > @@ -1093,7 +1092,7 @@ release_binding_lport(const struct sbrec_chassis *chassis_rec, > > > > > > static bool > > > consider_vif_lport_(const struct sbrec_port_binding *pb, > > > - bool can_bind, const char *vif_chassis, > > > + bool can_bind, > > > struct binding_ctx_in *b_ctx_in, > > > struct binding_ctx_out *b_ctx_out, > > > struct binding_lport *b_lport, > > > @@ -1134,7 +1133,8 @@ consider_vif_lport_(const struct sbrec_port_binding *pb, > > > "requested-chassis %s", > > > pb->logical_port, > > > b_ctx_in->chassis_rec->name, > > > - vif_chassis); > > > + pb->requested_chassis ? > > > + pb->requested_chassis->name : "(none)"); > > > } > > > } > > > > > > @@ -1157,9 +1157,7 @@ consider_vif_lport(const struct sbrec_port_binding *pb, > > > struct local_binding *lbinding, > > > struct hmap *qos_map) > > > { > > > - const char *vif_chassis = smap_get(&pb->options, "requested-chassis"); > > > - bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, > > > - vif_chassis); > > > + bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, pb); > > > > > > if (!lbinding) { > > > lbinding = local_binding_find(&b_ctx_out->lbinding_data->bindings, > > > @@ -1189,8 +1187,8 @@ consider_vif_lport(const struct sbrec_port_binding *pb, > > > } > > > } > > > > > > - return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, > > > - b_ctx_out, b_lport, qos_map); > > > + return consider_vif_lport_(pb, can_bind, b_ctx_in, b_ctx_out, > > > + b_lport, qos_map); > > > } > > > > > > static bool > > > @@ -1274,12 +1272,9 @@ consider_container_lport(const struct sbrec_port_binding *pb, > > > } > > > > > > ovs_assert(parent_b_lport && parent_b_lport->pb); > > > - const char *vif_chassis = smap_get(&parent_b_lport->pb->options, > > > - "requested-chassis"); > > > - bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, > > > - vif_chassis); > > > + bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, pb); > > > > > > - return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, b_ctx_out, > > > + return consider_vif_lport_(pb, can_bind, b_ctx_in, b_ctx_out, > > > container_b_lport, qos_map); > > > } > > > > > > @@ -1328,7 +1323,7 @@ consider_virtual_lport(const struct sbrec_port_binding *pb, > > > } > > > } > > > > > > - if (!consider_vif_lport_(pb, true, NULL, b_ctx_in, b_ctx_out, > > > + if (!consider_vif_lport_(pb, true, b_ctx_in, b_ctx_out, > > > virtual_b_lport, qos_map)) { > > > return false; > > > } > > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > > > index 0031a1035..7387a177b 100644 > > > --- a/controller/ovn-controller.c > > > +++ b/controller/ovn-controller.c > > > @@ -232,6 +232,9 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, > > > sbrec_port_binding_add_clause_chassis(&pb, OVSDB_F_EQ, > > > &chassis->header_.uuid); > > > > > > + sbrec_port_binding_add_clause_requested_chassis( > > > + &pb, OVSDB_F_EQ, &chassis->header_.uuid); > > > + > > > /* Ensure that we find out about l2gateway and l3gateway ports that > > > * should be present on this chassis. Otherwise, we might never find > > > * out about those ports, if their datapaths don't otherwise have a VIF > > > diff --git a/controller/physical.c b/controller/physical.c > > > index 6f2c1cea0..2f6bd0e91 100644 > > > --- a/controller/physical.c > > > +++ b/controller/physical.c > > > @@ -1066,11 +1066,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, > > > } else { > > > ofport = local_binding_get_lport_ofport(local_bindings, > > > binding->logical_port); > > > - const char *requested_chassis = smap_get(&binding->options, > > > - "requested-chassis"); > > > - if (ofport && requested_chassis && requested_chassis[0] && > > > - strcmp(requested_chassis, chassis->name) && > > > - strcmp(requested_chassis, chassis->hostname)) { > > > + if (ofport && binding->requested_chassis > > > + && binding->requested_chassis != chassis) { > > > /* Even though there is an ofport for this port_binding, it is > > > * requested on a different chassis. So ignore this ofport. > > > */ > > > -- > > > 2.32.0 > > > > > > _______________________________________________ > > > 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 >
On Thu, Sep 16, 2021 at 7:50 PM Numan Siddique <numans@ovn.org> wrote: > > On Thu, Sep 16, 2021 at 1:33 PM Frode Nordahl > <frode.nordahl@canonical.com> wrote: > > > > On Thu, Sep 16, 2021 at 6:12 PM Numan Siddique <numans@ovn.org> wrote: > > > > > > On Fri, Sep 3, 2021 at 3:28 PM Frode Nordahl > > > <frode.nordahl@canonical.com> wrote: > > > > > > > > Improve the efficiency of the requested-chassis feature by using > > > > the new Southbound Port_Binding:requested_chassis column instead > > > > of each chassis performing option processing and string comparison. > > > > > > > > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> > > > > > > I've one comment which seems to be discussed in v3 of the first patch. > > > > > > Looks to me this patch would break the existing behavior (which the > > > patch 2 addresses). > > > > > > Right now when CMS sets requested-chassis=foo for a logical port 'P1', > > > then other ovn-controllers > > > will not claim 'P1' if there is a VIF for this even if chassis 'foo' > > > doesn't exist. > > > > > > With this patch series, other ovn-controller will claim 'P1' for the > > > above scenario I mentioned. > > > > > > Is this going to be a problem ? Or are we going to break some existing > > > deployments ? > > > > I have to admit that I would find it unusual for a CMS to refer to > > non-existing chassis, but I do see that it could happen if the CMS > > knows about a hypervisor prior to OVN knowing about it, so I see what > > you mean. > > One usecase of this happening is when ovn-controller exits (without > --restart) option. > In this case, it deletes its own chassis record. That would indeed happen quite often! Thank you for pointing that out. > > > > > I'd suggest not breaking this scenario. If the > > > port_binding->options:requested-chassis is set, > > > then ovn-controller should not claim that port binding if the > > > port_binding->requested_chassis is > > > set. In other words ovn-controller should claim only if > > > port_binding->requested_chassis == this_chassis > > > when the port_binding->options:requested-chassis is set. > > > > > > Does this make sense ? > > > > Yes. I guess it's not really possible to point at a non-existing > > chassis at all with this approach, and having a phantom chassis for > > this would be entering hacky territory? I can drop this patch, but I > > would still need the column for the ovn-controller to be made aware of > > ports it should consider prior to any interface existing locally. > > I'm not suggesting to drop this patch but to also check if > port_binding->options:requested-chassis > is set or not. If CMS sets logical_port->options:requested-chassis, > this will be copied over > to port_binding->options and only then would ovn-northd set > port_binding->requested_chassis > if the chassis row exists in SB DB. Right, so use both to confirm, got it. That works for me! > Something like : > > if (smap_get(&port_binding->options, "requested-chassis") && > !port_binding->requested_chassis) > then don't claim the port binding if there is a VIF for this. > > If you do this way, then we would not need patch 2. > > Let me know if this is fine. Sounds good, I'll work on that approach. > > > > Would you be ok with keeping the name, or should I find a different > > name so that it does not conflict with the requested-chassis use case? > > I have no strong preference. If you can come up with a better name > that would do too :) If we're using it in conjunction with the requested-chassis option as discussed above, I guess it's fine to keep the name too. Thx!
diff --git a/controller/binding.c b/controller/binding.c index 34935bb9c..938e1d81d 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -1051,11 +1051,10 @@ is_binding_lport_this_chassis(struct binding_lport *b_lport, static bool can_bind_on_this_chassis(const struct sbrec_chassis *chassis_rec, - const char *requested_chassis) + const struct sbrec_port_binding *pb) { - return !requested_chassis || !requested_chassis[0] - || !strcmp(requested_chassis, chassis_rec->name) - || !strcmp(requested_chassis, chassis_rec->hostname); + return !pb->requested_chassis + || chassis_rec == pb->requested_chassis; } /* Returns 'true' if the 'lbinding' has binding lports of type LP_CONTAINER, @@ -1093,7 +1092,7 @@ release_binding_lport(const struct sbrec_chassis *chassis_rec, static bool consider_vif_lport_(const struct sbrec_port_binding *pb, - bool can_bind, const char *vif_chassis, + bool can_bind, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out, struct binding_lport *b_lport, @@ -1134,7 +1133,8 @@ consider_vif_lport_(const struct sbrec_port_binding *pb, "requested-chassis %s", pb->logical_port, b_ctx_in->chassis_rec->name, - vif_chassis); + pb->requested_chassis ? + pb->requested_chassis->name : "(none)"); } } @@ -1157,9 +1157,7 @@ consider_vif_lport(const struct sbrec_port_binding *pb, struct local_binding *lbinding, struct hmap *qos_map) { - const char *vif_chassis = smap_get(&pb->options, "requested-chassis"); - bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, - vif_chassis); + bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, pb); if (!lbinding) { lbinding = local_binding_find(&b_ctx_out->lbinding_data->bindings, @@ -1189,8 +1187,8 @@ consider_vif_lport(const struct sbrec_port_binding *pb, } } - return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, - b_ctx_out, b_lport, qos_map); + return consider_vif_lport_(pb, can_bind, b_ctx_in, b_ctx_out, + b_lport, qos_map); } static bool @@ -1274,12 +1272,9 @@ consider_container_lport(const struct sbrec_port_binding *pb, } ovs_assert(parent_b_lport && parent_b_lport->pb); - const char *vif_chassis = smap_get(&parent_b_lport->pb->options, - "requested-chassis"); - bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, - vif_chassis); + bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, pb); - return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, b_ctx_out, + return consider_vif_lport_(pb, can_bind, b_ctx_in, b_ctx_out, container_b_lport, qos_map); } @@ -1328,7 +1323,7 @@ consider_virtual_lport(const struct sbrec_port_binding *pb, } } - if (!consider_vif_lport_(pb, true, NULL, b_ctx_in, b_ctx_out, + if (!consider_vif_lport_(pb, true, b_ctx_in, b_ctx_out, virtual_b_lport, qos_map)) { return false; } diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 0031a1035..7387a177b 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -232,6 +232,9 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, sbrec_port_binding_add_clause_chassis(&pb, OVSDB_F_EQ, &chassis->header_.uuid); + sbrec_port_binding_add_clause_requested_chassis( + &pb, OVSDB_F_EQ, &chassis->header_.uuid); + /* Ensure that we find out about l2gateway and l3gateway ports that * should be present on this chassis. Otherwise, we might never find * out about those ports, if their datapaths don't otherwise have a VIF diff --git a/controller/physical.c b/controller/physical.c index 6f2c1cea0..2f6bd0e91 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -1066,11 +1066,8 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name, } else { ofport = local_binding_get_lport_ofport(local_bindings, binding->logical_port); - const char *requested_chassis = smap_get(&binding->options, - "requested-chassis"); - if (ofport && requested_chassis && requested_chassis[0] && - strcmp(requested_chassis, chassis->name) && - strcmp(requested_chassis, chassis->hostname)) { + if (ofport && binding->requested_chassis + && binding->requested_chassis != chassis) { /* Even though there is an ofport for this port_binding, it is * requested on a different chassis. So ignore this ofport. */
Improve the efficiency of the requested-chassis feature by using the new Southbound Port_Binding:requested_chassis column instead of each chassis performing option processing and string comparison. Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> --- controller/binding.c | 29 ++++++++++++----------------- controller/ovn-controller.c | 3 +++ controller/physical.c | 7 ++----- 3 files changed, 17 insertions(+), 22 deletions(-)