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

[Darrell Ball]

From: Mickey Spiegel <mickeys@gmail.com>
Date: Wednesday, December 7, 2016 at 3:50 PM
To: Darrell Ball <db...@vmware.com>
Cc: Darrell Ball <dlu...@gmail.com>, ovs dev <d...@openvswitch.org>
Subject: Re: [ovs-dev] [patch_v10] ovn: Add datapaths of interest filtering.


On Tue, Dec 6, 2016 at 7:53 PM, Darrell Ball 
<db...@vmware.com<mailto:db...@vmware.com>> wrote:


On 12/4/16, 9:48 PM, 
"ovs-dev-boun...@openvswitch.org<mailto:ovs-dev-boun...@openvswitch.org> on 
behalf of Mickey Spiegel" 
<ovs-dev-boun...@openvswitch.org<mailto:ovs-dev-boun...@openvswitch.org> on 
behalf of mickeys@gmail.com<mailto:mickeys@gmail.com>> wrote:

On Sun, Dec 4, 2016 at 4:13 PM, Darrell Ball 
<dlu...@gmail.com<mailto:dlu...@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.
>

This is getting close. I have a few questions and comments inline.

>
> 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.
>

I lack familiarity with conditional monitoring behavior when you do
not explicitly suppress as a starting point. A new hv comes up, and
starts downloading stuff from SB, including port_bindings. At some
point the first VIF on that hv comes up, as a result of which the hv's
controller calls sbrec_port_binding_add_clause_logical_port.
Do all other port_bindings get suppressed at that point?
If so, won't this lead to different behavior between the first time a
VIF comes up on a hv, and subsequent additions of VIFs on
unrelated and previously unseen datapaths?
Wouldn't the behavior be more consistent if you suppress from the
beginning?
I commented on this before; I wanted to push this part until later; but fine, 
let us push it now.
The issue is l2gateway logical ports which may legitimately exist as the only 
port in a LS datapath
on a given HV.
L2gateway LPs are only known on a HV when the LP l2gateway-chassis option is 
downloaded to a HV.
If we want initial suppression of LPs, then we need to do something special for 
l2gateway ports.

This is a valid point that I missed.

For now, monitor all l2gateway ports unconditionally.
L2gateways are both a logical port and a chassis; not a datapath as in the case 
of l3gateway
 (Possibly later, a physical HV level external_id could be used to specify the 
presence of the l2gateway logical port
similarly to VIFs.).
The number of l2gateway ports could be on the order of tenants, at least.

The code that you added for l3gateway looks good.

If you do not take care of optimizing l2gateway, I will take a look
later. In my distributed NAT patch set, I am adding another
port_binding type on distributed routers, that will be bound to a
chassis. Similar to l2gateway, this new port_binding type should
trigger a clause based on that datapath, and unlike l3gateway it
does not affect the flood fill calculation. To handle both of these,
a new datapath option "chassis-of-interest" will need to be
added.

I will be vetting the proposal for l2gateways I mentioned.


>
> 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 conservati

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

[Mickey Spiegel]

On Tue, Dec 6, 2016 at 7:53 PM, Darrell Ball  wrote:

>
>
> On 12/4/16, 9:48 PM, "ovs-dev-boun...@openvswitch.org on behalf of Mickey
> Spiegel"  mickeys@gmail.com> wrote:
>
> On Sun, Dec 4, 2016 at 4:13 PM, Darrell Ball  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.
> >
>
> This is getting close. I have a few questions and comments inline.
>
> >
> > 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.
> >
>
> I lack familiarity with conditional monitoring behavior when you do
> not explicitly suppress as a starting point. A new hv comes up, and
> starts downloading stuff from SB, including port_bindings. At some
> point the first VIF on that hv comes up, as a result of which the hv's
> controller calls sbrec_port_binding_add_clause_logical_port.
> Do all other port_bindings get suppressed at that point?
> If so, won't this lead to different behavior between the first time a
> VIF comes up on a hv, and subsequent additions of VIFs on
> unrelated and previously unseen datapaths?
> Wouldn't the behavior be more consistent if you suppress from the
> beginning?
>
> I commented on this before; I wanted to push this part until later; but
> fine, let us push it now.
> The issue is l2gateway logical ports which may legitimately exist as the
> only port in a LS datapath
> on a given HV.
> L2gateway LPs are only known on a HV when the LP l2gateway-chassis option
> is downloaded to a HV.
> If we want initial suppression of LPs, then we need to do something
> special for l2gateway ports.
>

