mbox series

[ovs-dev,v5,00/16] northd lflow incremental processing

Message ID 20240111152752.2789854-1-numans@ovn.org
Headers show
Series northd lflow incremental processing | expand

Message

Numan Siddique Jan. 11, 2024, 3:27 p.m. UTC
From: Numan Siddique <numans@ovn.org>

This patch series adds incremental processing in the lflow engine
node to handle changes to northd and other engine nodes.
Changed related to load balancers and NAT are mainly handled in
this patch series.

This patch series can also be found here - https://github.com/numansiddique/ovn/tree/northd_lbnatacl_lflow/v5

Prior to this patch series, most of the changes to northd engine
resulted in full recomputation of logical flows.  This series
aims to improve the performance of ovn-northd by adding the I-P
support.  In order to add this support, some of the northd engine
node data (from struct ovn_datapath) is split and moved over to
new engine nodes - mainly related to load balancers, NAT and ACLs.

Below are the scale testing results done with these patches applied
using ovn-heater.  The test ran the scenario  -
ocp-500-density-heavy.yml [1].

With all the lflow I-P patches applied, the resuts are:

-------------------------------------------------------------------------------------------------------------------------------------------------------
                        Min (s)         Median (s)      90%ile (s)      99%ile (s)      Max (s)         Mean (s)        Total (s)       Count   Failed
-------------------------------------------------------------------------------------------------------------------------------------------------------
Iteration Total         0.136883        1.129016        1.192001        1.204167        1.212728        0.665017        83.127099       125     0
Namespace.add_ports     0.005216        0.005736        0.007034        0.015486        0.018978        0.006211        0.776373        125     0
WorkerNode.bind_port    0.035030        0.046082        0.052469        0.058293        0.060311        0.045973        11.493259       250     0
WorkerNode.ping_port    0.005057        0.006727        1.047692        1.069253        1.071336        0.266896        66.724094       250     0
-------------------------------------------------------------------------------------------------------------------------------------------------------

The results with the present main are:

-------------------------------------------------------------------------------------------------------------------------------------------------------
                        Min (s)         Median (s)      90%ile (s)      99%ile (s)      Max (s)         Mean (s)        Total (s)       Count   Failed
-------------------------------------------------------------------------------------------------------------------------------------------------------
Iteration Total         0.135491        2.223805        3.311270        3.339078        3.345346        1.729172        216.146495      125     0
Namespace.add_ports     0.005380        0.005744        0.006819        0.018773        0.020800        0.006292        0.786532        125     0
WorkerNode.bind_port    0.034179        0.046055        0.053488        0.058801        0.071043        0.046117        11.529311       250     0
WorkerNode.ping_port    0.004956        0.006952        3.086952        3.191743        3.192807        0.791544        197.886026      250     0
-------------------------------------------------------------------------------------------------------------------------------------------------------

Please see the link [2] which has a high level description of the
changes done in this patch series.


[1] - https://github.com/ovn-org/ovn-heater/blob/main/test-scenarios/ocp-500-density-heavy.yml
[2] - https://mail.openvswitch.org/pipermail/ovs-dev/2023-December/410053.html

v4 -> v5
-------
   * Rebased to latest main and resolved the conflicts.

   * Addressed the review comments from Han in patch 15 (and in p8).  Removed the
     assert if SB dp group is missing and handled it by returning false
     so that lflow engine recomputes.  Added test cases to cover this
     scenario for both lflows (p8) and SB load balancers (p15) .

v3 -> v4
-------
   * Addressed most of the review comments from Dumitru and Han.

   * Found a couple of bugs in v3 patch 9 -
     "northd: Refactor lflow management into a separate module."
     and addressed them in v4.
     To brief  the issue, if a logical flow L(M, A) is referenced
     by 2 lflow_ref's which belong to the same datapath, then the lflow
     was deleted even if one lflow_ref was cleared due to any changes.
     It is addressed now by maintaining a reference count in the 'struct
     ovn_lflow' for each datapath it is used by.

   * Moved the v3 patch 14 ("northd:  Add I-P for NB_Global and SB_Global.")
     to patch 16 in v4.  There were comments in this patch to not add a
     full I-P for NB_Global and SB_Global.  Made this patch as the last
     in the series so that we can discuss further and not block other patches
     in case we want to drop this one.


