On Tue, Jun 25, 2019 at 1:23 AM Han Zhou <[email protected]> wrote: > > > On Mon, Jun 24, 2019 at 9:59 AM Han Zhou <[email protected]> wrote: > > > > > > > > On Mon, Jun 24, 2019 at 4:00 AM <[email protected]> wrote: > > > > > > From: Numan Siddique <[email protected]> > > > > > > Any changes for Port_Bindings rows of type - "chassisredirect", > "patch", "l3gateway" etc > > > which are not related to the chassis can be ignored in the function > > > 'binding_evaluate_port_binding_changes()'. Presently this returns true > and this results > > > in unnecessary flow computation on a chassis. > > > > > > Changes to these Port_Bindings (of type != "") will be handled by the > engine handler > > > flow_output_sb_port_binding_handler() for the engine node > 'en_sb_port_binding' (which > > > is input to 'en_flow_output'. > > > > > > For example, if a chassisredirect port is claimed by a gateway > chassis, the compute > > > nodes only need to update the flow in table 32 in the bundle action. > Where as presently > > > flow computation is triggered and this causes wastage of CPU. > > > > > > Signed-off-by: Numan Siddique <[email protected]> > > > --- > > > ovn/controller/binding.c | 16 +++++++++++++--- > > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > > > diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c > > > index 87d0b6d88..7d287ece9 100644 > > > --- a/ovn/controller/binding.c > > > +++ b/ovn/controller/binding.c > > > @@ -715,11 +715,21 @@ binding_evaluate_port_binding_changes( > > > * - If a regular VIF is unbound from this chassis, the local > ovsdb > > > * interface table will be updated, which will trigger > recompute. > > > * > > > - * - If the port is not a regular VIF, always trigger > recompute. */ > > > + * If the port is not a regular VIF, then ignore it. */ > > > if (binding_rec->chassis == chassis_rec > > > || is_our_chassis(chassis_rec, binding_rec, > > > - active_tunnels, &lport_to_iface, > local_lports) > > > - || strcmp(binding_rec->type, "")) { > > > + active_tunnels, &lport_to_iface, > > > + local_lports)) { > > > + changed = true; > > > + break; > > > + } > > > + > > > + /* Any changes to chassisredirect port (not claimed by this > chassis), > > > + * doesn't require any logical flow computation. It only > requires > > > + * physical flow update and thiss will be handled by > > > + * flow_output_sb_port_binding_handler() in ovn-controller.c > */ > > > + if (strcmp(binding_rec->type, "") && > > > + strcmp(binding_rec->type, "chassisredirect")) { > > > changed = true; > > > break; > > > } > > > -- > > > 2.21.0 > > > > > > _______________________________________________ > > > dev mailing list > > > [email protected] > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > Makes sense. Thanks for the improvement! > > > > Acked-by: Han Zhou <[email protected]> > > Sorry, I should not ack. I realized that this patch might cause problem. > There is a "XXX" need to be addressed before applying this patch. Please > read the "XXX" in ovn-controller.c L1401. Specifically this part: > --- > * - When is_chassis_resident(<lport>) is used in lflow. In this case > the > * port binding is not a regular VIF. It can be either "patch" or > * "external", with ha-chassis-group assigned. In current > * "runtime_data" handling, port-binding changes for these types > always > * trigger recomputing. So it is fine even if we do not handle it > here. > * (due to the ovsdb tracking support for referenced table changes, > * ha-chassis-group changes will appear as port-binding change). > --- > I guess it will cause fail-over not taking effect in the old active node, > thus it may continue sending GARP etc. It should be solved if we addressed > this "XXX" as mentioned: > ... One approach is to maintain a mapping between lport > * names and the lflows that uses them, and reprocess the related > lflows > * when related port-bindings change. > > Thanks for pointing this out. You are right.
> However, I wonder if it is really valuable to do this optimization just > for the "chassisredirect" case. How often would "chassisredirect" port > binding change happen in real world scenarios? > In real deployments, I would say this would be rare. But if you see the scale tests run by Daniel and Lucas, I noticed that this caused recomputations. However when I tested with these patches there were no improvements at all. Thanks Numan _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