This is a valid point that I missed.


> For now, monitor all l2gateway ports unconditionally.
> L2gateways are both a logical port and a chassis; not a datapath as in the
> case of l3gateway
>  (Possibly later, a physical HV level external_id could be used to specify
> the presence of the l2gateway logical port
> similarly to VIFs.).
> The number of l2gateway ports could be on the order of tenants, at least.
>

The code that you added for l3gateway looks good.

If you do not take care of optimizing l2gateway, I will take a look
later. In my distributed NAT patch set, I am adding another
port_binding type on distributed routers, that will be bound to a
chassis. Similar to l2gateway, this new port_binding type should
trigger a clause based on that datapath, and unlike l3gateway it
does not affect the flood fill calculation. To handle both of these,
a new datapath option "chassis-of-interest" will need to be
added.

>
>
> >
> > 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.
> >
>
> This description should be updated to mention the distributed
> logical router tests that you added.
>
>
> sure
>
> >
> > 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
> > 

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

[Darrell Ball]

On 12/6/16, 7:08 PM, "ovs-dev-boun...@openvswitch.org on behalf of Russell 
Bryant"  wrote:

On Tue, Dec 6, 2016 at 3:35 PM, Ben Pfaff  wrote:

> On Tue, Dec 06, 2016 at 02:45:19PM -0500, Russell Bryant wrote:
> > On Mon, Dec 5, 2016 at 2:22 AM, Ben Pfaff  wrote:
> >
> > > On Sun, Dec 04, 2016 at 04:13:44PM -0800, Darrell Ball 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.
> > >
> > > Hi Darrell, the series I just sent out has some relevance here.  It
> > > makes ovn-controller only implement the datapaths and ports flows that
> > > are relevant to a given hypervisor, even though it does not affect 
what
> > > part of the database is replicated.  The particularly relevant patch 
is
> > > this:
> > > 
https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_702608_=DgICAg=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=qWAPHpdqV7RcWokRLBU_mikSMVTnTefdf_BqOa0D9xE=iRVMW7EreZ7g-20aDnp497XNOeX0UbLTtBG0HHDPYXQ=
 
> > >
> > > I suggest having a look at the series.
> > >
> >
> > I'm in the middle of doing some control plane performance testing using
> > OpenStack and OVN.  I may have time to do another run with some
> additional
> > patches applied to see how they affect performance.  Ideally I'd be able
> to
> > do that this week, though.
> >
> > If I'm able to fit this in, what do you guys suggest if I've only got 
one
> > shot?  This patch or Ben's series?
>
> This series is aimed more at technical debt and code clarity than at
> performance.  It might help with performance, but that's not the main
> goal.
>
> Darrell/Liran's series is motivated by performance.  If it doesn't help
> performance, then probably it shouldn't be applied because it makes the
> code harder to understand.  (Disclaimer: I haven't read the recent
> versions.)
>

I just deployed this change in my environment and was not able to observe a
noticeable difference in CPU consumption of ovn-controller.  (ovn-northd
was updated, as well.)

The scenario was creating 500 VMs through OpenStack at a concurrency of 64
(up to 64 VMs at a time).  Each VM also received its own network.  The 500
VMs were distributed among 9 hypervisors.  Once all 500 VMs were up, they
were all deleted.

Your results seem to be quite different than Liran’s. What is the difference ?

