[ovs-dev] [PATCH] Use southbound database when saying Binding table
For the sake of clarity Signed-off-by: Hui Kang --- ovn/ovn-architecture.7.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml index 13acaf5..659c599 100644 --- a/ovn/ovn-architecture.7.xml +++ b/ovn/ovn-architecture.7.xml @@ -354,8 +354,8 @@ destined to the new port's MAC address should be delivered to it, and update the flow that delivers broadcast and multicast packets to include the new port. It also creates a record in the Binding table - and populates all its columns except the column that identifies the - chassis. + of the Southbound database and populates all its columns except the + column that identifies the chassis. -- 1.9.1 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] Use southbound database when saying Binding table
-Ben Pfaff wrote: - To: Hui Kang From: Ben Pfaff Date: 03/23/2016 02:22PM Cc: dev@openvswitch.org, Hui Kang/Watson/IBM@IBMUS Subject: Re: [ovs-dev] [PATCH] Use southbound database when saying Binding table On Wed, Mar 23, 2016 at 02:00:32PM -0400, Hui Kang wrote: > For the sake of clarity > > Signed-off-by: Hui Kang > --- > ovn/ovn-architecture.7.xml | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml > index 13acaf5..659c599 100644 > --- a/ovn/ovn-architecture.7.xml > +++ b/ovn/ovn-architecture.7.xml > @@ -354,8 +354,8 @@ >destined to the new port's MAC address should be delivered to it, and >update the flow that delivers broadcast and multicast packets to > include >the new port. It also creates a record in the Binding > table > - and populates all its columns except the column that identifies the > - chassis. > + of the Southbound database and populates all its columns except the > + column that identifies the chassis. This would be a better clarification if it corrected the name of the table. Hi, Ben, Thanks for your reply to my very first patch to OVS. Do you mean a different name rather than Binding table? - Hui ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ovn-architecture.7.xml: Clarify the definition of Binding table
For the sake of clarity Signed-off-by: Hui Kang --- ovn/ovn-architecture.7.xml | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml index 13acaf5..a588a0e 100644 --- a/ovn/ovn-architecture.7.xml +++ b/ovn/ovn-architecture.7.xml @@ -353,9 +353,10 @@ table to reflect the new port, e.g. add a flow to recognize that packets destined to the new port's MAC address should be delivered to it, and update the flow that delivers broadcast and multicast packets to include - the new port. It also creates a record in the Binding table - and populates all its columns except the column that identifies the - chassis. + the new port. It also creates a record in the Datapath_Binding, + Port_Binding, and MAC_Binding tables (hereafter, Binding + table) of the Southbound database and populates all its columns except + the column that identifies the chassis. -- 1.9.1 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ovn-architecture.7.xml: Clarify the definition of Binding table
"dev" wrote on 03/24/2016 03:29:28 PM: > From: Ben Pfaff > To: Hui Kang > Cc: dev@openvswitch.org > Date: 03/24/2016 03:29 PM > Subject: Re: [ovs-dev] [PATCH] ovn-architecture.7.xml: Clarify the > definition of Binding table > Sent by: "dev" > > On Thu, Mar 24, 2016 at 02:50:47PM -0400, Hui Kang wrote: > > For the sake of clarity > > > > Signed-off-by: Hui Kang > > --- > > ovn/ovn-architecture.7.xml | 7 --- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml > > index 13acaf5..a588a0e 100644 > > --- a/ovn/ovn-architecture.7.xml > > +++ b/ovn/ovn-architecture.7.xml > > @@ -353,9 +353,10 @@ > >table to reflect the new port, e.g. add a flow to recognize > that packets > >destined to the new port's MAC address should be delivered to it, and > >update the flow that delivers broadcast and multicast > packets to include > > - the new port. It also creates a record in the > Binding table > > - and populates all its columns except the column that identifies the > > - chassis. > > + the new port. It also creates a record in the Datapath_Binding, > > + Port_Binding, and MAC_Binding tables (hereafter, Binding > > + table) of the Southbound database and populates all its > columns except > > + the column that identifies the chassis. > > Now this doesn't make any sense at all. Hi, Ben, from my understanding, as a new VIF with MAC address and flow are added to to the southbound database, these tables seem need to be updated. Is there any misunderstanding here? Thanks. - Hui > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] Display correct ovnsb sock location in the ovnsb-ctl help message
Signed-off-by: Hui Kang
---
ovn/utilities/ovn-sbctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index a257729..a888333 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -329,7 +329,7 @@ Options:\n\
-t, --timeout=SECS wait at most SECS seconds\n\
--dry-run do not commit changes to database\n\
--oneline print exactly one line of output per command\n",
- program_name, program_name, ctl_get_db_cmd_usage(),
ctl_default_db());
+ program_name, program_name, ctl_get_db_cmd_usage(),
sbctl_default_db());
vlog_usage();
printf("\
--no-syslog equivalent to --verbose=sbctl:syslog:warn\n");
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ovn-architecture.7.xml: Fix ovn-controller behavior in VIF life cycle
Signed-off-by: Hui Kang --- ovn/ovn-architecture.7.xml | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml index f8a348b..72786bc 100644 --- a/ovn/ovn-architecture.7.xml +++ b/ovn/ovn-architecture.7.xml @@ -382,12 +382,12 @@ On the hypervisor where the VM is powered on, ovn-controller notices external-ids:iface-id in the new - Interface. In response, it updates the local hypervisor's OpenFlow - tables so that packets to and from the VIF are properly handled. - Afterward, in the OVN Southbound DB, it updates the + Interface. In response, in the OVN Southbound DB, it updates the Binding table's chassis column for the - row that links the logical port from - external-ids:iface-id to the hypervisor. + row that links the logical port from external-ids: + iface-id to the hypervisor. Afterward, ovn-controller + updates the local hypervisor's OpenFlow tables so that packets to and from + the VIF are properly handled. -- 1.9.1 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH monitor_cond V7 00/10] Conditional monitor implementation
"dev" wrote on 06/13/2016 07:19:25 AM: > From: Liran Schour > To: b...@ovn.org > Cc: d...@openvswitch.com > Date: 06/13/2016 08:29 AM > Subject: [ovs-dev] [PATCH monitor_cond V7 00/10] Conditional monitor > implementation > Sent by: "dev" > > This patch series implements conditional monitoring by introducing an OVSDB > RFC extension with 2 new JSON-RPC methods: "monitor_cond" and > "monitor_cond_change". Specification of this extension is defined in the > ovsdb-server (1) man page. > Monitor2 is now merged into monitor_cond. A monitor_cond session > with an empty > condition, will behave exactly like monitor2 and will get update2 > notifications > on all rows. > > This patch series is also available on: https://github.com/liranschour/ovs.git > branch monitor_cond_ovn. Hi, Liran, I am interested in cloning this patch from github. However, in your git repo, I see several branches name with monitor_cond_ovn, i.e., monitor_cond_ovn, monitor_cond_ovn_v7_ovn, monitor_cond_ovn_dev Which one shall I use? Thanks. - Hui > > OVN: > Last patch in this series is a RFC for OVN usage of conditional monitoring. > > Performance evaluation: > OVN is the main candidate for conditional monitoring usage. It is clear that > conditional monitoring reduces computation on the ovn-controller (client) side > due to the reduced size of flow tables and update messages. However, > performance evaluation shows also a reduction in computation on the SB > ovsdb-server side proportional to the degree that each logical network is > spread over physical hosts in the DC. > > Evaluation on simulated environment of 50 hosts and 1000 logical ports shows > the following results (cycles #): > > LN spread over # hosts|master| patch| change > - > 1 | 58855158082 | 38175941755 | 35.1% > 3 | 54816462604 | 40255584120 | 26.5% > 6 | 52972265506 | 39481653891 | 25.4% >12 | 57036827284 | 42008285519 | 26.3% >18 | 61900476558 | 45903107035 | 25.8% >24 | 64281399690 | 55617752599 | 13.4% >30 | 66905128558 | 61835913623 | 7.5% >42 | 76763742331 | 70522724721 | 8.1% >50 | 85372146321 | 80130285454 | 6.1% > > Changes V6 --> V7: > -- > * Change ovsdb-idl API to include only add and remove clause from condition. > * IDL maintain and track conditions. Send monitor_cond_change on condition > change. > * Due to IDL API change, OVN does not maintain and track condition. > * Report column duplication on ovsdv_monitor_add_column(). > * Split added documentation according to patches. > * Add to testing for monitor_cond_change method in ovsdb-client via unixctl > command. > * An update, if any, as a result of a condition change, will be sent to the > client before the reply to the "monitor_cond_update" request. > * Minor fixes due to review. > > Liran Schour (10): > ovsdb: create column index mapping between ovsdb row to monitor row > ovsdb: add conditions utilities to support monitor_cond > ovsdb: allow unmonitored columns in condition evaluation > ovsdb: generate update notifications for monitor_cond session > ovsdb: enable jsonrpc-server to service "monitor_cond_change" request > ovsdb-client: support monitor-cond method > lib: add to ovsdb-idl monitor_id > python: move Python idl to work with monitor_cond > lib: add monitor_cond_change API to C IDL lib > RFC OVN: Quick implementation of conditional monitoring > > NEWS| 3 +- > lib/automake.mk | 2 + > lib/ovsdb-condition.c | 47 > lib/ovsdb-condition.h | 45 +++ > lib/ovsdb-idl-provider.h| 2 + > lib/ovsdb-idl.c | 239 ++-- > lib/ovsdb-idl.h | 30 ++ > ovn/controller/binding.c| 108 +++- > ovn/controller/binding.h| 4 +- > ovn/controller/lport.c | 123 - > ovn/controller/lport.h | 10 +- > ovn/controller/ovn-controller.c | 23 +- > ovsdb/condition.c | 188 - > ovsdb/condition.h | 57 ++-- > ovsdb/jsonrpc-server.c | 233 +--- > ovsdb/jsonrpc-server.h | 2 +- > ovsdb/monitor.c | 592 + > +-- > ovsdb/monitor.h | 46 +++- > ovsdb/ovsdb-client.1.in | 37 ++- > ovsdb/ovsdb-client.c| 98 +-- > ovsdb/ovsdb-idlc.in | 364 +++- > ovsdb/ovsdb-server.1.in | 221 +-- > ovsdb/ovsdb-server.c| 20 +- > ovsdb/query.c | 6 +- > python/ovs/db/data.py | 16 +- > python/ovs/db/idl.py| 190 +++-- > tests/ovn-controller.at | 3 + > tests/ovs-vswi
Re: [ovs-dev] [PATCH V5] Function tracer to trace all function calls
Hi, Daniele, I am also interesting in profiling the ovs process and would like to participate in the discussion. I am using oprofile-1.1.0 to profiling the ovn-northd process. I believe oprofile-1.1.0 share the same underlying profiling mechanism as Linux perf (correct me if I am wrong). However, it is hard for me to understand the output of the profiled data. For example, in a highly overloaded ovn-northd process, the output of opreport --callgraph is === CPU: Intel Sandy Bridge microarchitecture, speed 3300 MHz (estimated) Counted CPU_CLK_UNHALTED events (Clock cycles when not halted) with a unit mask of 0x00 (No unit mask) count 9 samples %image name symbol name --- 1 0.3497 ovn-northd build_lswitch_flows.constprop.16 1 0.3497 ovn-northd sbrec_port_binding_set_options 1 0.3497 ovn-northd jsonrpc_session_run 1 0.3497 ovn-northd _init 1 0.3497 ovn-northd nbrec_logical_port_set_addresses 1 0.3497 ovn-northd nbrec_logical_router_port_get_mac 1 0.3497 ovn-northd sbrec_chassis_set_vtep_logical_switches 1 0.3497 ovn-northd describe_fd 1 0.3497 ovn-northd ovs_error 1 0.3497 ovn-northd vlog_valist 1 0.3497 ovn-northd make_unix_socket 1 0.3497 ovn-northd ofpbuf_use_const 2 0.6993 ovn-northd quicksort 2 0.6993 ovn-northd count_cpu_cores 7 2.4476 ovn-northd _fini 263 91.9580 ovn-northd ovs_scan__ 137264 58.5425 libc-2.19.so /lib/x86_64-linux-gnu/libc-2.19.so 137264 100.000 libc-2.19.so /lib/x86_64-linux-gnu/libc-2.19.so [self] --- 133. ovn-northd json_hash 266.6667 ovn-northd sbrec_chassis_set_external_ids 17467 7.4496 ovn-northd main 1746799.9828 ovn-northd main [self] 2 0.0114 kallsyms apic_timer_interrupt 1 0.0057 kallsyms reschedule_interrupt --- 13190 5.6255 ovn-northd hexit_value 1319099.9924 ovn-northd hexit_value [self] 1 0.0076 kallsyms apic_timer_interrupt --- 10227 4.3618 ovn-northd hash_bytes 1022799.9804 ovn-northd hash_bytes [self] 2 0.0196 kallsyms apic_timer_interrupt --- 9458 4.0338 ovn-northd scan_int === The callgraph seems not aligh with the ovn-northd implementation. Did I miss anything when compiling openvswitch source code (I used default ./configure; make to generate the ovn-northd execuable)? Thanks. - Hui > From: Daniele Di Proietto > To: Nirapada Ghosh/San Jose/IBM@IBMUS > Cc: "dev@openvswitch.org" > Date: 06/14/2016 09:58 PM > Subject: Re: [ovs-dev] [PATCH V5] Function tracer to trace all function calls > Sent by: "dev" > > Hi Nirapada, > > When optimizing for the DPDK datapath we have a very similar problem and, > usually, running a simple profiler like perf ( > https://perf.wiki.kernel.org/index.php/Main_Page) is enough to highlight > the bottlenecks in terms of CPU usage. > > Have you tried perf? Does this infrastructure provide advantages over > sample profiling? I'm not saying that one is better than the other, I'm > just curious! > > Thanks, > > Daniele > > 2016-06-14 18:05 GMT-07:00 : > > > In some circumstances, we might need to figure out where in > > code, the CPU time is being spent most, so as to pinpoint > > the bottleneck and thereby resolve it with proper changes. > > Using '-finstrument-functions' flag, that can be achieved, and > > this patch exactly does that. > > > > There is a python file [generate_ft_report.py] with the patch, > > that may be used to convert this trace output to a human readable > > format with symbol names instead of address and their execution > > times. This tool uses addr2line that expects the executable to > > be built with -g flag. > > > > To enable this feature, ovs needs needs to be configured with > > "--enable-ft" command line argument [i.e. configure --enable-ft] > > > > This instrumentation logs the tracing output in separate log files > > namely func_trace_.log. It does not use VLOG mechanism for > > logging as that will make the patch very complicat
Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine
Yusheng Wang wrote on 06/17/2016 04:30:02 AM: > From: Yusheng Wang > To: Hui Kang/Watson/IBM@IBMUS > Cc: "dev@openvswitch.org" > Date: 06/17/2016 04:40 AM > Subject: Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine > > > The idea is to write ovn-northd in nlog program. In order to do that, > we need a hook to ovs-db, which is the next step I am going to do. > Generally, a subset of ovs-db tables will be input tables to the nlog > engine and the engine's output table will also be written to ovs-db. > > If we think about ovn-northd as a data converter (function) between > north bound tables and south bound tables, it is straightforward that > any engine that works in this way could be used. Thanks for your detailed explanation. This plan is more ambition than I thought; it involves rewriting the ovn-northd. Look forward to that. > > The improvement comes from incremental computation. As for the > scalability, the paper [2] may not apply to OVN since they are of > totally different design. The paper explicitly says the nlog implements the incremental computation for any state change in the input table, "incremental computation allows us to recompute only the affected state and push the delta down to the network edge." Therefore, IMO your implementation is applicable to either OVN or the NVP [2]. Please correct me if I am wrong. > > In order to test the engine, we could try the interactive test case. There > is one combo test case in the patch and it has an interactive mode. We > could load a nlog program and run data on it. I will try the interactive test case in the patch. One minor thing: do you mind removing some trailing whitespace in these patches [5] [6] so that patching is easier. Thanks. Some error message is as follows: Applying: OVN: datalog man page /root/Downloads/ovs/.git/rebase-apply/patch:34: trailing whitespace. /root/Downloads/ovs/.git/rebase-apply/patch:58: trailing whitespace. specify the external function of the engine. /root/Downloads/ovs/.git/rebase-apply/patch:291: trailing whitespace. u(a,b) +---+---++---+---++---+---+---+---+ /root/Downloads/ovs/.git/rebase-apply/patch:293: trailing whitespace. +---+---++---+---++---+---+---+---+ /root/Downloads/ovs/.git/rebase-apply/patch:295: trailing whitespace. +---+---++---+---+---+---+ error: patch failed: ovn/lib/automake.mk:43 error: ovn/lib/automake.mk: patch does not apply [5] https://patchwork.ozlabs.org/patch/623323/ [6] https://patchwork.ozlabs.org/patch/636324/ - Hui > > -- Yusheng > > From: Hui Kang > Sent: Friday, June 17, 2016 3:28 PM > To: Yusheng Wang > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine > > Hi, Yusheng, > I am very intersted in testing your nice nlog implementation. Meanwhile, I > appreciate if you could look at some questions I have about this nlog. > > - Reading the early proposal of nlog [1], I understand that nlog will > be serving as a new computation model for ovn-northd. Furthermore, Ithink the > plan is to replace the existing join_XXX operations (e.g., [3][4]) in > populating tables in southbound database. Is my understanding correct? > > - How much performance improvement will you expect from using nlog? > The NVP paper [2] mentioned in section 8.2 that even with nlog, the > scalability issue still exists because Openflow in hypervisors requires > O(N^2). > > Thanks in advance. > > - Hui > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine
"dev" wrote on 06/16/2016 10:58:58 PM: > From: Yusheng Wang > To: Ryan Moats/Omaha/IBM@IBMUS > Cc: "dev@openvswitch.org" > Date: 06/16/2016 10:59 PM > Subject: Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine > Sent by: "dev" > > > I agree with all your comments but the last one. The benefit of > using one source file is > that you know everything is there as far as the engine is concerned. > We definitely need > to separate it once it is really large. > > I suppose it will take some time for the code to stabilize and the > engine will evolve over > time. The package right now is self contained and people could try > with it using the > interactive test case -- writing a datalog program, presenting it > with some input and > seeing what comes out. The previous man page have the implementation details > and it is a good start point before diving into the code. > > As for the prefix, I was thinking about 'dl' but it might be too > short to distinguish itself > while 'datalog' might be too long considering all functions will > carry it. Since test cases > are living in another source file, quite a few internal functions > have to be declared as > external so a lot more functions have to carry that name. > > I noticed the automake conflict when I sync my repo. I am not sure > if we have the guideline > of not changing the last line which does not have back slash char, > instead adding new lines > in the middle. Hope this will not prevent people from patching and > trying with it. The conflict is caused by a later change to ovn/lib/automake.mk. A quick fix is to change line 43 to line 45 in your patch. - Hui > > -- Yusheng > > From: Ryan Moats > Sent: Friday, June 17, 2016 12:03 AM > To: Yusheng Wang > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine > > This commit message is too bare for a commit this size. > Please provide some content for later readers that aren't > up to speed with the technology being introduced here. > > Because the patch is so big, I'm not including my comments > in-line, I'm going to put them all here: > > I know there was a separate patch for the ovn-datalog man patch, > I think that patch should be folded into this one for a more > atomic approach. > > Note: the log_ prefix is already used in lib/util.h and while this > patch introduces a private include file, I'd be more comfortable > if the log_ prefix were replaced with _datalog_ to be more consistent > with the ovs naming convention. > > Similarly, this patch overloads other prefixes that are used > elsewhere - I'd consider cleaning up the prefix usage to > avoid confusion later on. > > Nit: is there any reason why ENT_* values 0x0c through 0x0f > aren't in numerical order like the entries from 0 to 0x0b > appear to be? > > Nit (found in multiple places): comments should begin with a > capital letter and end with a period > > Also, I'm thinking that the datalog source file should > be broken into multiple files rather than concatenated together > (as it is over 3K lines long) > > Lastly, this patch set has a collision in the automake.mk file > with the current master and so could use a rebase. > > Ryan Moats > > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine
Hui Kang/Watson/IBM wrote on 06/17/2016 11:04:21 AM: > From: Hui Kang/Watson/IBM > To: Yusheng Wang > Cc: Ryan Moats/Omaha/IBM@IBMUS, "dev@openvswitch.org" > Date: 06/17/2016 11:04 AM > Subject: Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine > > "dev" wrote on 06/16/2016 10:58:58 PM: > > > From: Yusheng Wang > > To: Ryan Moats/Omaha/IBM@IBMUS > > Cc: "dev@openvswitch.org" > > Date: 06/16/2016 10:59 PM > > Subject: Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine > > Sent by: "dev" > > > > > > I agree with all your comments but the last one. The benefit of > > using one source file is > > that you know everything is there as far as the engine is concerned. > > We definitely need > > to separate it once it is really large. > > > > I suppose it will take some time for the code to stabilize and the > > engine will evolve over > > time. The package right now is self contained and people could try > > with it using the > > interactive test case -- writing a datalog program, presenting it > > with some input and > > seeing what comes out. The previous man page have the implementation details > > and it is a good start point before diving into the code. > > > > As for the prefix, I was thinking about 'dl' but it might be too > > short to distinguish itself > > while 'datalog' might be too long considering all functions will > > carry it. Since test cases > > are living in another source file, quite a few internal functions > > have to be declared as > > external so a lot more functions have to carry that name. > > > > I noticed the automake conflict when I sync my repo. I am not sure > > if we have the guideline > > of not changing the last line which does not have back slash char, > > instead adding new lines > > in the middle. Hope this will not prevent people from patching and > > trying with it. > > The conflict is caused by a later change to ovn/lib/automake.mk. > A quick fix is to change line 43 to line 45 in your patch. Sorry, this fix is to the automake.mk in a previous patch for the datalog man page. > > - Hui > > > > > -- Yusheng > > > > From: Ryan Moats > > Sent: Friday, June 17, 2016 12:03 AM > > To: Yusheng Wang > > Cc: dev@openvswitch.org > > Subject: Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine > > > > This commit message is too bare for a commit this size. > > Please provide some content for later readers that aren't > > up to speed with the technology being introduced here. > > > > Because the patch is so big, I'm not including my comments > > in-line, I'm going to put them all here: > > > > I know there was a separate patch for the ovn-datalog man patch, > > I think that patch should be folded into this one for a more > > atomic approach. > > > > Note: the log_ prefix is already used in lib/util.h and while this > > patch introduces a private include file, I'd be more comfortable > > if the log_ prefix were replaced with _datalog_ to be more consistent > > with the ovs naming convention. > > > > Similarly, this patch overloads other prefixes that are used > > elsewhere - I'd consider cleaning up the prefix usage to > > avoid confusion later on. > > > > Nit: is there any reason why ENT_* values 0x0c through 0x0f > > aren't in numerical order like the entries from 0 to 0x0b > > appear to be? > > > > Nit (found in multiple places): comments should begin with a > > capital letter and end with a period > > > > Also, I'm thinking that the datalog source file should > > be broken into multiple files rather than concatenated together > > (as it is over 3K lines long) > > > > Lastly, this patch set has a collision in the automake.mk file > > with the current master and so could use a rebase. > > > > Ryan Moats > > > > ___ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine
Hi, Yusheng, When running the datalog test_interactive program, I have some difficulty in generating the output of the incremental change. Could you help to check at which step I did something wrong? - Step 1: Create a rule of joining table r(a) with an empty table as in the example root@test:~/ovs/tests# ./ovstest test-datalog run Input rules, e.g. R(a):r(a). use EOF as end R(a):r(a). EOF Input changes, e.g. +:1:tbl_name|1:f0:f1|. use +, -, ? for add, remove, or query. could input multiple tables. the number in table name line indicates number of tuples to follow. '|' stands for new line. use '.' to exit for query, field value could be empty. Step 2: I tried to insert one tuple to table r in order to see the change datalog will make to table R. Since table r has one field name as a, I input the following line and I got this error message +:1:r|1|. Error in format --- Any idea? Thanks. - Hui Yusheng Wang wrote on 06/17/2016 04:30:02 AM: > From: Yusheng Wang > To: Hui Kang/Watson/IBM@IBMUS > Cc: "dev@openvswitch.org" > Date: 06/17/2016 04:40 AM > Subject: Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine > > > The idea is to write ovn-northd in nlog program. In order to do that, > we need a hook to ovs-db, which is the next step I am going to do. > Generally, a subset of ovs-db tables will be input tables to the nlog > engine and the engine's output table will also be written to ovs-db. > > If we think about ovn-northd as a data converter (function) between > north bound tables and south bound tables, it is straightforward that > any engine that works in this way could be used. > > The improvement comes from incremental computation. As for the > scalability, the paper [2] may not apply to OVN since they are of > totally different design. > > In order to test the engine, we could try the interactive test case. There > is one combo test case in the patch and it has an interactive mode. We > could load a nlog program and run data on it. > > -- Yusheng > > From: Hui Kang > Sent: Friday, June 17, 2016 3:28 PM > To: Yusheng Wang > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine > > Hi, Yusheng, > I am very intersted in testing your nice nlog implementation. Meanwhile, I > appreciate if you could look at some questions I have about this nlog. > > - Reading the early proposal of nlog [1], I understand that nlog will > be serving as a new computation model for ovn-northd. Furthermore, Ithink the > plan is to replace the existing join_XXX operations (e.g., [3][4]) in > populating tables in southbound database. Is my understanding correct? > > - How much performance improvement will you expect from using nlog? > The NVP paper [2] mentioned in section 8.2 that even with nlog, the > scalability issue still exists because Openflow in hypervisors requires > O(N^2). > > Thanks in advance. > > - Hui > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine
Yusheng Wang wrote on 06/19/2016 11:12:49 PM: > From: Yusheng Wang > To: Hui Kang/Watson/IBM@IBMUS > Cc: "dev@openvswitch.org" > Date: 06/19/2016 11:13 PM > Subject: Re: [ovs-dev] [PATCH] OVN: initial patch of datalog engine > > > > The paper explicitly says the nlog implements the incremental computation > > for any state change in the input table, "incremental computation allows us > > to recompute only the affected state and push the delta down to the network > > edge." Therefore, IMO your implementation is applicable to either OVN or > > the NVP [2]. Please correct me if I am wrong. > > You are correct. They are going to have similar behavior with > respect to computation > method. What I meant previously is that we could not compare the > performance just > based on the computation engine. Other factors, such as how database > interacts with > the engine and how the computation results are dispensed to hosts, > will also have impact. Thanks for the explanation and I agree with your above comments. In current OVN implementation, the ovn-controller takes the responsibility of generating local openflow rules in each hypervisor, translating from the logical flows in the southbound database. If all the work is performed at the nlog engine and pushed to other hypervisors, I suspect the nlog node could become the bottleneck. I am very interested in this work and please let me know if I could help in any case. Thanks. - Hui > > > Some error message is as follows: > > Applying: OVN: datalog man page > > /root/Downloads/ovs/.git/rebase-apply/patch:34: trailing whitespace. > > Thanks for pointing out this. I will remove those white spaces in later patch. > > -- Yusheng > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [OVN] Potential scalability bug in ovn-northd on creating and binding large number of lports
Hi, In our scalability test for OVN, we observed an in-scalable behaviour of the ovn-northd process: the time binding a logical port increases as # of large port increasing, regardless of whether logical ports belong to the same logical switch. The most suspicious function in causing this issue is build_ports() called by ovnnb_db_run() [1], as described below. Test description: step 1: Create 6 logical switches. For each logical switch, create 200 logical ports. step 2: Bind 200 lports from each logical switch on an OVN chassis. Test results for step 2: # of ports | # of ovn_ports| Cpu cycle spent in | | allocated in build_port() | built_port(), in million | 200 |200 | 25| 400 |400 | 50| 600 |600 | 75| 800 |800 | 93| 1000 | 1000 |108| 1200 | 1200 |125| We see that on binding each logical port on a hypervisor, join_logical_ports() in build_port allocates the number of (struct ovn_port) for all the existing ports in the southbound database [2], which causes the accumulated CPU cycles. My question is whether there is any particular reason to allocate that number of (struct ovn_port)? It seems to me there is room in this code to optimize for performance. Thanks. - Hui [1] https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L2529 [2] https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L571 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd on creating and binding large number of lports
Hi, Ryan, Thanks for your comments; reply inline. - Hui Ryan Moats/Omaha/IBM wrote on 06/24/2016 08:12:38 AM: > From: Ryan Moats/Omaha/IBM > To: Hui Kang/Watson/IBM@IBMUS, "Ben Pfaff" , "Justin > Pettit" , "Yusheng Wang" > Cc: dev@openvswitch.org > Date: 06/24/2016 08:12 AM > Subject: Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd > on creating and binding large number of lports > > "dev" wrote on 06/23/2016 12:56:59 PM: > > > From: Hui Kang/Watson/IBM@IBMUS > > To: dev@openvswitch.org > > Date: 06/23/2016 12:57 PM > > Subject: [ovs-dev] [OVN] Potential scalability bug in ovn-northd on > > creating and binding large number of lports > > Sent by: "dev" > > > > > > Hi, > > In our scalability test for OVN, we observed an in-scalable behaviour of > > the > > ovn-northd process: the time binding a logical port increases as # of large > > port increasing, regardless of whether logical ports belong to the same > > logical > > switch. The most suspicious function in causing this issue is build_ports() > > called by ovnnb_db_run() [1], as described below. > > > > Test description: > > step 1: Create 6 logical switches. For each logical switch, create 200 > > logical ports. > > step 2: Bind 200 lports from each logical switch on an OVN chassis. > > > > Test results for step 2: > > > > # of ports | # of ovn_ports| Cpu cycle spent in | > > | allocated in build_port() | built_port(), in million | > > 200 |200 | 25 | > > 400 |400 | 50 | > > 600 |600 | 75 | > > 800 |800 | 93 | > >1000 | 1000 |108 | > >1200 | 1200 |125 | > > > > We see that on binding each logical port on a hypervisor, > > join_logical_ports() > > in build_port allocates the number of (struct ovn_port) for all the > > existing > > ports in the southbound database [2], which causes the accumulated CPU > > cycles. > > > > My question is whether there is any particular reason to allocate that > > number > > of (struct ovn_port)? It seems to me there is room in this code to optimize > > for performance. Thanks. > > > > - Hui > > > > > > [1] > > https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L2529 > > [2] > > https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L571 > > Hui, ovn-northd's current design is that it processes the entire ovn nb db > each computational cycle, so I would expect to see what you are seeing, which > is the argument for converting ovn-northd to an incremental processing model. > As shown in the above table, in each computation loop (especially the ovnnb_db_run), there are N (i.e., the number of all logical ports in the southbound db) is created [3]. IMO, this is unnecessary at least for the binding operation, because binding a logical port impacts a limited number of ports (e.g., lports in the same logical switch). So for incremental processing model, I think we can created a global ovn_port list and only update the ports that is triggered by the ovsdb events. [3] https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L571 > Ben, Justin, Yusheng can one of you talk to an ETA for when the nlog > ovn-northd code base will start to land in the review queue? That will > provide input on whether doing an interim patch series is worth the effort > or not... What does ETA mean? I am not sure if you have read the thread about Yusheng's inital datalog patch[4] My understanding is that (1) it requires a radical redesign of the existing ovn-northd code; (2) the performance improvement is unclear. Really, the majority of the benefits of nlog comes from the feature of this programming language. Therefore, an interim patch fixing the current northd scalability issue makes sense to me. [4] http://openvswitch.org/pipermail/dev/2016-June/073176.html > > Ryan ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd on creating and binding large number of lports
> > My question is whether there is any particular reason to allocate that > > number > > of (struct ovn_port)? It seems to me there is room in this code to > optimize > > for performance. Thanks. > > > > - Hui > > > > > > [1] > > > https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L2529 > > > [2] > > > https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L571 > Hui, ovn-northd's current design is that it processes the entire ovn nb db > each computational cycle, so I would expect to see what you are seeing, > which > is the argument for converting ovn-northd to an incremental processing > model. > > Ben, Justin, Yusheng can one of you talk to an ETA for when the nlog > ovn-northd code base will start to land in the review queue? That will > provide input on whether doing an interim patch series is worth the effort > or not... > > Incremental processing is one angle. The other we need to have on > the roadmap is some type of sharding. This is important for HA > purposes, as well. We need to be able to run multiple instances of > ovn-northd that each only operate on a subset of the full data set. Hi, Russell Thanks for your comments. I agree that sharding is an important ascpect to improve the southdbound db scalability. In the NVP paper [5], sharding has been implemented within Onix controller. However, I have no idea how difficulty it is to import the implementation to OVN. - Hui [5] http://benpfaff.org/papers/net-virt.pdf > > -- > Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] [OVN-northd] Rename nbs/nbr to nbsp/nbrp
These variables indicate ports in nb switches or routers.
Signed-off-by: Hui Kang
---
ovn/northd/ovn-northd.c | 196
1 file changed, 98 insertions(+), 98 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 1599e18..5bc5408 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -486,9 +486,9 @@ struct ovn_port {
char *key; /* nbs->name, nbr->name, sb->logical_port. */
char *json_key; /* 'key', quoted for use in JSON. */
-const struct nbrec_logical_switch_port *nbs; /* May be NULL. */
-const struct nbrec_logical_router_port *nbr; /* May be NULL. */
-const struct sbrec_port_binding *sb; /* May be NULL. */
+const struct nbrec_logical_switch_port *nbsp; /* May be NULL. */
+const struct nbrec_logical_router_port *nbrp; /* May be NULL. */
+const struct sbrec_port_binding *sb; /* May be NULL. */
/* Logical router port data. */
ovs_be32 ip, mask; /* 192.168.10.123/24. */
@@ -504,8 +504,8 @@ struct ovn_port {
static struct ovn_port *
ovn_port_create(struct hmap *ports, const char *key,
-const struct nbrec_logical_switch_port *nbs,
-const struct nbrec_logical_router_port *nbr,
+const struct nbrec_logical_switch_port *nbsp,
+const struct nbrec_logical_router_port *nbrp,
const struct sbrec_port_binding *sb)
{
struct ovn_port *op = xzalloc(sizeof *op);
@@ -516,8 +516,8 @@ ovn_port_create(struct hmap *ports, const char *key,
op->key = xstrdup(key);
op->sb = sb;
-op->nbs = nbs;
-op->nbr = nbr;
+op->nbsp = nbsp;
+op->nbrp = nbrp;
hmap_insert(ports, &op->key_node, hash_string(op->key, 0));
return op;
}
@@ -578,21 +578,21 @@ join_logical_ports(struct northd_context *ctx,
HMAP_FOR_EACH (od, key_node, datapaths) {
if (od->nbs) {
for (size_t i = 0; i < od->nbs->n_ports; i++) {
-const struct nbrec_logical_switch_port *nbs =
od->nbs->ports[i];
-struct ovn_port *op = ovn_port_find(ports, nbs->name);
+const struct nbrec_logical_switch_port *nbsp =
od->nbs->ports[i];
+struct ovn_port *op = ovn_port_find(ports, nbsp->name);
if (op) {
-if (op->nbs || op->nbr) {
+if (op->nbsp || op->nbrp) {
static struct vlog_rate_limit rl
= VLOG_RATE_LIMIT_INIT(5, 1);
VLOG_WARN_RL(&rl, "duplicate logical port %s",
- nbs->name);
+ nbsp->name);
continue;
}
-op->nbs = nbs;
+op->nbsp = nbsp;
ovs_list_remove(&op->list);
ovs_list_push_back(both, &op->list);
} else {
-op = ovn_port_create(ports, nbs->name, nbs, NULL, NULL);
+op = ovn_port_create(ports, nbsp->name, nbsp, NULL, NULL);
ovs_list_push_back(nb_only, &op->list);
}
@@ -600,41 +600,41 @@ join_logical_ports(struct northd_context *ctx,
}
} else {
for (size_t i = 0; i < od->nbr->n_ports; i++) {
-const struct nbrec_logical_router_port *nbr
+const struct nbrec_logical_router_port *nbrp
= od->nbr->ports[i];
struct eth_addr mac;
-if (!eth_addr_from_string(nbr->mac, &mac)) {
+if (!eth_addr_from_string(nbrp->mac, &mac)) {
static struct vlog_rate_limit rl
= VLOG_RATE_LIMIT_INIT(5, 1);
-VLOG_WARN_RL(&rl, "bad 'mac' %s", nbr->mac);
+VLOG_WARN_RL(&rl, "bad 'mac' %s", nbrp->mac);
continue;
}
ovs_be32 ip, mask;
-char *error = ip_parse_masked(nbr->network, &ip, &mask);
+char *error = ip_parse_masked(nbrp->network, &ip, &mask);
if (error || mask == OVS_BE32_MAX || !ip_is_cidr(mask)) {
static struct vlog_rate_limit rl
= VLOG_RATE_LIMIT_INIT(5, 1);
-VLOG_WARN_RL(&rl, "bad 'network' %s", nbr->network);
+VLOG_WARN_RL(&rl, "bad 'network' %s", nbrp->network);
free(error);
continue;
}
-struct
Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd on creating and binding large number of lports
Ben Pfaff wrote on 06/24/2016 02:24:12 PM: > From: Ben Pfaff > To: Ryan Moats/Omaha/IBM@IBMUS > Cc: Hui Kang/Watson/IBM@IBMUS, Justin Pettit , > Yusheng Wang , dev@openvswitch.org > Date: 06/24/2016 02:24 PM > Subject: Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd > on creating and binding large number of lports > > On Fri, Jun 24, 2016 at 07:12:38AM -0500, Ryan Moats wrote: > > Ben, Justin, Yusheng can one of you talk to an ETA for when the nlog > > ovn-northd code base will start to land in the review queue? That will > > provide input on whether doing an interim patch series is worth the effort > > or not... > > My expectation is that nlog won't make it into OVS 2.6 next month, but > will make it into the following release 6 months later, so if there are > simple ways we can increase scalability in the meantime, let's take a > look at them. Sounds good. I will look into if a quick fix is possible to optimize this part. Thanks. - Hui > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd on creating and binding large number of lports
Ryan Moats/Omaha/IBM wrote on 06/24/2016 10:51:10 PM: > From: Ryan Moats/Omaha/IBM > To: Hui Kang/Watson/IBM@IBMUS > Cc: dev@openvswitch.org > Date: 06/24/2016 10:51 PM > Subject: Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd > on creating and binding large number of lports > > "dev" wrote on 06/23/2016 12:56:59 PM: > > > From: Hui Kang/Watson/IBM@IBMUS > > To: dev@openvswitch.org > > Date: 06/23/2016 12:57 PM > > Subject: [ovs-dev] [OVN] Potential scalability bug in ovn-northd on > > creating and binding large number of lports > > Sent by: "dev" > > > > > > Hi, > > In our scalability test for OVN, we observed an in-scalable behaviour of > > the > > ovn-northd process: the time binding a logical port increases as # of large > > port increasing, regardless of whether logical ports belong to the same > > logical > > switch. The most suspicious function in causing this issue is build_ports() > > called by ovnnb_db_run() [1], as described below. > > > > Test description: > > step 1: Create 6 logical switches. For each logical switch, create 200 > > logical ports. > > step 2: Bind 200 lports from each logical switch on an OVN chassis. > > > > Test results for step 2: > > > > # of ports | # of ovn_ports| Cpu cycle spent in | > > | allocated in build_port() | built_port(), in million | > > 200 |200 | 25 | > > 400 |400 | 50 | > > 600 |600 | 75 | > > 800 |800 | 93 | > >1000 | 1000 |108 | > >1200 | 1200 |125 | > > > > We see that on binding each logical port on a hypervisor, > > join_logical_ports() > > in build_port allocates the number of (struct ovn_port) for all the > > existing > > ports in the southbound database [2], which causes the accumulated CPU > > cycles. > > > > My question is whether there is any particular reason to allocate that > > number > > of (struct ovn_port)? It seems to me there is room in this code to optimize > > for performance. Thanks. > > > > - Hui > > > > > > [1] > > https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L2529 > > [2] > > https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L571 > > Hui- > > Since it looks like simple short term optimization of northd is a > "Good Thing" (TM) [1], Is the above the "long pole in the tent" or > are there other hot spots of similar brightness? > > I'm planning at looking at what might be possible, and want to know if > there are other spots that should be included in the pass. Hi, Ryan, Thanks for your attention to this problem. As of now, our profiling data shows that the build_ports (including its callee such as join_logical_port) is the hot spot. We can discuss on IRC or slack about how to optimize. Regards, - Hui > > Thanks in advance, > Ryan > > [1] http://openvswitch.org/pipermail/dev/2016-June/073574.html ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd on creating and binding large number of lports
> > > > Actually, I take that back. The cycles/port for all the cases above > > demonstrate only slightly nonlinear scaling: 200/25 is 8 Mcycles/port, > > 1200/125 is 9.6 Mcycles/port. > > > > So the issue is not that it does not scale. The issue is that it is > > slow. > > Er? When I do the ratios, I come up with 125 Kcycles/port at 200 ports going > down to slightly more than 104 Kcycles/port at 1200 ports, which is slightly > sub-linear (and I do think that's a good thing). > > However, I'm left wondering if it would be possible to make things even > better through judicial use of persistence and incremental processing. > > Right now the ports logic looks to me like: > - Build a list of all ports known via port bindings in the sb db. > - For each port known via the nb db: > - Look for the port in the sb list. > - If found, move the port from the sb list to the both list > - If not found, create a new entry in the nb_only list. > (After the above finishes, we have three lists: sb_only, nb_only, and both) > - For each entry in the both list, do modifications to align the port > binding with nb information. > - For each entry in the nb_only list, create port_binding information in > the sb db. > [If I were updating the port lists, I'd move the port from the nb_only > list to both list] > - For each entry in the sb_only list, remove from the port_binding table. > [If I were updating the sb_only list, I'd remove it from the sb_only > list] Hi, Ryan Thanks for drafting the pseudo-code. Please allow me to add number bullets in your original version to accommodate further discussions. 1. Build a list of all ports known via port bindings in the sb db. 2. For each port known via the nb db: 2.1 Look for the port in the sb list. 2.2 If found, move the port from the sb list to the both list 2.3 If not found, create a new entry in the nb_only list. (After the above finishes, we have three lists: sb_only, nb_only, and both) 3. For each entry in the both list, do modifications to align the port binding with nb information. 4. For each entry in the nb_only list, create port_binding information in the sb db. [If I were updating the port lists, I'd move the port from the nb_only list to both list] 5. For each entry in the sb_only list, remove from the port_binding table. [If I were updating the sb_only list, I'd remove it from the sb_only list] In square bracket of step 4., do you mean "If I were updating the nb_lists in step 2.3., ..."? Similarly, in step 5, do you mean "If I were updating the sb_only list in step 2.2,..."? In my opinion, step 4 and step 5 could be avoided with your logic in square bracket. Is my understanding correct? > > I *think* if I were to consider persisting the sb_only, nb_only, and both > lists and follow the extra logic I've added in square brackets above, I'd > only have entries in the both list at the end of the calculation set, so I > should only need to persist the both table. What do you mean by "persisting"? A global linked list to store the elements of struct ovn_ports? > > Further, I *think* if I were to then apply change tracking to the first > part of the process above, the logic changes to: Which step of the above pseudo-code should the following code be embedded into ? Thanks in advance. - Hui > > - For each tracked entry in the port bindings table > - if it is a deleted entry, remove from the both list (if there is still > a nb entry, we'll recreate it further on) > - if it is a new entry, add it to the sb_only list > - if it is a modified entry, find it in the both list and update the > sb information contained in the entry > - For each port known via the nb db: > - if the entry is found in the both list, update the nb data contained > in the entry > - if the entry is not in the both list, but is in the sb_only list, > move the entry from the sb_list to the both list > - if the entry is not in either the both or the sb_only list, create > a new entry in the nb_only list > - For each entry in the both list, do modifications to align the port > binding with nb information. > - For each entry in the nb_only list, create port_binding information in > the sb db and move the entry from the nb_only to the both list > - For each entry in the sb_only list, remove from the port_binding table. > > Now, I'm pretty sure this will cut down the number of cycles, but before > I go off and code it [and potentially break something ala yesterday's > excitement], I'm looking for some verification of both my conclusion of > persisting just the both list and the modified logic incorporating the > persisted both list and port binding change tracking adjustments). Do > these make sense or have I missed something? > > Ryan > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd on creating and binding large number of lports
Ryan Moats/Omaha/IBM wrote on 06/25/2016 09:07:39 PM: > From: Ryan Moats/Omaha/IBM > To: Hui Kang/Watson/IBM@IBMUS > Cc: Ben Pfaff , dev@openvswitch.org > Date: 06/25/2016 09:07 PM > Subject: Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd > on creating and binding large number of lports > > Hui Kang/Watson/IBM wrote on 06/25/2016 07:53:36 PM: > > > From: Hui Kang/Watson/IBM > > To: Ryan Moats/Omaha/IBM@IBMUS > > Cc: Ben Pfaff , dev@openvswitch.org > > Date: 06/25/2016 07:53 PM > > Subject: Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd > > on creating and binding large number of lports > > > > > > > > > > Actually, I take that back. The cycles/port for all the cases above > > > > demonstrate only slightly nonlinear scaling: 200/25 is 8 Mcycles/port, > > > > 1200/125 is 9.6 Mcycles/port. > > > > > > > > So the issue is not that it does not scale. The issue is that it is > > > > slow. > > > > > > Er? When I do the ratios, I come up with 125 Kcycles/port at 200 > ports going > > > down to slightly more than 104 Kcycles/port at 1200 ports, which > is slightly > > > sub-linear (and I do think that's a good thing). > > > > > > However, I'm left wondering if it would be possible to make things even > > > better through judicial use of persistence and incremental processing. > > > > > > Right now the ports logic looks to me like: > > > - Build a list of all ports known via port bindings in the sb db. > > > - For each port known via the nb db: > > > - Look for the port in the sb list. > > > - If found, move the port from the sb list to the both list > > > - If not found, create a new entry in the nb_only list. > > > (After the above finishes, we have three lists: sb_only, > nb_only, and both) > > > - For each entry in the both list, do modifications to align the port > > > binding with nb information. > > > - For each entry in the nb_only list, create port_binding information in > > > the sb db. > > > [If I were updating the port lists, I'd move the port from the nb_only > > > list to both list] > > > - For each entry in the sb_only list, remove from the port_binding table. > > > [If I were updating the sb_only list, I'd remove it from the sb_only > > > list] > > > > Hi, Ryan > > Thanks for drafting the pseudo-code. > > Please allow me to add number bullets in your original version to > accommodate > > further discussions. > > That's fine, I updated from sb list to sb_only to be more clear as well > > > > > 1. Build a list of all ports known via port bindings in the sb_only db. > > 2. For each port known via the nb db: > >2.1 Look for the port in the sb_only list. > >2.2 If found, move the port from the sb_only list to the both list > >2.3 If not found, create a new entry in the nb_only list. > > (After the above finishes, we have three lists: sb_only, nb_only, and both) > > 3. For each entry in the both list, do modifications to align the port > >binding with nb information. > > 4. For each entry in the nb_only list, create port_binding information in > >the sb db. > >[If I were updating the port lists, I'd move the port from the nb_only > >list to both list] > > 5. For each entry in the sb_only list, remove from the port_binding table. > >[If I were updating the sb_only list, I'd remove it from the sb_only > >list] > > > > In square bracket of step 4., do you mean "If I were updating the > nb_lists in > > step 2.3., ..."? > > No, that is part of the "if I were going to persist all the port lists, > what would I need to do" > > > Similarly, in step 5, do you mean "If I were updating the sb_only list in > > step 2.2,..."? > > Ditto the above explanation. > > > In my opinion, step 4 and step 5 could be avoided with your logic in square > > bracket. Is my understanding correct? > > No, as those both still need to be performed whether I persist the port lists > in ovn-northd or not. > > > > > > > I *think* if I were to consider persisting the sb_only, nb_only, and both > > > lists and follow the extra logic I've added in square brackets above, I'd > > > only have entries in the both list at the end of the calculationset, so I > > > should only need to persist the both table. > > > > What do you mean by "persisting
Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd on creating and binding large number of lports
Ryan Moats/Omaha/IBM wrote on 06/26/2016 08:53:19 PM: > From: Ryan Moats/Omaha/IBM > To: Hui Kang/Watson/IBM@IBMUS > Cc: Ben Pfaff , dev@openvswitch.org > Date: 06/26/2016 08:53 PM > Subject: Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd > on creating and binding large number of lports > > Hui Kang/Watson/IBM wrote on 06/26/2016 07:11:27 PM: > > > From: Hui Kang/Watson/IBM > > To: Ryan Moats/Omaha/IBM@IBMUS > > Cc: Ben Pfaff , dev@openvswitch.org > > Date: 06/26/2016 07:11 PM > > Subject: Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd > > on creating and binding large number of lports > > > > Ryan Moats/Omaha/IBM wrote on 06/25/2016 09:07:39 PM: > > > > > From: Ryan Moats/Omaha/IBM > > > To: Hui Kang/Watson/IBM@IBMUS > > > Cc: Ben Pfaff , dev@openvswitch.org > > > Date: 06/25/2016 09:07 PM > > > Subject: Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd > > > on creating and binding large number of lports > > > > > > Hui Kang/Watson/IBM wrote on 06/25/2016 07:53:36 PM: > > > > > > > From: Hui Kang/Watson/IBM > > > > To: Ryan Moats/Omaha/IBM@IBMUS > > > > Cc: Ben Pfaff , dev@openvswitch.org > > > > Date: 06/25/2016 07:53 PM > > > > Subject: Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd > > > > on creating and binding large number of lports > > > > > > > > > > > > > > > > Actually, I take that back. The cycles/port for all the cases above > > > > > > demonstrate only slightly nonlinear scaling: 200/25 is 8 > Mcycles/port, > > > > > > 1200/125 is 9.6 Mcycles/port. > > > > > > > > > > > > So the issue is not that it does not scale. The issue is that it is > > > > > > slow. > > > > > > > > > > Er? When I do the ratios, I come up with 125 Kcycles/port at 200 > > > ports going > > > > > down to slightly more than 104 Kcycles/port at 1200 ports, which > > > is slightly > > > > > sub-linear (and I do think that's a good thing). > > > > > > > > > > However, I'm left wondering if it would be possible to make > things even > > > > > better through judicial use of persistence and incremental processing. > > > > > > > > > > Right now the ports logic looks to me like: > > > > > - Build a list of all ports known via port bindings in the sb db. > > > > > - For each port known via the nb db: > > > > > - Look for the port in the sb list. > > > > > - If found, move the port from the sb list to the both list > > > > > - If not found, create a new entry in the nb_only list. > > > > > (After the above finishes, we have three lists: sb_only, > > > nb_only, and both) > > > > > - For each entry in the both list, do modifications to align the port > > > > > binding with nb information. > > > > > - For each entry in the nb_only list, create port_binding > > information in > > > > > the sb db. > > > > > [If I were updating the port lists, I'd move the port from > the nb_only > > > > > list to both list] > > > > > - For each entry in the sb_only list, remove from the > > port_binding table. > > > > > [If I were updating the sb_only list, I'd remove it from the sb_only > > > > > list] > > > > > > > > Hi, Ryan > > > > Thanks for drafting the pseudo-code. > > > > Please allow me to add number bullets in your original version to > > > accommodate > > > > further discussions. > > > > > > That's fine, I updated from sb list to sb_only to be more clear as well > > > > > > > > > > > 1. Build a list of all ports known via port bindings in the sb_only db. > > > > 2. For each port known via the nb db: > > > >2.1 Look for the port in the sb_only list. > > > >2.2 If found, move the port from the sb_only list to the both list > > > >2.3 If not found, create a new entry in the nb_only list. > > > > (After the above finishes, we have three lists: sb_only, > > nb_only, and both) > > > > 3. For each entry in the both list, do modifications to align the port > > > >binding with nb information. > > > > 4. For each entry in the nb_only list, create port_bind
Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd on creating and binding large number of lports
Ryan Moats/Omaha/IBM wrote on 06/25/2016 09:07:39 PM: > From: Ryan Moats/Omaha/IBM > To: Hui Kang/Watson/IBM@IBMUS > Cc: Ben Pfaff , dev@openvswitch.org > Date: 06/25/2016 09:07 PM > Subject: Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd > on creating and binding large number of lports > > > > > > > > I *think* if I were to consider persisting the sb_only, nb_only, and both > > > lists and follow the extra logic I've added in square brackets above, I'd > > > only have entries in the both list at the end of the calculationset, so I > > > should only need to persist the both table. > > > > What do you mean by "persisting"? A global linked list to store the elements > > of struct ovn_ports? > > That's exactly what I mean. I'm looking at trading memory for execution time. > > > > Further, I *think* if I were to then apply change tracking to the first > > > part of the process above, the logic changes to: > > > > Which step of the above pseudo-code should the following code be > > embedded into ? > > The following replaces the entire list above. The good thing about writing > this down is that I can come back to it later and realize where I goofed - > see below. > > > > > > > - For each tracked entry in the port bindings table > > > - if it is a deleted entry, remove from the both list (if there is still > > > a nb entry, we'll recreate it further on) > > > - if it is a new entry, add it to the sb_only list > > The above isn't quite right - since we create port binding entries ourself > in response to unmatched ports in the nb_only list, we need to check that > there isn't already a port in the both list. So the above changes to: > > - if it is a new entry, check for it in the both list > - if it is not there, then add it to the sb_only list > > > > - if it is a modified entry, find it in the both list and update the > > > sb information contained in the entry Do you mean updating the struct ovn_port in the both list? > > > - For each port known via the nb db: Should this be "For the port in each changed entry in nb db" due to the persisted both list? Thanks. - Hui > > > - if the entry is found in the both list, update the nb data contained > > > in the entry > > > - if the entry is not in the both list, but is in the sb_only list, > > > move the entry from the sb_list to the both list > > > - if the entry is not in either the both or the sb_only list, create > > > a new entry in the nb_only list > > > - For each entry in the both list, do modifications to align the port > > > binding with nb information. > > > - For each entry in the nb_only list, create port_binding information in > > > the sb db and move the entry from the nb_only to the both list > > > - For each entry in the sb_only list, remove from the port_binding table. > > > > > > Now, I'm pretty sure this will cut down the number of cycles, but before > > > I go off and code it [and potentially break something ala yesterday's > > > excitement], I'm looking for some verification of both my conclusion of > > > persisting just the both list and the modified logic incorporating the > > > persisted both list and port binding change tracking adjustments). Do > > > these make sense or have I missed something? > > Ryan ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] Scanning only changed entries in the ovnsb
Improve performance by scanning only changed entries in ovnsb
Signed-off-by: Hui Kang
---
ovn/northd/ovn-northd.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 7ce509d..8a73a3a 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2938,7 +2938,8 @@ ovnsb_db_run(struct northd_context *ctx)
hmap_insert(&lports_hmap, &hash_node->node, hash_string(nb->name, 0));
}
-SBREC_PORT_BINDING_FOR_EACH(sb, ctx->ovnsb_idl) {
+SBREC_PORT_BINDING_FOR_EACH_TRACKED (sb, ctx->ovnsb_idl) {
+// SBREC_PORT_BINDING_FOR_EACH(sb, ctx->ovnsb_idl) {
nb = NULL;
HMAP_FOR_EACH_WITH_HASH(hash_node, node,
hash_string(sb->logical_port, 0),
@@ -3139,7 +3140,7 @@ main(int argc, char *argv[])
add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_type);
add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_options);
add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_mac);
-ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_port_binding_col_chassis);
+ovsdb_idl_track_add_column(ovnsb_idl_loop.idl,
&sbrec_port_binding_col_chassis);
ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_address_set);
add_column_noalert(ovnsb_idl_loop.idl, &sbrec_address_set_col_name);
@@ -3165,6 +3166,7 @@ main(int argc, char *argv[])
}
ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop);
ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop);
+ovsdb_idl_track_clear(ctx.ovnsb_idl);
poll_block();
if (should_service_stop()) {
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] Scanning only changed entries in the ovnsb
Improve performance by scanning only changed entries in ovnsb
Signed-off-by: Hui Kang
---
ovn/northd/ovn-northd.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 7ce509d..bb6b853 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2938,7 +2938,7 @@ ovnsb_db_run(struct northd_context *ctx)
hmap_insert(&lports_hmap, &hash_node->node, hash_string(nb->name, 0));
}
-SBREC_PORT_BINDING_FOR_EACH(sb, ctx->ovnsb_idl) {
+SBREC_PORT_BINDING_FOR_EACH_TRACKED (sb, ctx->ovnsb_idl) {
nb = NULL;
HMAP_FOR_EACH_WITH_HASH(hash_node, node,
hash_string(sb->logical_port, 0),
@@ -3139,7 +3139,7 @@ main(int argc, char *argv[])
add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_type);
add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_options);
add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_mac);
-ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_port_binding_col_chassis);
+ovsdb_idl_track_add_column(ovnsb_idl_loop.idl,
&sbrec_port_binding_col_chassis);
ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_address_set);
add_column_noalert(ovnsb_idl_loop.idl, &sbrec_address_set_col_name);
@@ -3165,6 +3165,7 @@ main(int argc, char *argv[])
}
ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop);
ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop);
+ovsdb_idl_track_clear(ctx.ovnsb_idl);
poll_block();
if (should_service_stop()) {
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2] Scanning only changed entries in the ovnsb
Improve performance by scanning only changed entries in ovnsb
Signed-off-by: Hui Kang
---
ovn/northd/ovn-northd.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 7ce509d..bb6b853 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2938,7 +2938,7 @@ ovnsb_db_run(struct northd_context *ctx)
hmap_insert(&lports_hmap, &hash_node->node, hash_string(nb->name, 0));
}
-SBREC_PORT_BINDING_FOR_EACH(sb, ctx->ovnsb_idl) {
+SBREC_PORT_BINDING_FOR_EACH_TRACKED (sb, ctx->ovnsb_idl) {
nb = NULL;
HMAP_FOR_EACH_WITH_HASH(hash_node, node,
hash_string(sb->logical_port, 0),
@@ -3139,7 +3139,7 @@ main(int argc, char *argv[])
add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_type);
add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_options);
add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_mac);
-ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_port_binding_col_chassis);
+ovsdb_idl_track_add_column(ovnsb_idl_loop.idl,
&sbrec_port_binding_col_chassis);
ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_address_set);
add_column_noalert(ovnsb_idl_loop.idl, &sbrec_address_set_col_name);
@@ -3165,6 +3165,7 @@ main(int argc, char *argv[])
}
ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop);
ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop);
+ovsdb_idl_track_clear(ctx.ovnsb_idl);
poll_block();
if (should_service_stop()) {
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v3] Scanning only changed entries in the ovnsb
Improve performance by scanning only changed port binding entries
when determining whether to mark the logical switch port up or
down
Signed-off-by: Hui Kang
---
ovn/northd/ovn-northd.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 7ce509d..bb6b853 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2938,7 +2938,7 @@ ovnsb_db_run(struct northd_context *ctx)
hmap_insert(&lports_hmap, &hash_node->node, hash_string(nb->name, 0));
}
-SBREC_PORT_BINDING_FOR_EACH(sb, ctx->ovnsb_idl) {
+SBREC_PORT_BINDING_FOR_EACH_TRACKED (sb, ctx->ovnsb_idl) {
nb = NULL;
HMAP_FOR_EACH_WITH_HASH(hash_node, node,
hash_string(sb->logical_port, 0),
@@ -3139,7 +3139,7 @@ main(int argc, char *argv[])
add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_type);
add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_options);
add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_mac);
-ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_port_binding_col_chassis);
+ovsdb_idl_track_add_column(ovnsb_idl_loop.idl,
&sbrec_port_binding_col_chassis);
ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_address_set);
add_column_noalert(ovnsb_idl_loop.idl, &sbrec_address_set_col_name);
@@ -3165,6 +3165,7 @@ main(int argc, char *argv[])
}
ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop);
ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop);
+ovsdb_idl_track_clear(ctx.ovnsb_idl);
poll_block();
if (should_service_stop()) {
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] Scanning only changed entries in the ovnsb
"dev" wrote on 07/16/2016 10:47:14 PM: > From: Ryan Moats/Omaha/IBM@IBMUS > To: Hui Kang > Cc: dev@openvswitch.org > Date: 07/16/2016 10:47 PM > Subject: Re: [ovs-dev] [PATCH v2] Scanning only changed entries in the ovnsb > Sent by: "dev" > > "dev" wrote on 07/14/2016 05:59:29 PM: > > > From: Hui Kang > > To: dev@openvswitch.org > > Date: 07/14/2016 06:04 PM > > Subject: [ovs-dev] [PATCH v2] Scanning only changed entries in the ovnsb > > Sent by: "dev" > > > > Improve performance by scanning only changed entries in ovnsb > > > > Signed-off-by: Hui Kang > > --- > > It applies cleanly, it compiles, it passes unit tests and > it looks straightforward enough. The only nit I'd pick is > the commit description is a bit misleading - I think saying > > "Improve performance by scanning only changed port binding > entries when determining whether to mark the logical switch > port up or down" > > would be more accurate. On the assumption that the merge gets > a more descriptive commit message... Thanks, Ryan. I have updated the commit message in v3 in this trivial fix. - Hui > > Acked-by: Ryan Moats > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] [OVN-northd] Rename nbs/nbr to nbsp/nbrp
Make sense to me. I will update the patch. Thanks, Ryan. - Hui On Mon, Jul 18, 2016 at 10:37 AM, Ryan Moats wrote: > "dev" wrote on 07/02/2016 12:25:11 PM: > >> From: Ben Pfaff >> To: Justin Pettit >> Cc: dev@openvswitch.org, Hui Kang >> Date: 07/02/2016 12:25 PM >> Subject: Re: [ovs-dev] [PATCH] [OVN-northd] Rename nbs/nbr to nbsp/nbrp >> Sent by: "dev" > > >> >> On Fri, Jun 24, 2016 at 12:39:54PM -0400, Hui Kang wrote: >> > These variables indicate ports in nb switches or routers. >> > >> > Signed-off-by: Hui Kang >> >> Justin, do you want to review this? You like naming. > > I'll throw my $0.02 onto the table (and yes, it's only barely > worth $0.02) - if we are going to rename them, how about we > change them to nlsp and nlrp so that they align with the > structs that they are holding pointers to. > > Ryan > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2] [OVN-northd] Rename nbs/nbr port names to nbsp/nbrp
These variables indicate ports in nb switches or routers.
Signed-off-by: Hui Kang
--
v1->v2:
- modify commit message
---
ovn/northd/ovn-northd.c | 196
1 file changed, 98 insertions(+), 98 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 1599e18..5bc5408 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -486,9 +486,9 @@ struct ovn_port {
char *key; /* nbs->name, nbr->name, sb->logical_port. */
char *json_key; /* 'key', quoted for use in JSON. */
-const struct nbrec_logical_switch_port *nbs; /* May be NULL. */
-const struct nbrec_logical_router_port *nbr; /* May be NULL. */
-const struct sbrec_port_binding *sb; /* May be NULL. */
+const struct nbrec_logical_switch_port *nbsp; /* May be NULL. */
+const struct nbrec_logical_router_port *nbrp; /* May be NULL. */
+const struct sbrec_port_binding *sb; /* May be NULL. */
/* Logical router port data. */
ovs_be32 ip, mask; /* 192.168.10.123/24. */
@@ -504,8 +504,8 @@ struct ovn_port {
static struct ovn_port *
ovn_port_create(struct hmap *ports, const char *key,
-const struct nbrec_logical_switch_port *nbs,
-const struct nbrec_logical_router_port *nbr,
+const struct nbrec_logical_switch_port *nbsp,
+const struct nbrec_logical_router_port *nbrp,
const struct sbrec_port_binding *sb)
{
struct ovn_port *op = xzalloc(sizeof *op);
@@ -516,8 +516,8 @@ ovn_port_create(struct hmap *ports, const char *key,
op->key = xstrdup(key);
op->sb = sb;
-op->nbs = nbs;
-op->nbr = nbr;
+op->nbsp = nbsp;
+op->nbrp = nbrp;
hmap_insert(ports, &op->key_node, hash_string(op->key, 0));
return op;
}
@@ -578,21 +578,21 @@ join_logical_ports(struct northd_context *ctx,
HMAP_FOR_EACH (od, key_node, datapaths) {
if (od->nbs) {
for (size_t i = 0; i < od->nbs->n_ports; i++) {
-const struct nbrec_logical_switch_port *nbs =
od->nbs->ports[i];
-struct ovn_port *op = ovn_port_find(ports, nbs->name);
+const struct nbrec_logical_switch_port *nbsp =
od->nbs->ports[i];
+struct ovn_port *op = ovn_port_find(ports, nbsp->name);
if (op) {
-if (op->nbs || op->nbr) {
+if (op->nbsp || op->nbrp) {
static struct vlog_rate_limit rl
= VLOG_RATE_LIMIT_INIT(5, 1);
VLOG_WARN_RL(&rl, "duplicate logical port %s",
- nbs->name);
+ nbsp->name);
continue;
}
-op->nbs = nbs;
+op->nbsp = nbsp;
ovs_list_remove(&op->list);
ovs_list_push_back(both, &op->list);
} else {
-op = ovn_port_create(ports, nbs->name, nbs, NULL, NULL);
+op = ovn_port_create(ports, nbsp->name, nbsp, NULL, NULL);
ovs_list_push_back(nb_only, &op->list);
}
@@ -600,41 +600,41 @@ join_logical_ports(struct northd_context *ctx,
}
} else {
for (size_t i = 0; i < od->nbr->n_ports; i++) {
-const struct nbrec_logical_router_port *nbr
+const struct nbrec_logical_router_port *nbrp
= od->nbr->ports[i];
struct eth_addr mac;
-if (!eth_addr_from_string(nbr->mac, &mac)) {
+if (!eth_addr_from_string(nbrp->mac, &mac)) {
static struct vlog_rate_limit rl
= VLOG_RATE_LIMIT_INIT(5, 1);
-VLOG_WARN_RL(&rl, "bad 'mac' %s", nbr->mac);
+VLOG_WARN_RL(&rl, "bad 'mac' %s", nbrp->mac);
continue;
}
ovs_be32 ip, mask;
-char *error = ip_parse_masked(nbr->network, &ip, &mask);
+char *error = ip_parse_masked(nbrp->network, &ip, &mask);
if (error || mask == OVS_BE32_MAX || !ip_is_cidr(mask)) {
static struct vlog_rate_limit rl
= VLOG_RATE_LIMIT_INIT(5, 1);
-VLOG_WARN_RL(&rl, "bad 'network' %s", nbr->network);
+VLOG_WARN_RL(&rl, "bad 'network' %s", nbrp->network);
free(error);
continue;
[ovs-dev] [PATCH v3] [OVN-northd] Rename nbs/nbr port names to nbsp/nbrp
These variables indicate ports in nb switches or routers.
Signed-off-by: Hui Kang
--
v2->v3:
- rebase agains master branch
v1->v2:
- modify commit message
---
ovn/northd/ovn-northd.c | 184
1 file changed, 92 insertions(+), 92 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 7ce509d..4c5192b 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -483,7 +483,7 @@ struct ovn_port {
const struct sbrec_port_binding *sb; /* May be NULL. */
/* Logical switch port data. */
-const struct nbrec_logical_switch_port *nbs; /* May be NULL. */
+const struct nbrec_logical_switch_port *nbsp; /* May be NULL. */
struct lport_addresses *lsp_addrs; /* Logical switch port addresses. */
unsigned int n_lsp_addrs;
@@ -492,7 +492,7 @@ struct ovn_port {
unsigned int n_ps_addrs;
/* Logical router port data. */
-const struct nbrec_logical_router_port *nbr; /* May be NULL. */
+const struct nbrec_logical_router_port *nbrp; /* May be NULL. */
struct lport_addresses lrp_networks;
@@ -505,8 +505,8 @@ struct ovn_port {
static struct ovn_port *
ovn_port_create(struct hmap *ports, const char *key,
-const struct nbrec_logical_switch_port *nbs,
-const struct nbrec_logical_router_port *nbr,
+const struct nbrec_logical_switch_port *nbsp,
+const struct nbrec_logical_router_port *nbrp,
const struct sbrec_port_binding *sb)
{
struct ovn_port *op = xzalloc(sizeof *op);
@@ -517,8 +517,8 @@ ovn_port_create(struct hmap *ports, const char *key,
op->key = xstrdup(key);
op->sb = sb;
-op->nbs = nbs;
-op->nbr = nbr;
+op->nbsp = nbsp;
+op->nbrp = nbrp;
hmap_insert(ports, &op->key_node, hash_string(op->key, 0));
return op;
}
@@ -591,17 +591,17 @@ join_logical_ports(struct northd_context *ctx,
HMAP_FOR_EACH (od, key_node, datapaths) {
if (od->nbs) {
for (size_t i = 0; i < od->nbs->n_ports; i++) {
-const struct nbrec_logical_switch_port *nbs =
od->nbs->ports[i];
-struct ovn_port *op = ovn_port_find(ports, nbs->name);
+const struct nbrec_logical_switch_port *nbsp =
od->nbs->ports[i];
+struct ovn_port *op = ovn_port_find(ports, nbsp->name);
if (op) {
-if (op->nbs || op->nbr) {
+if (op->nbsp || op->nbrp) {
static struct vlog_rate_limit rl
= VLOG_RATE_LIMIT_INIT(5, 1);
VLOG_WARN_RL(&rl, "duplicate logical port %s",
- nbs->name);
+ nbsp->name);
continue;
}
-op->nbs = nbs;
+op->nbsp = nbsp;
ovs_list_remove(&op->list);
ovs_list_push_back(both, &op->list);
@@ -609,39 +609,39 @@ join_logical_ports(struct northd_context *ctx,
* not have been initialized fully. */
ovs_assert(!op->n_lsp_addrs && !op->n_ps_addrs);
} else {
-op = ovn_port_create(ports, nbs->name, nbs, NULL, NULL);
+op = ovn_port_create(ports, nbsp->name, nbsp, NULL, NULL);
ovs_list_push_back(nb_only, &op->list);
}
op->lsp_addrs
-= xmalloc(sizeof *op->lsp_addrs * nbs->n_addresses);
-for (size_t j = 0; j < nbs->n_addresses; j++) {
-if (!strcmp(nbs->addresses[j], "unknown")) {
+= xmalloc(sizeof *op->lsp_addrs * nbsp->n_addresses);
+for (size_t j = 0; j < nbsp->n_addresses; j++) {
+if (!strcmp(nbsp->addresses[j], "unknown")) {
continue;
}
-if (!extract_lsp_addresses(nbs->addresses[j],
+if (!extract_lsp_addresses(nbsp->addresses[j],
&op->lsp_addrs[op->n_lsp_addrs])) {
static struct vlog_rate_limit rl
= VLOG_RATE_LIMIT_INIT(1, 1);
VLOG_INFO_RL(&rl, "invalid syntax '%s' in logical "
"switch port addresses. No MAC "
"address found",
- op->nbs->addresses[j]);
+ op->nbsp->addresses[j]);
Re: [ovs-dev] [PATCH v3] [OVN-northd] Rename nbs/nbr port names tonbsp/nbrp
"dev" wrote on 07/19/2016 04:15:56 PM: > From: Justin Pettit > To: Hui Kang > Cc: dev@openvswitch.org > Date: 07/19/2016 04:16 PM > Subject: Re: [ovs-dev] [PATCH v3] [OVN-northd] Rename nbs/nbr port > names to nbsp/nbrp > Sent by: "dev" > > > > On Jul 19, 2016, at 11:36 AM, Hui Kang wrote: > > > > These variables indicate ports in nb switches or routers. > > > > Signed-off-by: Hui Kang > > > > -- > > v2->v3: > > - rebase agains master branch > > v1->v2: > > - modify commit message > > --- > > Thanks for the patch. Just a few things first, though: > >- A couple of lines you changed went over 80 columns. I'll wrap those. >- The way you wrote your revision history, it shows up in the > commit message. I'll fix it up. >- Your author email address and Signed-off-by don't match. Which > do you want to use? I'd like to use the corporate email account, which is ka...@us.ibm.com I will figure out how to setup the mail account in my dev box and keep them consistent next time. Thanks for your help. - Hui > > I'll merge it after I get clarification on that last question. > > --Justin > > > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the ovnsb
"dev" wrote on 07/26/2016 02:20:27 PM: > From: Ben Pfaff > To: Hui Kang > Cc: dev@openvswitch.org > Date: 07/26/2016 02:20 PM > Subject: Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the ovnsb > Sent by: "dev" > > On Sat, Jul 16, 2016 at 11:58:25PM -0400, Hui Kang wrote: > > Improve performance by scanning only changed port binding entries > > when determining whether to mark the logical switch port up or > > down > > > > Signed-off-by: Hui Kang > > Won't this skip an initial round of updates at ovn-northd startup time? > (Certainly ovn-northd might get killed and restarted occasionally, > especially if we're doing failover to a second host.) Good call, you are right. Do you think an initialization of iterating all entries in ovnsb before entering the main loop will correct this patch on northd restarting or failover? Thanks. - Hui > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the ovnsb
"dev" wrote on 07/26/2016 02:20:27 PM: > From: Ben Pfaff > To: Hui Kang > Cc: dev@openvswitch.org > Date: 07/26/2016 02:20 PM > Subject: Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the ovnsb > Sent by: "dev" > > On Sat, Jul 16, 2016 at 11:58:25PM -0400, Hui Kang wrote: > > Improve performance by scanning only changed port binding entries > > when determining whether to mark the logical switch port up or > > down > > > > Signed-off-by: Hui Kang > > Won't this skip an initial round of updates at ovn-northd startup time? > (Certainly ovn-northd might get killed and restarted occasionally, > especially if we're doing failover to a second host.) Hi, Ben, After second thought, I think skipping the initial round is the purpose of this patch. ovsdb_idl_create(ovsdb) copies the the Port_binding table from southbound database whenever ovn-northd gets started. In this case, the northbound DB and southbound db are synced. In ovnsb_db_run, ovn-northd only gets notified when there is change to the Chassis column [1]. Therefore, ovnsb_db_run should only look the entry that are changed with its Chassis column. There is no need to initialize by iterating every entry in the Port_binding table. Please correct me if my understanding is incorrect. Thanks. - Hui [1] https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L3034 > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the ovnsb
Russell Bryant wrote on 07/26/2016 03:50:44 PM: > From: Russell Bryant > To: Hui Kang/Watson/IBM@IBMUS > Cc: Ben Pfaff , Hui Kang , ovs > dev > Date: 07/26/2016 03:51 PM > Subject: Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the ovnsb > > On Tue, Jul 26, 2016 at 3:44 PM, Hui Kang wrote: > > > "dev" wrote on 07/26/2016 02:20:27 PM: > > > From: Ben Pfaff > > To: Hui Kang > > Cc: dev@openvswitch.org > > Date: 07/26/2016 02:20 PM > > Subject: Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the > ovnsb > > Sent by: "dev" > > > > On Sat, Jul 16, 2016 at 11:58:25PM -0400, Hui Kang wrote: > > > Improve performance by scanning only changed port binding entries > > > when determining whether to mark the logical switch port up or > > > down > > > > > > Signed-off-by: Hui Kang > > > > Won't this skip an initial round of updates at ovn-northd startup time? > > (Certainly ovn-northd might get killed and restarted occasionally, > > especially if we're doing failover to a second host.) > > Hi, Ben, > After second thought, I think skipping the initial round is the purpose of > this patch. > > ovsdb_idl_create(ovsdb) copies the the Port_binding table from southbound > database whenever ovn-northd gets started. In this case, the northbound > DB and southbound db are synced. In ovnsb_db_run, ovn-northd only gets > notified when there is change to the Chassis column [1]. Therefore, > ovnsb_db_run should only look the entry that are changed with its Chassis > column. There is no need to initialize by iterating every entry in the > Port_binding table. Please correct me if my understanding is incorrect. > Thanks. > > What if the Chassis column changes in some Port_Binding records > while ovn-northd isn't running? Hi, Russel, Thanks for correcting me. In this case, initialization is necessary each time ovn-northd restarts. - Hui > > -- > Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH, v4] Scanning only changed entries in the ovnsb
- Improve performance by scanning only changed port binding entries
when determining whether to mark the logical switch port up or
down
v3->v4:
- Add an initialization function to scan all entries in Port_binding table
when ovn-northd restarts or fails over
Signed-off-by: Hui Kang
---
ovn/northd/ovn-northd.c | 95 +
1 file changed, 65 insertions(+), 30 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 17fbf29..35566a7 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -53,6 +53,11 @@ struct northd_context {
struct ovsdb_idl_txn *ovnsb_txn;
};
+struct lport_hash_node {
+struct hmap_node node;
+const struct nbrec_logical_switch_port *nbsp;
+};
+
static const char *ovnnb_db;
static const char *ovnsb_db;
@@ -3196,13 +3201,47 @@ ovnnb_db_run(struct northd_context *ctx)
hmap_destroy(&ports);
}
+static void
+sb_chassis_update_nbsec(const struct sbrec_port_binding *sb,
+struct hmap *lports_hmap)
+
+{
+struct lport_hash_node *hash_node;
+const struct nbrec_logical_switch_port *nbsp;
+
+nbsp = NULL;
+HMAP_FOR_EACH_WITH_HASH(hash_node, node,
+hash_string(sb->logical_port, 0),
+lports_hmap) {
+if (!strcmp(sb->logical_port, hash_node->nbsp->name)) {
+nbsp = hash_node->nbsp;
+break;
+}
+}
+
+if (!nbsp) {
+/* The logical port doesn't exist for this port binding. This can
+ * happen under normal circumstances when ovn-northd hasn't gotten
+ * around to pruning the Port_Binding yet. */
+return;
+}
+
+if (sb->chassis && (!nbsp->up || !*nbsp->up)) {
+bool up = true;
+nbrec_logical_switch_port_set_up(nbsp, &up, 1);
+} else if (!sb->chassis && (!nbsp->up || *nbsp->up)) {
+bool up = false;
+nbrec_logical_switch_port_set_up(nbsp, &up, 1);
+}
+}
+
/*
* The only change we get notified about is if the 'chassis' column of the
* 'Port_Binding' table changes. When this column is not empty, it means we
* need to set the corresponding logical port as 'up' in the northbound DB.
*/
static void
-ovnsb_db_run(struct northd_context *ctx)
+ovnsb_db_run(struct northd_context *ctx, bool is_init)
{
if (!ctx->ovnnb_txn) {
return;
@@ -3211,10 +3250,7 @@ ovnsb_db_run(struct northd_context *ctx)
const struct sbrec_port_binding *sb;
const struct nbrec_logical_switch_port *nbsp;
-struct lport_hash_node {
-struct hmap_node node;
-const struct nbrec_logical_switch_port *nbsp;
-} *hash_node;
+struct lport_hash_node *hash_node;
hmap_init(&lports_hmap);
@@ -3224,30 +3260,13 @@ ovnsb_db_run(struct northd_context *ctx)
hmap_insert(&lports_hmap, &hash_node->node, hash_string(nbsp->name,
0));
}
-SBREC_PORT_BINDING_FOR_EACH(sb, ctx->ovnsb_idl) {
-nbsp = NULL;
-HMAP_FOR_EACH_WITH_HASH(hash_node, node,
-hash_string(sb->logical_port, 0),
-&lports_hmap) {
-if (!strcmp(sb->logical_port, hash_node->nbsp->name)) {
-nbsp = hash_node->nbsp;
-break;
-}
-}
-
-if (!nbsp) {
-/* The logical port doesn't exist for this port binding. This can
- * happen under normal circumstances when ovn-northd hasn't gotten
- * around to pruning the Port_Binding yet. */
-continue;
+if (is_init) {
+SBREC_PORT_BINDING_FOR_EACH(sb, ctx->ovnsb_idl) {
+sb_chassis_update_nbsec(sb, &lports_hmap);
}
-
-if (sb->chassis && (!nbsp->up || !*nbsp->up)) {
-bool up = true;
-nbrec_logical_switch_port_set_up(nbsp, &up, 1);
-} else if (!sb->chassis && (!nbsp->up || *nbsp->up)) {
-bool up = false;
-nbrec_logical_switch_port_set_up(nbsp, &up, 1);
+} else {
+SBREC_PORT_BINDING_FOR_EACH_TRACKED(sb, ctx->ovnsb_idl) {
+sb_chassis_update_nbsec(sb, &lports_hmap);
}
}
@@ -3417,6 +3436,19 @@ add_column_noalert(struct ovsdb_idl *idl,
ovsdb_idl_omit_alert(idl, column);
}
+static void
+ovnsb_db_run_init(struct ovsdb_idl_loop *ovnsb_idl_loop)
+{
+struct northd_context ctx = {
+.ovnnb_idl = NULL,
+.ovnnb_txn = NULL,
+.ovnsb_idl = ovnsb_idl_loop->idl,
+.ovnsb_txn = ovsdb_idl_loop_run(ovnsb_idl_loop),
+};
+
+ovnsb_db_run(&ctx, true);
+}
+
int
main(int argc, char *argv[])
{
@@ -3485,7 +3517,6 @@ main(int argc, char *argv[])
add_column_noalert(ovnsb_idl_loop.idl, &sb
Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the ovnsb
Ben Pfaff wrote on 07/27/2016 12:30:25 PM: > From: Ben Pfaff > To: Hui Kang/Watson/IBM@IBMUS > Cc: Hui Kang , dev@openvswitch.org > Date: 07/27/2016 12:30 PM > Subject: Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the ovnsb > > On Tue, Jul 26, 2016 at 03:44:57PM -0400, Hui Kang wrote: > > > > > > "dev" wrote on 07/26/2016 02:20:27 PM: > > > > > From: Ben Pfaff > > > To: Hui Kang > > > Cc: dev@openvswitch.org > > > Date: 07/26/2016 02:20 PM > > > Subject: Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the > > ovnsb > > > Sent by: "dev" > > > > > > On Sat, Jul 16, 2016 at 11:58:25PM -0400, Hui Kang wrote: > > > > Improve performance by scanning only changed port binding entries > > > > when determining whether to mark the logical switch port up or > > > > down > > > > > > > > Signed-off-by: Hui Kang > > > > > > Won't this skip an initial round of updates at ovn-northd startup time? > > > (Certainly ovn-northd might get killed and restarted occasionally, > > > especially if we're doing failover to a second host.) > > > > Hi, Ben, > > After second thought, I think skipping the initial round is the purpose of > > this patch. > > > > ovsdb_idl_create(ovsdb) copies the the Port_binding table from southbound > > database whenever ovn-northd gets started. In this case, the northbound > > DB and southbound db are synced. > > I don't understand this statement. ovsdb_idl_create() doesn't sync > anything between nb and sb dbs. Sorry for the confusion. I have updated the patch to v4 addressing your comment. - Hui > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [ovs-dev,ovn-controller] Physical: persist tunnels
"dev" wrote on 07/26/2016 02:37:30 PM:
> From: "Liran Schour"
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: dev@openvswitch.org
> Date: 07/26/2016 02:38 PM
> Subject: Re: [ovs-dev] [ovs-dev,ovn-controller] Physical: persist tunnels
> Sent by: "dev"
>
> "dev" wrote on 26/07/2016 12:02:37 AM:
>
> > From: Flavio Fernandes
> > To: Ryan Moats
> > Cc: dev@openvswitch.org
> > Date: 26/07/2016 12:06 AM
> > Subject: Re: [ovs-dev] [ovs-dev,ovn-controller] Physical: persist
> tunnels
> > Sent by: "dev"
> >
> >
> > > On Jul 25, 2016, at 12:28 PM, Ryan Moats wrote:
> > >
> > > While commit ab39371d68842b7e4000cc5d8718e6fc04e92795
> > > (ovn-controller: Handle physical changes correctly) addressed
> > > unit test failures, it did so at the cost of performance: [1]
> > > notes that ovn-controller cpu usage is now pegged at 100%.
> > >
> > > Root cause of this is that while the storage for tunnels is
> > > persisted, their creation is not (which the above changed
> > > incorrectly assumed was the case). This patch persists
> > > tunneled data across invocations of physical_run. A side
> > > effect is that renaming of localfvif_map_changed variable
> > > to physical_map_changed and extending its scope to include
> > > tunnel changes.
> > >
> >
> > I tested this fix by using a Vagrant file that stamps out vms as
> > compute nodes.
> > To deploy ovn, call the script ?/vagrant/scripts/setup-ovn-
> > cluster.sh? and that
> > would render ovn-controller in compute nodes to peg the cpu at 100%
> before the
> > changes.
> >
> > More info on _easily_ deploying this cluster is available here:
> >
https://github.com/flavio-fernandes/just-ovn-nodes/blob/master/README.md
> >
> >
> > > [1] http://openvswitch.org/pipermail/dev/2016-July/076058.html
> > >
> > > Signed-off-by: Ryan Moats
> >
> > Acked-by: Flavio Fernandes
> > Tested-by: Flavio Fernandes
> >
>
> Tested this fix with a cluster of 50 hosts.
>
> Acked-by: Liran Schour
> Tested-by: Liran Schour
This patch fixes the 100% CPU unitization of ovn-controller. Without this
patch
the ovn-controller reaches 100% CPU in fresh deployment using
ovn-scale-test [1].
After applying this patch, the CPU utilization looks good.
[1] https://github.com/openvswitch/ovn-scale-test
Acked-by: Hui Kang
Tested-by: Hui Kang
>
> >
> > > ---
> > > ovn/controller/physical.c | 59
> > +--
> > > 1 file changed, 37 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> > > index a104e33..e788fe5 100644
> > > --- a/ovn/controller/physical.c
> > > +++ b/ovn/controller/physical.c
> > > @@ -605,8 +605,13 @@ physical_run(struct controller_ctx *ctx, enum
> > mf_field_id mff_ovn_geneve,
> > > uuid_generate(hc_uuid);
> > > }
> > >
> > > +/* This bool tracks physical mapping changes. */
> > > +bool physical_map_changed = false;
> > > +
> > > struct simap new_localvif_to_ofport =
> > > SIMAP_INITIALIZER(&new_localvif_to_ofport);
> > > +struct simap new_tunnel_to_ofport =
> > > +SIMAP_INITIALIZER(&new_tunnel_to_ofport);
> > > for (int i = 0; i < br_int->n_ports; i++) {
> > > const struct ovsrec_port *port_rec = br_int->ports[i];
> > > if (!strcmp(port_rec->name, br_int->name)) {
> > > @@ -668,18 +673,23 @@ physical_run(struct controller_ctx *ctx,
> > enum mf_field_id mff_ovn_geneve,
> > > continue;
> > > }
> > >
> > > -struct chassis_tunnel *tun = xmalloc(sizeof *tun);
> > > -hmap_insert(&tunnels, &tun->hmap_node,
> > > -hash_string(chassis_id, 0));
> > > -tun->chassis_id = chassis_id;
> > > -tun->ofport = u16_to_ofp(ofport);
> > > -tun->type = tunnel_type;
> > > -full_binding_processing = true;
> > > -binding_reset_processing();
> > > -
> > > -/* Reprocess logical flow table immediately. */
> > > -lflow_reset_processing();
> > > -poll_immediate_wake();
> > > +simap_put(&new_tunne
Re: [ovs-dev] [PATCH, v4] Scanning only changed entries in the ovnsb
"dev" wrote on 07/27/2016 05:53:17 PM: > From: Ben Pfaff > To: Hui Kang > Cc: dev@openvswitch.org > Date: 07/27/2016 05:53 PM > Subject: Re: [ovs-dev] [PATCH, v4] Scanning only changed entries in the ovnsb > Sent by: "dev" > > On Tue, Jul 26, 2016 at 10:25:07PM -0400, Hui Kang wrote: > > - Improve performance by scanning only changed port binding entries > > when determining whether to mark the logical switch port up or > > down > > > > v3->v4: > > - Add an initialization function to scan all entries in Port_binding table > > when ovn-northd restarts or fails over > > > > Signed-off-by: Hui Kang > > Hi, thanks for the revision. This gets a few patch rejects, would you > mind rebasing? Not a problem; will submit an update soon. - Hui > > Thanks, > > Ben. > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH, v5] Scanning only changed entries in the ovnsb
- Improve performance by scanning only changed port binding entries
when determining whether to mark the logical switch port up or
down
---
v4->v5:
- rebase
v3->v4:
- Add an initialization function to scan all entries in Port_binding table
when ovn-northd restarts or fails over
Signed-off-by: Hui Kang
---
ovn/northd/ovn-northd.c | 100
1 file changed, 68 insertions(+), 32 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 716f123..32c14a2 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -53,6 +53,11 @@ struct northd_context {
struct ovsdb_idl_txn *ovnsb_txn;
};
+struct lport_hash_node {
+struct hmap_node node;
+const struct nbrec_logical_switch_port *nbsp;
+};
+
static const char *ovnnb_db;
static const char *ovnsb_db;
@@ -3206,20 +3211,51 @@ ovnnb_db_run(struct northd_context *ctx, struct
ovsdb_idl_loop *sb_loop)
}
}
+static void
+sb_chassis_update_nbsec(const struct sbrec_port_binding *sb,
+struct hmap *lports_hmap)
+
+{
+struct lport_hash_node *hash_node;
+const struct nbrec_logical_switch_port *nbsp;
+
+nbsp = NULL;
+HMAP_FOR_EACH_WITH_HASH(hash_node, node,
+hash_string(sb->logical_port, 0),
+lports_hmap) {
+if (!strcmp(sb->logical_port, hash_node->nbsp->name)) {
+nbsp = hash_node->nbsp;
+break;
+}
+}
+
+if (!nbsp) {
+/* The logical port doesn't exist for this port binding. This can
+ * happen under normal circumstances when ovn-northd hasn't gotten
+ * around to pruning the Port_Binding yet. */
+return;
+}
+
+if (sb->chassis && (!nbsp->up || !*nbsp->up)) {
+bool up = true;
+nbrec_logical_switch_port_set_up(nbsp, &up, 1);
+} else if (!sb->chassis && (!nbsp->up || *nbsp->up)) {
+bool up = false;
+nbrec_logical_switch_port_set_up(nbsp, &up, 1);
+}
+}
+
/* Handle changes to the 'chassis' column of the 'Port_Binding' table. When
* this column is not empty, it means we need to set the corresponding logical
* port as 'up' in the northbound DB. */
static void
-update_logical_port_status(struct northd_context *ctx)
+update_logical_port_status(struct northd_context *ctx, bool is_init)
{
struct hmap lports_hmap;
const struct sbrec_port_binding *sb;
const struct nbrec_logical_switch_port *nbsp;
-struct lport_hash_node {
-struct hmap_node node;
-const struct nbrec_logical_switch_port *nbsp;
-} *hash_node;
+struct lport_hash_node *hash_node;
hmap_init(&lports_hmap);
@@ -3229,30 +3265,13 @@ update_logical_port_status(struct northd_context *ctx)
hmap_insert(&lports_hmap, &hash_node->node, hash_string(nbsp->name,
0));
}
-SBREC_PORT_BINDING_FOR_EACH(sb, ctx->ovnsb_idl) {
-nbsp = NULL;
-HMAP_FOR_EACH_WITH_HASH(hash_node, node,
-hash_string(sb->logical_port, 0),
-&lports_hmap) {
-if (!strcmp(sb->logical_port, hash_node->nbsp->name)) {
-nbsp = hash_node->nbsp;
-break;
-}
-}
-
-if (!nbsp) {
-/* The logical port doesn't exist for this port binding. This can
- * happen under normal circumstances when ovn-northd hasn't gotten
- * around to pruning the Port_Binding yet. */
-continue;
+if (is_init) {
+SBREC_PORT_BINDING_FOR_EACH(sb, ctx->ovnsb_idl) {
+sb_chassis_update_nbsec(sb, &lports_hmap);
}
-
-if (sb->chassis && (!nbsp->up || !*nbsp->up)) {
-bool up = true;
-nbrec_logical_switch_port_set_up(nbsp, &up, 1);
-} else if (!sb->chassis && (!nbsp->up || *nbsp->up)) {
-bool up = false;
-nbrec_logical_switch_port_set_up(nbsp, &up, 1);
+} else {
+SBREC_PORT_BINDING_FOR_EACH_TRACKED(sb, ctx->ovnsb_idl) {
+sb_chassis_update_nbsec(sb, &lports_hmap);
}
}
@@ -3354,13 +3373,14 @@ update_northbound_cfg(struct northd_context *ctx,
/* Handle a fairly small set of changes in the southbound database. */
static void
-ovnsb_db_run(struct northd_context *ctx, struct ovsdb_idl_loop *sb_loop)
+ovnsb_db_run(struct northd_context *ctx, struct ovsdb_idl_loop *sb_loop,
+ bool is_init)
{
if (!ctx->ovnnb_txn || !ovsdb_idl_has_ever_connected(ctx->ovnsb_idl)) {
return;
}
-update_logical_port_status(ctx);
+update_logical_port_status(ctx, is_init);
update_northbound_cfg(ctx, sb_loop);
}
@@ -3463,6 +3483,19 @@ add_col
Re: [ovs-dev] [PATCH, v5] Scanning only changed entries in the ovnsb
"dev" wrote on 08/06/2016 08:38:27 AM:
> From: Ryan Moats/Omaha/IBM@IBMUS
> To: Hui Kang
> Cc: dev@openvswitch.org
> Date: 08/06/2016 08:38 AM
> Subject: Re: [ovs-dev] [PATCH, v5] Scanning only changed entries in the
ovnsb
> Sent by: "dev"
>
>
>
>
> "dev" wrote on 07/27/2016 08:11:11 PM:
>
> > From: Hui Kang
> > To: dev@openvswitch.org
> > Date: 07/27/2016 08:11 PM
> > Subject: [ovs-dev] [PATCH, v5] Scanning only changed entries in the
ovnsb
> > Sent by: "dev"
> >
> > - Improve performance by scanning only changed port binding entries
> > when determining whether to mark the logical switch port up or
> > down
> >
> > ---
> > v4->v5:
> > - rebase
> >
> > v3->v4:
> > - Add an initialization function to scan all entries in Port_binding
> table
> > when ovn-northd restarts or fails over
> >
> > Signed-off-by: Hui Kang
> > ---
>
>
> While git am doesn't apply this patch cleanly, patch -p1 appears to
> and all the functional tests pass.
>
> There are a couple of places I would change things, shown in line
> below...
>
> > ovn/northd/ovn-northd.c | 100 +++
> > +
> > 1 file changed, 68 insertions(+), 32 deletions(-)
> >
> > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> > index 716f123..32c14a2 100644
> > --- a/ovn/northd/ovn-northd.c
> > +++ b/ovn/northd/ovn-northd.c
> > @@ -53,6 +53,11 @@ struct northd_context {
> > struct ovsdb_idl_txn *ovnsb_txn;
> > };
> >
> > +struct lport_hash_node {
> > +struct hmap_node node;
> > +const struct nbrec_logical_switch_port *nbsp;
> > +};
> > +
> > static const char *ovnnb_db;
> > static const char *ovnsb_db;
> >
> > @@ -3206,20 +3211,51 @@ ovnnb_db_run(struct northd_context *ctx,
> > struct ovsdb_idl_loop *sb_loop)
> > }
> > }
> >
> > +static void
> > +sb_chassis_update_nbsec(const struct sbrec_port_binding *sb,
> > +struct hmap *lports_hmap)
> > +
>
> Style: the blank line can be removed
>
> > +{
> > +struct lport_hash_node *hash_node;
> > +const struct nbrec_logical_switch_port *nbsp;
> > +
> > +nbsp = NULL;
> > +HMAP_FOR_EACH_WITH_HASH(hash_node, node,
> > +hash_string(sb->logical_port, 0),
> > +lports_hmap) {
> > +if (!strcmp(sb->logical_port, hash_node->nbsp->name)) {
> > +nbsp = hash_node->nbsp;
> > +break;
> > +}
> > +}
> > +
> > +if (!nbsp) {
> > +/* The logical port doesn't exist for this port binding. This
> can
> > + * happen under normal circumstances when ovn-northd hasn't
> gotten
> > + * around to pruning the Port_Binding yet. */
> > +return;
> > +}
> > +
> > +if (sb->chassis && (!nbsp->up || !*nbsp->up)) {
> > +bool up = true;
> > +nbrec_logical_switch_port_set_up(nbsp, &up, 1);
> > +} else if (!sb->chassis && (!nbsp->up || *nbsp->up)) {
> > +bool up = false;
> > +nbrec_logical_switch_port_set_up(nbsp, &up, 1);
> > +}
> > +}
> > +
> > /* Handle changes to the 'chassis' column of the 'Port_Binding' table.
> When
> > * this column is not empty, it means we need to set the
> > corresponding logical
> > * port as 'up' in the northbound DB. */
> > static void
> > -update_logical_port_status(struct northd_context *ctx)
> > +update_logical_port_status(struct northd_context *ctx, bool is_init)
> > {
> > struct hmap lports_hmap;
> > const struct sbrec_port_binding *sb;
> > const struct nbrec_logical_switch_port *nbsp;
> >
> > -struct lport_hash_node {
> > -struct hmap_node node;
> > -const struct nbrec_logical_switch_port *nbsp;
> > -} *hash_node;
> > +struct lport_hash_node *hash_node;
> >
> > hmap_init(&lports_hmap);
> >
> > @@ -3229,30 +3265,13 @@ update_logical_port_status(struct
northd_context
> *ctx)
> > hmap_insert(&lports_hmap, &hash_node->node,
> > hash_string(nbsp->name, 0));
> > }
> >
> > -SBREC_PORT_BINDING_FOR_EACH(sb, ctx->ovnsb_idl) {
> > -nbsp = NULL;
> > -HMAP_FOR
Re: [ovs-dev] [PATCH] ovn: Delete stale MAC_Bindings that result in Referential Integrity Violation
"dev" wrote on 08/24/2016 03:10:29 AM:
> From: Chandra Sekhar Vejendla/San Jose/IBM@IBMUS
> To: dev@openvswitch.org
> Date: 08/24/2016 03:11 AM
> Subject: [ovs-dev] [PATCH] ovn: Delete stale MAC_Bindings that
> result in Referential Integrity Violation
> Sent by: "dev"
>
> The MAC_Bindings have a strong reference to the Datapath_Binding. However
the
> MAC_Bindings are never deleted anywhere, and when the Datapath
(associated
> with a MAC_Binding) is deleted, the ovsdb-server returns Referential
> Integrity Violation. This prevents newer operations initiated from the
CMS
> from being committed to the Southbound DB.
>
> The patch fixes this by deleting the MAC_Binding entry when the
> logical_port referred in the mac_binding entry is deleted.
>
> Signed-off-by: Chandra Sekhar Vejendla
> ---
> ovn/northd/ovn-northd.c | 20
> tests/ovn.at| 31 +++
> 2 files changed, 51 insertions(+)
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 0874a9c..2e81390 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -3921,6 +3921,19 @@ build_lflows(struct northd_context *ctx,
> struct hmap *datapaths,
> hmap_destroy(&mcgroups);
> }
>
> +/* Remove mac_binding entries that refer to logical_ports which are
> + * deleted. */
> +static void
> +cleanup_mac_bindings(struct northd_context *ctx, struct hmap *ports)
> +{
> +const struct sbrec_mac_binding *b, *n;
> +SBREC_MAC_BINDING_FOR_EACH_SAFE (b, n, ctx->ovnsb_idl) {
> +if (!ovn_port_find(ports, b->logical_port)) {
> +sbrec_mac_binding_delete(b);
> +}
> +}
> +}
> +
> /* OVN_Northbound and OVN_Southbound have an identical Address_Set
table.
> * We always update OVN_Southbound to match the current data in
> * OVN_Northbound, so that the address sets used in Logical_Flows in
> @@ -3969,6 +3982,7 @@ ovnnb_db_run(struct northd_context *ctx,
> struct ovsdb_idl_loop *sb_loop)
> build_ports(ctx, &datapaths, &ports);
> build_ipam(ctx, &datapaths, &ports);
> build_lflows(ctx, &datapaths, &ports);
> +cleanup_mac_bindings(ctx, &ports);
Hi, Chandra,
Should the function be called inside build_ports() for the sake the
cleanness?
In addition, all port processing in that function.
Thanks
- Hui
>
> sync_address_sets(ctx);
>
> @@ -4347,6 +4361,12 @@ main(int argc, char *argv[])
> add_column_noalert(ovnsb_idl_loop.idl,
&sbrec_port_binding_col_options);
> add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_mac);
> ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> &sbrec_port_binding_col_chassis);
> +ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_mac_binding);
> +add_column_noalert(ovnsb_idl_loop.idl,
&sbrec_mac_binding_col_datapath);
> +add_column_noalert(ovnsb_idl_loop.idl, &sbrec_mac_binding_col_ip);
> +add_column_noalert(ovnsb_idl_loop.idl, &sbrec_mac_binding_col_mac);
> +add_column_noalert(ovnsb_idl_loop.idl,
> + &sbrec_mac_binding_col_logical_port);
> ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_dhcp_options);
> add_column_noalert(ovnsb_idl_loop.idl,
&sbrec_dhcp_options_col_code);
> add_column_noalert(ovnsb_idl_loop.idl,
&sbrec_dhcp_options_col_type);
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 216bb07..4804f35 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -4965,3 +4965,34 @@ cat packets
> OVN_CLEANUP([hv1])
>
> AT_CLEANUP
> +
> +AT_SETUP([ovn -- delete mac bindings])
> +AT_KEYWORDS([ovn])
> +ovn_start
> +net_add n1
> +sim_add hv1
> +as hv1
> +ovs-vsctl -- add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +# Create logical switch ls0
> +ovn-nbctl ls-add ls0
> +# Create port lp0 in ls0
> +ovn-nbctl lsp-add ls0 lp1
> +ovn-nbctl lsp-set-addresses lp0 "f0:00:00:00:00:01 192.168.0.1"
> +ovn-nbctl lsp-set-addresses lp1 "f0:00:00:00:00:02 192.168.0.2"
> +dp_uuid=`ovn-sbctl find datapath | grep uuid | cut -f2 -d ":" | cut
> -f2 -d " "`
> +ovn-sbctl create MAC_Binding ip=10.0.0.1 datapath=$dp_uuid
> logical_port=lp0 mac="mac1"
> +ovn-sbctl create MAC_Binding ip=10.0.0.1 datapath=$dp_uuid
> logical_port=lp1 mac="mac2"
> +ovn-sbctl find MAC_Binding
> +#Delete port lp0
> +ovn-nbctl lsp-del lp0
> +ovn-sbctl find MAC_Binding
> +AT_CHECK([ovn-sbctl find MAC_Binding logical_port=lp0], [0], [])
> +#Delete ls0
> +ovn-nbctl ls-del ls0
> +ovn-sbctl find MAC_Binding
> +AT_CHECK([ovn-sbctl find MAC_Binding], [0], [])
> +
> +OVN_CLEANUP([hv1])
> +
> +AT_CLEANUP
> --
> 1.9.1
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Difference between hmap and cmap
Hi, OVS developers, I am studying the hashing functions in OVS and find these two implementations: hmap and cmap. I understand that cmap is the improved cuckoo hashing and cmap supports multiple-reader and single writer. My questions are: 1. What does hmap stand for? hash map? 2. hmap looks much simpler than cmap; does it support multi-reader and single writers? 3. If cmap performs better than hmap, why not using cmap all over the OVS code? Thanks in advance. - Hui ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] Fix missing system-id in INSTALL.Docker
Signed-off-by: Hui Kang --- INSTALL.Docker.md | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/INSTALL.Docker.md b/INSTALL.Docker.md index b62922d..5cb49d0 100644 --- a/INSTALL.Docker.md +++ b/INSTALL.Docker.md @@ -69,6 +69,8 @@ gets cleared. It is harmless to run it again in any case.) $LOCAL_IP in the below command is the IP address via which other hosts can reach this host. This acts as your local tunnel endpoint. +$SYSTEM_ID in the below command is the unique identifier for the docker host. + $ENCAP_TYPE is the type of tunnel that you would like to use for overlay networking. The options are "geneve" or "stt". (Please note that your kernel should have support for your chosen $ENCAP_TYPE. Both geneve @@ -79,8 +81,10 @@ support in upstream Linux. You can verify whether you have the support in your kernel by doing a "lsmod | grep $ENCAP_TYPE".) ``` -ovs-vsctl set Open_vSwitch . external_ids:ovn-remote="tcp:$CENTRAL_IP:6642" \ - external_ids:ovn-nb="tcp:$CENTRAL_IP:6641" external_ids:ovn-encap-ip=$LOCAL_IP external_ids:ovn-encap-type="$ENCAP_TYPE" +ovs-vsctl set Open_vSwitch . external-ids:system-id=$SYSTEM_ID \ + external_ids:ovn-remote="tcp:$CENTRAL_IP:6642" \ + external_ids:ovn-nb="tcp:$CENTRAL_IP:6641" \ + external_ids:ovn-encap-ip=$LOCAL_IP external_ids:ovn-encap-type="$ENCAP_TYPE" ``` And finally, start the ovn-controller. (You need to run the below command -- 1.9.1 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] Fix missing system-id in INSTALL.Docker
"dev" wrote on 10/03/2016 03:57:21 PM: > From: Ben Pfaff > To: Hui Kang > Cc: dev@openvswitch.org > Date: 10/03/2016 03:57 PM > Subject: Re: [ovs-dev] [PATCH] Fix missing system-id in INSTALL.Docker > Sent by: "dev" > > On Mon, Oct 03, 2016 at 03:30:51PM -0400, Hui Kang wrote: > > Signed-off-by: Hui Kang > > --- > > INSTALL.Docker.md | 8 ++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/INSTALL.Docker.md b/INSTALL.Docker.md > > index b62922d..5cb49d0 100644 > > --- a/INSTALL.Docker.md > > +++ b/INSTALL.Docker.md > > @@ -69,6 +69,8 @@ gets cleared. It is harmless to run it again inany case.) > > $LOCAL_IP in the below command is the IP address via which other hosts > > can reach this host. This acts as your local tunnel endpoint. > > > > +$SYSTEM_ID in the below command is the unique identifier for the > docker host. > > + > > $ENCAP_TYPE is the type of tunnel that you would like to use for overlay > > networking. The options are "geneve" or "stt". (Please note that your > > kernel should have support for your chosen $ENCAP_TYPE. Both geneve > > @@ -79,8 +81,10 @@ support in upstream Linux. You can verify > whether you have the support in your > > kernel by doing a "lsmod | grep $ENCAP_TYPE".) > > > > ``` > > -ovs-vsctl set Open_vSwitch . external_ids:ovn-remote="tcp: > $CENTRAL_IP:6642" \ > > - external_ids:ovn-nb="tcp:$CENTRAL_IP:6641" external_ids:ovn- > encap-ip=$LOCAL_IP external_ids:ovn-encap-type="$ENCAP_TYPE" > > +ovs-vsctl set Open_vSwitch . external-ids:system-id=$SYSTEM_ID \ > > + external_ids:ovn-remote="tcp:$CENTRAL_IP:6642" \ > > + external_ids:ovn-nb="tcp:$CENTRAL_IP:6641" \ > > + external_ids:ovn-encap-ip=$LOCAL_IP external_ids:ovn-encap- > type="$ENCAP_TYPE" > > Usually the OVS system startup script should ensure that the system has > a system-id. You don't see that behavior in your environment? No. I followed the instruction in [1] and [2] to setup the OVN. Without system-id in the ovs-vsctl command, I saw the following message and "ovn-sbctl show" does not show any connected chassis. With the system-id in the command, "ovn-sbctl show" outputs correctly. 2016-10-03T19:10:56.892Z|8|main|WARN|'system-id' in Open_vSwitch database is missing. 2016-10-03T19:11:11.902Z|9|main|WARN|Dropped 6 log messages in last 15 seconds (most recently, 5 seconds ago) due to excessive rate 2016-10-03T19:11:11.902Z|00010|main|WARN|'system-id' in Open_vSwitch database is missing. 2016-10-03T19:11:26.912Z|00011|main|WARN|Dropped 6 log messages in last 15 seconds (most recently, 5 seconds ago) due to excessive rate 2016-10-03T19:11:26.912Z|00012|main|WARN|'system-id' in Open_vSwitch database is missing. 2016-10-03T19:11:41.927Z|00013|main|WARN|Dropped 5 log messages in last 15 seconds (most recently, 5 seconds ago) due to excessive rate Thanks. - Hui > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] Fix missing system-id in INSTALL.Docker
Ben Pfaff wrote on 10/03/2016 09:23:29 PM: > From: Ben Pfaff > To: Hui Kang/Watson/IBM@IBMUS > Cc: Hui Kang , dev@openvswitch.org > Date: 10/03/2016 09:23 PM > Subject: Re: [ovs-dev] [PATCH] Fix missing system-id in INSTALL.Docker > > On Mon, Oct 03, 2016 at 04:43:20PM -0400, Hui Kang wrote: > > > > > > "dev" wrote on 10/03/2016 03:57:21 PM: > > > > > From: Ben Pfaff > > > To: Hui Kang > > > Cc: dev@openvswitch.org > > > Date: 10/03/2016 03:57 PM > > > Subject: Re: [ovs-dev] [PATCH] Fix missing system-id in INSTALL.Docker > > > Sent by: "dev" > > > > > > On Mon, Oct 03, 2016 at 03:30:51PM -0400, Hui Kang wrote: > > > > Signed-off-by: Hui Kang > > > > --- > > > > INSTALL.Docker.md | 8 ++-- > > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/INSTALL.Docker.md b/INSTALL.Docker.md > > > > index b62922d..5cb49d0 100644 > > > > --- a/INSTALL.Docker.md > > > > +++ b/INSTALL.Docker.md > > > > @@ -69,6 +69,8 @@ gets cleared. It is harmless to run it again inany > > case.) > > > > $LOCAL_IP in the below command is the IP address via which other hosts > > > > can reach this host. This acts as your local tunnel endpoint. > > > > > > > > +$SYSTEM_ID in the below command is the unique identifier for the > > > docker host. > > > > + > > > > $ENCAP_TYPE is the type of tunnel that you would like to use for > > overlay > > > > networking. The options are "geneve" or "stt". (Please note that > > your > > > > kernel should have support for your chosen $ENCAP_TYPE. Both geneve > > > > @@ -79,8 +81,10 @@ support in upstream Linux. You can verify > > > whether you have the support in your > > > > kernel by doing a "lsmod | grep $ENCAP_TYPE".) > > > > > > > > ``` > > > > -ovs-vsctl set Open_vSwitch . external_ids:ovn-remote="tcp: > > > $CENTRAL_IP:6642" \ > > > > - external_ids:ovn-nb="tcp:$CENTRAL_IP:6641" external_ids:ovn- > > > encap-ip=$LOCAL_IP external_ids:ovn-encap-type="$ENCAP_TYPE" > > > > +ovs-vsctl set Open_vSwitch . external-ids:system-id=$SYSTEM_ID \ > > > > + external_ids:ovn-remote="tcp:$CENTRAL_IP:6642" \ > > > > + external_ids:ovn-nb="tcp:$CENTRAL_IP:6641" \ > > > > + external_ids:ovn-encap-ip=$LOCAL_IP external_ids:ovn-encap- > > > type="$ENCAP_TYPE" > > > > > > Usually the OVS system startup script should ensure that the system has > > > a system-id. You don't see that behavior in your environment? > > > > No. I followed the instruction in [1] and [2] to setup the OVN. Without > > system-id in the ovs-vsctl command, I saw the following message and > > "ovn-sbctl show" does not show any connected chassis. With the system-id > > in the command, "ovn-sbctl show" outputs correctly. > > > > 2016-10-03T19:10:56.892Z|8|main|WARN|'system-id' in Open_vSwitch > > database is missing. > > 2016-10-03T19:11:11.902Z|9|main|WARN|Dropped 6 log messages in last 15 > > seconds (most recently, 5 seconds ago) due to excessive rate > > 2016-10-03T19:11:11.902Z|00010|main|WARN|'system-id' in Open_vSwitch > > database is missing. > > 2016-10-03T19:11:26.912Z|00011|main|WARN|Dropped 6 log messages in last 15 > > seconds (most recently, 5 seconds ago) due to excessive rate > > 2016-10-03T19:11:26.912Z|00012|main|WARN|'system-id' in Open_vSwitch > > database is missing. > > 2016-10-03T19:11:41.927Z|00013|main|WARN|Dropped 5 log messages in last 15 > > seconds (most recently, 5 seconds ago) due to excessive rate > > It's probably best to fix the instructions so that they say how to set > up Open vSwitch itself (rather than OVN) to include a system-id. > > You said that the instructions are in "[1] and [2]" but I don't know > what those reference. My bad. They are [1] https://github.com/openvswitch/ovs/blob/master/INSTALL.md [2] https://github.com/openvswitch/ovs/blob/master/INSTALL.Docker.md There is no where in [1] or [2] mentioning system-id. Thanks. - Hui > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] Fix missing system-id in INSTALL.Docker
Ben Pfaff wrote on 10/04/2016 12:25:50 AM: > From: Ben Pfaff > To: Hui Kang/Watson/IBM@IBMUS > Cc: dev@openvswitch.org, Hui Kang > Date: 10/04/2016 12:26 AM > Subject: Re: [ovs-dev] [PATCH] Fix missing system-id in INSTALL.Docker > > On Mon, Oct 03, 2016 at 10:15:21PM -0400, Hui Kang wrote: > > > > > > Ben Pfaff wrote on 10/03/2016 09:23:29 PM: > > > > > From: Ben Pfaff > > > To: Hui Kang/Watson/IBM@IBMUS > > > Cc: Hui Kang , dev@openvswitch.org > > > Date: 10/03/2016 09:23 PM > > > Subject: Re: [ovs-dev] [PATCH] Fix missing system-id in INSTALL.Docker > > > > > > On Mon, Oct 03, 2016 at 04:43:20PM -0400, Hui Kang wrote: > > > > > > > > > > > > "dev" wrote on 10/03/2016 03:57:21 PM: > > > > > > > > > From: Ben Pfaff > > > > > To: Hui Kang > > > > > Cc: dev@openvswitch.org > > > > > Date: 10/03/2016 03:57 PM > > > > > Subject: Re: [ovs-dev] [PATCH] Fix missing system-id in > > INSTALL.Docker > > > > > Sent by: "dev" > > > > > > > > > > On Mon, Oct 03, 2016 at 03:30:51PM -0400, Hui Kang wrote: > > > > > > Signed-off-by: Hui Kang > > > > > > --- > > > > > > INSTALL.Docker.md | 8 ++-- > > > > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/INSTALL.Docker.md b/INSTALL.Docker.md > > > > > > index b62922d..5cb49d0 100644 > > > > > > --- a/INSTALL.Docker.md > > > > > > +++ b/INSTALL.Docker.md > > > > > > @@ -69,6 +69,8 @@ gets cleared. It is harmless to run it again > > inany > > > > case.) > > > > > > $LOCAL_IP in the below command is the IP address via which other > > hosts > > > > > > can reach this host. This acts as your local tunnel endpoint. > > > > > > > > > > > > +$SYSTEM_ID in the below command is the unique identifier for the > > > > > docker host. > > > > > > + > > > > > > $ENCAP_TYPE is the type of tunnel that you would like to use for > > > > overlay > > > > > > networking. The options are "geneve" or "stt". (Please note that > > > > your > > > > > > kernel should have support for your chosen $ENCAP_TYPE. Both > > geneve > > > > > > @@ -79,8 +81,10 @@ support in upstream Linux. You can verify > > > > > whether you have the support in your > > > > > > kernel by doing a "lsmod | grep $ENCAP_TYPE".) > > > > > > > > > > > > ``` > > > > > > -ovs-vsctl set Open_vSwitch . external_ids:ovn-remote="tcp: > > > > > $CENTRAL_IP:6642" \ > > > > > > - external_ids:ovn-nb="tcp:$CENTRAL_IP:6641" external_ids:ovn- > > > > > encap-ip=$LOCAL_IP external_ids:ovn-encap-type="$ENCAP_TYPE" > > > > > > +ovs-vsctl set Open_vSwitch . external-ids:system-id=$SYSTEM_ID \ > > > > > > + external_ids:ovn-remote="tcp:$CENTRAL_IP:6642" \ > > > > > > + external_ids:ovn-nb="tcp:$CENTRAL_IP:6641" \ > > > > > > + external_ids:ovn-encap-ip=$LOCAL_IP external_ids:ovn-encap- > > > > > type="$ENCAP_TYPE" > > > > > > > > > > Usually the OVS system startup script should ensure that the system > > has > > > > > a system-id. You don't see that behavior in your environment? > > > > > > > > No. I followed the instruction in [1] and [2] to setup the OVN. Without > > > > system-id in the ovs-vsctl command, I saw the following message and > > > > "ovn-sbctl show" does not show any connected chassis. With the > > system-id > > > > in the command, "ovn-sbctl show" outputs correctly. > > > > > > > > 2016-10-03T19:10:56.892Z|8|main|WARN|'system-id' in Open_vSwitch > > > > database is missing. > > > > 2016-10-03T19:11:11.902Z|9|main|WARN|Dropped 6 log messages in last > > 15 > > > > seconds (most recently, 5 seconds ago) due to excessive rate > > > > 2016-10-03T19:11:11.902Z|00010|main|WARN|'system-id' in Open_vSwitch > > > > database is missing. > > > > 2016-10-03T19:11:26.912Z|00011|main|WARN|Dropp
[ovs-dev] [PATCH v2] Fix missing system-id in INSTALL.Docker
Signed-off-by: Hui Kang --- INSTALL.Docker.md | 8 1 file changed, 8 insertions(+) diff --git a/INSTALL.Docker.md b/INSTALL.Docker.md index b62922d..4606354 100644 --- a/INSTALL.Docker.md +++ b/INSTALL.Docker.md @@ -83,6 +83,14 @@ ovs-vsctl set Open_vSwitch . external_ids:ovn-remote="tcp:$CENTRAL_IP:6642" \ external_ids:ovn-nb="tcp:$CENTRAL_IP:6641" external_ids:ovn-encap-ip=$LOCAL_IP external_ids:ovn-encap-type="$ENCAP_TYPE" ``` +Note that if Open vSwitch is started manually (without the assistance of system +startup scripts provided by packages like .deb or .rpm), you should provide a +unique identification, $SYSTEM_ID for this host: + +``` +ovs-vsctl set Open_vSwitch . external_ids:system-id=$SYSTEM_ID +``` + And finally, start the ovn-controller. (You need to run the below command on every boot) -- 1.9.1 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] Fix missing system-id in INSTALL.Docker
I like your version better. I am ok abandoning this patch. Thanks. - Hui On Tue, Oct 4, 2016 at 7:01 PM, Ben Pfaff wrote: > On Tue, Oct 04, 2016 at 04:33:02PM -0400, Hui Kang wrote: >> Signed-off-by: Hui Kang >> --- >> INSTALL.Docker.md | 8 >> 1 file changed, 8 insertions(+) >> >> diff --git a/INSTALL.Docker.md b/INSTALL.Docker.md >> index b62922d..4606354 100644 >> --- a/INSTALL.Docker.md >> +++ b/INSTALL.Docker.md >> @@ -83,6 +83,14 @@ ovs-vsctl set Open_vSwitch . >> external_ids:ovn-remote="tcp:$CENTRAL_IP:6642" \ >>external_ids:ovn-nb="tcp:$CENTRAL_IP:6641" >> external_ids:ovn-encap-ip=$LOCAL_IP external_ids:ovn-encap-type="$ENCAP_TYPE" >> ``` >> >> +Note that if Open vSwitch is started manually (without the assistance of >> system >> +startup scripts provided by packages like .deb or .rpm), you should provide >> a >> +unique identification, $SYSTEM_ID for this host: >> + >> +``` >> +ovs-vsctl set Open_vSwitch . external_ids:system-id=$SYSTEM_ID >> +``` >> + >> And finally, start the ovn-controller. (You need to run the below command >> on every boot) > > Thanks for working on this. > > I'm concerned about this wording, because the system-id should be > persistent and unique and this isn't specific enough to encourage people > to do it the right way. > > I posted my version, see what you think: > https://patchwork.ozlabs.org/patch/678263/ ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
