Re: [ovs-dev] [PATCH ovn] northd: Change the default value of ignore_lsp_down to true.

2023-08-24 Thread Jakub Libosvar


> On Aug 24, 2023, at 12:18 PM, Han Zhou  wrote:
> 
> 
> 
> On Thu, Aug 24, 2023 at 5:34 AM Jakub Libosvar  wrote:
> >
> > Hello,
> >
> > we use OpenStack (Neutron) as CMS and we have some customers reporting 
> > problems with their use case because of this change. I was wondering about 
> > the benefits described below in the commit message, are there any public 
> > data that back up the statements about the control plane cost at scale and 
> > latency? It would be great to have those to help with the decision whether 
> > we want to enable or disable this knob.
> 
> Hi Jakub,
> 
> Thanks for reporting this. I am curious about the use case that is impacted 
> by this behavior. Could you share more details?
> With regard to the control plane cost, I don't have data at hand, but it 
> should be easy to test either with a scale test environment or with an actual 
> deployment by enabling and disabling this setting and run the same test to 
> compare. You should see obvious northd CPU time increase because for each LSP 
> creation and binding there will be one more control plane round trip and 
> recompute in northd, and if you check the ARP responder flow installation on 
> the chassis, you should see its installation latency is much bigger. Of 
> course, this depends on the scale under test which determines how long it 
> takes for ovn-northd to recompute.
> 
> Thanks,
> Han
> 

Hi Han,

thanks for your reply. I don’t know much of details, the best of information I 
have is that they use OpenStack Manila with NetApp cluster. They’ve been using 
Neutron for IP allocation and the same IP is then configured on the NetApp 
storage as a share export. With the OVN option they get ARP replies for the 
unbound port instead of the real storage export, which uses the port's IP. 
That’s the best of my understanding. They fixed their environment by setting 
ignore_lsp_down to false, thus I wanted to know the benefits.

Thank you again.

Jakub

