Hi Dumitru,
Overall, this looks like a good patch series. I don't have any specific
findings, but I have some general points/questions.
1) There are no tests. With the isolated lflow-cache.[hc] API, adding
unit tests seems like it wouldn't be too difficult.
2) Has there been any proven benefit to LCACHE_T_CONJ_ID? Reading
through consider_logical_flow(), LCACHE_T_CONJ_ID and LCACHE_T_NONE
require the same work to be done. I suppose it allows more time before
the conj_id overflows, but is that really worth taking up potentially
limited cache space?
3) It seems like cached lflows of type LCACHE_T_MATCHES are the most
valuable, since caching them saves the most amount of work in
consider_logical_flow(). Is there any way to give preference to these
types of lflows over others? It would be a shame if the cache filled up
with LCACHE_T_EXPR and LCACHE_T_CONJ_ID lflows, and that prevented
LCACHE_T_MATCHES flows from being cached as a result.
4) We calculate the memory of each lflow_cache_value whenever it is
added to the cache. Does this incur a noticeable cost? If the
lflow_cache has no memory limit, then would it be more beneficial not to
calculate the memory when the value is added, but instead wait until a
memory report is requested?
On 1/28/21 11:25 AM, Dumitru Ceara wrote:
Scale tests have identified the lflow cache to be one of the main memory
consumers in ovn-controller. This series refactors the lflow cache code
and adds configuration knobs to limit the size (in lines and/or memory)
of the cache.
Dumitru Ceara (6):
lflow: Refactor convert_match_to_expr() to explicitly consume prereqs.
lflow-cache: Move the lflow cache to its own module.
lflow-cache: Add coverage counters.
lflow-cache: Reclaim heap memory after cache flush.
lflow-cache: Make maximum number of cache entries configurable.
lflow-cache: Make max cache memory usage configurable.
NEWS | 5 +
configure.ac | 1
controller/automake.mk | 2
controller/chassis.c | 44 +++++
controller/lflow-cache.c | 282 ++++++++++++++++++++++++++++++++
controller/lflow-cache.h | 81 +++++++++
controller/lflow.c | 349 +++++++++++++--------------------------
controller/lflow.h | 6 -
controller/ovn-controller.8.xml | 23 +++
controller/ovn-controller.c | 65 ++++---
include/ovn/expr.h | 2
lib/expr.c | 46 +++++
12 files changed, 635 insertions(+), 271 deletions(-)
create mode 100644 controller/lflow-cache.c
create mode 100644 controller/lflow-cache.h
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
