[ovs-dev] [patch_v9] ovn: Add datapaths of interest filtering.

[Mickey Spiegel]

On Thu, Dec 1, 2016 at 8:07 PM, Darrell Ball <dlu998 at gmail.com> wrote:

> This patch adds datapaths of interest support where only datapaths of
> local interest are monitored by the ovn-controller ovsdb client.  The
> idea is to do a flood fill in ovn-controller of datapath associations
> calculated by northd. A new column is added to the SB database
> datapath_binding table - related_datapaths to facilitate this so all
> datapaths associations are known quickly in ovn-controller.  This
> allows monitoring to adapt quickly with a single new monitor setting
> for all datapaths of interest locally.
>

I agree that the flood fill approach is the right way to go. As you suggest
this allows monitoring to adapt quickly, compared to previous alternative
approaches.

I just wanted to raise one high level design implication for explicit
discussion, even though the high level design approach in this patch
is probably the right way to go.

This patch splits the flood fill processing between northd and the
controller. It is northd's responsibility to populate all neighbor
datapaths in the related_datapaths column of SB Datapath_Binding.
The controller then runs the flood fill algorithm based on these
related_datapaths, recursing from each Datapath_Binding through
related_datapaths to other Datapath_Bindings. In order for this to
succeed in one shot, it is necessary to access the full
Datapath_Binding table. The high level design implication is that
conditional monitoring cannot be applied to the Datapath_Binding
table itself. There should be no issue extending conditional
monitoring to other SB tables such as Multicast_Group and
MAC_Binding, in addition to the Logical_Flow and Port_Binding
covered in this patch.

One alternative approach would be to run the flood fill algorithm
in northd, populating all results (not just neighbor datapaths) in the
related_datapaths column of SB Datapath_Binding. However, that
has a different high level design implication. Since northd
processing is not chassis specific, it would not be possible to have
conditional monitoring filter out chassis-specific datapaths and
datapaths behind chassis-specific datapaths (e.g. l3gateway
router flow suppression mentioned below).

Filtering out chassis-specific datapaths / l3gateway router flow
suppression should be possible using the approach in this patch,
by having northd add the chassis name to the external_ids
column of SB Datapath_Binding and then checking that value
on each controller in add_datapath_of_interest. The alternative
would be to allow those Logical Flows to go to the controller,
instead preventing such flows from being installed by making a
small change in lflows.c.

My previous comments on v8 are still relevant to v9. I hope that
they can be resolved in the next version.

Mickey



>
> We monitor both logical flows and logical ports. The mac bindings
> will likely go away and multicast groups are relatively few.
> To reduce risk and overall latency we only suppress logical flows as
> a starting point. Logical ports are not suppressed as a starting
> point and but then are monitor suppressed as appropriate from that
> point on. The initial suppression has little practical gain and not
> doing it allows for us to be flexible in terms of how different port
> types are supported.
>
> To validate the gain, we take the approach to verify the l3gateway
> router flow suppression from the non-l3gateway HVs POV and also add
> a separate test to verify the flow and port handling scale advantages.
> The separate test added verifies the logical port and logical flow
> suppression advantage. This measures the difference in flows and ports
> sent to the HV clients where the far majority of the overall work is.
> The less ports and flows that are unnecessarily processed on the HVs,
> the more benefit, excluding latency skew. The separate test is
> simple for interpretation and shows an order of magnitude advantage;
> the test uses only logical switches. The advantage would increase with
> logical routers. The test is conservative in estimating the numerical
> advantage to increase the probability of the check passing the first
> time, since there are some timing conssiderations that affect the numbers.
>
> Liran Schour contributed enhancements to the conditional
> monitoring granularity in ovs and also submitted patches
> for ovn usage of conditional monitoring which aided this patch
> though sharing of concepts through code review work.
>
> Ben Pfaff suggested that northd could be used to pre-populate
> related datapaths for ovn-controller to use.  That idea is
> used as part of this patch.
>
> CC: Ben Pfaff <blp at ovn.org>
> CC: Liran Schour <LIRANS at il.ibm.com>
> Signed-off-by: Darrell Ball <dlu998 at gmail.com>
> ---
>
> v8->v9: Reinstate logical port monitoring from patch V3
>         now to handle some LP high influence topologies, especially
>         in lieu of a lack of incremental processing right now.
>         The original plan was to forgo monitoring logical ports
>         in the first commit, but without incremental processing,
>         port processing gains importance.
>
>         Parse out some functions
>
>         Add a separate test for conditional monitoring to
>         guesstimate the reduction in logical port and flow events
>         seen at the HVs.
>
> v7->v8: Start wth logical flow conditional monitoring off.
>         Remove deprecated code.
>         Recover SB DB version number change.
>         Change test to be more definitive.
>
> v6->v7: Rebase
>
> v5->v6: Rebase; fix stale handling.
>
> v4->v5: Correct cleanup of monitors.
>         Fix warning.
>
> v3->v4: Refactor after incremental processing backout.
>         Limit filtering to logical flows to limit risk.
>
> v2->v3: Line length violation fixups :/
>
> v1->v2: Added logical port removal monitoring handling, factoring
>         in recent changes for incremental processing.  Added some
>         comments in the code regarding initial conditions for
>         database conditional monitoring.
>
>  ovn/controller/binding.c        |  24 +++++--
>  ovn/controller/lflow.c          |   5 ++
>  ovn/controller/ovn-controller.c | 128 +++++++++++++++++++++++++++++++++-
>  ovn/controller/ovn-controller.h |   5 ++
>  ovn/controller/patch.c          |  34 ++++++++-
>  ovn/northd/ovn-northd.c         |  76 ++++++++++++++++++++
>  ovn/ovn-sb.ovsschema            |  11 ++-
>  ovn/ovn-sb.xml                  |   9 +++
>  tests/ovn.at                    | 149 ++++++++++++++++++++++++++++++
> ++++++++++
>  9 files changed, 427 insertions(+), 14 deletions(-)
>