How many:
1) logical routers
2) gateway routers
 do you have ?

Sounds like this test boots up 500 VMs, checking for all to be created and then 
deletes them
and then ends ? 

The CPU consumption looked roughly the same both before and after the patch
and time to complete the test scenario was also similar.

ovn-controller hits 100% CPU about 30% of the way into creating VMs and
stays there until we finish creating them and start deleting them instead.

-- 
Russell Bryant
___
dev mailing list
d...@openvswitch.org

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DgICAg=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=qWAPHpdqV7RcWokRLBU_mikSMVTnTefdf_BqOa0D9xE=INRPrNb-XW4vMEJXjpU5SuTBNF_snlyvngHM3fevc_Y=
 


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Darrell Ball]

On 12/4/16, 9:48 PM, "ovs-dev-boun...@openvswitch.org on behalf of Mickey 
Spiegel"  
wrote:

On Sun, Dec 4, 2016 at 4:13 PM, Darrell Ball  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.
>

This is getting close. I have a few questions and comments inline.

>
> 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.
>

I lack familiarity with conditional monitoring behavior when you do
not explicitly suppress as a starting point. A new hv comes up, and
starts downloading stuff from SB, including port_bindings. At some
point the first VIF on that hv comes up, as a result of which the hv's
controller calls sbrec_port_binding_add_clause_logical_port.
Do all other port_bindings get suppressed at that point?
If so, won't this lead to different behavior between the first time a
VIF comes up on a hv, and subsequent additions of VIFs on
unrelated and previously unseen datapaths?
Wouldn't the behavior be more consistent if you suppress from the
beginning?

I commented on this before; I wanted to push this part until later; but fine, 
let us push it now.
The issue is l2gateway logical ports which may legitimately exist as the only 
port in a LS datapath
on a given HV.
L2gateway LPs are only known on a HV when the LP l2gateway-chassis option is 
downloaded to a HV.
If we want initial suppression of LPs, then we need to do something special for 
l2gateway ports.
For now, monitor all l2gateway ports unconditionally.
L2gateways are both a logical port and a chassis; not a datapath as in the case 
of l3gateway
 (Possibly later, a physical HV level external_id could be used to specify the 
presence of the l2gateway logical port
similarly to VIFs.).
The number of l2gateway ports could be on the order of tenants, at least.


>
> 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.
>

This description should be updated to mention the distributed
logical router tests that you added.


sure

>
> 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 
> CC: Liran Schour 
> Signed-off-by: Darrell Ball 
> ---
>
> v9->v10: Fix ordering issue on northd as pointed out by Mickey.
>
>  Remove patch flood fill triggering optimization to
>  make the code easier to verify and harder to break.
>
>  Fix a couple issues on ovn-controller side, including
>  stale checking.
>
>  Add lots more focused testing - now 2 separate 

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

[Darrell Ball]

On 12/6/16, 2:03 PM, "ovs-dev-boun...@openvswitch.org on behalf of Russell 
Bryant"  wrote:

On Sun, Dec 4, 2016 at 7:13 PM, Darrell Ball  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.
>
> 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 
> CC: Liran Schour 
> Signed-off-by: Darrell Ball 
> ---
>

When I apply this patch to master, one of the tests is failing for me.
This test passes if I drop the patch.

2296: ovn-controller - ovn-bridge-mappingsFAILED (
ovn-controller.at:45)

I am looking at it
This test checks for patch ports to be created unconditionally, which changes
with conditional monitoring of ports.


-- 
Russell Bryant
___
dev mailing list
d...@openvswitch.org

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DgICAg=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=wr5myh5e_jqsSNotKbCHZ0h1vKRN4DgPmWbXLwFUuaE=CxrUAL7Q_YpMPdeAJdUd7pX3EnhKNIzs3DLj9cF7xzE=
 


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Russell Bryant]

On Sun, Dec 4, 2016 at 7:13 PM, Darrell Ball  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.
>
> 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 
> CC: Liran Schour 
> Signed-off-by: Darrell Ball 
> ---
>

When I apply this patch to master, one of the tests is failing for me.
This test passes if I drop the patch.

2296: ovn-controller - ovn-bridge-mappingsFAILED (
ovn-controller.at:45)

-- 
Russell Bryant
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Russell Bryant]

On Tue, Dec 6, 2016 at 3:36 PM, Ben Pfaff  wrote:

> On Tue, Dec 06, 2016 at 12:35:16PM -0800, Ben Pfaff wrote:
> > On Tue, Dec 06, 2016 at 02:45:19PM -0500, Russell Bryant wrote:
> > > On Mon, Dec 5, 2016 at 2:22 AM, Ben Pfaff  wrote:
> > >
> > > > On Sun, Dec 04, 2016 at 04:13:44PM -0800, Darrell Ball 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.
> > > >
> > > > Hi Darrell, the series I just sent out has some relevance here.  It
> > > > makes ovn-controller only implement the datapaths and ports flows
> that
> > > > are relevant to a given hypervisor, even though it does not affect
> what
> > > > part of the database is replicated.  The particularly relevant patch
> is
> > > > this:
> > > > https://patchwork.ozlabs.org/patch/702608/
> > > >
> > > > I suggest having a look at the series.
> > > >
> > >
> > > I'm in the middle of doing some control plane performance testing using
> > > OpenStack and OVN.  I may have time to do another run with some
> additional
> > > patches applied to see how they affect performance.  Ideally I'd be
> able to
> > > do that this week, though.
> > >
> > > If I'm able to fit this in, what do you guys suggest if I've only got
> one
> > > shot?  This patch or Ben's series?
> >
> > This series is aimed more at technical debt and code clarity than at
>   ^^^
> I meant "my series", not this one.
>
> > performance.  It might help with performance, but that's not the main
> > goal.
> >
> > Darrell/Liran's series is motivated by performance.  If it doesn't help
> > performance, then probably it shouldn't be applied because it makes the
> > code harder to understand.  (Disclaimer: I haven't read the recent
> > versions.)
>

OK, thanks for confirming.

I'll do a run with this included if we have time, since this seems to be
the main pending change related to OVN control plane performance.  Let me
know if I missed something, though.

-- 
Russell Bryant
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Ben Pfaff]

On Tue, Dec 06, 2016 at 12:35:16PM -0800, Ben Pfaff wrote:
> On Tue, Dec 06, 2016 at 02:45:19PM -0500, Russell Bryant wrote:
> > On Mon, Dec 5, 2016 at 2:22 AM, Ben Pfaff  wrote:
> > 
> > > On Sun, Dec 04, 2016 at 04:13:44PM -0800, Darrell Ball 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.
> > >
> > > Hi Darrell, the series I just sent out has some relevance here.  It
> > > makes ovn-controller only implement the datapaths and ports flows that
> > > are relevant to a given hypervisor, even though it does not affect what
> > > part of the database is replicated.  The particularly relevant patch is
> > > this:
> > > https://patchwork.ozlabs.org/patch/702608/
> > >
> > > I suggest having a look at the series.
> > >
> > 
> > I'm in the middle of doing some control plane performance testing using
> > OpenStack and OVN.  I may have time to do another run with some additional
> > patches applied to see how they affect performance.  Ideally I'd be able to
> > do that this week, though.
> > 
> > If I'm able to fit this in, what do you guys suggest if I've only got one
> > shot?  This patch or Ben's series?
> 
> This series is aimed more at technical debt and code clarity than at
  ^^^
I meant "my series", not this one.

> performance.  It might help with performance, but that's not the main
> goal.
> 
> Darrell/Liran's series is motivated by performance.  If it doesn't help
> performance, then probably it shouldn't be applied because it makes the
> code harder to understand.  (Disclaimer: I haven't read the recent
> versions.)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Ben Pfaff]

