[ovs-git] [ovn-org/ovn] dce49a: ovn-ctl: Handle cluster db upgrades for run_(nb/sb...
numansiddique
noreply at github.com
Tue Mar 2 18:32:40 UTC 2021
Branch: refs/heads/branch-20.03
Home: https://github.com/ovn-org/ovn
Commit: dce49ab7bf0365e50e9660098f743b340d444fbe
https://github.com/ovn-org/ovn/commit/dce49ab7bf0365e50e9660098f743b340d444fbe
Author: Numan Siddique <numans at ovn.org>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M utilities/ovn-ctl
Log Message:
-----------
ovn-ctl: Handle cluster db upgrades for run_(nb/sb)_ovsdb
when ovn-ctl run_(nb_sb)_ovsdb is called, the ovsdb-server is started without
passing --detach and --monoitor and the process is exec'd.
For cluster mode, upgrade_cluster is never called and hence the dbs are not upraded
to new schema. CMS has to handle the db upgrade separately.
This patch handles the db upgrade by starting ovsdb-server in background and then
waits on ovsdb-server to complete.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1868392
Acked-by: Mark Michelson <mmichels at redhat.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
(cherry picked from commit 67e2f386cc838d0b0f9b4b5da7fe611e1113b70c)
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Commit: f56d6421ffedbc8111447edb61a571fa7ebef5b1
https://github.com/ovn-org/ovn/commit/f56d6421ffedbc8111447edb61a571fa7ebef5b1
Author: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M northd/ovn-northd.8.xml
M northd/ovn-northd.c
M tests/ovn.at
Log Message:
-----------
Revert "Manage ARP process locally in a DVR scenario"
This reverts commit c0bf32d72f8b893bbe3cb64912b0fd259d71555f.
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
Signed-off-by: Han Zhou <hzhou at ovn.org>
(cherry picked from commit d9ed450713eda62af1bec5009694b2d206c9f435)
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
Commit: cc68620af4d2b0b5fb6e7ebb8d1b781830e56df1
https://github.com/ovn-org/ovn/commit/cc68620af4d2b0b5fb6e7ebb8d1b781830e56df1
Author: Han Zhou <hzhou at ovn.org>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M controller/ofctrl.c
Log Message:
-----------
ofctrl.c: Maintain references between installed flows and desired flows.
Currently there is no link maintained between installed flows and desired
flows. This patch maintains the mapping between them, which will be useful
for a future patch that incrementally processes the flow installation without
having to do the full comparison between them.
This patch also refactors the struct ovn_flow with two different wrapper
types: desired_flow and installed_flow, and the related static functions,
to make the code easier to read and avoid misuses of the struct.
Acked-by: Mark Michelson <mmichels at redhat.com>
Signed-off-by: Han Zhou <hzhou at ovn.org>
(cherry picked from commit 354d3853d40cbce89a434632f67daed7fc992d8b)
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
Commit: c17b73d172139b9395595d86cf4d3309c2207ca9
https://github.com/ovn-org/ovn/commit/c17b73d172139b9395595d86cf4d3309c2207ca9
Author: Han Zhou <hzhou at ovn.org>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M controller/ofctrl.c
Log Message:
-----------
ofctrl.c: Refactor - move openflow msg construction to functions.
These functions will be reused in multiple places in a future patch.
Acked-by: Mark Michelson <mmichels at redhat.com>
Signed-off-by: Han Zhou <hzhou at ovn.org>
(cherry picked from commit 23063cf4178c05f5d6b3e4ec6d323ccc88df6101)
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
Commit: a4809a59175903a7276b3ca6d36a23dabb5fa363
https://github.com/ovn-org/ovn/commit/a4809a59175903a7276b3ca6d36a23dabb5fa363
Author: Han Zhou <hzhou at ovn.org>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M controller/ofctrl.c
M controller/ofctrl.h
Log Message:
-----------
ofctrl: Incremental processing for flow installation by tracking.
With incremental processing for flow computation, the bottle neck of
ovn-controller in large scale environment is in the flow installation
(ofctrl_put()), which does full comparison between the two big flow tables: the
installed flows and desired flows.
This patch implements tracking desired flow changes when flows are
incrementally computed by I-P engine, and then incrementally processing the
flow installation using the tracked information in ofctrl_put(). It falls back
to the full comparison whenever tracking information is unavailable, e.g. when
I-P engine triggers full recompute.
In ovn-scale-test with 3000 HVs and 30k lports, the end-to-end latency between
the moment a lflow is updated in SB DB and the moment when all the 3K HVs
complete OVS flow updating has reduced around 60% (from 1s to 400ms).
Below is the perf result for a ovn-controller processing a new port-binding:
Beore:
+ 96.76% 0.00% ovn-controller [unknown] [k] 0xffffffffffffffff
+ 90.21% 0.00% ovn-controller ovn-controller [.] main
+ 39.93% 1.19% ovn-controller ovn-controller [.] ofctrl_put
+ 31.27% 12.47% ovn-controller ovn-controller [.] ovn_flow_lookup
+ 22.12% 3.12% ovn-controller ovn-controller [.] encaps_run
+ 18.69% 2.77% ovn-controller ovn-controller [.] minimatch_equal
+ 17.63% 4.11% ovn-controller ovn-controller [.] patch_run
+ 15.91% 0.00% ovn-controller ovn-controller [.] add_bridge_mappings (inlined)
+ 14.03% 12.08% ovn-controller ovn-controller [.] minimask_equal
+ 12.41% 0.00% ovn-controller ovn-controller [.] chassis_tunnel_add (inlined)
+ 11.40% 0.00% ovn-controller ovn-controller [.] tunnel_add (inlined)
After:
+ 94.59% 0.00% ovn-controller [unknown] [k] 0xffffffffffffffff
+ 83.56% 0.09% ovn-controller ovn-controller [.] main
+ 35.37% 3.13% ovn-controller ovn-controller [.] encaps_run
+ 27.54% 7.53% ovn-controller ovn-controller [.] patch_run
+ 24.86% 0.00% ovn-controller ovn-controller [.] add_bridge_mappings (inlined)
+ 20.01% 0.00% ovn-controller ovn-controller [.] chassis_tunnel_add (inlined)
+ 18.51% 0.00% ovn-controller ovn-controller [.] tunnel_add (inlined)
+ 14.08% 0.17% ovn-controller ovn-controller [.] physical_run
+ 11.37% 11.28% ovn-controller ovn-controller [.] next_real_row
+ 10.50% 2.59% ovn-controller ovn-controller [.] consider_port_binding
...
+ 2.14% 0.32% ovn-controller ovn-controller [.] ofctrl_put ▒
Before the optimization, ofctrl_put took 40% of CPU, and now it disappears from
the hot spots.
Acked-by: Mark Michelson <mmichels at redhat.com>
Signed-off-by: Han Zhou <hzhou at ovn.org>
(cherry picked from commit 6f0b1e02d9ab3a94048c4818f2d382938cad4b71)
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
Commit: d7cb15d333aef3ee9f1db0f0860c4ee9e345e70c
https://github.com/ovn-org/ovn/commit/d7cb15d333aef3ee9f1db0f0860c4ee9e345e70c
Author: Han Zhou <hzhou at ovn.org>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M controller/ofctrl.c
Log Message:
-----------
ofctrl.c: Merge opposite changes of tracked flows before installing.
This patch optimizes the previous patch that incrementally processes
flow installation by merging the "add-after-delete" flow changes as
much as possible to avoid unnecessary OpenFlow updates.
Acked-by: Mark Michelson <mmichels at redhat.com>
Signed-off-by: Han Zhou <hzhou at ovn.org>
(cherry picked from commit f4e508dd7a6cfbfc2e3250a8c11a8d0fdc1dfdd0)
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
Commit: 0c388ade25e3cb0f8f322680df15707231a206e5
https://github.com/ovn-org/ovn/commit/0c388ade25e3cb0f8f322680df15707231a206e5
Author: Han Zhou <hzhou at ovn.org>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M controller/ofctrl.c
M tests/ovn.at
Log Message:
-----------
ofctrl.c: Fix duplicated flow handling in I-P while merging opposite changes.
In a scenario in I-P when a desired flow is removed and an exactly same flow is
added back in the same iteration, the merge_tracked_flows() function will merge
the operation without firing any OpenFlow action. However, if there are
multiple desired flows (e.g. F1 and F2) matching the same installed flow, and
if the one being re-added (say, F1) is the one currently installed in OVS, the
current implementation will update the currently installed flow to F2 in the
data structure while the actual OVS flow is not updated (still F1). So far
there is still no problem, but the member desired_flow of the installed flow
is pointing to the wrong desired flow.
Now there is only one place where the desired_flow member is consulted, in
update_installed_flows_by_track() for handling a tracked *modified* flow. In
reality flow modification happens only when conjunction flows need to be
combined, which would never happen together with the above scenario of
merge_tracked_flows(). So this wrong desired_flow pointer doesn't cause any
real problem yet.
However, there is a place that can already utilize this active desired_flow
information, which is when handling a tracked flow deletion in
update_installed_flows_by_track(): we can check if the flow being deleted is
the currently active one installed in OVS. If not, we don't need to send and
OpenFlow modify to OVS. This optimization is added in this patch, apart from
fixing the problem of merge_tracked_flows(). Without the fix, this optimization
would cause the test case added in this patch fail.
Fixes: f4e508dd7 ("ofctrl.c: Merge opposite changes of tracked flows before installing.")
Acked-by: Dumitru Ceara <dceara at redhat.com>
Signed-off-by: Han Zhou <hzhou at ovn.org>
(cherry picked from commit 9d2e8d32fb9865513b70408a665184a67564390d)
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
Commit: ebf1929bd48cb05c961a6b8bbdb02598435d1c68
https://github.com/ovn-org/ovn/commit/ebf1929bd48cb05c961a6b8bbdb02598435d1c68
Author: Han Zhou <hzhou at ovn.org>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M controller/ofctrl.c
Log Message:
-----------
ofctrl.c: Avoid repeatedly linking an installed flow and a desired flow.
In update_installed_flows_by_compare() there are two loops. The first
loop iterates the installed flows and find its peer in desired flows to:
1. uninstall flows that are not needed anymore
2. update flows if needed
At the same time, it links the desired flow found for the installed flow
which also set the desired flow as the current active installed flow.
The second loop iterates the desired flows and find its peer in installed
flows to install missing flows. At the same time it will detect if there
are conflict desired flows matching same installed flow then just link
them.
However, currently in the second loop, it blindly link the desired flows to the
installed flows, without checking if it is already linked in the first loop.
Lucky enough, this won't cause any real problem so far, because when there are
conflict flows, the one found in the first loop will be set as active in the
installed_flow, and in the function link_installed_to_desired() checks if it is
already the active desired flow it just does nothing but return. However, the
check in the link_installed_to_desired() is confusing because a desired_flow
may be linked to the installed_flow already but not the active flow, and the
check is insufficient. It should be rather an assertion and let the caller
ensure that a pair of desired_flow and installed_flow is never linked twice.
For the above reason, this patch does the following changes:
1. Removes the check in link_installed_to_desired() and convert it to an assert.
2. Before calling link_installed_to_desired() in the above mentioned loop,
check if the desired flow is already installed.
Acked-by: Dumitru Ceara <dceara at redhat.com>
Signed-off-by: Han Zhou <hzhou at ovn.org>
(cherry picked from commit 7cab7bd1268ba67429954da4f73de91090acf779)
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
Commit: 2cff4639076b1d719db6ff3b480d9ed7468fc816
https://github.com/ovn-org/ovn/commit/2cff4639076b1d719db6ff3b480d9ed7468fc816
Author: Dumitru Ceara <dceara at redhat.com>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M controller/ofctrl.c
Log Message:
-----------
ofctrl.c: Only merge actions for conjunctive flows.
In ofctrl_add_or_append_flow() when merging flow actions make sure we only
do that for conjunctive flows. All other actions can not be merged with
action "conjunction".
CC: Mark Michelson <mmichels at redhat.com>
Fixes: e659bab31a91 ("Combine conjunctions with identical matches into one flow.")
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
Signed-off-by: Han Zhou <hzhou at ovn.org>
(cherry picked from commit dadae4f800ccb1f2759378f0bd804dd002e31605)
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
Commit: fa08a1a155c6538c672714b57aa9aa2749352c4f
https://github.com/ovn-org/ovn/commit/fa08a1a155c6538c672714b57aa9aa2749352c4f
Author: Dumitru Ceara <dceara at redhat.com>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M controller/ofctrl.c
Log Message:
-----------
ofctrl.c: Do not change flow ordering when merging opposite changes.
Instead of removing the old desired flow from the list and inserting the new
one at the end we now directly replace the old flow with the new one while
maintaining the same ordering.
For now order of the flows is not relevant but further commits require
maintaining the order of desired flows.
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
Signed-off-by: Han Zhou <hzhou at ovn.org>
(cherry picked from commit e49ce9a33f38f29c44e3c30afcc189b5f6a9ef8e)
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
Commit: 589e5227572e9da9593664eeea4e6c8ac82be9f3
https://github.com/ovn-org/ovn/commit/589e5227572e9da9593664eeea4e6c8ac82be9f3
Author: Dumitru Ceara <dceara at redhat.com>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M controller/ofctrl.c
Log Message:
-----------
ofctrl.c: Simplify active desired flow selection.
Always consider the first "desired flow" in the list of flows that refer an
"installed flow" as the active flow. This simplifies the logic of the flow
update code and is used in a further commit to implement a partial ordering
of desired flows within installed flows.
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
Signed-off-by: Han Zhou <hzhou at ovn.org>
(cherry picked from commit 107bb25029350bd0f7dfeeb0ef3053adbd504e3e)
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
Commit: d11e11cca987618cf544942e8ae62f4d64a1bf08
https://github.com/ovn-org/ovn/commit/d11e11cca987618cf544942e8ae62f4d64a1bf08
Author: Dumitru Ceara <dceara at redhat.com>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M controller/ofctrl.c
Log Message:
-----------
ofctrl.c: Always log the most recent flow changes.
Fixes: 6f0b1e02d9ab ("ofctrl: Incremental processing for flow installation by tracking.")
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
Signed-off-by: Han Zhou <hzhou at ovn.org>
(cherry picked from commit 33c15c145988daa6172928dc870f3a0225515f50)
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
Commit: 0280878addd9d852449ee9ee6bf2d7e5461ff7e5
https://github.com/ovn-org/ovn/commit/0280878addd9d852449ee9ee6bf2d7e5461ff7e5
Author: Dumitru Ceara <dceara at redhat.com>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M controller/ofctrl.c
M tests/ovn.at
Log Message:
-----------
ofctrl.c: Add a predictable resolution for conflicting flow actions.
Until now, in case the ACL configuration generates openflows that have
the same match but different actions, ovn-controller was using the
following approach:
1. If the flow being added contains conjunctive actions, merge its
actions with the already existing flow.
2. Otherwise, if the flow is being added incrementally
(update_installed_flows_by_track), don't install the new flow but
instead keep the old one.
3. Otherwise, (update_installed_flows_by_compare), don't install the
new flow but instead keep the old one.
Even though one can argue that having an ACL with a match that includes
the match of another ACL is a misconfiguration, it can happen that the
users provision OVN like this. Depending on the order of reading and
installing the logical flows, the above operations can yield
unpredictable results, e.g., allow specific traffic but then after
ovn-controller is restarted (or a recompute happens) that specific
traffic starts being dropped.
A simple example of ACL configuration is:
ovn-nbctl acl-add ls to-lport 3 '(ip4.src==10.0.0.1 ||
ip4.src==10.0.0.2) && (ip4.dst == 10.0.0.3 || ip4.dst == 10.0.0.4)' allow
ovn-nbctl acl-add ls to-lport 3 'ip4.src==10.0.0.1' allow
ovn-nbctl acl-add ls to-lport 2 'arp' allow
ovn-nbctl acl-add ls to-lport 1 'ip4' drop
This is a pattern used by most CMSs:
- define a default deny policy.
- punch holes in the default deny policy based on user specific
configurations.
Without this commit the behavior for traffic from 10.0.0.1 to 10.0.0.5
is unpredictable. Depending on the order of operations traffic might be
dropped or allowed.
It's also quite hard to force the CMS to ensure that such match overlaps
never occur.
To address this issue we now ensure that all desired flows refering the
same installed flow are partially sorted in the following way:
- first all flows with action "allow".
- then all flows with action "drop".
- then a single flow with action "conjunction" (resulting from merging
all flows with the same match and action conjunction).
This ensures that "allow" flows have precedence over "drop" flows which
in turn have precedence over "conjunction" flows. Essentially less
restrictive flows are always preferred over more restrictive flows whenever a match
conflict happens.
CC: Daniel Alvarez <dalvarez at redhat.com>
Reported-at: https://bugzilla.redhat.com/1871931
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
Acked-by: Mark Gray <mark.d.gray at redhat.com>
Signed-off-by: Han Zhou <hzhou at ovn.org>
(cherry picked from commit 986b3d5e4ad6f05245d021ba699c957246294a22)
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
Commit: e868c78bd7bdbd39e34f126524c1fa7ce03e8fd8
https://github.com/ovn-org/ovn/commit/e868c78bd7bdbd39e34f126524c1fa7ce03e8fd8
Author: Dumitru Ceara <dceara at redhat.com>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M tests/ovn.at
Log Message:
-----------
ovn.at: Make some of the tests more predictable.
Fix some of the race conditions that are present in the current OVN test
suite.
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
(cherry picked from commit d6594a4695084e1b0f4fb3170547942876d3a81a)
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
Commit: d3ae7ebef13ca6b2648f22cac1d73c519573dc76
https://github.com/ovn-org/ovn/commit/d3ae7ebef13ca6b2648f22cac1d73c519573dc76
Author: Dumitru Ceara <dceara at redhat.com>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M tests/ofproto-macros.at
Log Message:
-----------
tests: Add ofctl_strip_all() to filter OVS flow outputs.
Extend the already existing ofctl_strip() to also ignore n_packets,
n_bytes, cookie. Use some helper functions from
tests/system-userspace-packet-type-aware.at, which will be removed
by a future commit.
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
(cherry picked from commit 0879a0aa9f332bdc689769a1bc2f156f1c3149a5)
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Commit: 0ef44958bb1c28721dc86aa21757077fc0b24c12
https://github.com/ovn-org/ovn/commit/0ef44958bb1c28721dc86aa21757077fc0b24c12
Author: Dumitru Ceara <dceara at redhat.com>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M tests/ovn.at
Log Message:
-----------
tests: Fix test "ovn -- Superseding ACLs with conjunction".
The test was checking the output of "ovs-ofctl dump-flows" without
taking into account that some fields don't have predictable values,
e.g., hard_age.
Instead, use ofctl_strip_all().
Fixes: 986b3d5e4ad6 ("ofctrl.c: Add a predictable resolution for conflicting flow actions.")
Reported-by: Numan Siddique <numans at ovn.org>
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
(cherry picked from commit 6e3d69e6b5153e26c869a03ec7dd1aaa40488005)
Signed-off-by: Frode Nordahl <frode.nordahl at canonical.com>
Commit: 4b6dc713c5fb5790b135cdd6606b6893117a3fe8
https://github.com/ovn-org/ovn/commit/4b6dc713c5fb5790b135cdd6606b6893117a3fe8
Author: Numan Siddique <numans at ovn.org>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M controller/ofctrl.c
M tests/ovn.at
Log Message:
-----------
ofctrl: Fix the assert seen when flood removing flows.
In one of the scaled deployments, ovn-controller is asserting with the
below stack trace
***
(gdb) bt
0 raise () from /lib64/libc.so.6
1 abort () from /lib64/libc.so.6
2 ovs_abort_valist ("%s: assertion %s failed in %s()") at lib/util.c:419
3 vlog_abort_valist ("%s: assertion %s failed in %s()") at lib/vlog.c:1249
4 vlog_abort ("%s: assertion %s failed in %s()") at lib/vlog.c:1263
5 ovs_assert_failure (where="controller/ofctrl.c:1198",
function="flood_remove_flows_for_sb_uuid",
condition="ovs_list_is_empty(&f->list_node)") at lib/util.c:86
6 flood_remove_flows_for_sb_uuid (sb_uuid=...538,
flood_remove_nodes=...ed0) at controller/ofctrl.c:1205
7 flood_remove_flows_for_sb_uuid (sb_uuid=...898,
flood_remove_nodes=...ed0) at controller/ofctrl.c:1230
8 flood_remove_flows_for_sb_uuid (sb_uuid=...bf0,
flood_remove_nodes=...ed0) at controller/ofctrl.c:1230
9 ofctrl_flood_remove_flows (flood_remove_nodes=...ed0) at controller/ofctrl.c:1250
10 lflow_handle_changed_ref (ref_type=REF_TYPE_PORTGROUP,
ref_name= "5564_pg_64...bac") at controller/lflow.c:612
11 _flow_output_resource_ref_handler (ref_type=REF_TYPE_PORTGROUP)
at controller/ovn-controller.c:2181
12 engine_compute () at lib/inc-proc-eng.c:306
13 engine_run_node (recompute_allowed=true) at lib/inc-proc-eng.c:352
14 engine_run (recompute_allowed=true) at lib/inc-proc-eng.c:377
15 main () at controller/ovn-controller.c:2794
***
This assertion is seen when a port group gets updated and it is referenced by many
logical flows (with conj actions). The function ofctrl_flood_remove_flows(), calls
flood_remove_flows_for_sb_uuid() for each sb uuid in the hmap - flood_remove_nodes
using HMAP_FOR_EACH (flood_remove_nodes). flood_remove_flows_for_sb_uuid() also takes
the hmap 'flood_remove_nodes' as an argument and it inserts few items into it when
it has to call itself recursively. When an item is inserted, its possible that the
hmap may get expanded. And if this happens, the HMAP_FOR_EACH () skips few entries
causing some of the desired flows not getting cleared.
Later when ofctrl_add_or_append_flow() is called, there would be multiple
'struct sb_flow_ref' references for the same desired flow. And this causes the
above assertion later when the same port group gets updated.
This patch fixes this issue by cloning the hmap 'flood_remove_nodes' and using it to
iterate the flood remove nodes. Also a test case is added to cover this scenario.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1928012
Fixes: 580aea72e26f ("ovn-controller: Fix conjunction handling with incremental processing.")
Suggested-by: Ilya Maximetes <i.maximets at ovn.org>
Acked-by: Ilya Maximetes <i.maximets at ovn.org>
Signed-off-by: Numan Siddique <numans at ovn.org>
(cherry-picked from master commit 858d1dd716db1a1e664a7c1737fd34f04fcbda5e)
Commit: 824249dc2b616e62089c9038e18da149a3494bb9
https://github.com/ovn-org/ovn/commit/824249dc2b616e62089c9038e18da149a3494bb9
Author: Dumitru Ceara <dceara at redhat.com>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M controller/ofctrl.c
M tests/ovn.at
Log Message:
-----------
ofctrl: Do not link a desired flow twice.
Depending on the logical flow matches, multiple SB flows can point to
the same desired flow. If it happens that the desired flow conflicts
with a more restrictive (already installed) flow, then we shouldn't try
to add the desired flow multiple times to the list maintained for the
installed flow.
Fixes: 6f0b1e02d9ab ("ofctrl: Incremental processing for flow installation by tracking.")
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
Acked-by: Mark Gray <mark.d.gray at redhat.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
(cherry-picked from master commit 6975c649f932633042ca54df2d8f8f0eb866c344)
Commit: 2283617adc564164b9655187c505c9558e67db33
https://github.com/ovn-org/ovn/commit/2283617adc564164b9655187c505c9558e67db33
Author: Numan Siddique <numans at ovn.org>
Date: 2021-03-02 (Tue, 02 Mar 2021)
Changed paths:
M controller/lflow.c
M tests/ovn.at
Log Message:
-----------
ofctrl: Fix the assert seen when flood removing flows with conj actions.
In one of the scaled deployments, ovn-controller is asserting with the
below stack trace
***
(gdb) bt
0 raise () from /lib64/libc.so.6
1 abort () from /lib64/libc.so.6
2 ovs_abort_valist ("%s: assertion %s failed in %s()") at lib/util.c:419
3 vlog_abort_valist ("%s: assertion %s failed in %s()") at lib/vlog.c:1249
4 vlog_abort ("%s: assertion %s failed in %s()") at lib/vlog.c:1263
5 ovs_assert_failure (where="controller/ofctrl.c:1216",
function="flood_remove_flows_for_sb_uuid",
condition="ovs_list_is_empty(&f->list_node)") at lib/util.c:86
6 flood_remove_flows_for_sb_uuid (sb_uuid=...134,
flood_remove_nodes=...ef0) at controller/ofctrl.c:1216
9 ofctrl_flood_remove_flows (flood_remove_nodes=...ef0) at controller/ofctrl.c:1280
10 lflow_handle_changed_ref (ref_type=REF_TYPE_PORTGROUP,
ref_name= "5564_pg_14...aac") at controller/lflow.c:522
11 _flow_output_resource_ref_handler (ref_type=REF_TYPE_PORTGROUP)
at controller/ovn-controller.c:2220
12 engine_compute () at lib/inc-proc-eng.c:306
13 engine_run_node (recompute_allowed=true) at lib/inc-proc-eng.c:352
14 engine_run (recompute_allowed=true) at lib/inc-proc-eng.c:377
15 main () at controller/ovn-controller.c:2887
***
The reason for this assert is different from the assertion bug fixed in
the commit 858d1dd716("ofctrl: Fix the assert seen when flood removing
flows.")
The above assertion is seen if lflow.c calls ofctrl_add_or_append_flow()
twice for the same flow uuid 'S'. When this function is called for
the first time, a new desired flow 'f' is created and 'f->references'
will have [(S, f)]. When it is called for the 2nd time, the list
'f->references' is updated as [(S, f), (S,f)]. Later when
flood_remove_flows_for_sb_uuid() is called for 'S', the above assertion
is seen.
This scenarion can happen when ovn-controller receives an update message
from SB ovsdb-server in which a new logical flow 'F' belonging to a
datapath 'D' is added which would result in conjunction flows. If this
datapath 'D' is added to 'local_datapaths' in the same loop, then
logical flow 'F' is first processed in lflow_add_flows_for_datapath()
and the second time in lflow_handle_changed_flows().
This issue can be fixed in 2 ways
1. Flood remove all the tracked flows in lflow_handle_changed_flows()
first and then process the modified or newly added logical flows.
2. In the function ofctrl_add_or_append_flow(), check if there is
already a reference for the flow uuid 'S' in the existing desired
flows and only append the actions if it doesn't exist.
This patch has taken the approach (1) to ensure correctness of flows.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1929978
Signed-off-by: Numan Siddique <numans at ovn.org>
Acked-by: Dumitru Ceara <dceara at redhat.com>
Signed-off-by: Numan Siddique <numans at ovn.org>
(cherry-picked from master commit c6c61b4e3462fb5201a61a226c2acaf6f4caf917)
Compare: https://github.com/ovn-org/ovn/compare/aa8ef5588c11...2283617adc56
More information about the git
mailing list