> >
> > Thank you very much.
> >
> > Jakub
> >
> > > On Oct 2, 2021, at 3:11 AM, Han Zhou  wrote:
> > >
> > > The current default behavior is that ARP responder flows for VIFs are
> > > added by northd after the port-binding state is UP, which creates more
> > > trouble than benefit in most use cases. To make the default behavior
> > > desirable for majority of the use cases, set the option ignore_lsp_down
> > > to true by default. This would help saving the control plane cost in
> > > large scale environment, reduce the e2e latency for all flows to be
> > > installed for a VIF, and making the VIF readiness checking more convenient
> > > in test cases and likely in CMS as well. User can still set it to false
> > > in circumstances (if any) when this behavior is not desired.
> > >
> > > Signed-off-by: Han Zhou 
> > > ---
> > > NEWS | 4 
> > > northd/northd.c  | 2 +-
> > > northd/ovn_northd.dl | 2 +-
> > > ovn-nb.xml   | 2 +-
> > > tests/ovn-northd.at  | 3 ++-
> > > 5 files changed, 9 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/NEWS b/NEWS
> > > index 8a21c029e..348f3f548 100644
> > > --- a/NEWS
> > > +++ b/NEWS
> > > @@ -12,6 +12,10 @@ OVN v21.09.0 - xx xxx 
> > >   - Allow static routes without nexthops.
> > >   - Enabled logical dp groups as a default.  CMS should disable it if not
> > > desired.
> > > +  - Set ignore_lsp_down to true as default, so that ARP responder flows 
> > > are
> > > +installed together with other flows when a logical switch port is 
> > > created,
> > > +without having to wait for the port to be UP.  CMS should set it to 
> > > false
> > > +if not desired.
> > >
> > > OVN v21.06.0 - 18 Jun 2021
> > > -
> > > diff --git a/northd/northd.c b/northd/northd.c
> > > index cf2467fe1..5ffd6b8eb 100644
> > > --- a/northd/northd.c
> > > +++ b/northd/northd.c
> > > @@ -14304,7 +14304,7 @@ ovnnb_db_run(struct northd_context *ctx,
> > > controller_event_en = smap_get_bool(>options,
> > > "controller_event", false);
> > > check_lsp_is_up = !smap_get_bool(>options,
> > > - "ignore_lsp_down", false);
> > > + "ignore_lsp_down", true);
> > >
> > > build_datapaths(ctx, datapaths, lr_list);
> > > build_ovn_lbs(ctx

Re: [ovs-dev] [PATCH ovn] northd: Change the default value of ignore_lsp_down to true.

2023-08-24 Thread Jakub Libosvar
Hello,

we use OpenStack (Neutron) as CMS and we have some customers reporting problems 
with their use case because of this change. I was wondering about the benefits 
described below in the commit message, are there any public data that back up 
the statements about the control plane cost at scale and latency? It would be 
great to have those to help with the decision whether we want to enable or 
disable this knob.

Thank you very much.

Jakub

> On Oct 2, 2021, at 3:11 AM, Han Zhou  wrote:
> 
> The current default behavior is that ARP responder flows for VIFs are
> added by northd after the port-binding state is UP, which creates more
> trouble than benefit in most use cases. To make the default behavior
> desirable for majority of the use cases, set the option ignore_lsp_down
> to true by default. This would help saving the control plane cost in
> large scale environment, reduce the e2e latency for all flows to be
> installed for a VIF, and making the VIF readiness checking more convenient
> in test cases and likely in CMS as well. User can still set it to false
> in circumstances (if any) when this behavior is not desired.
> 
> Signed-off-by: Han Zhou 
> ---
> NEWS | 4 
> northd/northd.c  | 2 +-
> northd/ovn_northd.dl | 2 +-
> ovn-nb.xml   | 2 +-
> tests/ovn-northd.at  | 3 ++-
> 5 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 8a21c029e..348f3f548 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -12,6 +12,10 @@ OVN v21.09.0 - xx xxx 
>   - Allow static routes without nexthops.
>   - Enabled logical dp groups as a default.  CMS should disable it if not
> desired.
> +  - Set ignore_lsp_down to true as default, so that ARP responder flows are
> +installed together with other flows when a logical switch port is 
> created,
> +without having to wait for the port to be UP.  CMS should set it to false
> +if not desired.
> 
> OVN v21.06.0 - 18 Jun 2021
> -
> diff --git a/northd/northd.c b/northd/northd.c
> index cf2467fe1..5ffd6b8eb 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -14304,7 +14304,7 @@ ovnnb_db_run(struct northd_context *ctx,
> controller_event_en = smap_get_bool(>options,
> "controller_event", false);
> check_lsp_is_up = !smap_get_bool(>options,
> - "ignore_lsp_down", false);
> + "ignore_lsp_down", true);
> 
> build_datapaths(ctx, datapaths, lr_list);
> build_ovn_lbs(ctx, datapaths, );
> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> index 22913f05a..7154fed13 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -753,7 +753,7 @@ Northd_Probe_Interval[interval] :-
> relation CheckLspIsUp[bool]
> CheckLspIsUp[check_lsp_is_up] :-
> nb in nb::NB_Global(),
> -var check_lsp_is_up = not nb.options.get_bool_def(i"ignore_lsp_down", 
> false).
> +var check_lsp_is_up = not nb.options.get_bool_def(i"ignore_lsp_down", 
> true).
> CheckLspIsUp[true] :-
> Unit(),
> not nb in nb::NB_Global().
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 390cc5a44..d8266ed4d 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -236,7 +236,7 @@
>   resolved even before the relevant VM/container is running. For
>   environments where this is not an issue, setting it to
>   true can reduce the load and latency of the control
> -  plane. The default value is false.
> +  plane. The default value is true.
> 
>   
> 
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index c5400d806..3eebb55b6 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -1918,6 +1918,7 @@ OVN_FOR_EACH_NORTHD([
> AT_SETUP([ignore_lsp_down])
> ovn_start
> 
> +ovn-nbctl set NB_Global . options:ignore_lsp_down=false
> ovn-nbctl ls-add sw0
> ovn-nbctl lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1 "aa:aa:aa:aa:aa:aa 
> 10.0.0.1"
> 
> @@ -4922,7 +4923,7 @@ check ovn-nbctl lsp-add sw0 lsp0 \
> 
> check ovn-nbctl --wait=sb sync
> AT_CHECK([ovn-sbctl --columns=tags list logical_flow | grep lsp0 -c], [0], 
> [dnl
> -9
> +10
> ])
> 
> AT_CLEANUP
> -- 
> 2.30.2
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 

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


Re: [ovs-dev] [PATCH ovn] controller: Restore MAC and vlan for DVR scenario

2022-12-16 Thread Jakub Libosvar
On 12/12/22 7:41 AM, Dumitru Ceara wrote:
> On 12/5/22 17:07, Dumitru Ceara wrote:
>> On 9/30/22 12:43, Dumitru Ceara wrote:
>>> On 9/20/22 22:18, Mark Michelson wrote:
 Thanks Ales,

 Acked-by: Mark Michelson 

>>>
>>> I applied this to the main branch and backported it to all stable
>>> branches down to branch-22.03.
>>>
>>
>> Hi all,
>>
>> There was an internal request from within Red Hat (from Jakub in CC) to
>> backport this fix to branch-21.12 too.  If nobody is it I can take care

Hi,

is there a BZ/Jira that I can set our OSP bug as depending on?

Thank you very much for the backport!

Kuba

> 
> Oops, s/is it/is against it/
> 
> But I hope the intention of the question was clear.
> 
>> of porting the patch to 21.12.
>>
>> I'll wait a day or two to give people time to reply.
>>
> 
> I went ahead and applied this to branch 21.12 too.
> 
> Regards,
> Dumitru
> 

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


Re: [ovs-dev] [PATCH ovn] controller: Restore MAC and vlan for DVR scenario

2022-12-15 Thread Jakub Libosvar
Thanks Dumitru! Is there any BZ/Jira ticket I could follow?

Thanks,
Kuba

On 12/12/22 7:41 AM, Dumitru Ceara wrote:
> On 12/5/22 17:07, Dumitru Ceara wrote:
>> On 9/30/22 12:43, Dumitru Ceara wrote:
>>> On 9/20/22 22:18, Mark Michelson wrote:
 Thanks Ales,

 Acked-by: Mark Michelson 

>>>
>>> I applied this to the main branch and backported it to all stable
>>> branches down to branch-22.03.
>>>
>>
>> Hi all,
>>
>> There was an internal request from within Red Hat (from Jakub in CC) to
>> backport this fix to branch-21.12 too.  If nobody is it I can take care
> 
> Oops, s/is it/is against it/
> 
> But I hope the intention of the question was clear.
> 
>> of porting the patch to 21.12.
>>
>> I'll wait a day or two to give people time to reply.
>>
> 
> I went ahead and applied this to branch 21.12 too.
> 
> Regards,
> Dumitru
> 

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


Re: [ovs-dev] [PATCH ovn] ovn-northd: Increase ECMP route priority with 400.

2020-05-18 Thread Jakub Libosvar
On 15/05/2020 18:00, Lorenzo Bianconi wrote:
>>> On Thu, May 14, 2020 at 1:12 AM Dumitru Ceara  wrote:

 On 5/13/20 11:51 PM, Han Zhou wrote:
> In commit c0bf32d the priorities of the regular routes were increased by
> 400, but ECMP routes were not updated. This patch fixes it. Since
> for ECMP routes the outport is unknown (there can be many different
> outports) the condition check of whether the outport is distributed
> gateway port is skipped.
>
> Fixes: c0bf32d ("Manage ARP process locally in a DVR scenario")
> Cc: Lorenzo Bianconi 
> Signed-off-by: Han Zhou 
>>
>> [...]
>>
>>> So I wonder if we should change the solution of the commit c0bf32d, instead
>>> of just increasing the ECMP routes priority only. I don't completely
>>> understand the problem that commit was trying to solve. Is there an example
>>> that describes the scenario in more detail and the reason why the route
>>> priorities have to be different?
>>
>> Hi Han,
>>
>> this morning Dumitru and me worked trying to find a solution for this issue.
>> We though to restore the original configuration in table 9
>> and add a new table used only to take care of FIP/DVR traffic.
>> Does it work for you?
>> I sent a RFC with the basic implementation. I tested it and it works fine
>> regarding to FIP traffic. I will work on unitest waiting for feedbacks.
>>
>> Regards,
>> Lorenzo
> 
> ops I forgot, this is the patch:
> https://patchwork.ozlabs.org/project/openvswitch/patch/a99d46b275a10a6d8278434f7cefa0466bb78af5.1589557561.git.lorenzo.bianc...@redhat.com/
> 
> Regards,
> Lorenzo

I just cherry-picked this patch to OVN master on my devstack environment
and tested it fixes the problem with Neutron and OpenStack.

Kuba

> 
>>
>>>

> ---
>  northd/ovn-northd.8.xml |  3 ++-
>  northd/ovn-northd.c | 22 --
>  2 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 8f224b0..b0475d0 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -2601,7 +2601,8 @@ next;
>ids MID1, MID2, ..., a logical flow
>>> with match
>in CIDR notation ip4.dst ==
>>> N/M,
>or ip6.dst == N/M, whose
>>> priority
> -  is the integer value of M, has the following
>>> actions:
> +  is 400 + the integer value of M, has
>>> the
> +  following actions:
>  
>
>  
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index b25152d..c02e305 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -7337,7 +7337,8 @@ build_route_prefix_s(const struct v46_ip *prefix,
>>> unsigned int plen)
>  }
>
>  static void
> -build_route_match(const struct ovn_port *op_inport, const char
>>> *network_s,
> +build_route_match(const struct ovn_port *op_inport,
> +  const struct ovn_port *op_outport, const char
>>> *network_s,
>int plen, bool is_src_route, bool is_ipv4, struct ds
>>> *match,
>uint16_t *priority)
>  {
> @@ -7351,6 +7352,13 @@ build_route_match(const struct ovn_port
>>> *op_inport, const char *network_s,
>  dir = "dst";
>  *priority = (plen * 2) + 1;
>  }
> +/* traffic for internal IPs of logical switch ports must be sent to
> + * the gw controller through the overlay tunnels
> + */
> +if (!op_outport ||
> +(op_outport->nbrp && !op_outport->nbrp->n_gateway_chassis)) {
> +priority += DROUTE_PRIO;
> +}
>
>  if (op_inport) {
>  ds_put_format(match, "inport == %s && ", op_inport->json_key);
> @@ -7434,8 +7442,8 @@ build_ecmp_route_flow(struct hmap *lflows, struct
>>> ovn_datapath *od,
>  uint16_t priority;
>
>  char *prefix_s = build_route_prefix_s(>prefix, eg->plen);
> -build_route_match(NULL, prefix_s, eg->plen, eg->is_src_route,
>>> is_ipv4,
> -  , );
> +build_route_match(NULL, NULL, prefix_s, eg->plen, eg->is_src_route,
> +  is_ipv4, , );
>  free(prefix_s);
>
>  struct ds actions = DS_EMPTY_INITIALIZER;
> @@ -7548,14 +7556,8 @@ add_route(struct hmap *lflows, const struct
>>> ovn_port *op,
>  op_inport = op;
>  }
>  }
> -build_route_match(op_inport, network_s, plen, is_src_route,
>>> is_ipv4,
> +build_route_match(op_inport, op, network_s, plen, is_src_route,
>>> is_ipv4,
>, );
> -/* traffic for internal IPs of logical switch ports must be sent to
> - * the gw controller through the overlay tunnels
> - */
> -if (op->nbrp && !op->nbrp->n_gateway_chassis) {
> -priority += DROUTE_PRIO;
> -}
>
> 

[ovs-dev] [PATCH] packaging: Add hostname as dependency

2017-02-13 Thread Jakub Libosvar
ovs-ctl script uses hostname. This patch adds dependency for debian and
rhel systems.

Signed-off-by: Jakub Libosvar <libos...@redhat.com>
---
 debian/control  | 2 +-
 rhel/openvswitch-fedora.spec.in | 2 +-
 rhel/openvswitch.spec.in| 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/debian/control b/debian/control
index 0b75f2b..25dc9b6 100644
--- a/debian/control
+++ b/debian/control
@@ -22,7 +22,7 @@ Homepage: http://openvswitch.org/
 
 Package: openvswitch-datapath-source
 Architecture: all
-Depends: bzip2, debhelper (>= 5.0.37), module-assistant, ${misc:Depends}
+Depends: bzip2, debhelper (>= 5.0.37), hostname, module-assistant, 
${misc:Depends}
 Suggests: openvswitch-switch
 Description: Open vSwitch datapath module source - module-assistant version
  Open vSwitch is a production quality, multilayer, software-based,
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index b395613..79212ad 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -68,7 +68,7 @@ BuildRequires: dpdk-devel >= 16.11
 Provides: %{name}-dpdk = %{version}-%{release}
 %endif
 
-Requires: openssl iproute module-init-tools
+Requires: openssl hostname iproute module-init-tools
 #Upstream kernel commit 4f647e0a3c37b8d5086214128614a136064110c3
 #Requires: kernel >= 3.15.0-0
 
diff --git a/rhel/openvswitch.spec.in b/rhel/openvswitch.spec.in
index 6aa23de..31a535c 100644
--- a/rhel/openvswitch.spec.in
+++ b/rhel/openvswitch.spec.in
@@ -32,7 +32,7 @@ License: ASL 2.0
 Release: 1
 Source: openvswitch-%{version}.tar.gz
 Buildroot: /tmp/openvswitch-rpm
-Requires: logrotate, python >= 2.7, python-six
+Requires: logrotate, hostname, python >= 2.7, python-six
 BuildRequires: python-six
 BuildRequires: openssl-devel
 BuildRequires: checkpolicy, selinux-policy-devel
-- 
1.8.3.1

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