On Tue, Dec 06, 2016 at 02:45:19PM -0500, Russell Bryant wrote:
> On Mon, Dec 5, 2016 at 2:22 AM, Ben Pfaff  wrote:
> 
> > On Sun, Dec 04, 2016 at 04:13:44PM -0800, Darrell Ball 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.
> >
> > Hi Darrell, the series I just sent out has some relevance here.  It
> > makes ovn-controller only implement the datapaths and ports flows that
> > are relevant to a given hypervisor, even though it does not affect what
> > part of the database is replicated.  The particularly relevant patch is
> > this:
> > https://patchwork.ozlabs.org/patch/702608/
> >
> > I suggest having a look at the series.
> >
> 
> I'm in the middle of doing some control plane performance testing using
> OpenStack and OVN.  I may have time to do another run with some additional
> patches applied to see how they affect performance.  Ideally I'd be able to
> do that this week, though.
> 
> If I'm able to fit this in, what do you guys suggest if I've only got one
> shot?  This patch or Ben's series?

This series is aimed more at technical debt and code clarity than at
performance.  It might help with performance, but that's not the main
goal.

Darrell/Liran's series is motivated by performance.  If it doesn't help
performance, then probably it shouldn't be applied because it makes the
code harder to understand.  (Disclaimer: I haven't read the recent
versions.)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Russell Bryant]

On Mon, Dec 5, 2016 at 2:22 AM, Ben Pfaff  wrote:

> On Sun, Dec 04, 2016 at 04:13:44PM -0800, Darrell Ball 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.
>
> Hi Darrell, the series I just sent out has some relevance here.  It
> makes ovn-controller only implement the datapaths and ports flows that
> are relevant to a given hypervisor, even though it does not affect what
> part of the database is replicated.  The particularly relevant patch is
> this:
> https://patchwork.ozlabs.org/patch/702608/
>
> I suggest having a look at the series.
>

I'm in the middle of doing some control plane performance testing using
OpenStack and OVN.  I may have time to do another run with some additional
patches applied to see how they affect performance.  Ideally I'd be able to
do that this week, though.

If I'm able to fit this in, what do you guys suggest if I've only got one
shot?  This patch or Ben's series?

-- 
Russell Bryant
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Mickey Spiegel]

On Sun, Dec 4, 2016 at 4:13 PM, Darrell Ball  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.
>

This is getting close. I have a few questions and comments inline.

>
> 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.
>

I lack familiarity with conditional monitoring behavior when you do
not explicitly suppress as a starting point. A new hv comes up, and
starts downloading stuff from SB, including port_bindings. At some
point the first VIF on that hv comes up, as a result of which the hv's
controller calls sbrec_port_binding_add_clause_logical_port.
Do all other port_bindings get suppressed at that point?
If so, won't this lead to different behavior between the first time a
VIF comes up on a hv, and subsequent additions of VIFs on
unrelated and previously unseen datapaths?
Wouldn't the behavior be more consistent if you suppress from the
beginning?

>
> 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.
>

This description should be updated to mention the distributed
logical router tests that you added.

>
> 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 
> CC: Liran Schour 
> Signed-off-by: Darrell Ball 
> ---
>
> v9->v10: Fix ordering issue on northd as pointed out by Mickey.
>
>  Remove patch flood fill triggering optimization to
>  make the code easier to verify and harder to break.
>
>  Fix a couple issues on ovn-controller side, including
>  stale checking.
>
>  Add lots more focused testing - now 2 separate extensive
>  tests and beefed up l3gateway test.
>
>  Remove dependency on port filtering for l3gateways filtering
>  and associate l3gateway chassis name as part of datapath,
>  similarly to logical ports. Some agreement on adding this
>  extra field to the datapath binding table.
>

I certainly agree. It is good to have the filtering depend only
on the contents of datapath_binding, with no dependencies
on port_bindings. This way there is no issue with ordering
dependencies when filtering port_bindings.

>
> 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.
>