v2 -> v3
-------
   * Addressed some of the review comments from Han and Dumitru.  There
     are still a few pending review comments which needs to be addressed
     or discussed.

   * Renamed the engine node from "lr_lbnat_data" to "lr_stateful"
     (v3 patch 5).

   * Renamed the engine node from "ls_lbacls" to "ls_stateful" (v3 patch 8).

   * Removed v2 patch 2 from the series (northd: Track ovn_datapaths in
     northd engine track data.").  This patch is now part of v3 patch 7
     (northd: Add a new node 'ls_stateful').

   * Squashed v2 patch 8 (northd: Don't commit dhcp response flows in
     the conntrack.) into v3 patch 7 (northd: Add a new node
     'ls_stateful'.)


v1 -> v2
--------
   * Now also maintaing array indexes for ls_lbacls, lr_nat and
     lr_lb_nat_data tables (similar to ovn_datapaths->array) to
     make the lookup effecient.  The same ovn_datapath->index
     is reused.

   * Made some signficant changes to 'struct lflow_ref' in lflow-mgr.c.
     In v2 we don't use objdep_mgr to maintain the resource to lflow
     references.  Instead we maintain the 'struct lflow' pointer.
     With this we don't need to maintain additional hmap of lflows.


Numan Siddique (16):
  northd: Refactor the northd change tracking.
  tests: Add a couple of tests in ovn-northd for I-P.
  northd: Move router ports SB PB options sync to sync_to_sb_pb node.
  northd: Add a new engine 'lr_nat' to manage lr NAT data.
  northd: Add a new engine 'lr_stateful' to manage lr's stateful data.
  northd:  Generate router's stateful flows using lr_stateful data.
  northd: Add a new node 'ls_stateful'.
  northd: Refactor lflow management into a separate module.
  northd: Use lflow_ref when adding all logical flows.
  northd:  Move ovn_lb_datapaths from lib to northd module.
  northd: Handle lb changes in lflow engine.
  northd: Add lr_stateful handler for lflow engine node.
  northd: Add ls_stateful handler for lflow engine node.
  northd: Add a noop handler for northd SB mac binding.
  northd: Add northd change handler for sync_to_sb_lb node.
  northd:  Add I-P for NB_Global and SB_Global.

 lib/lb.c                  |   96 -
 lib/lb.h                  |   57 -
 lib/ovn-util.c            |   24 +-
 lib/ovn-util.h            |    4 +-
 lib/stopwatch-names.h     |    3 +
 northd/aging.c            |   21 +-
 northd/automake.mk        |   14 +-
 northd/en-global-config.c |  588 ++++
 northd/en-global-config.h |   65 +
 northd/en-lflow.c         |  115 +-
 northd/en-lflow.h         |    8 +
 northd/en-lr-nat.c        |  423 +++
 northd/en-lr-nat.h        |  130 +
 northd/en-lr-stateful.c   |  669 ++++
 northd/en-lr-stateful.h   |  137 +
 northd/en-ls-stateful.c   |  443 +++
 northd/en-ls-stateful.h   |  104 +
 northd/en-northd.c        |   70 +-
 northd/en-northd.h        |    2 +-
 northd/en-port-group.h    |    3 +
 northd/en-sync-from-sb.c  |    2 +-
 northd/en-sync-sb.c       |  572 +++-
 northd/inc-proc-northd.c  |   74 +-
 northd/lb.c               |  124 +
 northd/lb.h               |  108 +
 northd/lflow-mgr.c        | 1230 +++++++
 northd/lflow-mgr.h        |  189 ++
 northd/northd.c           | 6343 ++++++++++++++++---------------------
 northd/northd.h           |  540 +++-
 northd/ovn-northd.c       |    7 +
 tests/ovn-macros.at       |   44 +
 tests/ovn-northd.at       | 1235 +++++++-
 32 files changed, 9338 insertions(+), 4106 deletions(-)
 create mode 100644 northd/en-global-config.c
 create mode 100644 northd/en-global-config.h
 create mode 100644 northd/en-lr-nat.c
 create mode 100644 northd/en-lr-nat.h
 create mode 100644 northd/en-lr-stateful.c
 create mode 100644 northd/en-lr-stateful.h
 create mode 100644 northd/en-ls-stateful.c
 create mode 100644 northd/en-ls-stateful.h
 create mode 100644 northd/lb.c
 create mode 100644 northd/lb.h
 create mode 100644 northd/lflow-mgr.c
 create mode 100644 northd/lflow-mgr.h

Comments

Han Zhou Jan. 25, 2024, 6:39 a.m. UTC | #1
On Thu, Jan 11, 2024 at 7:28 AM <numans@ovn.org> wrote:
>
> From: Numan Siddique <numans@ovn.org>
>
> This patch series adds incremental processing in the lflow engine
> node to handle changes to northd and other engine nodes.
> Changed related to load balancers and NAT are mainly handled in
> this patch series.
>
> This patch series can also be found here -
https://github.com/numansiddique/ovn/tree/northd_lbnatacl_lflow/v5
>
> Prior to this patch series, most of the changes to northd engine
> resulted in full recomputation of logical flows.  This series
> aims to improve the performance of ovn-northd by adding the I-P
> support.  In order to add this support, some of the northd engine
> node data (from struct ovn_datapath) is split and moved over to
> new engine nodes - mainly related to load balancers, NAT and ACLs.
>
> Below are the scale testing results done with these patches applied
> using ovn-heater.  The test ran the scenario  -
> ocp-500-density-heavy.yml [1].
>
> With all the lflow I-P patches applied, the resuts are:
>
>
-------------------------------------------------------------------------------------------------------------------------------------------------------
>                         Min (s)         Median (s)      90%ile (s)
 99%ile (s)      Max (s)         Mean (s)        Total (s)       Count
Failed
>
-------------------------------------------------------------------------------------------------------------------------------------------------------
> Iteration Total         0.136883        1.129016        1.192001
 1.204167        1.212728        0.665017        83.127099       125     0
> Namespace.add_ports     0.005216        0.005736        0.007034
 0.015486        0.018978        0.006211        0.776373        125     0
> WorkerNode.bind_port    0.035030        0.046082        0.052469
 0.058293        0.060311        0.045973        11.493259       250     0
> WorkerNode.ping_port    0.005057        0.006727        1.047692
 1.069253        1.071336        0.266896        66.724094       250     0
>
-------------------------------------------------------------------------------------------------------------------------------------------------------
>
> The results with the present main are:
>
>
-------------------------------------------------------------------------------------------------------------------------------------------------------
>                         Min (s)         Median (s)      90%ile (s)
 99%ile (s)      Max (s)         Mean (s)        Total (s)       Count
Failed
>
-------------------------------------------------------------------------------------------------------------------------------------------------------
> Iteration Total         0.135491        2.223805        3.311270
 3.339078        3.345346        1.729172        216.146495      125     0
> Namespace.add_ports     0.005380        0.005744        0.006819
 0.018773        0.020800        0.006292        0.786532        125     0
> WorkerNode.bind_port    0.034179        0.046055        0.053488
 0.058801        0.071043        0.046117        11.529311       250     0
> WorkerNode.ping_port    0.004956        0.006952        3.086952
 3.191743        3.192807        0.791544        197.886026      250     0
>
-------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Please see the link [2] which has a high level description of the
> changes done in this patch series.
>
>
> [1] -
https://github.com/ovn-org/ovn-heater/blob/main/test-scenarios/ocp-500-density-heavy.yml
> [2] -
https://mail.openvswitch.org/pipermail/ovs-dev/2023-December/410053.html
>
> v4 -> v5
> -------
>    * Rebased to latest main and resolved the conflicts.
>
>    * Addressed the review comments from Han in patch 15 (and in p8).
Removed the
>      assert if SB dp group is missing and handled it by returning false
>      so that lflow engine recomputes.  Added test cases to cover this
>      scenario for both lflows (p8) and SB load balancers (p15) .

Thanks Numan. I went through this version of the series. I tried my best to
review in details but I can't say I examined every lines of the changes.
The major comments are about the implicit dependency related to p4, p5, p6,
p7, and some pending discussions for p8 (for which I am also going to do
more performance test). For the rest of patches, please consider them as:
Acked-by: Han Zhou <hzhou@ovn.org>

Thanks,
Han

>
> v3 -> v4
> -------
>    * Addressed most of the review comments from Dumitru and Han.
>
>    * Found a couple of bugs in v3 patch 9 -
>      "northd: Refactor lflow management into a separate module."
>      and addressed them in v4.
>      To brief  the issue, if a logical flow L(M, A) is referenced
>      by 2 lflow_ref's which belong to the same datapath, then the lflow
>      was deleted even if one lflow_ref was cleared due to any changes.
>      It is addressed now by maintaining a reference count in the 'struct
>      ovn_lflow' for each datapath it is used by.
>
>    * Moved the v3 patch 14 ("northd:  Add I-P for NB_Global and
SB_Global.")
>      to patch 16 in v4.  There were comments in this patch to not add a
>      full I-P for NB_Global and SB_Global.  Made this patch as the last
>      in the series so that we can discuss further and not block other
patches
>      in case we want to drop this one.
>
>
> v2 -> v3
> -------
>    * Addressed some of the review comments from Han and Dumitru.  There
>      are still a few pending review comments which needs to be addressed
>      or discussed.
>
>    * Renamed the engine node from "lr_lbnat_data" to "lr_stateful"
>      (v3 patch 5).
>
>    * Renamed the engine node from "ls_lbacls" to "ls_stateful" (v3 patch
8).
>
>    * Removed v2 patch 2 from the series (northd: Track ovn_datapaths in
>      northd engine track data.").  This patch is now part of v3 patch 7
>      (northd: Add a new node 'ls_stateful').
>
>    * Squashed v2 patch 8 (northd: Don't commit dhcp response flows in
>      the conntrack.) into v3 patch 7 (northd: Add a new node
>      'ls_stateful'.)
>
>
> v1 -> v2
> --------
>    * Now also maintaing array indexes for ls_lbacls, lr_nat and
>      lr_lb_nat_data tables (similar to ovn_datapaths->array) to
>      make the lookup effecient.  The same ovn_datapath->index
>      is reused.
>
>    * Made some signficant changes to 'struct lflow_ref' in lflow-mgr.c.
>      In v2 we don't use objdep_mgr to maintain the resource to lflow
>      references.  Instead we maintain the 'struct lflow' pointer.
>      With this we don't need to maintain additional hmap of lflows.
>
>
> Numan Siddique (16):
>   northd: Refactor the northd change tracking.
>   tests: Add a couple of tests in ovn-northd for I-P.
>   northd: Move router ports SB PB options sync to sync_to_sb_pb node.
>   northd: Add a new engine 'lr_nat' to manage lr NAT data.
>   northd: Add a new engine 'lr_stateful' to manage lr's stateful data.
>   northd:  Generate router's stateful flows using lr_stateful data.
>   northd: Add a new node 'ls_stateful'.
>   northd: Refactor lflow management into a separate module.
>   northd: Use lflow_ref when adding all logical flows.
>   northd:  Move ovn_lb_datapaths from lib to northd module.
>   northd: Handle lb changes in lflow engine.
>   northd: Add lr_stateful handler for lflow engine node.
>   northd: Add ls_stateful handler for lflow engine node.
>   northd: Add a noop handler for northd SB mac binding.
>   northd: Add northd change handler for sync_to_sb_lb node.
>   northd:  Add I-P for NB_Global and SB_Global.
>
>  lib/lb.c                  |   96 -
>  lib/lb.h                  |   57 -
>  lib/ovn-util.c            |   24 +-
>  lib/ovn-util.h            |    4 +-
>  lib/stopwatch-names.h     |    3 +
>  northd/aging.c            |   21 +-
>  northd/automake.mk        |   14 +-
>  northd/en-global-config.c |  588 ++++
>  northd/en-global-config.h |   65 +
>  northd/en-lflow.c         |  115 +-
>  northd/en-lflow.h         |    8 +
>  northd/en-lr-nat.c        |  423 +++
>  northd/en-lr-nat.h        |  130 +
>  northd/en-lr-stateful.c   |  669 ++++
>  northd/en-lr-stateful.h   |  137 +
>  northd/en-ls-stateful.c   |  443 +++
>  northd/en-ls-stateful.h   |  104 +
>  northd/en-northd.c        |   70 +-
>  northd/en-northd.h        |    2 +-
>  northd/en-port-group.h    |    3 +
>  northd/en-sync-from-sb.c  |    2 +-
>  northd/en-sync-sb.c       |  572 +++-
>  northd/inc-proc-northd.c  |   74 +-
>  northd/lb.c               |  124 +
>  northd/lb.h               |  108 +
>  northd/lflow-mgr.c        | 1230 +++++++
>  northd/lflow-mgr.h        |  189 ++
>  northd/northd.c           | 6343 ++++++++++++++++---------------------
>  northd/northd.h           |  540 +++-
>  northd/ovn-northd.c       |    7 +
>  tests/ovn-macros.at       |   44 +
>  tests/ovn-northd.at       | 1235 +++++++-
>  32 files changed, 9338 insertions(+), 4106 deletions(-)
>  create mode 100644 northd/en-global-config.c
>  create mode 100644 northd/en-global-config.h
>  create mode 100644 northd/en-lr-nat.c
>  create mode 100644 northd/en-lr-nat.h
>  create mode 100644 northd/en-lr-stateful.c
>  create mode 100644 northd/en-lr-stateful.h
>  create mode 100644 northd/en-ls-stateful.c
>  create mode 100644 northd/en-ls-stateful.h
>  create mode 100644 northd/lb.c
>  create mode 100644 northd/lb.h
>  create mode 100644 northd/lflow-mgr.c
>  create mode 100644 northd/lflow-mgr.h
>
> --
> 2.43.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev