[ovs-git] [ovn-org/ovn] 0d6dba: I-P engine: Provide the option to store client dat...

[numansiddique]

  Branch: refs/heads/master
  Home:   https://github.com/ovn-org/ovn
  Commit: 0d6dbac7ce5ae8b6aea486de14cbc2eef6a88d43
      https://github.com/ovn-org/ovn/commit/0d6dbac7ce5ae8b6aea486de14cbc2eef6a88d43
  Author: Numan Siddique <numans at ovn.org>
  Date:   2020-09-10 (Thu, 10 Sep 2020)

  Changed paths:
    M lib/inc-proc-eng.h

  Log Message:
  -----------
  I-P engine: Provide the option to store client data in engine ctx.

There can be some client specific data which could change from one engine run
to another. Adding a 'void *' data in 'struct engine_ctx' will be useful.
One such usecase is to provide a config option in ovn-controller to turn on or
off logical flow expr caching. And this config knob can be stored in the engine_ctx.
An upcoming patch will make use of this.

Acked-by: Mark Michelson <mmichels at redhat.com>
Signed-off-by: Numan Siddique <numans at ovn.org>


  Commit: 2662498bfd13ccb13defd1c800f449be7e271abe
      https://github.com/ovn-org/ovn/commit/2662498bfd13ccb13defd1c800f449be7e271abe
  Author: Numan Siddique <numans at ovn.org>
  Date:   2020-09-10 (Thu, 10 Sep 2020)

  Changed paths:
    M controller/chassis.c
    M controller/lflow.c
    M controller/lflow.h
    M controller/ovn-controller.c
    M tests/ovn.at

  Log Message:
  -----------
  ovn-controller: Persist the conjunction ids allocated for conjuctive matches.

For a logical flow which results in conjuctive OF matches, we are not persisting
the allocated conjunction ids for it. There are few side effects because of this.
  - When a port group or address set gets modified, the logical flows which references
    these port groups/address sets gets reprocessed again and the resulting OpenvSwitch flows
    with conjunctive matches gets modified in the vswitchd if the conjunction id changes.

  - And because of this there is small probability of a packet getting dropped when the
    OF flows gets updated with different conjunction ids.

This patch fixes this issue by persisting the conjunction ids. Earlier, logical flow caching
support was added [1] to ovn-controller and then reverted [2] later due to some issues.

This patch takes the lflow caching approach to persist the conjunction ids. But it only
creates the cache for logical flows which result in conjunctive matches. And it doesn't
cache the expr tree.

The lflow caching is made configurable and enabled by default. Any user can disable caching
by setting 'ovn-enable-lflow-cache' to 'false' in the OVS db.
 - ovs-vsctl set open . external_ids:ovn-enable-lflow-cache=false

Note: Changing the option 'ovn-enable-lflow-cache' doesn't result in full recompute of
I-P engine. But ovn-controller updates the chassis.other_config column. And when it
receives the update2/update3 message, this results in full recompute (as any chassis
changes results in full recompute). This is the case with all the config options in OVS db.

An upcoming patch series, will attempt to add back the expr caching (addressing all the issues.)

[1] - 8795bec737b("ovn-controller: Cache logical flow expr tree for each lflow.)
[2] - 065bcf46218("ovn-controller: Revert lflow expr caching")

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1858878
Acked-by: Mark Michelson <mmichels at redhat.com>
Signed-off-by: Numan Siddique <numans at ovn.org>


  Commit: 024500aeab9a3eb8d6ae9e8a17d2ec2a28369a99
      https://github.com/ovn-org/ovn/commit/024500aeab9a3eb8d6ae9e8a17d2ec2a28369a99
  Author: Numan Siddique <numans at ovn.org>
  Date:   2020-09-10 (Thu, 10 Sep 2020)

  Changed paths:
    M controller/lflow.c
    M include/ovn/expr.h
    M lib/expr.c
    M tests/test-ovn.c
    M utilities/ovn-trace.c

  Log Message:
  -----------
  expr: Evaluate the condition expression in a separate step.

A similar patch was committed earlier [1] and then reverted [2].
This patch is similar to [1], but not exactly same.

A new function is added - expr_evaluate_condition() which evaluates
the condition expression - is_chassis_resident. expr_simplify() will
no longer evaluates this condition. expr_normalize() is still expected
to be called after expr_evaluate_condition(). Otherwise it will assert
if 'is_chassis_resident()' is not evaluated.

An upcoming commit needs this in order to cache the logical flow expressions
for logical flows having 'is_chassis_resident()' condition in their match.
The expr tree after expr_simplify()  for such logical flows is cached. Since
we can't cache the is_chassis_resident condition, it is separated out.
(For logical flows which do not have this condition in their match, the expr matches
will be cached.)

[1] - 99e3a145927("expr: Evaluate the condition expression in a separate step.")
[2] - 065bcf46218("ovn-controller: Revert lflow expr caching")

Acked-by: Mark Michelson <mmichels at redhat.com>
Signed-off-by: Numan Siddique <numans at ovn.org>


  Commit: 1213bc827040d7f59444d94f1776a75dd7dd62f2
      https://github.com/ovn-org/ovn/commit/1213bc827040d7f59444d94f1776a75dd7dd62f2
  Author: Numan Siddique <numans at ovn.org>
  Date:   2020-09-10 (Thu, 10 Sep 2020)

  Changed paths:
    M controller/lflow.c
    M include/ovn/expr.h
    M lib/expr.c
    M tests/test-ovn.c
    M utilities/ovn-trace.c

  Log Message:
  -----------
  ovn-controller: Cache logical flow expr matches.

This patch is a second attempt in caching the expr tree of logical flows.
Earlier attemp [1] was reverted.

In this approach, we do the following

  - If a logical flow match has any port group/address set references, we don't
    cache expr match or expr tree.

  - If a logical flow match doesn't have a port group/address set reference and
    expr condition match - is_chassis_resident, we cache the expr matches
    (which is hmap of struct expr_match *).

  - If a logical flow match has expr condition match - is_chassis_resident we
    cache the simplified 'expr' tree.

Caching is configurable and can be disabled if desired. The second patch of this series
added this support.

In my testing, a complete lflow_run() took around 5 seconds for around 90k logical flows,
where as it took around 2 seconds with caching.

Acked-by: Mark Michelson <mmichels at redhat.com>
Signed-off-by: Numan Siddique <numans at ovn.org>


Compare: https://github.com/ovn-org/ovn/compare/58611a51051e...1213bc827040