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.
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?
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
