Merge ~fnordahl/ubuntu/+source/ovn:ubuntu/focal into ~ubuntu-server-dev/ubuntu/+source/ovn:ubuntu/focal
- Git
- lp:~fnordahl/ubuntu/+source/ovn
- ubuntu/focal
- Merge into ubuntu/focal
Proposed by
Frode Nordahl
Status: | Merged | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Merged at revision: | bd1c02325d9bfc0e746f65e573d98f3415cbb5b2 | ||||||||||||
Proposed branch: | ~fnordahl/ubuntu/+source/ovn:ubuntu/focal | ||||||||||||
Merge into: | ~ubuntu-server-dev/ubuntu/+source/ovn:ubuntu/focal | ||||||||||||
Diff against target: |
4592 lines (+4464/-0) 21 files modified
debian/changelog (+12/-0) debian/patches/ovn-ctl-cluster-db-upgrades.patch (+63/-0) debian/patches/ovn-northd-revert-manage-arp-process-locally-dvr.patch (+198/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-01.patch (+66/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-02.patch (+892/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-03.patch (+719/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-04.patch (+157/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-05.patch (+484/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-06.patch (+112/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-07.patch (+236/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-08.patch (+93/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-09.patch (+220/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-10.patch (+80/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-11.patch (+218/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-12.patch (+43/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-13.patch (+419/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-14.patch (+126/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-15.patch (+66/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-16.patch (+135/-0) debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-17.patch (+106/-0) debian/patches/series (+19/-0) |
||||||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Ubuntu Server Developers | Pending | ||
Review via email: mp+394584@code.launchpad.net |
Commit message
Description of the change
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/debian/changelog b/debian/changelog |
2 | index 647f58a..dc6ede0 100644 |
3 | --- a/debian/changelog |
4 | +++ b/debian/changelog |
5 | @@ -1,3 +1,15 @@ |
6 | +ovn (20.03.1-0ubuntu1.2) focal; urgency=medium |
7 | + |
8 | + * d/p/ovn-northd-revert-manage-arp-process-locally-dvr.patch: Cherry pick |
9 | + fix for incorrect ARP processing with DVR enabled (LP: #1905933). |
10 | + * d/p/ovn-ctl-cluster-db-upgrades.patch: Cherry pick fix for upgrading |
11 | + database schema of clustered databases on package upgrade (LP: #1907081) |
12 | + * d/p/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-*: Cherry |
13 | + pick fixes for predictable resolution for conflicting flow actions. |
14 | + (LP: #1906922) |
15 | + |
16 | + -- Frode Nordahl <frode.nordahl@canonical.com> Fri, 27 Nov 2020 09:16:07 +0000 |
17 | + |
18 | ovn (20.03.1-0ubuntu1.1) focal; urgency=medium |
19 | |
20 | * d/p/ovn-controller-ofctrl-probe-interval.patch: Cherry pick |
21 | diff --git a/debian/patches/ovn-ctl-cluster-db-upgrades.patch b/debian/patches/ovn-ctl-cluster-db-upgrades.patch |
22 | new file mode 100644 |
23 | index 0000000..8ca5ac8 |
24 | --- /dev/null |
25 | +++ b/debian/patches/ovn-ctl-cluster-db-upgrades.patch |
26 | @@ -0,0 +1,63 @@ |
27 | +Description: Cherry-pick fix applied to master |
28 | +Origin: https://github.com/ovn-org/ovn/commit/67e2f386cc838d0b0f9b4b5da7fe611e1113b70c |
29 | +Applied-Upstream: commit: 67e2f386cc838d0b0f9b4b5da7fe611e1113b70c |
30 | + |
31 | +From 67e2f386cc838d0b0f9b4b5da7fe611e1113b70c Mon Sep 17 00:00:00 2001 |
32 | +From: Numan Siddique <numans@ovn.org> |
33 | +Date: Wed, 9 Sep 2020 12:49:39 +0530 |
34 | +Subject: [PATCH] ovn-ctl: Handle cluster db upgrades for run_(nb/sb)_ovsdb |
35 | + |
36 | +when ovn-ctl run_(nb_sb)_ovsdb is called, the ovsdb-server is started without |
37 | +passing --detach and --monoitor and the process is exec'd. |
38 | + |
39 | +For cluster mode, upgrade_cluster is never called and hence the dbs are not upraded |
40 | +to new schema. CMS has to handle the db upgrade separately. |
41 | + |
42 | +This patch handles the db upgrade by starting ovsdb-server in background and then |
43 | +waits on ovsdb-server to complete. |
44 | + |
45 | +Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1868392 |
46 | +Acked-by: Mark Michelson <mmichels@redhat.com> |
47 | +Signed-off-by: Numan Siddique <numans@ovn.org> |
48 | +--- |
49 | + utilities/ovn-ctl | 20 +++++++++++++++++++- |
50 | + 1 file changed, 19 insertions(+), 1 deletion(-) |
51 | + |
52 | +diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl |
53 | +index c7cb42bc1..a763f7750 100755 |
54 | +--- a/utilities/ovn-ctl |
55 | ++++ b/utilities/ovn-ctl |
56 | +@@ -284,7 +284,21 @@ $cluster_remote_port |
57 | + set "$@" --sync-from=`cat $active_conf_file` |
58 | + fi |
59 | + |
60 | +- "$@" "$file" |
61 | ++ local run_ovsdb_in_bg="no" |
62 | ++ local process_id= |
63 | ++ if test X$detach = Xno && test $mode = cluster && test -z "$cluster_remote_addr" ; then |
64 | ++ # When detach is no (for run_nb_ovsdb/run_sb_ovsdb commands) |
65 | ++ # we want to run ovsdb-server in background rather than running it in |
66 | ++ # foreground so that the OVN dbs are upgraded for the cluster mode. |
67 | ++ # Otherwise, CMS has to take the responsibility of upgrading the dbs. |
68 | ++ # Note: We run only the ovsdb-server in backgroud which created the |
69 | ++ # cluster (i.e cluster_remote_addr is not set.). |
70 | ++ run_ovsdb_in_bg="yes" |
71 | ++ "$@" $file & |
72 | ++ process_id=$! |
73 | ++ else |
74 | ++ "$@" "$file" |
75 | ++ fi |
76 | + |
77 | + # Initialize the database if it's running standalone, |
78 | + # active-passive, or is the first server in a cluster. |
79 | +@@ -295,6 +309,10 @@ $cluster_remote_port |
80 | + if test $mode = cluster; then |
81 | + upgrade_cluster "$schema" "unix:$sock" |
82 | + fi |
83 | ++ |
84 | ++ if test $run_ovsdb_in_bg = yes; then |
85 | ++ wait $process_id |
86 | ++ fi |
87 | + } |
88 | + |
89 | + start_nb_ovsdb() { |
90 | diff --git a/debian/patches/ovn-northd-revert-manage-arp-process-locally-dvr.patch b/debian/patches/ovn-northd-revert-manage-arp-process-locally-dvr.patch |
91 | new file mode 100644 |
92 | index 0000000..796db51 |
93 | --- /dev/null |
94 | +++ b/debian/patches/ovn-northd-revert-manage-arp-process-locally-dvr.patch |
95 | @@ -0,0 +1,198 @@ |
96 | +Description: Cherry-pick fix applied to master, branch-20.09 and branch-20.06 but not branch-20.03 |
97 | +Origin: https://github.com/ovn-org/ovn/commit/d9ed450713eda62af1bec5009694b2d206c9f435 |
98 | +Applied-Upstream: commit: d9ed450713eda62af1bec5009694b2d206c9f435 |
99 | + |
100 | +From d9ed450713eda62af1bec5009694b2d206c9f435 Mon Sep 17 00:00:00 2001 |
101 | +From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> |
102 | +Date: Mon, 25 May 2020 23:55:06 +0200 |
103 | +Subject: [PATCH] Revert "Manage ARP process locally in a DVR scenario" |
104 | + |
105 | +This reverts commit c0bf32d72f8b893bbe3cb64912b0fd259d71555f. |
106 | + |
107 | +Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> |
108 | +Signed-off-by: Han Zhou <hzhou@ovn.org> |
109 | +--- |
110 | + northd/ovn-northd.8.xml | 37 ++-------------------------- |
111 | + northd/ovn-northd.c | 53 +---------------------------------------- |
112 | + tests/ovn.at | 14 ----------- |
113 | + 3 files changed, 3 insertions(+), 101 deletions(-) |
114 | + |
115 | + |
116 | +diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml |
117 | +index b611f985a..944bb41ec 100644 |
118 | +--- a/northd/ovn-northd.8.xml |
119 | ++++ b/northd/ovn-northd.8.xml |
120 | +@@ -2475,46 +2475,13 @@ output; |
121 | + </p> |
122 | + </li> |
123 | + |
124 | +- <li> |
125 | +- <p> |
126 | +- For distributed logical routers where one of the logical router ports |
127 | +- specifies a <code>redirect-chassis</code>, a priority-400 logical |
128 | +- flow for each <code>dnat_and_snat</code> NAT rules configured. |
129 | +- These flows will allow to properly forward traffic to the external |
130 | +- connections if available and avoid sending it through the tunnel. |
131 | +- Assuming the following NAT rule has been configured: |
132 | +- </p> |
133 | +- |
134 | +- <pre> |
135 | +-external_ip = <var>A</var>; |
136 | +-external_mac = <var>B</var>; |
137 | +-logical_ip = <var>C</var>; |
138 | +- </pre> |
139 | +- |
140 | +- <p> |
141 | +- the following action will be applied: |
142 | +- </p> |
143 | +- |
144 | +- <pre> |
145 | +-ip.ttl--; |
146 | +-reg0 = <var>ip.dst</var>; |
147 | +-reg1 = <var>A</var>; |
148 | +-eth.src = <var>B</var>; |
149 | +-outport = <var>router-port</var>; |
150 | +-next; |
151 | +- </pre> |
152 | +- |
153 | +- </li> |
154 | +- |
155 | + <li> |
156 | + <p> |
157 | + IPv4 routing table. For each route to IPv4 network <var>N</var> with |
158 | + netmask <var>M</var>, on router port <var>P</var> with IP address |
159 | + <var>A</var> and Ethernet |
160 | + address <var>E</var>, a logical flow with match <code>ip4.dst == |
161 | +- <var>N</var>/<var>M</var></code>, whose priority is <code>400</code> |
162 | +- + the number of 1-bits in <var>M</var> if the router port is not a |
163 | +- distributed gateway port, else the priority is the number of |
164 | ++ <var>N</var>/<var>M</var></code>, whose priority is the number of |
165 | + 1-bits in <var>M</var>, has the following actions: |
166 | + </p> |
167 | + |
168 | +@@ -2913,7 +2880,7 @@ icmp4 { |
169 | + |
170 | + <li> |
171 | + For each NAT rule in the OVN Northbound database that can |
172 | +- be handled in a distributed manner, a priority-200 logical |
173 | ++ be handled in a distributed manner, a priority-100 logical |
174 | + flow with match <code>ip4.src == <var>B</var> && |
175 | + outport == <var>GW</var></code>, where <var>GW</var> is |
176 | + the logical router distributed gateway port, with actions |
177 | +diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c |
178 | +index 26a6441ce..f8e743503 100644 |
179 | +--- a/northd/ovn-northd.c |
180 | ++++ b/northd/ovn-northd.c |
181 | +@@ -7031,8 +7031,6 @@ build_routing_policy_flow(struct hmap *lflows, struct ovn_datapath *od, |
182 | + ds_destroy(&actions); |
183 | + } |
184 | + |
185 | +-/* default logical flow prioriry for distributed routes */ |
186 | +-#define DROUTE_PRIO 400 |
187 | + struct parsed_route { |
188 | + struct ovs_list list_node; |
189 | + struct v46_ip prefix; |
190 | +@@ -7420,40 +7418,6 @@ build_ecmp_route_flow(struct hmap *lflows, struct ovn_datapath *od, |
191 | + ds_destroy(&actions); |
192 | + } |
193 | + |
194 | +-static void |
195 | +-add_distributed_routes(struct hmap *lflows, struct ovn_datapath *od) |
196 | +-{ |
197 | +- struct ds actions = DS_EMPTY_INITIALIZER; |
198 | +- struct ds match = DS_EMPTY_INITIALIZER; |
199 | +- |
200 | +- for (size_t i = 0; i < od->nbr->n_nat; i++) { |
201 | +- const struct nbrec_nat *nat = od->nbr->nat[i]; |
202 | +- |
203 | +- if (strcmp(nat->type, "dnat_and_snat") || |
204 | +- !nat->external_mac) { |
205 | +- continue; |
206 | +- } |
207 | +- |
208 | +- bool is_ipv4 = strchr(nat->logical_ip, '.') ? true : false; |
209 | +- ds_put_format(&match, "ip%s.src == %s && is_chassis_resident(\"%s\")", |
210 | +- is_ipv4 ? "4" : "6", nat->logical_ip, |
211 | +- nat->logical_port); |
212 | +- char *prefix = is_ipv4 ? "" : "xx"; |
213 | +- ds_put_format(&actions, "outport = %s; eth.src = %s; " |
214 | +- "%sreg0 = ip%s.dst; %sreg1 = %s; next;", |
215 | +- od->l3dgw_port->json_key, nat->external_mac, |
216 | +- prefix, is_ipv4 ? "4" : "6", |
217 | +- prefix, nat->external_ip); |
218 | +- ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, DROUTE_PRIO, |
219 | +- ds_cstr(&match), ds_cstr(&actions)); |
220 | +- ds_clear(&match); |
221 | +- ds_clear(&actions); |
222 | +- } |
223 | +- |
224 | +- ds_destroy(&actions); |
225 | +- ds_destroy(&match); |
226 | +-} |
227 | +- |
228 | + static void |
229 | + add_route(struct hmap *lflows, const struct ovn_port *op, |
230 | + const char *lrp_addr_s, const char *network_s, int plen, |
231 | +@@ -7475,12 +7439,6 @@ add_route(struct hmap *lflows, const struct ovn_port *op, |
232 | + } |
233 | + build_route_match(op_inport, network_s, plen, is_src_route, is_ipv4, |
234 | + &match, &priority); |
235 | +- /* traffic for internal IPs of logical switch ports must be sent to |
236 | +- * the gw controller through the overlay tunnels |
237 | +- */ |
238 | +- if (op->nbrp && !op->nbrp->n_gateway_chassis) { |
239 | +- priority += DROUTE_PRIO; |
240 | +- } |
241 | + |
242 | + struct ds actions = DS_EMPTY_INITIALIZER; |
243 | + ds_put_format(&actions, "ip.ttl--; "REG_ECMP_GROUP_ID" = 0; %sreg0 = ", |
244 | +@@ -9090,7 +9048,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, |
245 | + nat->logical_ip, |
246 | + od->l3dgw_port->json_key); |
247 | + ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_GW_REDIRECT, |
248 | +- 200, ds_cstr(&match), "next;", |
249 | ++ 100, ds_cstr(&match), "next;", |
250 | + &nat->header_); |
251 | + } |
252 | + |
253 | +@@ -9373,15 +9331,6 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, |
254 | + ovn_lflow_add(lflows, od, S_ROUTER_IN_ND_RA_RESPONSE, 0, "1", "next;"); |
255 | + } |
256 | + |
257 | +- /* Logical router ingress table IP_ROUTING - IP routing for distributed |
258 | +- * logical router |
259 | +- */ |
260 | +- HMAP_FOR_EACH (od, key_node, datapaths) { |
261 | +- if (od->nbr && od->l3dgw_port) { |
262 | +- add_distributed_routes(lflows, od); |
263 | +- } |
264 | +- } |
265 | +- |
266 | + /* Logical router ingress table IP_ROUTING & IP_ROUTING_ECMP: IP Routing. |
267 | + * |
268 | + * A packet that arrives at this table is an IP packet that should be |
269 | +diff --git a/tests/ovn.at b/tests/ovn.at |
270 | +index 423d8a507..341e9c2ab 100644 |
271 | +--- a/tests/ovn.at |
272 | ++++ b/tests/ovn.at |
273 | +@@ -9551,20 +9551,6 @@ AT_CHECK([as hv3 ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings=p |
274 | + OVS_WAIT_UNTIL([test 1 = `as hv3 ovs-vsctl show | \ |
275 | + grep "Port patch-br-int-to-ln_port" | wc -l`]) |
276 | + |
277 | +-AT_CHECK([test 1 = `ovn-sbctl dump-flows lr0 | grep lr_in_ip_routing | \ |
278 | +-grep "ip4.src == 10.0.0.3 && is_chassis_resident(\"foo1\")" -c`]) |
279 | +-AT_CHECK([test 1 = `ovn-sbctl dump-flows lr0 | grep lr_in_ip_routing | \ |
280 | +-grep "ip4.src == 10.0.0.4 && is_chassis_resident(\"foo2\")" -c`]) |
281 | +- |
282 | +-key=`ovn-sbctl --bare --columns tunnel_key list datapath_Binding lr0` |
283 | +-# Check that the OVS flows appear for the dnat_and_snat entries in |
284 | +-# lr_in_ip_routing table. |
285 | +-OVS_WAIT_UNTIL([test 1 = `as hv3 ovs-ofctl dump-flows br-int table=17 | \ |
286 | +-grep "priority=400,ip,metadata=0x$key,nw_src=10.0.0.3" -c`]) |
287 | +- |
288 | +-OVS_WAIT_UNTIL([test 1 = `as hv3 ovs-ofctl dump-flows br-int table=17 | \ |
289 | +-grep "priority=400,ip,metadata=0x$key,nw_src=10.0.0.4" -c`]) |
290 | +- |
291 | + # Re-add nat-addresses option |
292 | + ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0 nat-addresses="router" |
293 | + |
294 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-01.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-01.patch |
295 | new file mode 100644 |
296 | index 0000000..e4b3831 |
297 | --- /dev/null |
298 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-01.patch |
299 | @@ -0,0 +1,66 @@ |
300 | +Description: Cherry-pick fix applied to master and branch-20.12 |
301 | +Origin: https://github.com/ovn-org/ovn/commit/69b56114bd253531d2dab7336a99d1c7563eb7be |
302 | +Applied-Upstream: commit: 69b56114bd253531d2dab7336a99d1c7563eb7be |
303 | + |
304 | +From 6d5adcd482b8f525e383c13e714826165d86d33f Mon Sep 17 00:00:00 2001 |
305 | +From: Han Zhou <hzhou@ovn.org> |
306 | +Date: Thu, 13 Aug 2020 12:06:09 -0700 |
307 | +Subject: [PATCH 01/15] ofctrl: change ofctrl_dup_flow to module internal |
308 | + function |
309 | + |
310 | +Acked-by: Mark Michelson <mmichels@redhat.com> |
311 | +Signed-off-by: Han Zhou <hzhou@ovn.org> |
312 | +--- |
313 | + controller/ofctrl.c | 8 +++++--- |
314 | + controller/ofctrl.h | 2 -- |
315 | + 2 files changed, 5 insertions(+), 5 deletions(-) |
316 | + |
317 | +diff --git a/controller/ofctrl.c b/controller/ofctrl.c |
318 | +index 966620fe0..2dd6445ae 100644 |
319 | +--- a/controller/ofctrl.c |
320 | ++++ b/controller/ofctrl.c |
321 | +@@ -169,6 +169,8 @@ static struct ofpbuf *encode_meter_mod(const struct ofputil_meter_mod *); |
322 | + |
323 | + static void ovn_installed_flow_table_clear(void); |
324 | + static void ovn_installed_flow_table_destroy(void); |
325 | ++static struct ovn_flow *ovn_flow_dup(struct ovn_flow *source); |
326 | ++ |
327 | + |
328 | + static void ofctrl_recv(const struct ofp_header *, enum ofptype); |
329 | + |
330 | +@@ -787,8 +789,8 @@ ovn_flow_match_hash(const struct ovn_flow *f) |
331 | + } |
332 | + |
333 | + /* Duplicate an ovn_flow structure. */ |
334 | +-struct ovn_flow * |
335 | +-ofctrl_dup_flow(struct ovn_flow *src) |
336 | ++static struct ovn_flow * |
337 | ++ovn_flow_dup(struct ovn_flow *src) |
338 | + { |
339 | + struct ovn_flow *dst = xmalloc(sizeof *dst); |
340 | + dst->table_id = src->table_id; |
341 | +@@ -1229,7 +1231,7 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, |
342 | + ovn_flow_log(d, "adding installed"); |
343 | + |
344 | + /* Copy 'd' from 'flow_table' to installed_flows. */ |
345 | +- struct ovn_flow *new_node = ofctrl_dup_flow(d); |
346 | ++ struct ovn_flow *new_node = ovn_flow_dup(d); |
347 | + hmap_insert(&installed_flows, &new_node->match_hmap_node, |
348 | + new_node->match_hmap_node.hash); |
349 | + } |
350 | +diff --git a/controller/ofctrl.h b/controller/ofctrl.h |
351 | +index 21d2ce648..37f06db86 100644 |
352 | +--- a/controller/ofctrl.h |
353 | ++++ b/controller/ofctrl.h |
354 | +@@ -56,8 +56,6 @@ void ofctrl_wait(void); |
355 | + void ofctrl_destroy(void); |
356 | + int64_t ofctrl_get_cur_cfg(void); |
357 | + |
358 | +-struct ovn_flow *ofctrl_dup_flow(struct ovn_flow *source); |
359 | +- |
360 | + void ofctrl_ct_flush_zone(uint16_t zone_id); |
361 | + |
362 | + char *ofctrl_inject_pkt(const struct ovsrec_bridge *br_int, |
363 | +-- |
364 | +2.29.2 |
365 | + |
366 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-02.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-02.patch |
367 | new file mode 100644 |
368 | index 0000000..f64a23a |
369 | --- /dev/null |
370 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-02.patch |
371 | @@ -0,0 +1,892 @@ |
372 | +Description: Cherry-pick fix applied to master and branch-20.12 |
373 | +Origin: https://github.com/ovn-org/ovn/commit/580aea72e26f53deb72a0d0f5516327a4af7c8fe |
374 | +Applied-Upstream: commit: 580aea72e26f53deb72a0d0f5516327a4af7c8fe |
375 | + |
376 | +From 273c8cad7bce2a2ff8569e9128f266bc0000221b Mon Sep 17 00:00:00 2001 |
377 | +From: Han Zhou <hzhou@ovn.org> |
378 | +Date: Thu, 20 Aug 2020 22:55:39 -0700 |
379 | +Subject: [PATCH 02/15] ovn-controller: Fix conjunction handling with |
380 | + incremental processing. |
381 | + |
382 | +When translating lflows to OVS flows, different lflows can refer to same OVS |
383 | +flow as a result of calling ofctrl_add_or_append_flow(), particularly for |
384 | +conjunction combinding. However, the implementation doesn't work with |
385 | +incremental processing, because when any of the lflows are removed, we rely on |
386 | +the lflow's uuid to remove the OVS flow in the desired flow table. Currently |
387 | +only one single lflow uuid is maintained in the desired flow, so removing one |
388 | +of the lflows that references to the same desired flow resulted in wrong |
389 | +behavior: either removing flows that are used by other lflows, or the existing |
390 | +flows are not updated (part of the conjunction actions should be removed from |
391 | +the flow). |
392 | + |
393 | +To solve the problem, this patch maintains the cross reference (M:N) between |
394 | +lflows (and other sb objects) and desired flows, and handles the flow removal |
395 | +and updates with a flood-removal and re-add approach. |
396 | + |
397 | +Fixes: e659bab31a9 ("Combine conjunctions with identical matches into one flow.") |
398 | +Cc: Mark Michelson <mmichels@redhat.com> |
399 | +Acked-by: Mark Michelson <mmichels@redhat.com> |
400 | +Signed-off-by: Han Zhou <hzhou@ovn.org> |
401 | +--- |
402 | + controller/lflow.c | 103 ++++++++----- |
403 | + controller/ofctrl.c | 351 +++++++++++++++++++++++++++++++++++--------- |
404 | + controller/ofctrl.h | 31 +++- |
405 | + tests/ovn.at | 91 ++++++++++++ |
406 | + 4 files changed, 470 insertions(+), 106 deletions(-) |
407 | + |
408 | +diff --git a/controller/lflow.c b/controller/lflow.c |
409 | +index 01214a3a6..b4016bd08 100644 |
410 | +--- a/controller/lflow.c |
411 | ++++ b/controller/lflow.c |
412 | +@@ -328,41 +328,56 @@ lflow_handle_changed_flows(struct lflow_ctx_in *l_ctx_in, |
413 | + struct hmap nd_ra_opts = HMAP_INITIALIZER(&nd_ra_opts); |
414 | + nd_ra_opts_init(&nd_ra_opts); |
415 | + |
416 | +- /* Handle removed flows first, and then other flows, so that when |
417 | +- * the flows being added and removed have same match conditions |
418 | +- * can be processed in the proper order */ |
419 | ++ struct controller_event_options controller_event_opts; |
420 | ++ controller_event_opts_init(&controller_event_opts); |
421 | ++ |
422 | ++ /* Handle flow removing first (for deleted or updated lflows), and then |
423 | ++ * handle reprocessing or adding flows, so that when the flows being |
424 | ++ * removed and added with same match conditions can be processed in the |
425 | ++ * proper order */ |
426 | ++ |
427 | ++ struct hmap flood_remove_nodes = HMAP_INITIALIZER(&flood_remove_nodes); |
428 | ++ struct ofctrl_flood_remove_node *ofrn, *next; |
429 | + SBREC_LOGICAL_FLOW_TABLE_FOR_EACH_TRACKED (lflow, |
430 | + l_ctx_in->logical_flow_table) { |
431 | +- /* Remove any flows that should be removed. */ |
432 | +- if (sbrec_logical_flow_is_deleted(lflow)) { |
433 | +- VLOG_DBG("handle deleted lflow "UUID_FMT, |
434 | ++ if (!sbrec_logical_flow_is_new(lflow)) { |
435 | ++ VLOG_DBG("delete lflow "UUID_FMT, |
436 | + UUID_ARGS(&lflow->header_.uuid)); |
437 | +- ofctrl_remove_flows(l_ctx_out->flow_table, &lflow->header_.uuid); |
438 | +- /* Delete entries from lflow resource reference. */ |
439 | +- lflow_resource_destroy_lflow(l_ctx_out->lfrr, |
440 | +- &lflow->header_.uuid); |
441 | ++ ofrn = xmalloc(sizeof *ofrn); |
442 | ++ ofrn->sb_uuid = lflow->header_.uuid; |
443 | ++ hmap_insert(&flood_remove_nodes, &ofrn->hmap_node, |
444 | ++ uuid_hash(&ofrn->sb_uuid)); |
445 | + } |
446 | + } |
447 | ++ ofctrl_flood_remove_flows(l_ctx_out->flow_table, &flood_remove_nodes); |
448 | ++ HMAP_FOR_EACH (ofrn, hmap_node, &flood_remove_nodes) { |
449 | ++ /* Delete entries from lflow resource reference. */ |
450 | ++ lflow_resource_destroy_lflow(l_ctx_out->lfrr, &ofrn->sb_uuid); |
451 | ++ /* Reprocessing the lflow if the sb record is not deleted. */ |
452 | ++ lflow = sbrec_logical_flow_table_get_for_uuid( |
453 | ++ l_ctx_in->logical_flow_table, &ofrn->sb_uuid); |
454 | ++ if (lflow) { |
455 | ++ VLOG_DBG("re-add lflow "UUID_FMT, |
456 | ++ UUID_ARGS(&lflow->header_.uuid)); |
457 | ++ if (!consider_logical_flow(lflow, &dhcp_opts, &dhcpv6_opts, |
458 | ++ &nd_ra_opts, &controller_event_opts, |
459 | ++ l_ctx_in, l_ctx_out)) { |
460 | ++ ret = false; |
461 | ++ break; |
462 | ++ } |
463 | ++ } |
464 | ++ } |
465 | ++ HMAP_FOR_EACH_SAFE (ofrn, next, hmap_node, &flood_remove_nodes) { |
466 | ++ hmap_remove(&flood_remove_nodes, &ofrn->hmap_node); |
467 | ++ free(ofrn); |
468 | ++ } |
469 | ++ hmap_destroy(&flood_remove_nodes); |
470 | + |
471 | +- struct controller_event_options controller_event_opts; |
472 | +- controller_event_opts_init(&controller_event_opts); |
473 | +- |
474 | ++ /* Now handle new lflows only. */ |
475 | + SBREC_LOGICAL_FLOW_TABLE_FOR_EACH_TRACKED (lflow, |
476 | + l_ctx_in->logical_flow_table) { |
477 | +- if (!sbrec_logical_flow_is_deleted(lflow)) { |
478 | +- /* Now, add/modify existing flows. If the logical |
479 | +- * flow is a modification, just remove the flows |
480 | +- * for this row, and then add new flows. */ |
481 | +- if (!sbrec_logical_flow_is_new(lflow)) { |
482 | +- VLOG_DBG("handle updated lflow "UUID_FMT, |
483 | +- UUID_ARGS(&lflow->header_.uuid)); |
484 | +- ofctrl_remove_flows(l_ctx_out->flow_table, |
485 | +- &lflow->header_.uuid); |
486 | +- /* Delete entries from lflow resource reference. */ |
487 | +- lflow_resource_destroy_lflow(l_ctx_out->lfrr, |
488 | +- &lflow->header_.uuid); |
489 | +- } |
490 | +- VLOG_DBG("handle new lflow "UUID_FMT, |
491 | ++ if (sbrec_logical_flow_is_new(lflow)) { |
492 | ++ VLOG_DBG("add lflow "UUID_FMT, |
493 | + UUID_ARGS(&lflow->header_.uuid)); |
494 | + if (!consider_logical_flow(lflow, &dhcp_opts, &dhcpv6_opts, |
495 | + &nd_ra_opts, &controller_event_opts, |
496 | +@@ -406,7 +421,6 @@ lflow_handle_changed_ref(enum ref_type ref_type, const char *ref_name, |
497 | + * when reparsing the lflows. */ |
498 | + LIST_FOR_EACH (lrln, ref_list, &rlfn->ref_lflow_head) { |
499 | + ovs_list_remove(&lrln->lflow_list); |
500 | +- lflow_resource_destroy_lflow(l_ctx_out->lfrr, &lrln->lflow_uuid); |
501 | + } |
502 | + |
503 | + struct hmap dhcp_opts = HMAP_INITIALIZER(&dhcp_opts); |
504 | +@@ -432,22 +446,32 @@ lflow_handle_changed_ref(enum ref_type ref_type, const char *ref_name, |
505 | + controller_event_opts_init(&controller_event_opts); |
506 | + |
507 | + /* Re-parse the related lflows. */ |
508 | ++ /* Firstly, flood remove the flows from desired flow table. */ |
509 | ++ struct hmap flood_remove_nodes = HMAP_INITIALIZER(&flood_remove_nodes); |
510 | ++ struct ofctrl_flood_remove_node *ofrn, *ofrn_next; |
511 | + LIST_FOR_EACH (lrln, ref_list, &rlfn->ref_lflow_head) { |
512 | ++ VLOG_DBG("Reprocess lflow "UUID_FMT" for resource type: %d," |
513 | ++ " name: %s.", |
514 | ++ UUID_ARGS(&lrln->lflow_uuid), |
515 | ++ ref_type, ref_name); |
516 | ++ ofctrl_flood_remove_add_node(&flood_remove_nodes, &lrln->lflow_uuid); |
517 | ++ } |
518 | ++ ofctrl_flood_remove_flows(l_ctx_out->flow_table, &flood_remove_nodes); |
519 | ++ |
520 | ++ /* Secondly, for each lflow that is actually removed, reprocessing it. */ |
521 | ++ HMAP_FOR_EACH (ofrn, hmap_node, &flood_remove_nodes) { |
522 | ++ lflow_resource_destroy_lflow(l_ctx_out->lfrr, &ofrn->sb_uuid); |
523 | ++ |
524 | + const struct sbrec_logical_flow *lflow = |
525 | + sbrec_logical_flow_table_get_for_uuid(l_ctx_in->logical_flow_table, |
526 | +- &lrln->lflow_uuid); |
527 | ++ &ofrn->sb_uuid); |
528 | + if (!lflow) { |
529 | +- VLOG_DBG("Reprocess lflow "UUID_FMT" for resource type: %d," |
530 | +- " name: %s - not found.", |
531 | +- UUID_ARGS(&lrln->lflow_uuid), |
532 | ++ VLOG_DBG("lflow "UUID_FMT" not found while reprocessing for" |
533 | ++ " resource type: %d, name: %s.", |
534 | ++ UUID_ARGS(&ofrn->sb_uuid), |
535 | + ref_type, ref_name); |
536 | + continue; |
537 | + } |
538 | +- VLOG_DBG("Reprocess lflow "UUID_FMT" for resource type: %d," |
539 | +- " name: %s.", |
540 | +- UUID_ARGS(&lrln->lflow_uuid), |
541 | +- ref_type, ref_name); |
542 | +- ofctrl_remove_flows(l_ctx_out->flow_table, &lrln->lflow_uuid); |
543 | + |
544 | + if (!consider_logical_flow(lflow, &dhcp_opts, &dhcpv6_opts, |
545 | + &nd_ra_opts, &controller_event_opts, |
546 | +@@ -457,6 +481,11 @@ lflow_handle_changed_ref(enum ref_type ref_type, const char *ref_name, |
547 | + } |
548 | + *changed = true; |
549 | + } |
550 | ++ HMAP_FOR_EACH_SAFE (ofrn, ofrn_next, hmap_node, &flood_remove_nodes) { |
551 | ++ hmap_remove(&flood_remove_nodes, &ofrn->hmap_node); |
552 | ++ free(ofrn); |
553 | ++ } |
554 | ++ hmap_destroy(&flood_remove_nodes); |
555 | + |
556 | + LIST_FOR_EACH_SAFE (lrln, next, ref_list, &rlfn->ref_lflow_head) { |
557 | + ovs_list_remove(&lrln->ref_list); |
558 | +diff --git a/controller/ofctrl.c b/controller/ofctrl.c |
559 | +index 2dd6445ae..609d5db5f 100644 |
560 | +--- a/controller/ofctrl.c |
561 | ++++ b/controller/ofctrl.c |
562 | +@@ -51,11 +51,45 @@ |
563 | + |
564 | + VLOG_DEFINE_THIS_MODULE(ofctrl); |
565 | + |
566 | +-/* An OpenFlow flow. */ |
567 | ++/* An OpenFlow flow. |
568 | ++ * |
569 | ++ * Links are maintained between desired flows and SB data. The relationship |
570 | ++ * is M to N. The struct sb_flow_ref is used to link a pair of desired flow |
571 | ++ * and SB UUID. The below diagram depicts the data structure. |
572 | ++ * |
573 | ++ * SB UUIDs |
574 | ++ * +-----+-----+-----+-----+-----+-----+-----+ |
575 | ++ * | | | | | | | | |
576 | ++ * +--+--+--+--+--+--+-----+--+--+--+--+--+--+ |
577 | ++ * | | | | | | |
578 | ++ * Desired Flows | | | | | | |
579 | ++ * +----+ +-+-+ | +-+-+ | +-+-+ | |
580 | ++ * | +-------+ +-------+ +-------------+ | | |
581 | ++ * +----+ +---+ | +-+-+ | +---+ | |
582 | ++ * | | | | | | |
583 | ++ * +----+ | | +-+-+ | |
584 | ++ * | +-------------------------------+ | | |
585 | ++ * +----+ +---+ | +---+ | |
586 | ++ * | +-------------+ | | | |
587 | ++ * +----+ +---+ | | |
588 | ++ * | | | | |
589 | ++ * +----+ +-+-+ +-+-+ |
590 | ++ * | +-------------------+ +-------------------+ | |
591 | ++ * +----+ +---+ +---+ |
592 | ++ * | | |
593 | ++ * +----+ |
594 | ++ * |
595 | ++ * The links are updated whenever there is a change in desired flows, which is |
596 | ++ * usually triggered by a SB data change in I-P engine. |
597 | ++ */ |
598 | + struct ovn_flow { |
599 | + struct hmap_node match_hmap_node; /* For match based hashing. */ |
600 | +- struct hindex_node uuid_hindex_node; /* For uuid based hashing. */ |
601 | + struct ovs_list list_node; /* For handling lists of flows. */ |
602 | ++ struct ovs_list references; /* A list of struct sb_flow_ref nodes, which |
603 | ++ references this flow. (There are cases |
604 | ++ that multiple SB entities share the same |
605 | ++ desired OpenFlow flow, e.g. when |
606 | ++ conjunction is used.) */ |
607 | + |
608 | + /* Key. */ |
609 | + uint8_t table_id; |
610 | +@@ -63,21 +97,34 @@ struct ovn_flow { |
611 | + struct minimatch match; |
612 | + |
613 | + /* Data. */ |
614 | +- struct uuid sb_uuid; |
615 | + struct ofpact *ofpacts; |
616 | + size_t ofpacts_len; |
617 | + uint64_t cookie; |
618 | + }; |
619 | + |
620 | ++struct sb_to_flow { |
621 | ++ struct hmap_node hmap_node; /* Node in |
622 | ++ ovn_desired_flow_table.uuid_flow_table. */ |
623 | ++ struct uuid sb_uuid; |
624 | ++ struct ovs_list flows; /* A list of struct sb_flow_ref nodes that are |
625 | ++ referenced by the sb_uuid. */ |
626 | ++}; |
627 | ++ |
628 | ++struct sb_flow_ref { |
629 | ++ struct ovs_list sb_list; /* List node in ovn_flow.references. */ |
630 | ++ struct ovs_list flow_list; /* List node in sb_to_flow.ovn_flows. */ |
631 | ++ struct ovn_flow *flow; |
632 | ++ struct uuid sb_uuid; |
633 | ++}; |
634 | ++ |
635 | + static struct ovn_flow *ovn_flow_alloc(uint8_t table_id, uint16_t priority, |
636 | + uint64_t cookie, |
637 | + const struct match *match, |
638 | +- const struct ofpbuf *actions, |
639 | +- const struct uuid *sb_uuid); |
640 | ++ const struct ofpbuf *actions); |
641 | + static uint32_t ovn_flow_match_hash(const struct ovn_flow *); |
642 | + static struct ovn_flow *ovn_flow_lookup(struct hmap *flow_table, |
643 | + const struct ovn_flow *target, |
644 | +- bool cmp_sb_uuid); |
645 | ++ const struct uuid *sb_uuid); |
646 | + static char *ovn_flow_to_string(const struct ovn_flow *); |
647 | + static void ovn_flow_log(const struct ovn_flow *, const char *action); |
648 | + static void ovn_flow_destroy(struct ovn_flow *); |
649 | +@@ -644,13 +691,46 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype type) |
650 | + } |
651 | + } |
652 | + |
653 | |
654 | ++static struct sb_to_flow * |
655 | ++sb_to_flow_find(struct hmap *uuid_flow_table, const struct uuid *sb_uuid) |
656 | ++{ |
657 | ++ struct sb_to_flow *stf; |
658 | ++ HMAP_FOR_EACH_WITH_HASH (stf, hmap_node, uuid_hash(sb_uuid), |
659 | ++ uuid_flow_table) { |
660 | ++ if (uuid_equals(sb_uuid, &stf->sb_uuid)) { |
661 | ++ return stf; |
662 | ++ } |
663 | ++ } |
664 | ++ return NULL; |
665 | ++} |
666 | ++ |
667 | ++static void |
668 | ++link_flow_to_sb(struct ovn_desired_flow_table *flow_table, |
669 | ++ struct ovn_flow *f, const struct uuid *sb_uuid) |
670 | ++{ |
671 | ++ struct sb_flow_ref *sfr = xmalloc(sizeof *sfr); |
672 | ++ sfr->flow = f; |
673 | ++ sfr->sb_uuid = *sb_uuid; |
674 | ++ ovs_list_insert(&f->references, &sfr->sb_list); |
675 | ++ struct sb_to_flow *stf = sb_to_flow_find(&flow_table->uuid_flow_table, |
676 | ++ sb_uuid); |
677 | ++ if (!stf) { |
678 | ++ stf = xmalloc(sizeof *stf); |
679 | ++ stf->sb_uuid = *sb_uuid; |
680 | ++ ovs_list_init(&stf->flows); |
681 | ++ hmap_insert(&flow_table->uuid_flow_table, &stf->hmap_node, |
682 | ++ uuid_hash(sb_uuid)); |
683 | ++ } |
684 | ++ ovs_list_insert(&stf->flows, &sfr->flow_list); |
685 | ++} |
686 | ++ |
687 | + /* Flow table interfaces to the rest of ovn-controller. */ |
688 | + |
689 | + /* Adds a flow to 'desired_flows' with the specified 'match' and 'actions' to |
690 | + * the OpenFlow table numbered 'table_id' with the given 'priority' and |
691 | + * OpenFlow 'cookie'. The caller retains ownership of 'match' and 'actions'. |
692 | + * |
693 | +- * The flow is also added to a hash index based on sb_uuid. |
694 | ++ * The flow is also linked to the sb_uuid that generates it. |
695 | + * |
696 | + * This just assembles the desired flow table in memory. Nothing is actually |
697 | + * sent to the switch until a later call to ofctrl_put(). |
698 | +@@ -665,11 +745,9 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table *flow_table, |
699 | + bool log_duplicate_flow) |
700 | + { |
701 | + struct ovn_flow *f = ovn_flow_alloc(table_id, priority, cookie, match, |
702 | +- actions, sb_uuid); |
703 | ++ actions); |
704 | + |
705 | +- ovn_flow_log(f, "ofctrl_add_flow"); |
706 | +- |
707 | +- if (ovn_flow_lookup(&flow_table->match_flow_table, f, true)) { |
708 | ++ if (ovn_flow_lookup(&flow_table->match_flow_table, f, sb_uuid)) { |
709 | + if (log_duplicate_flow) { |
710 | + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); |
711 | + if (!VLOG_DROP_DBG(&rl)) { |
712 | +@@ -684,31 +762,8 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table *flow_table, |
713 | + |
714 | + hmap_insert(&flow_table->match_flow_table, &f->match_hmap_node, |
715 | + f->match_hmap_node.hash); |
716 | +- hindex_insert(&flow_table->uuid_flow_table, &f->uuid_hindex_node, |
717 | +- f->uuid_hindex_node.hash); |
718 | +-} |
719 | +- |
720 | +-/* Removes a bundles of flows from the flow table. */ |
721 | +-void |
722 | +-ofctrl_remove_flows(struct ovn_desired_flow_table *flow_table, |
723 | +- const struct uuid *sb_uuid) |
724 | +-{ |
725 | +- struct ovn_flow *f, *next; |
726 | +- HINDEX_FOR_EACH_WITH_HASH_SAFE (f, next, uuid_hindex_node, |
727 | +- uuid_hash(sb_uuid), |
728 | +- &flow_table->uuid_flow_table) { |
729 | +- if (uuid_equals(&f->sb_uuid, sb_uuid)) { |
730 | +- ovn_flow_log(f, "ofctrl_remove_flow"); |
731 | +- hmap_remove(&flow_table->match_flow_table, |
732 | +- &f->match_hmap_node); |
733 | +- hindex_remove(&flow_table->uuid_flow_table, &f->uuid_hindex_node); |
734 | +- ovn_flow_destroy(f); |
735 | +- } |
736 | +- } |
737 | +- |
738 | +- /* remove any related group and meter info */ |
739 | +- ovn_extend_table_remove_desired(groups, sb_uuid); |
740 | +- ovn_extend_table_remove_desired(meters, sb_uuid); |
741 | ++ link_flow_to_sb(flow_table, f, sb_uuid); |
742 | ++ ovn_flow_log(f, "ofctrl_add_flow"); |
743 | + } |
744 | + |
745 | + void |
746 | +@@ -721,6 +776,9 @@ ofctrl_add_flow(struct ovn_desired_flow_table *desired_flows, |
747 | + match, actions, sb_uuid, true); |
748 | + } |
749 | + |
750 | ++/* Either add a new flow, or append actions on an existing flow. If the |
751 | ++ * flow existed, a new link will also be created between the new sb_uuid |
752 | ++ * and the existing flow. */ |
753 | + void |
754 | + ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, |
755 | + uint8_t table_id, uint16_t priority, uint64_t cookie, |
756 | +@@ -729,12 +787,10 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, |
757 | + const struct uuid *sb_uuid) |
758 | + { |
759 | + struct ovn_flow *f = ovn_flow_alloc(table_id, priority, cookie, match, |
760 | +- actions, sb_uuid); |
761 | +- |
762 | +- ovn_flow_log(f, "ofctrl_add_or_append_flow"); |
763 | ++ actions); |
764 | + |
765 | + struct ovn_flow *existing; |
766 | +- existing = ovn_flow_lookup(&desired_flows->match_flow_table, f, false); |
767 | ++ existing = ovn_flow_lookup(&desired_flows->match_flow_table, f, NULL); |
768 | + if (existing) { |
769 | + /* There's already a flow with this particular match. Append the |
770 | + * action to that flow rather than adding a new flow |
771 | +@@ -751,11 +807,169 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, |
772 | + |
773 | + ofpbuf_uninit(&compound); |
774 | + ovn_flow_destroy(f); |
775 | ++ f = existing; |
776 | + } else { |
777 | + hmap_insert(&desired_flows->match_flow_table, &f->match_hmap_node, |
778 | + f->match_hmap_node.hash); |
779 | +- hindex_insert(&desired_flows->uuid_flow_table, &f->uuid_hindex_node, |
780 | +- f->uuid_hindex_node.hash); |
781 | ++ } |
782 | ++ link_flow_to_sb(desired_flows, f, sb_uuid); |
783 | ++ |
784 | ++ if (existing) { |
785 | ++ ovn_flow_log(f, "ofctrl_add_or_append_flow (append)"); |
786 | ++ } else { |
787 | ++ ovn_flow_log(f, "ofctrl_add_or_append_flow (add)"); |
788 | ++ } |
789 | ++} |
790 | ++ |
791 | ++/* Remove ovn_flows for the given "sb_to_flow" node in the uuid_flow_table. |
792 | ++ * Optionally log the message for each flow that is acturally removed, if |
793 | ++ * log_msg is not NULL. */ |
794 | ++static void |
795 | ++remove_flows_from_sb_to_flow(struct ovn_desired_flow_table *flow_table, |
796 | ++ struct sb_to_flow *stf, |
797 | ++ const char *log_msg) |
798 | ++{ |
799 | ++ struct sb_flow_ref *sfr, *next; |
800 | ++ LIST_FOR_EACH_SAFE (sfr, next, flow_list, &stf->flows) { |
801 | ++ ovs_list_remove(&sfr->sb_list); |
802 | ++ ovs_list_remove(&sfr->flow_list); |
803 | ++ struct ovn_flow *f = sfr->flow; |
804 | ++ free(sfr); |
805 | ++ |
806 | ++ if (ovs_list_is_empty(&f->references)) { |
807 | ++ if (log_msg) { |
808 | ++ ovn_flow_log(f, log_msg); |
809 | ++ } |
810 | ++ hmap_remove(&flow_table->match_flow_table, |
811 | ++ &f->match_hmap_node); |
812 | ++ ovn_flow_destroy(f); |
813 | ++ } |
814 | ++ } |
815 | ++ hmap_remove(&flow_table->uuid_flow_table, &stf->hmap_node); |
816 | ++ free(stf); |
817 | ++} |
818 | ++ |
819 | ++void |
820 | ++ofctrl_remove_flows(struct ovn_desired_flow_table *flow_table, |
821 | ++ const struct uuid *sb_uuid) |
822 | ++{ |
823 | ++ struct sb_to_flow *stf = sb_to_flow_find(&flow_table->uuid_flow_table, |
824 | ++ sb_uuid); |
825 | ++ if (stf) { |
826 | ++ remove_flows_from_sb_to_flow(flow_table, stf, "ofctrl_remove_flow"); |
827 | ++ } |
828 | ++ |
829 | ++ /* remove any related group and meter info */ |
830 | ++ ovn_extend_table_remove_desired(groups, sb_uuid); |
831 | ++ ovn_extend_table_remove_desired(meters, sb_uuid); |
832 | ++} |
833 | ++ |
834 | ++static struct ofctrl_flood_remove_node * |
835 | ++flood_remove_find_node(struct hmap *flood_remove_nodes, struct uuid *sb_uuid) |
836 | ++{ |
837 | ++ struct ofctrl_flood_remove_node *ofrn; |
838 | ++ HMAP_FOR_EACH_WITH_HASH (ofrn, hmap_node, uuid_hash(sb_uuid), |
839 | ++ flood_remove_nodes) { |
840 | ++ if (uuid_equals(&ofrn->sb_uuid, sb_uuid)) { |
841 | ++ return ofrn; |
842 | ++ } |
843 | ++ } |
844 | ++ return NULL; |
845 | ++} |
846 | ++ |
847 | ++void |
848 | ++ofctrl_flood_remove_add_node(struct hmap *flood_remove_nodes, |
849 | ++ const struct uuid *sb_uuid) |
850 | ++{ |
851 | ++ struct ofctrl_flood_remove_node *ofrn = xmalloc(sizeof *ofrn); |
852 | ++ ofrn->sb_uuid = *sb_uuid; |
853 | ++ hmap_insert(flood_remove_nodes, &ofrn->hmap_node, uuid_hash(sb_uuid)); |
854 | ++} |
855 | ++ |
856 | ++static void |
857 | ++flood_remove_flows_for_sb_uuid(struct ovn_desired_flow_table *flow_table, |
858 | ++ const struct uuid *sb_uuid, |
859 | ++ struct hmap *flood_remove_nodes) |
860 | ++{ |
861 | ++ struct sb_to_flow *stf = sb_to_flow_find(&flow_table->uuid_flow_table, |
862 | ++ sb_uuid); |
863 | ++ if (!stf) { |
864 | ++ return; |
865 | ++ } |
866 | ++ |
867 | ++ /* ovn_flows that have other references and waiting to be removed. */ |
868 | ++ struct ovs_list to_be_removed = OVS_LIST_INITIALIZER(&to_be_removed); |
869 | ++ |
870 | ++ /* Traverse all flows for the given sb_uuid. */ |
871 | ++ struct sb_flow_ref *sfr, *next; |
872 | ++ LIST_FOR_EACH_SAFE (sfr, next, flow_list, &stf->flows) { |
873 | ++ struct ovn_flow *f = sfr->flow; |
874 | ++ ovn_flow_log(f, "flood remove"); |
875 | ++ |
876 | ++ ovs_list_remove(&sfr->sb_list); |
877 | ++ ovs_list_remove(&sfr->flow_list); |
878 | ++ free(sfr); |
879 | ++ |
880 | ++ ovs_assert(ovs_list_is_empty(&f->list_node)); |
881 | ++ if (ovs_list_is_empty(&f->references)) { |
882 | ++ /* This is to optimize the case when most flows have only |
883 | ++ * one referencing sb_uuid, so to_be_removed list should |
884 | ++ * be empty in most cases. */ |
885 | ++ hmap_remove(&flow_table->match_flow_table, |
886 | ++ &f->match_hmap_node); |
887 | ++ ovn_flow_destroy(f); |
888 | ++ } else { |
889 | ++ ovs_list_insert(&to_be_removed, &f->list_node); |
890 | ++ } |
891 | ++ } |
892 | ++ hmap_remove(&flow_table->uuid_flow_table, &stf->hmap_node); |
893 | ++ free(stf); |
894 | ++ |
895 | ++ /* Traverse other referencing sb_uuids for the flows in the to_be_removed |
896 | ++ * list. */ |
897 | ++ |
898 | ++ /* Detach the items in f->references from the sfr.flow_list lists, |
899 | ++ * so that recursive calls will not mess up the sfr.sb_list list. */ |
900 | ++ struct ovn_flow *f, *f_next; |
901 | ++ LIST_FOR_EACH (f, list_node, &to_be_removed) { |
902 | ++ ovs_assert(!ovs_list_is_empty(&f->references)); |
903 | ++ LIST_FOR_EACH (sfr, sb_list, &f->references) { |
904 | ++ ovs_list_remove(&sfr->flow_list); |
905 | ++ } |
906 | ++ } |
907 | ++ LIST_FOR_EACH_SAFE (f, f_next, list_node, &to_be_removed) { |
908 | ++ LIST_FOR_EACH_SAFE (sfr, next, sb_list, &f->references) { |
909 | ++ if (!flood_remove_find_node(flood_remove_nodes, &sfr->sb_uuid)) { |
910 | ++ ofctrl_flood_remove_add_node(flood_remove_nodes, |
911 | ++ &sfr->sb_uuid); |
912 | ++ flood_remove_flows_for_sb_uuid(flow_table, &sfr->sb_uuid, |
913 | ++ flood_remove_nodes); |
914 | ++ } |
915 | ++ ovs_list_remove(&sfr->sb_list); |
916 | ++ free(sfr); |
917 | ++ } |
918 | ++ ovs_list_remove(&f->list_node); |
919 | ++ hmap_remove(&flow_table->match_flow_table, |
920 | ++ &f->match_hmap_node); |
921 | ++ ovn_flow_destroy(f); |
922 | ++ } |
923 | ++ |
924 | ++} |
925 | ++ |
926 | ++void |
927 | ++ofctrl_flood_remove_flows(struct ovn_desired_flow_table *flow_table, |
928 | ++ struct hmap *flood_remove_nodes) |
929 | ++{ |
930 | ++ struct ofctrl_flood_remove_node *ofrn; |
931 | ++ HMAP_FOR_EACH (ofrn, hmap_node, flood_remove_nodes) { |
932 | ++ flood_remove_flows_for_sb_uuid(flow_table, &ofrn->sb_uuid, |
933 | ++ flood_remove_nodes); |
934 | ++ } |
935 | ++ |
936 | ++ /* remove any related group and meter info */ |
937 | ++ HMAP_FOR_EACH (ofrn, hmap_node, flood_remove_nodes) { |
938 | ++ ovn_extend_table_remove_desired(groups, &ofrn->sb_uuid); |
939 | ++ ovn_extend_table_remove_desired(meters, &ofrn->sb_uuid); |
940 | + } |
941 | + } |
942 | + |
943 | |
944 | +@@ -763,18 +977,17 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, |
945 | + |
946 | + static struct ovn_flow * |
947 | + ovn_flow_alloc(uint8_t table_id, uint16_t priority, uint64_t cookie, |
948 | +- const struct match *match, const struct ofpbuf *actions, |
949 | +- const struct uuid *sb_uuid) |
950 | ++ const struct match *match, const struct ofpbuf *actions) |
951 | + { |
952 | + struct ovn_flow *f = xmalloc(sizeof *f); |
953 | ++ ovs_list_init(&f->references); |
954 | ++ ovs_list_init(&f->list_node); |
955 | + f->table_id = table_id; |
956 | + f->priority = priority; |
957 | + minimatch_init(&f->match, match); |
958 | + f->ofpacts = xmemdup(actions->data, actions->size); |
959 | + f->ofpacts_len = actions->size; |
960 | +- f->sb_uuid = *sb_uuid; |
961 | + f->match_hmap_node.hash = ovn_flow_match_hash(f); |
962 | +- f->uuid_hindex_node.hash = uuid_hash(&f->sb_uuid); |
963 | + f->cookie = cookie; |
964 | + |
965 | + return f; |
966 | +@@ -793,23 +1006,27 @@ static struct ovn_flow * |
967 | + ovn_flow_dup(struct ovn_flow *src) |
968 | + { |
969 | + struct ovn_flow *dst = xmalloc(sizeof *dst); |
970 | ++ ovs_list_init(&dst->references); |
971 | + dst->table_id = src->table_id; |
972 | + dst->priority = src->priority; |
973 | + minimatch_clone(&dst->match, &src->match); |
974 | + dst->ofpacts = xmemdup(src->ofpacts, src->ofpacts_len); |
975 | + dst->ofpacts_len = src->ofpacts_len; |
976 | +- dst->sb_uuid = src->sb_uuid; |
977 | + dst->match_hmap_node.hash = src->match_hmap_node.hash; |
978 | +- dst->uuid_hindex_node.hash = uuid_hash(&src->sb_uuid); |
979 | + dst->cookie = src->cookie; |
980 | + return dst; |
981 | + } |
982 | + |
983 | + /* Finds and returns an ovn_flow in 'flow_table' whose key is identical to |
984 | +- * 'target''s key, or NULL if there is none. */ |
985 | ++ * 'target''s key, or NULL if there is none. |
986 | ++ * |
987 | ++ * If sb_uuid is not NULL, the function will also check if the found flow is |
988 | ++ * referenced by the sb_uuid. |
989 | ++ * |
990 | ++ * NOTE: sb_uuid can only be used for ovn_desired_flow_table lookup. */ |
991 | + static struct ovn_flow * |
992 | + ovn_flow_lookup(struct hmap *flow_table, const struct ovn_flow *target, |
993 | +- bool cmp_sb_uuid) |
994 | ++ const struct uuid *sb_uuid) |
995 | + { |
996 | + struct ovn_flow *f; |
997 | + |
998 | +@@ -818,9 +1035,16 @@ ovn_flow_lookup(struct hmap *flow_table, const struct ovn_flow *target, |
999 | + if (f->table_id == target->table_id |
1000 | + && f->priority == target->priority |
1001 | + && minimatch_equal(&f->match, &target->match)) { |
1002 | +- if (!cmp_sb_uuid || uuid_equals(&target->sb_uuid, &f->sb_uuid)) { |
1003 | ++ if (!sb_uuid) { |
1004 | + return f; |
1005 | + } |
1006 | ++ ovs_assert(flow_table != &installed_flows); |
1007 | ++ struct sb_flow_ref *sfr; |
1008 | ++ LIST_FOR_EACH (sfr, sb_list, &f->references) { |
1009 | ++ if (uuid_equals(sb_uuid, &sfr->sb_uuid)) { |
1010 | ++ return f; |
1011 | ++ } |
1012 | ++ } |
1013 | + } |
1014 | + } |
1015 | + return NULL; |
1016 | +@@ -830,7 +1054,7 @@ static char * |
1017 | + ovn_flow_to_string(const struct ovn_flow *f) |
1018 | + { |
1019 | + struct ds s = DS_EMPTY_INITIALIZER; |
1020 | +- ds_put_format(&s, "sb_uuid="UUID_FMT", ", UUID_ARGS(&f->sb_uuid)); |
1021 | ++ |
1022 | + ds_put_format(&s, "cookie=%"PRIx64", ", f->cookie); |
1023 | + ds_put_format(&s, "table_id=%"PRIu8", ", f->table_id); |
1024 | + ds_put_format(&s, "priority=%"PRIu16", ", f->priority); |
1025 | +@@ -855,6 +1079,7 @@ static void |
1026 | + ovn_flow_destroy(struct ovn_flow *f) |
1027 | + { |
1028 | + if (f) { |
1029 | ++ ovs_assert(ovs_list_is_empty(&f->references)); |
1030 | + minimatch_destroy(&f->match); |
1031 | + free(f->ofpacts); |
1032 | + free(f); |
1033 | +@@ -866,18 +1091,16 @@ void |
1034 | + ovn_desired_flow_table_init(struct ovn_desired_flow_table *flow_table) |
1035 | + { |
1036 | + hmap_init(&flow_table->match_flow_table); |
1037 | +- hindex_init(&flow_table->uuid_flow_table); |
1038 | ++ hmap_init(&flow_table->uuid_flow_table); |
1039 | + } |
1040 | + |
1041 | + void |
1042 | + ovn_desired_flow_table_clear(struct ovn_desired_flow_table *flow_table) |
1043 | + { |
1044 | +- struct ovn_flow *f, *next; |
1045 | +- HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, |
1046 | +- &flow_table->match_flow_table) { |
1047 | +- hmap_remove(&flow_table->match_flow_table, &f->match_hmap_node); |
1048 | +- hindex_remove(&flow_table->uuid_flow_table, &f->uuid_hindex_node); |
1049 | +- ovn_flow_destroy(f); |
1050 | ++ struct sb_to_flow *stf, *next; |
1051 | ++ HMAP_FOR_EACH_SAFE (stf, next, hmap_node, |
1052 | ++ &flow_table->uuid_flow_table) { |
1053 | ++ remove_flows_from_sb_to_flow(flow_table, stf, NULL); |
1054 | + } |
1055 | + } |
1056 | + |
1057 | +@@ -886,7 +1109,7 @@ ovn_desired_flow_table_destroy(struct ovn_desired_flow_table *flow_table) |
1058 | + { |
1059 | + ovn_desired_flow_table_clear(flow_table); |
1060 | + hmap_destroy(&flow_table->match_flow_table); |
1061 | +- hindex_destroy(&flow_table->uuid_flow_table); |
1062 | ++ hmap_destroy(&flow_table->uuid_flow_table); |
1063 | + } |
1064 | + |
1065 | + static void |
1066 | +@@ -1159,7 +1382,7 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, |
1067 | + struct ovn_flow *i, *next; |
1068 | + HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, &installed_flows) { |
1069 | + struct ovn_flow *d = ovn_flow_lookup(&flow_table->match_flow_table, |
1070 | +- i, false); |
1071 | ++ i, NULL); |
1072 | + if (!d) { |
1073 | + /* Installed flow is no longer desirable. Delete it from the |
1074 | + * switch and from installed_flows. */ |
1075 | +@@ -1175,10 +1398,6 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, |
1076 | + hmap_remove(&installed_flows, &i->match_hmap_node); |
1077 | + ovn_flow_destroy(i); |
1078 | + } else { |
1079 | +- if (!uuid_equals(&i->sb_uuid, &d->sb_uuid)) { |
1080 | +- /* Update installed flow's UUID. */ |
1081 | +- i->sb_uuid = d->sb_uuid; |
1082 | +- } |
1083 | + if (!ofpacts_equal(i->ofpacts, i->ofpacts_len, |
1084 | + d->ofpacts, d->ofpacts_len) || |
1085 | + i->cookie != d->cookie) { |
1086 | +@@ -1215,7 +1434,7 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, |
1087 | + * in the installed flow table. */ |
1088 | + struct ovn_flow *d; |
1089 | + HMAP_FOR_EACH (d, match_hmap_node, &flow_table->match_flow_table) { |
1090 | +- i = ovn_flow_lookup(&installed_flows, d, false); |
1091 | ++ i = ovn_flow_lookup(&installed_flows, d, NULL); |
1092 | + if (!i) { |
1093 | + /* Send flow_mod to add flow. */ |
1094 | + struct ofputil_flow_mod fm = { |
1095 | +diff --git a/controller/ofctrl.h b/controller/ofctrl.h |
1096 | +index 37f06db86..8789ce490 100644 |
1097 | +--- a/controller/ofctrl.h |
1098 | ++++ b/controller/ofctrl.h |
1099 | +@@ -35,8 +35,9 @@ struct ovn_desired_flow_table { |
1100 | + /* Hash map flow table using flow match conditions as hash key.*/ |
1101 | + struct hmap match_flow_table; |
1102 | + |
1103 | +- /* SB uuid index for the nodes in match_flow_table.*/ |
1104 | +- struct hindex uuid_flow_table; |
1105 | ++ /* SB uuid index for the cross reference nodes that link to the nodes in |
1106 | ++ * match_flow_table.*/ |
1107 | ++ struct hmap uuid_flow_table; |
1108 | + }; |
1109 | + |
1110 | + /* Interface for OVN main loop. */ |
1111 | +@@ -74,7 +75,30 @@ void ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, |
1112 | + const struct ofpbuf *actions, |
1113 | + const struct uuid *sb_uuid); |
1114 | + |
1115 | +-void ofctrl_remove_flows(struct ovn_desired_flow_table *, const struct uuid *); |
1116 | ++/* Removes a bundles of flows from the flow table for a specific sb_uuid. The |
1117 | ++ * flows are removed only if they are not referenced by any other sb_uuid(s). |
1118 | ++ * For flood-removing all related flows referenced by other sb_uuid(s), use |
1119 | ++ * ofctrl_flood_remove_flows(). */ |
1120 | ++void ofctrl_remove_flows(struct ovn_desired_flow_table *, |
1121 | ++ const struct uuid *sb_uuid); |
1122 | ++ |
1123 | ++/* The function ofctrl_flood_remove_flows flood-removes flows from the desired |
1124 | ++ * flow table for the sb_uuids provided in the flood_remove_nodes argument. |
1125 | ++ * For each given sb_uuid in flood_remove_nodes, it removes all the flows |
1126 | ++ * generated by the sb_uuid, and if any of the flows are referenced by another |
1127 | ++ * sb_uuid, it continues removing all the flows used by that sb_uuid as well, |
1128 | ++ * and so on, recursively. |
1129 | ++ * |
1130 | ++ * It adds all the sb_uuids that are actually removed in the |
1131 | ++ * flood_remove_nodes. */ |
1132 | ++struct ofctrl_flood_remove_node { |
1133 | ++ struct hmap_node hmap_node; |
1134 | ++ struct uuid sb_uuid; |
1135 | ++}; |
1136 | ++void ofctrl_flood_remove_flows(struct ovn_desired_flow_table *, |
1137 | ++ struct hmap *flood_remove_nodes); |
1138 | ++void ofctrl_flood_remove_add_node(struct hmap *flood_remove_nodes, |
1139 | ++ const struct uuid *sb_uuid); |
1140 | + |
1141 | + void ovn_desired_flow_table_init(struct ovn_desired_flow_table *); |
1142 | + void ovn_desired_flow_table_clear(struct ovn_desired_flow_table *); |
1143 | +@@ -86,6 +110,7 @@ void ofctrl_check_and_add_flow(struct ovn_desired_flow_table *, |
1144 | + const struct ofpbuf *ofpacts, |
1145 | + const struct uuid *, bool log_duplicate_flow); |
1146 | + |
1147 | ++ |
1148 | + bool ofctrl_is_connected(void); |
1149 | + void ofctrl_set_probe_interval(int probe_interval); |
1150 | + |
1151 | +diff --git a/tests/ovn.at b/tests/ovn.at |
1152 | +index 341e9c2ab..62cec0871 100644 |
1153 | +--- a/tests/ovn.at |
1154 | ++++ b/tests/ovn.at |
1155 | +@@ -12543,6 +12543,8 @@ AT_CHECK([cat 2.packets], [0], [expout]) |
1156 | + |
1157 | + OVS_WAIT_UNTIL([test 9 = `as hv1 ovs-ofctl dump-flows br-int | \ |
1158 | + grep conjunction | wc -l`]) |
1159 | ++OVS_WAIT_UNTIL([test 3 = `as hv1 ovs-ofctl dump-flows br-int | \ |
1160 | ++grep conjunction.*conjunction | wc -l`]) |
1161 | + OVS_WAIT_UNTIL([test 2 = `as hv1 ovs-ofctl dump-flows br-int | \ |
1162 | + grep conj_id | wc -l`]) |
1163 | + |
1164 | +@@ -12560,9 +12562,98 @@ sip=`ip_to_hex 10 0 0 4` |
1165 | + dip=`ip_to_hex 10 0 0 7` |
1166 | + |
1167 | + test_ip 1 f00000000001 f00000000002 $sip $dip |
1168 | ++ |
1169 | + $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets |
1170 | + AT_CHECK([cat 2.packets], [0], []) |
1171 | + |
1172 | ++# Remove the first ACL, and verify that the conjunction flows are updated |
1173 | ++# properly. |
1174 | ++# There should be total of 6 flows present with conjunction action and 1 flow |
1175 | ++# with conj match. Eg. |
1176 | ++# table=44, priority=2001,conj_id=3,metadata=0x1 actions=drop |
1177 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.7 actions=conjunction(4,2/2) |
1178 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 actions=conjunction(4,2/2) |
1179 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 actions=conjunction(4,2/2) |
1180 | ++# priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 actions=conjunction(4,1/2) |
1181 | ++# priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 actions=conjunction(4,1/2) |
1182 | ++# priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 actions=conjunction(4,1/2) |
1183 | ++ |
1184 | ++ovn-nbctl acl-del ls1 to-lport 1001 \ |
1185 | ++'ip4 && ip4.src == $set1 && ip4.dst == $set1' |
1186 | ++ |
1187 | ++OVS_WAIT_UNTIL([test 6 = `as hv1 ovs-ofctl dump-flows br-int | \ |
1188 | ++grep conjunction | wc -l`]) |
1189 | ++OVS_WAIT_UNTIL([test 0 = `as hv1 ovs-ofctl dump-flows br-int | \ |
1190 | ++grep conjunction.*conjunction | wc -l`]) |
1191 | ++OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-ofctl dump-flows br-int | \ |
1192 | ++grep conj_id | wc -l`]) |
1193 | ++ |
1194 | ++# Add the ACL back |
1195 | ++ovn-nbctl acl-add ls1 to-lport 1001 \ |
1196 | ++'ip4 && ip4.src == $set1 && ip4.dst == $set1' allow |
1197 | ++# Add one more ACL with more overlapping |
1198 | ++ovn-nbctl acl-add ls1 to-lport 1001 \ |
1199 | ++'ip4 && ip4.src == $set1 && ip4.dst == {10.0.0.9, 10.0.0.10}' drop |
1200 | ++ |
1201 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 actions=conjunction(4,1/2) |
1202 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.7 actions=conjunction(4,1/2) |
1203 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(5,1/2) |
1204 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.5 actions=conjunction(5,1/2) |
1205 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.6 actions=conjunction(5,1/2) |
1206 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 actions=conjunction(4,1/2),conjunction(6,1/2) |
1207 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.10 actions=conjunction(6,1/2) |
1208 | ++# priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 actions=conjunction(4,2/2),conjunction(5,2/2),conjunction(6,2/2) |
1209 | ++# priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 actions=conjunction(4,2/2),conjunction(5,2/2),conjunction(6,2/2) |
1210 | ++# priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 actions=conjunction(4,2/2),conjunction(5,2/2),conjunction(6,2/2) |
1211 | ++ |
1212 | ++OVS_WAIT_UNTIL([test 10 = `as hv1 ovs-ofctl dump-flows br-int | \ |
1213 | ++grep conjunction | wc -l`]) |
1214 | ++OVS_WAIT_UNTIL([test 4 = `as hv1 ovs-ofctl dump-flows br-int | \ |
1215 | ++grep conjunction.*conjunction | wc -l`]) |
1216 | ++OVS_WAIT_UNTIL([test 3 = `as hv1 ovs-ofctl dump-flows br-int | \ |
1217 | ++grep conjunction.*conjunction.*conjunction | wc -l`]) |
1218 | ++ |
1219 | ++# Remove 10.0.0.7 from address set2. All flows should be updated properly. |
1220 | ++ovn-nbctl set Address_Set set2 \ |
1221 | ++addresses=\"10.0.0.8\",\"10.0.0.9\" |
1222 | ++ |
1223 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(9,1/2) |
1224 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.10 actions=conjunction(7,1/2) |
1225 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 actions=conjunction(8,1/2) |
1226 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.5 actions=conjunction(9,1/2) |
1227 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 actions=conjunction(7,1/2),conjunction(8,1/2) |
1228 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.6 actions=conjunction(9,1/2) |
1229 | ++# priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 actions=conjunction(7,2/2),conjunction(8,2/2),conjunction(9,2/2) |
1230 | ++# priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 actions=conjunction(7,2/2),conjunction(8,2/2),conjunction(9,2/2) |
1231 | ++# priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 actions=conjunction(7,2/2),conjunction(8,2/2),conjunction(9,2/2) |
1232 | ++ |
1233 | ++OVS_WAIT_UNTIL([test 9 = `as hv1 ovs-ofctl dump-flows br-int | \ |
1234 | ++grep conjunction | wc -l`]) |
1235 | ++OVS_WAIT_UNTIL([test 4 = `as hv1 ovs-ofctl dump-flows br-int | \ |
1236 | ++grep conjunction.*conjunction | wc -l`]) |
1237 | ++OVS_WAIT_UNTIL([test 3 = `as hv1 ovs-ofctl dump-flows br-int | \ |
1238 | ++grep conjunction.*conjunction.*conjunction | wc -l`]) |
1239 | ++ |
1240 | ++# Remove an ACL again |
1241 | ++ovn-nbctl acl-del ls1 to-lport 1001 \ |
1242 | ++'ip4 && ip4.src == $set1 && ip4.dst == $set1' |
1243 | ++ |
1244 | ++ovn-nbctl --wait=hv sync |
1245 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.10 actions=conjunction(10,1/2) |
1246 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.8 actions=conjunction(11,1/2) |
1247 | ++# priority=2001,ip,metadata=0x1,nw_dst=10.0.0.9 actions=conjunction(10,1/2),conjunction(11,1/2) |
1248 | ++# priority=2001,ip,metadata=0x1,nw_src=10.0.0.5 actions=conjunction(10,2/2),conjunction(11,2/2) |
1249 | ++# priority=2001,ip,metadata=0x1,nw_src=10.0.0.6 actions=conjunction(10,2/2),conjunction(11,2/2) |
1250 | ++# priority=2001,ip,metadata=0x1,nw_src=10.0.0.4 actions=conjunction(10,2/2),conjunction(11,2/2) |
1251 | ++ |
1252 | ++OVS_WAIT_UNTIL([test 6 = `as hv1 ovs-ofctl dump-flows br-int | \ |
1253 | ++grep conjunction | wc -l`]) |
1254 | ++OVS_WAIT_UNTIL([test 4 = `as hv1 ovs-ofctl dump-flows br-int | \ |
1255 | ++grep conjunction.*conjunction | wc -l`]) |
1256 | ++OVS_WAIT_UNTIL([test 0 = `as hv1 ovs-ofctl dump-flows br-int | \ |
1257 | ++grep conjunction.*conjunction.*conjunction | wc -l`]) |
1258 | ++ |
1259 | ++OVN_CLEANUP([hv1]) |
1260 | + AT_CLEANUP |
1261 | + |
1262 | + # 3 hypervisors, one logical switch, 3 logical ports per hypervisor |
1263 | +-- |
1264 | +2.29.2 |
1265 | + |
1266 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-03.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-03.patch |
1267 | new file mode 100644 |
1268 | index 0000000..88e91be |
1269 | --- /dev/null |
1270 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-03.patch |
1271 | @@ -0,0 +1,719 @@ |
1272 | +Description: Cherry-pick fix applied to master and branch-20.12 |
1273 | +Origin: https://github.com/ovn-org/ovn/commit/354d3853d40cbce89a434632f67daed7fc992d8b |
1274 | +Applied-Upstream: commit: 354d3853d40cbce89a434632f67daed7fc992d8b |
1275 | + |
1276 | +From d18e5cf1ec9ad111796889194d29d9f5648a1e5e Mon Sep 17 00:00:00 2001 |
1277 | +From: Han Zhou <hzhou@ovn.org> |
1278 | +Date: Wed, 19 Aug 2020 18:37:51 -0700 |
1279 | +Subject: [PATCH 03/15] ofctrl.c: Maintain references between installed flows |
1280 | + and desired flows. |
1281 | + |
1282 | +Currently there is no link maintained between installed flows and desired |
1283 | +flows. This patch maintains the mapping between them, which will be useful |
1284 | +for a future patch that incrementally processes the flow installation without |
1285 | +having to do the full comparison between them. |
1286 | + |
1287 | +This patch also refactors the struct ovn_flow with two different wrapper |
1288 | +types: desired_flow and installed_flow, and the related static functions, |
1289 | +to make the code easier to read and avoid misuses of the struct. |
1290 | + |
1291 | +Acked-by: Mark Michelson <mmichels@redhat.com> |
1292 | +Signed-off-by: Han Zhou <hzhou@ovn.org> |
1293 | +--- |
1294 | + controller/ofctrl.c | 428 ++++++++++++++++++++++++++++++-------------- |
1295 | + 1 file changed, 294 insertions(+), 134 deletions(-) |
1296 | + |
1297 | +diff --git a/controller/ofctrl.c b/controller/ofctrl.c |
1298 | +index 609d5db5f..c6823e941 100644 |
1299 | +--- a/controller/ofctrl.c |
1300 | ++++ b/controller/ofctrl.c |
1301 | +@@ -51,7 +51,27 @@ |
1302 | + |
1303 | + VLOG_DEFINE_THIS_MODULE(ofctrl); |
1304 | + |
1305 | +-/* An OpenFlow flow. |
1306 | ++/* An OpenFlow flow. */ |
1307 | ++struct ovn_flow { |
1308 | ++ /* Key. */ |
1309 | ++ uint8_t table_id; |
1310 | ++ uint16_t priority; |
1311 | ++ struct minimatch match; |
1312 | ++ |
1313 | ++ /* Hash. */ |
1314 | ++ uint32_t hash; |
1315 | ++ |
1316 | ++ /* Data. */ |
1317 | ++ struct ofpact *ofpacts; |
1318 | ++ size_t ofpacts_len; |
1319 | ++ uint64_t cookie; |
1320 | ++}; |
1321 | ++ |
1322 | ++/* A desired flow, in struct ovn_desired_flow_table, calculated by the |
1323 | ++ * incremental processing engine. |
1324 | ++ * - They are added/removed incrementally when I-P engine is able to process |
1325 | ++ * the changes incrementally, or |
1326 | ++ * - Completely cleared and recomputed by I-P engine when recompute happens. |
1327 | + * |
1328 | + * Links are maintained between desired flows and SB data. The relationship |
1329 | + * is M to N. The struct sb_flow_ref is used to link a pair of desired flow |
1330 | +@@ -82,52 +102,92 @@ VLOG_DEFINE_THIS_MODULE(ofctrl); |
1331 | + * The links are updated whenever there is a change in desired flows, which is |
1332 | + * usually triggered by a SB data change in I-P engine. |
1333 | + */ |
1334 | +-struct ovn_flow { |
1335 | ++struct desired_flow { |
1336 | ++ struct ovn_flow flow; |
1337 | + struct hmap_node match_hmap_node; /* For match based hashing. */ |
1338 | + struct ovs_list list_node; /* For handling lists of flows. */ |
1339 | +- struct ovs_list references; /* A list of struct sb_flow_ref nodes, which |
1340 | +- references this flow. (There are cases |
1341 | +- that multiple SB entities share the same |
1342 | +- desired OpenFlow flow, e.g. when |
1343 | +- conjunction is used.) */ |
1344 | + |
1345 | +- /* Key. */ |
1346 | +- uint8_t table_id; |
1347 | +- uint16_t priority; |
1348 | +- struct minimatch match; |
1349 | ++ /* A list of struct sb_flow_ref nodes, which references this flow. (There |
1350 | ++ * are cases that multiple SB entities share the same desired OpenFlow |
1351 | ++ * flow, e.g. when conjunction is used.) */ |
1352 | ++ struct ovs_list references; |
1353 | + |
1354 | +- /* Data. */ |
1355 | +- struct ofpact *ofpacts; |
1356 | +- size_t ofpacts_len; |
1357 | +- uint64_t cookie; |
1358 | ++ /* The corresponding flow in installed table. */ |
1359 | ++ struct installed_flow *installed_flow; |
1360 | ++ |
1361 | ++ /* Node in installed_flow.desired_refs list. */ |
1362 | ++ struct ovs_list installed_ref_list_node; |
1363 | + }; |
1364 | + |
1365 | + struct sb_to_flow { |
1366 | + struct hmap_node hmap_node; /* Node in |
1367 | + ovn_desired_flow_table.uuid_flow_table. */ |
1368 | + struct uuid sb_uuid; |
1369 | +- struct ovs_list flows; /* A list of struct sb_flow_ref nodes that are |
1370 | +- referenced by the sb_uuid. */ |
1371 | ++ struct ovs_list flows; /* A list of struct sb_flow_ref nodes that |
1372 | ++ are referenced by the sb_uuid. */ |
1373 | + }; |
1374 | + |
1375 | + struct sb_flow_ref { |
1376 | +- struct ovs_list sb_list; /* List node in ovn_flow.references. */ |
1377 | +- struct ovs_list flow_list; /* List node in sb_to_flow.ovn_flows. */ |
1378 | +- struct ovn_flow *flow; |
1379 | ++ struct ovs_list sb_list; /* List node in desired_flow.references. */ |
1380 | ++ struct ovs_list flow_list; /* List node in sb_to_flow.desired_flows. */ |
1381 | ++ struct desired_flow *flow; |
1382 | + struct uuid sb_uuid; |
1383 | + }; |
1384 | + |
1385 | +-static struct ovn_flow *ovn_flow_alloc(uint8_t table_id, uint16_t priority, |
1386 | +- uint64_t cookie, |
1387 | +- const struct match *match, |
1388 | +- const struct ofpbuf *actions); |
1389 | ++/* A installed flow, in static variable installed_flows. |
1390 | ++ * |
1391 | ++ * Installed flows are updated in ofctrl_put for maintaining the flow |
1392 | ++ * installation to OVS. They are updated according to desired flows: either by |
1393 | ++ * processing the tracked desired flow changes, or by comparing desired flows |
1394 | ++ * with currently installed flows when tracked desired flows changes are not |
1395 | ++ * available. |
1396 | ++ * |
1397 | ++ * In addition, when ofctrl state machine enters S_CLEAR, the installed flows |
1398 | ++ * will be cleared. (This happens in initialization phase and also when |
1399 | ++ * ovs-vswitchd is disconnected/reconnected). |
1400 | ++ * |
1401 | ++ * Links are maintained between installed flows and desired flows. The |
1402 | ++ * relationship is 1 to N. A link is added when a flow addition is processed. |
1403 | ++ * A link is removed when a flow deletion is processed, the desired flow |
1404 | ++ * table is cleared, or the installed flow table is cleared. |
1405 | ++ */ |
1406 | ++struct installed_flow { |
1407 | ++ struct ovn_flow flow; |
1408 | ++ struct hmap_node match_hmap_node; /* For match based hashing. */ |
1409 | ++ |
1410 | ++ /* A list of desired ovn_flow nodes (linked by |
1411 | ++ * desired_flow.installed_ref_list_node), which reference this installed |
1412 | ++ * flow. (There are cases that multiple desired flows reference the same |
1413 | ++ * installed flow, e.g. when there are conflict/duplicated ACLs that |
1414 | ++ * generates same match conditions). */ |
1415 | ++ struct ovs_list desired_refs; |
1416 | ++ |
1417 | ++ /* The corresponding flow in desired table. It must be one of the flows in |
1418 | ++ * desired_refs list. If there are more than one flows in references list, |
1419 | ++ * this is the one that is actually installed. */ |
1420 | ++ struct desired_flow *desired_flow; |
1421 | ++}; |
1422 | ++ |
1423 | ++static struct desired_flow *desired_flow_alloc( |
1424 | ++ uint8_t table_id, |
1425 | ++ uint16_t priority, |
1426 | ++ uint64_t cookie, |
1427 | ++ const struct match *match, |
1428 | ++ const struct ofpbuf *actions); |
1429 | ++static struct desired_flow *desired_flow_lookup( |
1430 | ++ struct ovn_desired_flow_table *, |
1431 | ++ const struct ovn_flow *target, |
1432 | ++ const struct uuid *sb_uuid); |
1433 | ++static void desired_flow_destroy(struct desired_flow *); |
1434 | ++ |
1435 | ++static struct installed_flow *installed_flow_lookup( |
1436 | ++ const struct ovn_flow *target); |
1437 | ++static void installed_flow_destroy(struct installed_flow *); |
1438 | ++static struct installed_flow *installed_flow_dup(struct desired_flow *); |
1439 | ++ |
1440 | + static uint32_t ovn_flow_match_hash(const struct ovn_flow *); |
1441 | +-static struct ovn_flow *ovn_flow_lookup(struct hmap *flow_table, |
1442 | +- const struct ovn_flow *target, |
1443 | +- const struct uuid *sb_uuid); |
1444 | + static char *ovn_flow_to_string(const struct ovn_flow *); |
1445 | + static void ovn_flow_log(const struct ovn_flow *, const char *action); |
1446 | +-static void ovn_flow_destroy(struct ovn_flow *); |
1447 | + |
1448 | + /* OpenFlow connection to the switch. */ |
1449 | + static struct rconn *swconn; |
1450 | +@@ -216,7 +276,6 @@ static struct ofpbuf *encode_meter_mod(const struct ofputil_meter_mod *); |
1451 | + |
1452 | + static void ovn_installed_flow_table_clear(void); |
1453 | + static void ovn_installed_flow_table_destroy(void); |
1454 | +-static struct ovn_flow *ovn_flow_dup(struct ovn_flow *source); |
1455 | + |
1456 | + |
1457 | + static void ofctrl_recv(const struct ofp_header *, enum ofptype); |
1458 | +@@ -691,6 +750,45 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype type) |
1459 | + } |
1460 | + } |
1461 | + |
1462 | |
1463 | ++static void |
1464 | ++link_installed_to_desired(struct installed_flow *i, struct desired_flow *d) |
1465 | ++{ |
1466 | ++ if (i->desired_flow == d) { |
1467 | ++ return; |
1468 | ++ } |
1469 | ++ |
1470 | ++ if (ovs_list_is_empty(&i->desired_refs)) { |
1471 | ++ ovs_assert(!i->desired_flow); |
1472 | ++ i->desired_flow = d; |
1473 | ++ } |
1474 | ++ ovs_list_insert(&i->desired_refs, &d->installed_ref_list_node); |
1475 | ++ d->installed_flow = i; |
1476 | ++} |
1477 | ++ |
1478 | ++static void |
1479 | ++unlink_installed_to_desired(struct installed_flow *i, struct desired_flow *d) |
1480 | ++{ |
1481 | ++ ovs_assert(i && i->desired_flow && !ovs_list_is_empty(&i->desired_refs)); |
1482 | ++ ovs_assert(d && d->installed_flow == i); |
1483 | ++ ovs_list_remove(&d->installed_ref_list_node); |
1484 | ++ d->installed_flow = NULL; |
1485 | ++ if (i->desired_flow == d) { |
1486 | ++ i->desired_flow = ovs_list_is_empty(&i->desired_refs) ? NULL : |
1487 | ++ CONTAINER_OF(ovs_list_front(&i->desired_refs), |
1488 | ++ struct desired_flow, |
1489 | ++ installed_ref_list_node); |
1490 | ++ } |
1491 | ++} |
1492 | ++ |
1493 | ++static void |
1494 | ++unlink_all_refs_for_installed_flow(struct installed_flow *i) |
1495 | ++{ |
1496 | ++ struct desired_flow *d, *next; |
1497 | ++ LIST_FOR_EACH_SAFE (d, next, installed_ref_list_node, &i->desired_refs) { |
1498 | ++ unlink_installed_to_desired(i, d); |
1499 | ++ } |
1500 | ++} |
1501 | ++ |
1502 | |
1503 | + static struct sb_to_flow * |
1504 | + sb_to_flow_find(struct hmap *uuid_flow_table, const struct uuid *sb_uuid) |
1505 | + { |
1506 | +@@ -706,7 +804,7 @@ sb_to_flow_find(struct hmap *uuid_flow_table, const struct uuid *sb_uuid) |
1507 | + |
1508 | + static void |
1509 | + link_flow_to_sb(struct ovn_desired_flow_table *flow_table, |
1510 | +- struct ovn_flow *f, const struct uuid *sb_uuid) |
1511 | ++ struct desired_flow *f, const struct uuid *sb_uuid) |
1512 | + { |
1513 | + struct sb_flow_ref *sfr = xmalloc(sizeof *sfr); |
1514 | + sfr->flow = f; |
1515 | +@@ -744,26 +842,26 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table *flow_table, |
1516 | + const struct uuid *sb_uuid, |
1517 | + bool log_duplicate_flow) |
1518 | + { |
1519 | +- struct ovn_flow *f = ovn_flow_alloc(table_id, priority, cookie, match, |
1520 | +- actions); |
1521 | ++ struct desired_flow *f = desired_flow_alloc(table_id, priority, cookie, |
1522 | ++ match, actions); |
1523 | + |
1524 | +- if (ovn_flow_lookup(&flow_table->match_flow_table, f, sb_uuid)) { |
1525 | ++ if (desired_flow_lookup(flow_table, &f->flow, sb_uuid)) { |
1526 | + if (log_duplicate_flow) { |
1527 | + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); |
1528 | + if (!VLOG_DROP_DBG(&rl)) { |
1529 | +- char *s = ovn_flow_to_string(f); |
1530 | ++ char *s = ovn_flow_to_string(&f->flow); |
1531 | + VLOG_DBG("dropping duplicate flow: %s", s); |
1532 | + free(s); |
1533 | + } |
1534 | + } |
1535 | +- ovn_flow_destroy(f); |
1536 | ++ desired_flow_destroy(f); |
1537 | + return; |
1538 | + } |
1539 | + |
1540 | + hmap_insert(&flow_table->match_flow_table, &f->match_hmap_node, |
1541 | +- f->match_hmap_node.hash); |
1542 | ++ f->flow.hash); |
1543 | + link_flow_to_sb(flow_table, f, sb_uuid); |
1544 | +- ovn_flow_log(f, "ofctrl_add_flow"); |
1545 | ++ ovn_flow_log(&f->flow, "ofctrl_add_flow"); |
1546 | + } |
1547 | + |
1548 | + void |
1549 | +@@ -786,11 +884,11 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, |
1550 | + const struct ofpbuf *actions, |
1551 | + const struct uuid *sb_uuid) |
1552 | + { |
1553 | +- struct ovn_flow *f = ovn_flow_alloc(table_id, priority, cookie, match, |
1554 | +- actions); |
1555 | ++ struct desired_flow *f = desired_flow_alloc(table_id, priority, cookie, |
1556 | ++ match, actions); |
1557 | + |
1558 | +- struct ovn_flow *existing; |
1559 | +- existing = ovn_flow_lookup(&desired_flows->match_flow_table, f, NULL); |
1560 | ++ struct desired_flow *existing; |
1561 | ++ existing = desired_flow_lookup(desired_flows, &f->flow, NULL); |
1562 | + if (existing) { |
1563 | + /* There's already a flow with this particular match. Append the |
1564 | + * action to that flow rather than adding a new flow |
1565 | +@@ -798,26 +896,27 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, |
1566 | + uint64_t compound_stub[64 / 8]; |
1567 | + struct ofpbuf compound; |
1568 | + ofpbuf_use_stub(&compound, compound_stub, sizeof(compound_stub)); |
1569 | +- ofpbuf_put(&compound, existing->ofpacts, existing->ofpacts_len); |
1570 | +- ofpbuf_put(&compound, f->ofpacts, f->ofpacts_len); |
1571 | ++ ofpbuf_put(&compound, existing->flow.ofpacts, |
1572 | ++ existing->flow.ofpacts_len); |
1573 | ++ ofpbuf_put(&compound, f->flow.ofpacts, f->flow.ofpacts_len); |
1574 | + |
1575 | +- free(existing->ofpacts); |
1576 | +- existing->ofpacts = xmemdup(compound.data, compound.size); |
1577 | +- existing->ofpacts_len = compound.size; |
1578 | ++ free(existing->flow.ofpacts); |
1579 | ++ existing->flow.ofpacts = xmemdup(compound.data, compound.size); |
1580 | ++ existing->flow.ofpacts_len = compound.size; |
1581 | + |
1582 | + ofpbuf_uninit(&compound); |
1583 | +- ovn_flow_destroy(f); |
1584 | ++ desired_flow_destroy(f); |
1585 | + f = existing; |
1586 | + } else { |
1587 | + hmap_insert(&desired_flows->match_flow_table, &f->match_hmap_node, |
1588 | +- f->match_hmap_node.hash); |
1589 | ++ f->flow.hash); |
1590 | + } |
1591 | + link_flow_to_sb(desired_flows, f, sb_uuid); |
1592 | + |
1593 | + if (existing) { |
1594 | +- ovn_flow_log(f, "ofctrl_add_or_append_flow (append)"); |
1595 | ++ ovn_flow_log(&f->flow, "ofctrl_add_or_append_flow (append)"); |
1596 | + } else { |
1597 | +- ovn_flow_log(f, "ofctrl_add_or_append_flow (add)"); |
1598 | ++ ovn_flow_log(&f->flow, "ofctrl_add_or_append_flow (add)"); |
1599 | + } |
1600 | + } |
1601 | + |
1602 | +@@ -833,16 +932,19 @@ remove_flows_from_sb_to_flow(struct ovn_desired_flow_table *flow_table, |
1603 | + LIST_FOR_EACH_SAFE (sfr, next, flow_list, &stf->flows) { |
1604 | + ovs_list_remove(&sfr->sb_list); |
1605 | + ovs_list_remove(&sfr->flow_list); |
1606 | +- struct ovn_flow *f = sfr->flow; |
1607 | ++ struct desired_flow *f = sfr->flow; |
1608 | + free(sfr); |
1609 | + |
1610 | + if (ovs_list_is_empty(&f->references)) { |
1611 | + if (log_msg) { |
1612 | +- ovn_flow_log(f, log_msg); |
1613 | ++ ovn_flow_log(&f->flow, log_msg); |
1614 | + } |
1615 | + hmap_remove(&flow_table->match_flow_table, |
1616 | + &f->match_hmap_node); |
1617 | +- ovn_flow_destroy(f); |
1618 | ++ if (f->installed_flow) { |
1619 | ++ unlink_installed_to_desired(f->installed_flow, f); |
1620 | ++ } |
1621 | ++ desired_flow_destroy(f); |
1622 | + } |
1623 | + } |
1624 | + hmap_remove(&flow_table->uuid_flow_table, &stf->hmap_node); |
1625 | +@@ -903,8 +1005,8 @@ flood_remove_flows_for_sb_uuid(struct ovn_desired_flow_table *flow_table, |
1626 | + /* Traverse all flows for the given sb_uuid. */ |
1627 | + struct sb_flow_ref *sfr, *next; |
1628 | + LIST_FOR_EACH_SAFE (sfr, next, flow_list, &stf->flows) { |
1629 | +- struct ovn_flow *f = sfr->flow; |
1630 | +- ovn_flow_log(f, "flood remove"); |
1631 | ++ struct desired_flow *f = sfr->flow; |
1632 | ++ ovn_flow_log(&f->flow, "flood remove"); |
1633 | + |
1634 | + ovs_list_remove(&sfr->sb_list); |
1635 | + ovs_list_remove(&sfr->flow_list); |
1636 | +@@ -917,7 +1019,10 @@ flood_remove_flows_for_sb_uuid(struct ovn_desired_flow_table *flow_table, |
1637 | + * be empty in most cases. */ |
1638 | + hmap_remove(&flow_table->match_flow_table, |
1639 | + &f->match_hmap_node); |
1640 | +- ovn_flow_destroy(f); |
1641 | ++ if (f->installed_flow) { |
1642 | ++ unlink_installed_to_desired(f->installed_flow, f); |
1643 | ++ } |
1644 | ++ desired_flow_destroy(f); |
1645 | + } else { |
1646 | + ovs_list_insert(&to_be_removed, &f->list_node); |
1647 | + } |
1648 | +@@ -930,7 +1035,7 @@ flood_remove_flows_for_sb_uuid(struct ovn_desired_flow_table *flow_table, |
1649 | + |
1650 | + /* Detach the items in f->references from the sfr.flow_list lists, |
1651 | + * so that recursive calls will not mess up the sfr.sb_list list. */ |
1652 | +- struct ovn_flow *f, *f_next; |
1653 | ++ struct desired_flow *f, *f_next; |
1654 | + LIST_FOR_EACH (f, list_node, &to_be_removed) { |
1655 | + ovs_assert(!ovs_list_is_empty(&f->references)); |
1656 | + LIST_FOR_EACH (sfr, sb_list, &f->references) { |
1657 | +@@ -951,7 +1056,10 @@ flood_remove_flows_for_sb_uuid(struct ovn_desired_flow_table *flow_table, |
1658 | + ovs_list_remove(&f->list_node); |
1659 | + hmap_remove(&flow_table->match_flow_table, |
1660 | + &f->match_hmap_node); |
1661 | +- ovn_flow_destroy(f); |
1662 | ++ if (f->installed_flow) { |
1663 | ++ unlink_installed_to_desired(f->installed_flow, f); |
1664 | ++ } |
1665 | ++ desired_flow_destroy(f); |
1666 | + } |
1667 | + |
1668 | + } |
1669 | +@@ -973,22 +1081,32 @@ ofctrl_flood_remove_flows(struct ovn_desired_flow_table *flow_table, |
1670 | + } |
1671 | + } |
1672 | + |
1673 | |
1674 | +-/* ovn_flow. */ |
1675 | ++/* flow operations. */ |
1676 | + |
1677 | +-static struct ovn_flow * |
1678 | +-ovn_flow_alloc(uint8_t table_id, uint16_t priority, uint64_t cookie, |
1679 | +- const struct match *match, const struct ofpbuf *actions) |
1680 | ++static void |
1681 | ++ovn_flow_init(struct ovn_flow *f, uint8_t table_id, uint16_t priority, |
1682 | ++ uint64_t cookie, const struct match *match, |
1683 | ++ const struct ofpbuf *actions) |
1684 | + { |
1685 | +- struct ovn_flow *f = xmalloc(sizeof *f); |
1686 | +- ovs_list_init(&f->references); |
1687 | +- ovs_list_init(&f->list_node); |
1688 | + f->table_id = table_id; |
1689 | + f->priority = priority; |
1690 | + minimatch_init(&f->match, match); |
1691 | + f->ofpacts = xmemdup(actions->data, actions->size); |
1692 | + f->ofpacts_len = actions->size; |
1693 | +- f->match_hmap_node.hash = ovn_flow_match_hash(f); |
1694 | ++ f->hash = ovn_flow_match_hash(f); |
1695 | + f->cookie = cookie; |
1696 | ++} |
1697 | ++ |
1698 | ++static struct desired_flow * |
1699 | ++desired_flow_alloc(uint8_t table_id, uint16_t priority, uint64_t cookie, |
1700 | ++ const struct match *match, const struct ofpbuf *actions) |
1701 | ++{ |
1702 | ++ struct desired_flow *f = xmalloc(sizeof *f); |
1703 | ++ ovs_list_init(&f->references); |
1704 | ++ ovs_list_init(&f->list_node); |
1705 | ++ ovs_list_init(&f->installed_ref_list_node); |
1706 | ++ f->installed_flow = NULL; |
1707 | ++ ovn_flow_init(&f->flow, table_id, priority, cookie, match, actions); |
1708 | + |
1709 | + return f; |
1710 | + } |
1711 | +@@ -1001,48 +1119,47 @@ ovn_flow_match_hash(const struct ovn_flow *f) |
1712 | + minimatch_hash(&f->match, 0)); |
1713 | + } |
1714 | + |
1715 | +-/* Duplicate an ovn_flow structure. */ |
1716 | +-static struct ovn_flow * |
1717 | +-ovn_flow_dup(struct ovn_flow *src) |
1718 | ++/* Duplicate a desired flow to an installed flow. */ |
1719 | ++static struct installed_flow * |
1720 | ++installed_flow_dup(struct desired_flow *src) |
1721 | + { |
1722 | +- struct ovn_flow *dst = xmalloc(sizeof *dst); |
1723 | +- ovs_list_init(&dst->references); |
1724 | +- dst->table_id = src->table_id; |
1725 | +- dst->priority = src->priority; |
1726 | +- minimatch_clone(&dst->match, &src->match); |
1727 | +- dst->ofpacts = xmemdup(src->ofpacts, src->ofpacts_len); |
1728 | +- dst->ofpacts_len = src->ofpacts_len; |
1729 | +- dst->match_hmap_node.hash = src->match_hmap_node.hash; |
1730 | +- dst->cookie = src->cookie; |
1731 | ++ struct installed_flow *dst = xmalloc(sizeof *dst); |
1732 | ++ ovs_list_init(&dst->desired_refs); |
1733 | ++ dst->desired_flow = NULL; |
1734 | ++ dst->flow.table_id = src->flow.table_id; |
1735 | ++ dst->flow.priority = src->flow.priority; |
1736 | ++ minimatch_clone(&dst->flow.match, &src->flow.match); |
1737 | ++ dst->flow.ofpacts = xmemdup(src->flow.ofpacts, src->flow.ofpacts_len); |
1738 | ++ dst->flow.ofpacts_len = src->flow.ofpacts_len; |
1739 | ++ dst->flow.hash = src->flow.hash; |
1740 | ++ dst->flow.cookie = src->flow.cookie; |
1741 | + return dst; |
1742 | + } |
1743 | + |
1744 | +-/* Finds and returns an ovn_flow in 'flow_table' whose key is identical to |
1745 | ++/* Finds and returns a desired_flow in 'flow_table' whose key is identical to |
1746 | + * 'target''s key, or NULL if there is none. |
1747 | + * |
1748 | + * If sb_uuid is not NULL, the function will also check if the found flow is |
1749 | +- * referenced by the sb_uuid. |
1750 | +- * |
1751 | +- * NOTE: sb_uuid can only be used for ovn_desired_flow_table lookup. */ |
1752 | +-static struct ovn_flow * |
1753 | +-ovn_flow_lookup(struct hmap *flow_table, const struct ovn_flow *target, |
1754 | +- const struct uuid *sb_uuid) |
1755 | ++ * referenced by the sb_uuid. */ |
1756 | ++static struct desired_flow * |
1757 | ++desired_flow_lookup(struct ovn_desired_flow_table *flow_table, |
1758 | ++ const struct ovn_flow *target, |
1759 | ++ const struct uuid *sb_uuid) |
1760 | + { |
1761 | +- struct ovn_flow *f; |
1762 | +- |
1763 | +- HMAP_FOR_EACH_WITH_HASH (f, match_hmap_node, target->match_hmap_node.hash, |
1764 | +- flow_table) { |
1765 | ++ struct desired_flow *d; |
1766 | ++ HMAP_FOR_EACH_WITH_HASH (d, match_hmap_node, target->hash, |
1767 | ++ &flow_table->match_flow_table) { |
1768 | ++ struct ovn_flow *f = &d->flow; |
1769 | + if (f->table_id == target->table_id |
1770 | + && f->priority == target->priority |
1771 | + && minimatch_equal(&f->match, &target->match)) { |
1772 | + if (!sb_uuid) { |
1773 | +- return f; |
1774 | ++ return d; |
1775 | + } |
1776 | +- ovs_assert(flow_table != &installed_flows); |
1777 | + struct sb_flow_ref *sfr; |
1778 | +- LIST_FOR_EACH (sfr, sb_list, &f->references) { |
1779 | ++ LIST_FOR_EACH (sfr, sb_list, &d->references) { |
1780 | + if (uuid_equals(sb_uuid, &sfr->sb_uuid)) { |
1781 | +- return f; |
1782 | ++ return d; |
1783 | + } |
1784 | + } |
1785 | + } |
1786 | +@@ -1050,6 +1167,24 @@ ovn_flow_lookup(struct hmap *flow_table, const struct ovn_flow *target, |
1787 | + return NULL; |
1788 | + } |
1789 | + |
1790 | ++/* Finds and returns an installed_flow in installed_flows whose key is |
1791 | ++ * identical to 'target''s key, or NULL if there is none. */ |
1792 | ++static struct installed_flow * |
1793 | ++installed_flow_lookup(const struct ovn_flow *target) |
1794 | ++{ |
1795 | ++ struct installed_flow *i; |
1796 | ++ HMAP_FOR_EACH_WITH_HASH (i, match_hmap_node, target->hash, |
1797 | ++ &installed_flows) { |
1798 | ++ struct ovn_flow *f = &i->flow; |
1799 | ++ if (f->table_id == target->table_id |
1800 | ++ && f->priority == target->priority |
1801 | ++ && minimatch_equal(&f->match, &target->match)) { |
1802 | ++ return i; |
1803 | ++ } |
1804 | ++ } |
1805 | ++ return NULL; |
1806 | ++} |
1807 | ++ |
1808 | + static char * |
1809 | + ovn_flow_to_string(const struct ovn_flow *f) |
1810 | + { |
1811 | +@@ -1076,17 +1211,35 @@ ovn_flow_log(const struct ovn_flow *f, const char *action) |
1812 | + } |
1813 | + |
1814 | + static void |
1815 | +-ovn_flow_destroy(struct ovn_flow *f) |
1816 | ++ovn_flow_uninit(struct ovn_flow *f) |
1817 | ++{ |
1818 | ++ minimatch_destroy(&f->match); |
1819 | ++ free(f->ofpacts); |
1820 | ++} |
1821 | ++ |
1822 | ++static void |
1823 | ++desired_flow_destroy(struct desired_flow *f) |
1824 | + { |
1825 | + if (f) { |
1826 | + ovs_assert(ovs_list_is_empty(&f->references)); |
1827 | +- minimatch_destroy(&f->match); |
1828 | +- free(f->ofpacts); |
1829 | ++ ovs_assert(!f->installed_flow); |
1830 | ++ ovn_flow_uninit(&f->flow); |
1831 | ++ free(f); |
1832 | ++ } |
1833 | ++} |
1834 | ++ |
1835 | ++static void |
1836 | ++installed_flow_destroy(struct installed_flow *f) |
1837 | ++{ |
1838 | ++ if (f) { |
1839 | ++ ovs_assert(ovs_list_is_empty(&f->desired_refs)); |
1840 | ++ ovs_assert(!f->desired_flow); |
1841 | ++ ovn_flow_uninit(&f->flow); |
1842 | + free(f); |
1843 | + } |
1844 | + } |
1845 | + |
1846 | |
1847 | +-/* Flow tables of struct ovn_flow. */ |
1848 | ++/* Desired flow table operations. */ |
1849 | + void |
1850 | + ovn_desired_flow_table_init(struct ovn_desired_flow_table *flow_table) |
1851 | + { |
1852 | +@@ -1112,13 +1265,16 @@ ovn_desired_flow_table_destroy(struct ovn_desired_flow_table *flow_table) |
1853 | + hmap_destroy(&flow_table->uuid_flow_table); |
1854 | + } |
1855 | + |
1856 | ++ |
1857 | |
1858 | ++/* Installed flow table operations. */ |
1859 | + static void |
1860 | + ovn_installed_flow_table_clear(void) |
1861 | + { |
1862 | +- struct ovn_flow *f, *next; |
1863 | ++ struct installed_flow *f, *next; |
1864 | + HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, &installed_flows) { |
1865 | + hmap_remove(&installed_flows, &f->match_hmap_node); |
1866 | +- ovn_flow_destroy(f); |
1867 | ++ unlink_all_refs_for_installed_flow(f); |
1868 | ++ installed_flow_destroy(f); |
1869 | + } |
1870 | + } |
1871 | + |
1872 | +@@ -1379,81 +1535,85 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, |
1873 | + /* Iterate through all of the installed flows. If any of them are no |
1874 | + * longer desired, delete them; if any of them should have different |
1875 | + * actions, update them. */ |
1876 | +- struct ovn_flow *i, *next; |
1877 | ++ struct installed_flow *i, *next; |
1878 | + HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, &installed_flows) { |
1879 | +- struct ovn_flow *d = ovn_flow_lookup(&flow_table->match_flow_table, |
1880 | +- i, NULL); |
1881 | ++ unlink_all_refs_for_installed_flow(i); |
1882 | ++ struct desired_flow *d = desired_flow_lookup(flow_table, &i->flow, |
1883 | ++ NULL); |
1884 | + if (!d) { |
1885 | + /* Installed flow is no longer desirable. Delete it from the |
1886 | + * switch and from installed_flows. */ |
1887 | + struct ofputil_flow_mod fm = { |
1888 | +- .match = i->match, |
1889 | +- .priority = i->priority, |
1890 | +- .table_id = i->table_id, |
1891 | ++ .match = i->flow.match, |
1892 | ++ .priority = i->flow.priority, |
1893 | ++ .table_id = i->flow.table_id, |
1894 | + .command = OFPFC_DELETE_STRICT, |
1895 | + }; |
1896 | + add_flow_mod(&fm, &msgs); |
1897 | +- ovn_flow_log(i, "removing installed"); |
1898 | ++ ovn_flow_log(&i->flow, "removing installed"); |
1899 | + |
1900 | + hmap_remove(&installed_flows, &i->match_hmap_node); |
1901 | +- ovn_flow_destroy(i); |
1902 | ++ installed_flow_destroy(i); |
1903 | + } else { |
1904 | +- if (!ofpacts_equal(i->ofpacts, i->ofpacts_len, |
1905 | +- d->ofpacts, d->ofpacts_len) || |
1906 | +- i->cookie != d->cookie) { |
1907 | ++ if (!ofpacts_equal(i->flow.ofpacts, i->flow.ofpacts_len, |
1908 | ++ d->flow.ofpacts, d->flow.ofpacts_len) || |
1909 | ++ i->flow.cookie != d->flow.cookie) { |
1910 | + /* Update actions in installed flow. */ |
1911 | + struct ofputil_flow_mod fm = { |
1912 | +- .match = i->match, |
1913 | +- .priority = i->priority, |
1914 | +- .table_id = i->table_id, |
1915 | +- .ofpacts = d->ofpacts, |
1916 | +- .ofpacts_len = d->ofpacts_len, |
1917 | ++ .match = i->flow.match, |
1918 | ++ .priority = i->flow.priority, |
1919 | ++ .table_id = i->flow.table_id, |
1920 | ++ .ofpacts = d->flow.ofpacts, |
1921 | ++ .ofpacts_len = d->flow.ofpacts_len, |
1922 | + .command = OFPFC_MODIFY_STRICT, |
1923 | + }; |
1924 | + /* Update cookie if it is changed. */ |
1925 | +- if (i->cookie != d->cookie) { |
1926 | ++ if (i->flow.cookie != d->flow.cookie) { |
1927 | + fm.modify_cookie = true; |
1928 | +- fm.new_cookie = htonll(d->cookie); |
1929 | ++ fm.new_cookie = htonll(d->flow.cookie); |
1930 | + /* Use OFPFC_ADD so that cookie can be updated. */ |
1931 | + fm.command = OFPFC_ADD, |
1932 | +- i->cookie = d->cookie; |
1933 | ++ i->flow.cookie = d->flow.cookie; |
1934 | + } |
1935 | + add_flow_mod(&fm, &msgs); |
1936 | +- ovn_flow_log(i, "updating installed"); |
1937 | ++ ovn_flow_log(&i->flow, "updating installed"); |
1938 | + |
1939 | + /* Replace 'i''s actions by 'd''s. */ |
1940 | +- free(i->ofpacts); |
1941 | +- i->ofpacts = xmemdup(d->ofpacts, d->ofpacts_len); |
1942 | +- i->ofpacts_len = d->ofpacts_len; |
1943 | ++ free(i->flow.ofpacts); |
1944 | ++ i->flow.ofpacts = xmemdup(d->flow.ofpacts, |
1945 | ++ d->flow.ofpacts_len); |
1946 | ++ i->flow.ofpacts_len = d->flow.ofpacts_len; |
1947 | + } |
1948 | ++ link_installed_to_desired(i, d); |
1949 | + |
1950 | + } |
1951 | + } |
1952 | + |
1953 | + /* Iterate through the desired flows and add those that aren't found |
1954 | + * in the installed flow table. */ |
1955 | +- struct ovn_flow *d; |
1956 | ++ struct desired_flow *d; |
1957 | + HMAP_FOR_EACH (d, match_hmap_node, &flow_table->match_flow_table) { |
1958 | +- i = ovn_flow_lookup(&installed_flows, d, NULL); |
1959 | ++ i = installed_flow_lookup(&d->flow); |
1960 | + if (!i) { |
1961 | + /* Send flow_mod to add flow. */ |
1962 | + struct ofputil_flow_mod fm = { |
1963 | +- .match = d->match, |
1964 | +- .priority = d->priority, |
1965 | +- .table_id = d->table_id, |
1966 | +- .ofpacts = d->ofpacts, |
1967 | +- .ofpacts_len = d->ofpacts_len, |
1968 | +- .new_cookie = htonll(d->cookie), |
1969 | ++ .match = d->flow.match, |
1970 | ++ .priority = d->flow.priority, |
1971 | ++ .table_id = d->flow.table_id, |
1972 | ++ .ofpacts = d->flow.ofpacts, |
1973 | ++ .ofpacts_len = d->flow.ofpacts_len, |
1974 | ++ .new_cookie = htonll(d->flow.cookie), |
1975 | + .command = OFPFC_ADD, |
1976 | + }; |
1977 | + add_flow_mod(&fm, &msgs); |
1978 | +- ovn_flow_log(d, "adding installed"); |
1979 | ++ ovn_flow_log(&d->flow, "adding installed"); |
1980 | + |
1981 | + /* Copy 'd' from 'flow_table' to installed_flows. */ |
1982 | +- struct ovn_flow *new_node = ovn_flow_dup(d); |
1983 | +- hmap_insert(&installed_flows, &new_node->match_hmap_node, |
1984 | +- new_node->match_hmap_node.hash); |
1985 | ++ i = installed_flow_dup(d); |
1986 | ++ hmap_insert(&installed_flows, &i->match_hmap_node, |
1987 | ++ i->flow.hash); |
1988 | + } |
1989 | ++ link_installed_to_desired(i, d); |
1990 | + } |
1991 | + |
1992 | + /* Iterate through the installed groups from previous runs. If they |
1993 | +-- |
1994 | +2.29.2 |
1995 | + |
1996 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-04.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-04.patch |
1997 | new file mode 100644 |
1998 | index 0000000..ec51814 |
1999 | --- /dev/null |
2000 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-04.patch |
2001 | @@ -0,0 +1,157 @@ |
2002 | +Description: Cherry-pick fix applied to master and branch-20.12 |
2003 | +Origin: https://github.com/ovn-org/ovn/commit/23063cf4178c05f5d6b3e4ec6d323ccc88df6101 |
2004 | +Applied-Upstream: commit: 23063cf4178c05f5d6b3e4ec6d323ccc88df6101 |
2005 | + |
2006 | +From 93c15acf2587d6d703ce0c05fd5500a7c1fcbb31 Mon Sep 17 00:00:00 2001 |
2007 | +From: Han Zhou <hzhou@ovn.org> |
2008 | +Date: Thu, 20 Aug 2020 21:38:30 -0700 |
2009 | +Subject: [PATCH 04/15] ofctrl.c: Refactor - move openflow msg construction to |
2010 | + functions. |
2011 | + |
2012 | +These functions will be reused in multiple places in a future patch. |
2013 | + |
2014 | +Acked-by: Mark Michelson <mmichels@redhat.com> |
2015 | +Signed-off-by: Han Zhou <hzhou@ovn.org> |
2016 | +--- |
2017 | + controller/ofctrl.c | 103 ++++++++++++++++++++++++++------------------ |
2018 | + 1 file changed, 60 insertions(+), 43 deletions(-) |
2019 | + |
2020 | +diff --git a/controller/ofctrl.c b/controller/ofctrl.c |
2021 | +index c6823e941..e94e5914e 100644 |
2022 | +--- a/controller/ofctrl.c |
2023 | ++++ b/controller/ofctrl.c |
2024 | +@@ -1418,6 +1418,63 @@ add_meter(struct ovn_extend_table_info *m_desired, |
2025 | + free(mm.meter.bands); |
2026 | + } |
2027 | + |
2028 | ++static void |
2029 | ++installed_flow_add(struct ovn_flow *d, struct ovs_list *msgs) |
2030 | ++{ |
2031 | ++ /* Send flow_mod to add flow. */ |
2032 | ++ struct ofputil_flow_mod fm = { |
2033 | ++ .match = d->match, |
2034 | ++ .priority = d->priority, |
2035 | ++ .table_id = d->table_id, |
2036 | ++ .ofpacts = d->ofpacts, |
2037 | ++ .ofpacts_len = d->ofpacts_len, |
2038 | ++ .new_cookie = htonll(d->cookie), |
2039 | ++ .command = OFPFC_ADD, |
2040 | ++ }; |
2041 | ++ add_flow_mod(&fm, msgs); |
2042 | ++} |
2043 | ++ |
2044 | ++static void |
2045 | ++installed_flow_mod(struct ovn_flow *i, struct ovn_flow *d, |
2046 | ++ struct ovs_list *msgs) |
2047 | ++{ |
2048 | ++ /* Update actions in installed flow. */ |
2049 | ++ struct ofputil_flow_mod fm = { |
2050 | ++ .match = i->match, |
2051 | ++ .priority = i->priority, |
2052 | ++ .table_id = i->table_id, |
2053 | ++ .ofpacts = d->ofpacts, |
2054 | ++ .ofpacts_len = d->ofpacts_len, |
2055 | ++ .command = OFPFC_MODIFY_STRICT, |
2056 | ++ }; |
2057 | ++ /* Update cookie if it is changed. */ |
2058 | ++ if (i->cookie != d->cookie) { |
2059 | ++ fm.modify_cookie = true; |
2060 | ++ fm.new_cookie = htonll(d->cookie); |
2061 | ++ /* Use OFPFC_ADD so that cookie can be updated. */ |
2062 | ++ fm.command = OFPFC_ADD; |
2063 | ++ } |
2064 | ++ add_flow_mod(&fm, msgs); |
2065 | ++ |
2066 | ++ /* Replace 'i''s actions and cookie by 'd''s. */ |
2067 | ++ free(i->ofpacts); |
2068 | ++ i->ofpacts = xmemdup(d->ofpacts, d->ofpacts_len); |
2069 | ++ i->ofpacts_len = d->ofpacts_len; |
2070 | ++ i->cookie = d->cookie; |
2071 | ++} |
2072 | ++ |
2073 | ++static void |
2074 | ++installed_flow_del(struct ovn_flow *i, struct ovs_list *msgs) |
2075 | ++{ |
2076 | ++ struct ofputil_flow_mod fm = { |
2077 | ++ .match = i->match, |
2078 | ++ .priority = i->priority, |
2079 | ++ .table_id = i->table_id, |
2080 | ++ .command = OFPFC_DELETE_STRICT, |
2081 | ++ }; |
2082 | ++ add_flow_mod(&fm, msgs); |
2083 | ++} |
2084 | ++ |
2085 | + /* The flow table can be updated if the connection to the switch is up and |
2086 | + * in the correct state and not backlogged with existing flow_mods. (Our |
2087 | + * criteria for being backlogged appear very conservative, but the socket |
2088 | +@@ -1543,46 +1600,16 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, |
2089 | + if (!d) { |
2090 | + /* Installed flow is no longer desirable. Delete it from the |
2091 | + * switch and from installed_flows. */ |
2092 | +- struct ofputil_flow_mod fm = { |
2093 | +- .match = i->flow.match, |
2094 | +- .priority = i->flow.priority, |
2095 | +- .table_id = i->flow.table_id, |
2096 | +- .command = OFPFC_DELETE_STRICT, |
2097 | +- }; |
2098 | +- add_flow_mod(&fm, &msgs); |
2099 | ++ installed_flow_del(&i->flow, &msgs); |
2100 | + ovn_flow_log(&i->flow, "removing installed"); |
2101 | +- |
2102 | + hmap_remove(&installed_flows, &i->match_hmap_node); |
2103 | + installed_flow_destroy(i); |
2104 | + } else { |
2105 | + if (!ofpacts_equal(i->flow.ofpacts, i->flow.ofpacts_len, |
2106 | + d->flow.ofpacts, d->flow.ofpacts_len) || |
2107 | + i->flow.cookie != d->flow.cookie) { |
2108 | +- /* Update actions in installed flow. */ |
2109 | +- struct ofputil_flow_mod fm = { |
2110 | +- .match = i->flow.match, |
2111 | +- .priority = i->flow.priority, |
2112 | +- .table_id = i->flow.table_id, |
2113 | +- .ofpacts = d->flow.ofpacts, |
2114 | +- .ofpacts_len = d->flow.ofpacts_len, |
2115 | +- .command = OFPFC_MODIFY_STRICT, |
2116 | +- }; |
2117 | +- /* Update cookie if it is changed. */ |
2118 | +- if (i->flow.cookie != d->flow.cookie) { |
2119 | +- fm.modify_cookie = true; |
2120 | +- fm.new_cookie = htonll(d->flow.cookie); |
2121 | +- /* Use OFPFC_ADD so that cookie can be updated. */ |
2122 | +- fm.command = OFPFC_ADD, |
2123 | +- i->flow.cookie = d->flow.cookie; |
2124 | +- } |
2125 | +- add_flow_mod(&fm, &msgs); |
2126 | + ovn_flow_log(&i->flow, "updating installed"); |
2127 | +- |
2128 | +- /* Replace 'i''s actions by 'd''s. */ |
2129 | +- free(i->flow.ofpacts); |
2130 | +- i->flow.ofpacts = xmemdup(d->flow.ofpacts, |
2131 | +- d->flow.ofpacts_len); |
2132 | +- i->flow.ofpacts_len = d->flow.ofpacts_len; |
2133 | ++ installed_flow_mod(&i->flow, &d->flow, &msgs); |
2134 | + } |
2135 | + link_installed_to_desired(i, d); |
2136 | + |
2137 | +@@ -1595,17 +1622,7 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, |
2138 | + HMAP_FOR_EACH (d, match_hmap_node, &flow_table->match_flow_table) { |
2139 | + i = installed_flow_lookup(&d->flow); |
2140 | + if (!i) { |
2141 | +- /* Send flow_mod to add flow. */ |
2142 | +- struct ofputil_flow_mod fm = { |
2143 | +- .match = d->flow.match, |
2144 | +- .priority = d->flow.priority, |
2145 | +- .table_id = d->flow.table_id, |
2146 | +- .ofpacts = d->flow.ofpacts, |
2147 | +- .ofpacts_len = d->flow.ofpacts_len, |
2148 | +- .new_cookie = htonll(d->flow.cookie), |
2149 | +- .command = OFPFC_ADD, |
2150 | +- }; |
2151 | +- add_flow_mod(&fm, &msgs); |
2152 | ++ installed_flow_add(&d->flow, &msgs); |
2153 | + ovn_flow_log(&d->flow, "adding installed"); |
2154 | + |
2155 | + /* Copy 'd' from 'flow_table' to installed_flows. */ |
2156 | +-- |
2157 | +2.29.2 |
2158 | + |
2159 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-05.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-05.patch |
2160 | new file mode 100644 |
2161 | index 0000000..6100fc2 |
2162 | --- /dev/null |
2163 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-05.patch |
2164 | @@ -0,0 +1,484 @@ |
2165 | +Description: Cherry-pick fix applied to master and branch-20.12 |
2166 | +Origin: https://github.com/ovn-org/ovn/commit/6f0b1e02d9ab3a94048c4818f2d382938cad4b71 |
2167 | +Applied-Upstream: commit: 6f0b1e02d9ab3a94048c4818f2d382938cad4b71 |
2168 | + |
2169 | +From 2029955c4932f5e28ed37ab6570bad6178c15054 Mon Sep 17 00:00:00 2001 |
2170 | +From: Han Zhou <hzhou@ovn.org> |
2171 | +Date: Mon, 17 Aug 2020 15:49:18 -0700 |
2172 | +Subject: [PATCH 05/15] ofctrl: Incremental processing for flow installation by |
2173 | + tracking. |
2174 | +MIME-Version: 1.0 |
2175 | +Content-Type: text/plain; charset=UTF-8 |
2176 | +Content-Transfer-Encoding: 8bit |
2177 | + |
2178 | +With incremental processing for flow computation, the bottle neck of |
2179 | +ovn-controller in large scale environment is in the flow installation |
2180 | +(ofctrl_put()), which does full comparison between the two big flow tables: the |
2181 | +installed flows and desired flows. |
2182 | + |
2183 | +This patch implements tracking desired flow changes when flows are |
2184 | +incrementally computed by I-P engine, and then incrementally processing the |
2185 | +flow installation using the tracked information in ofctrl_put(). It falls back |
2186 | +to the full comparison whenever tracking information is unavailable, e.g. when |
2187 | +I-P engine triggers full recompute. |
2188 | + |
2189 | +In ovn-scale-test with 3000 HVs and 30k lports, the end-to-end latency between |
2190 | +the moment a lflow is updated in SB DB and the moment when all the 3K HVs |
2191 | +complete OVS flow updating has reduced around 60% (from 1s to 400ms). |
2192 | + |
2193 | +Below is the perf result for a ovn-controller processing a new port-binding: |
2194 | + |
2195 | +Beore: |
2196 | ++ 96.76% 0.00% ovn-controller [unknown] [k] 0xffffffffffffffff |
2197 | ++ 90.21% 0.00% ovn-controller ovn-controller [.] main |
2198 | ++ 39.93% 1.19% ovn-controller ovn-controller [.] ofctrl_put |
2199 | ++ 31.27% 12.47% ovn-controller ovn-controller [.] ovn_flow_lookup |
2200 | ++ 22.12% 3.12% ovn-controller ovn-controller [.] encaps_run |
2201 | ++ 18.69% 2.77% ovn-controller ovn-controller [.] minimatch_equal |
2202 | ++ 17.63% 4.11% ovn-controller ovn-controller [.] patch_run |
2203 | ++ 15.91% 0.00% ovn-controller ovn-controller [.] add_bridge_mappings (inlined) |
2204 | ++ 14.03% 12.08% ovn-controller ovn-controller [.] minimask_equal |
2205 | ++ 12.41% 0.00% ovn-controller ovn-controller [.] chassis_tunnel_add (inlined) |
2206 | ++ 11.40% 0.00% ovn-controller ovn-controller [.] tunnel_add (inlined) |
2207 | + |
2208 | +After: |
2209 | ++ 94.59% 0.00% ovn-controller [unknown] [k] 0xffffffffffffffff |
2210 | ++ 83.56% 0.09% ovn-controller ovn-controller [.] main |
2211 | ++ 35.37% 3.13% ovn-controller ovn-controller [.] encaps_run |
2212 | ++ 27.54% 7.53% ovn-controller ovn-controller [.] patch_run |
2213 | ++ 24.86% 0.00% ovn-controller ovn-controller [.] add_bridge_mappings (inlined) |
2214 | ++ 20.01% 0.00% ovn-controller ovn-controller [.] chassis_tunnel_add (inlined) |
2215 | ++ 18.51% 0.00% ovn-controller ovn-controller [.] tunnel_add (inlined) |
2216 | ++ 14.08% 0.17% ovn-controller ovn-controller [.] physical_run |
2217 | ++ 11.37% 11.28% ovn-controller ovn-controller [.] next_real_row |
2218 | ++ 10.50% 2.59% ovn-controller ovn-controller [.] consider_port_binding |
2219 | +... |
2220 | ++ 2.14% 0.32% ovn-controller ovn-controller [.] ofctrl_put â–’ |
2221 | + |
2222 | +Before the optimization, ofctrl_put took 40% of CPU, and now it disappears from |
2223 | +the hot spots. |
2224 | + |
2225 | +Acked-by: Mark Michelson <mmichels@redhat.com> |
2226 | +Signed-off-by: Han Zhou <hzhou@ovn.org> |
2227 | +--- |
2228 | + controller/ofctrl.c | 289 +++++++++++++++++++++++++++++++++++--------- |
2229 | + controller/ofctrl.h | 6 +- |
2230 | + 2 files changed, 240 insertions(+), 55 deletions(-) |
2231 | + |
2232 | +diff --git a/controller/ofctrl.c b/controller/ofctrl.c |
2233 | +index e94e5914e..75b05f122 100644 |
2234 | +--- a/controller/ofctrl.c |
2235 | ++++ b/controller/ofctrl.c |
2236 | +@@ -101,6 +101,39 @@ struct ovn_flow { |
2237 | + * |
2238 | + * The links are updated whenever there is a change in desired flows, which is |
2239 | + * usually triggered by a SB data change in I-P engine. |
2240 | ++ * |
2241 | ++ * ** Tracking ** |
2242 | ++ * |
2243 | ++ * A desired flow can be tracked - listed in ovn_desired_flow_table's |
2244 | ++ * tracked_flows. |
2245 | ++ * |
2246 | ++ * Tracked flows is initially empty, and stays empty after the first run of I-P |
2247 | ++ * engine when installed flows are initially populated. After that, flow |
2248 | ++ * changes are tracked when I-P engine incrementally computes flow changes. |
2249 | ++ * Tracked flows are then processed and removed completely in ofctrl_put. |
2250 | ++ * ("processed" means OpenFlow change messages are composed and sent/queued to |
2251 | ++ * OVS, which ensures flows in OVS is always in sync (eventually) with the |
2252 | ++ * installed flows table). |
2253 | ++ * |
2254 | ++ * In case of full recompute of I-P engine, tracked flows are not |
2255 | ++ * added/removed, and ofctrl_put will not rely on tracked flows. (It is I-P |
2256 | ++ * engine's responsibility to ensure the tracked flows are cleared before |
2257 | ++ * recompute). |
2258 | ++ * |
2259 | ++ * Tracked flows can be preserved across multiple I-P engine runs - if in some |
2260 | ++ * iterations ofctrl_put() is skipped. Tracked flows are cleared only when it |
2261 | ++ * is consumed or when flow recompute happens. |
2262 | ++ * |
2263 | ++ * The "change_tracked" member of desired flow table maintains the status of |
2264 | ++ * whether flow changes are tracked or not. It is always set to true when |
2265 | ++ * ofctrl_put is completed, and transition to false whenever |
2266 | ++ * ovn_desired_flow_table_clear is called. |
2267 | ++ * |
2268 | ++ * NOTE: A tracked flow is just a reference to a desired flow, instead of a new |
2269 | ++ * copy. When a desired flow is removed and tracked, it is removed from the |
2270 | ++ * match_flow_table and uuid_flow_table indexes, and added to the tracked_flows |
2271 | ++ * list, marking is_deleted = true, but not immediately destroyed. It is |
2272 | ++ * destroyed when the tracking is processed for installed flow updates. |
2273 | + */ |
2274 | + struct desired_flow { |
2275 | + struct ovn_flow flow; |
2276 | +@@ -117,6 +150,11 @@ struct desired_flow { |
2277 | + |
2278 | + /* Node in installed_flow.desired_refs list. */ |
2279 | + struct ovs_list installed_ref_list_node; |
2280 | ++ |
2281 | ++ /* For tracking. */ |
2282 | ++ struct ovs_list track_list_node; /* node in ovn_desired_flow_table's |
2283 | ++ * tracked_flows list. */ |
2284 | ++ bool is_deleted; /* If the tracked flow is deleted. */ |
2285 | + }; |
2286 | + |
2287 | + struct sb_to_flow { |
2288 | +@@ -789,6 +827,62 @@ unlink_all_refs_for_installed_flow(struct installed_flow *i) |
2289 | + } |
2290 | + } |
2291 | + |
2292 | |
2293 | ++static void |
2294 | ++track_flow_add_or_modify(struct ovn_desired_flow_table *flow_table, |
2295 | ++ struct desired_flow *f) |
2296 | ++{ |
2297 | ++ if (!flow_table->change_tracked) { |
2298 | ++ return; |
2299 | ++ } |
2300 | ++ |
2301 | ++ /* If same node (flow adding/modifying) was tracked, remove it from |
2302 | ++ * tracking first. */ |
2303 | ++ if (!ovs_list_is_empty(&f->track_list_node)) { |
2304 | ++ ovs_list_remove(&f->track_list_node); |
2305 | ++ } |
2306 | ++ f->is_deleted = false; |
2307 | ++ ovs_list_push_back(&flow_table->tracked_flows, &f->track_list_node); |
2308 | ++ |
2309 | ++} |
2310 | ++ |
2311 | ++static void |
2312 | ++track_flow_del(struct ovn_desired_flow_table *flow_table, |
2313 | ++ struct desired_flow *f) |
2314 | ++{ |
2315 | ++ if (!flow_table->change_tracked) { |
2316 | ++ return; |
2317 | ++ } |
2318 | ++ /* If same node (flow adding/modifying) was tracked, remove it from |
2319 | ++ * tracking first. */ |
2320 | ++ if (!ovs_list_is_empty(&f->track_list_node)) { |
2321 | ++ ovs_list_remove(&f->track_list_node); |
2322 | ++ if (!f->installed_flow) { |
2323 | ++ /* If it is not installed yet, simply destroy it. */ |
2324 | ++ desired_flow_destroy(f); |
2325 | ++ return; |
2326 | ++ } |
2327 | ++ } |
2328 | ++ f->is_deleted = true; |
2329 | ++ ovs_list_push_back(&flow_table->tracked_flows, &f->track_list_node); |
2330 | ++} |
2331 | ++ |
2332 | ++/* When a desired flow is being removed, depending on "change_tracked", this |
2333 | ++ * function either unlinks a desired flow from installed flow and destroy it, |
2334 | ++ * or do nothing but track it. */ |
2335 | ++static void |
2336 | ++track_or_destroy_for_flow_del(struct ovn_desired_flow_table *flow_table, |
2337 | ++ struct desired_flow *f) |
2338 | ++{ |
2339 | ++ if (flow_table->change_tracked) { |
2340 | ++ track_flow_del(flow_table, f); |
2341 | ++ } else { |
2342 | ++ if (f->installed_flow) { |
2343 | ++ unlink_installed_to_desired(f->installed_flow, f); |
2344 | ++ } |
2345 | ++ desired_flow_destroy(f); |
2346 | ++ } |
2347 | ++} |
2348 | ++ |
2349 | |
2350 | + static struct sb_to_flow * |
2351 | + sb_to_flow_find(struct hmap *uuid_flow_table, const struct uuid *sb_uuid) |
2352 | + { |
2353 | +@@ -861,6 +955,7 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table *flow_table, |
2354 | + hmap_insert(&flow_table->match_flow_table, &f->match_hmap_node, |
2355 | + f->flow.hash); |
2356 | + link_flow_to_sb(flow_table, f, sb_uuid); |
2357 | ++ track_flow_add_or_modify(flow_table, f); |
2358 | + ovn_flow_log(&f->flow, "ofctrl_add_flow"); |
2359 | + } |
2360 | + |
2361 | +@@ -912,6 +1007,7 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, |
2362 | + f->flow.hash); |
2363 | + } |
2364 | + link_flow_to_sb(desired_flows, f, sb_uuid); |
2365 | ++ track_flow_add_or_modify(desired_flows, f); |
2366 | + |
2367 | + if (existing) { |
2368 | + ovn_flow_log(&f->flow, "ofctrl_add_or_append_flow (append)"); |
2369 | +@@ -941,10 +1037,7 @@ remove_flows_from_sb_to_flow(struct ovn_desired_flow_table *flow_table, |
2370 | + } |
2371 | + hmap_remove(&flow_table->match_flow_table, |
2372 | + &f->match_hmap_node); |
2373 | +- if (f->installed_flow) { |
2374 | +- unlink_installed_to_desired(f->installed_flow, f); |
2375 | +- } |
2376 | +- desired_flow_destroy(f); |
2377 | ++ track_or_destroy_for_flow_del(flow_table, f); |
2378 | + } |
2379 | + } |
2380 | + hmap_remove(&flow_table->uuid_flow_table, &stf->hmap_node); |
2381 | +@@ -1019,10 +1112,7 @@ flood_remove_flows_for_sb_uuid(struct ovn_desired_flow_table *flow_table, |
2382 | + * be empty in most cases. */ |
2383 | + hmap_remove(&flow_table->match_flow_table, |
2384 | + &f->match_hmap_node); |
2385 | +- if (f->installed_flow) { |
2386 | +- unlink_installed_to_desired(f->installed_flow, f); |
2387 | +- } |
2388 | +- desired_flow_destroy(f); |
2389 | ++ track_or_destroy_for_flow_del(flow_table, f); |
2390 | + } else { |
2391 | + ovs_list_insert(&to_be_removed, &f->list_node); |
2392 | + } |
2393 | +@@ -1056,10 +1146,7 @@ flood_remove_flows_for_sb_uuid(struct ovn_desired_flow_table *flow_table, |
2394 | + ovs_list_remove(&f->list_node); |
2395 | + hmap_remove(&flow_table->match_flow_table, |
2396 | + &f->match_hmap_node); |
2397 | +- if (f->installed_flow) { |
2398 | +- unlink_installed_to_desired(f->installed_flow, f); |
2399 | +- } |
2400 | +- desired_flow_destroy(f); |
2401 | ++ track_or_destroy_for_flow_del(flow_table, f); |
2402 | + } |
2403 | + |
2404 | + } |
2405 | +@@ -1105,7 +1192,9 @@ desired_flow_alloc(uint8_t table_id, uint16_t priority, uint64_t cookie, |
2406 | + ovs_list_init(&f->references); |
2407 | + ovs_list_init(&f->list_node); |
2408 | + ovs_list_init(&f->installed_ref_list_node); |
2409 | ++ ovs_list_init(&f->track_list_node); |
2410 | + f->installed_flow = NULL; |
2411 | ++ f->is_deleted = false; |
2412 | + ovn_flow_init(&f->flow, table_id, priority, cookie, match, actions); |
2413 | + |
2414 | + return f; |
2415 | +@@ -1245,11 +1334,27 @@ ovn_desired_flow_table_init(struct ovn_desired_flow_table *flow_table) |
2416 | + { |
2417 | + hmap_init(&flow_table->match_flow_table); |
2418 | + hmap_init(&flow_table->uuid_flow_table); |
2419 | ++ ovs_list_init(&flow_table->tracked_flows); |
2420 | ++ flow_table->change_tracked = false; |
2421 | + } |
2422 | + |
2423 | + void |
2424 | + ovn_desired_flow_table_clear(struct ovn_desired_flow_table *flow_table) |
2425 | + { |
2426 | ++ flow_table->change_tracked = false; |
2427 | ++ |
2428 | ++ struct desired_flow *f, *f_next; |
2429 | ++ LIST_FOR_EACH_SAFE (f, f_next, track_list_node, |
2430 | ++ &flow_table->tracked_flows) { |
2431 | ++ ovs_list_remove(&f->track_list_node); |
2432 | ++ if (f->is_deleted) { |
2433 | ++ if (f->installed_flow) { |
2434 | ++ unlink_installed_to_desired(f->installed_flow, f); |
2435 | ++ } |
2436 | ++ desired_flow_destroy(f); |
2437 | ++ } |
2438 | ++ } |
2439 | ++ |
2440 | + struct sb_to_flow *stf, *next; |
2441 | + HMAP_FOR_EACH_SAFE (stf, next, hmap_node, |
2442 | + &flow_table->uuid_flow_table) { |
2443 | +@@ -1475,6 +1580,117 @@ installed_flow_del(struct ovn_flow *i, struct ovs_list *msgs) |
2444 | + add_flow_mod(&fm, msgs); |
2445 | + } |
2446 | + |
2447 | ++static void |
2448 | ++update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table, |
2449 | ++ struct ovs_list *msgs) |
2450 | ++{ |
2451 | ++ ovs_assert(ovs_list_is_empty(&flow_table->tracked_flows)); |
2452 | ++ /* Iterate through all of the installed flows. If any of them are no |
2453 | ++ * longer desired, delete them; if any of them should have different |
2454 | ++ * actions, update them. */ |
2455 | ++ struct installed_flow *i, *next; |
2456 | ++ HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, &installed_flows) { |
2457 | ++ unlink_all_refs_for_installed_flow(i); |
2458 | ++ struct desired_flow *d = |
2459 | ++ desired_flow_lookup(flow_table, &i->flow, NULL); |
2460 | ++ if (!d) { |
2461 | ++ /* Installed flow is no longer desirable. Delete it from the |
2462 | ++ * switch and from installed_flows. */ |
2463 | ++ installed_flow_del(&i->flow, msgs); |
2464 | ++ ovn_flow_log(&i->flow, "removing installed"); |
2465 | ++ |
2466 | ++ hmap_remove(&installed_flows, &i->match_hmap_node); |
2467 | ++ installed_flow_destroy(i); |
2468 | ++ } else { |
2469 | ++ if (!ofpacts_equal(i->flow.ofpacts, i->flow.ofpacts_len, |
2470 | ++ d->flow.ofpacts, d->flow.ofpacts_len) || |
2471 | ++ i->flow.cookie != d->flow.cookie) { |
2472 | ++ installed_flow_mod(&i->flow, &d->flow, msgs); |
2473 | ++ ovn_flow_log(&i->flow, "updating installed"); |
2474 | ++ } |
2475 | ++ link_installed_to_desired(i, d); |
2476 | ++ |
2477 | ++ } |
2478 | ++ } |
2479 | ++ |
2480 | ++ /* Iterate through the desired flows and add those that aren't found |
2481 | ++ * in the installed flow table. */ |
2482 | ++ struct desired_flow *d; |
2483 | ++ HMAP_FOR_EACH (d, match_hmap_node, &flow_table->match_flow_table) { |
2484 | ++ i = installed_flow_lookup(&d->flow); |
2485 | ++ if (!i) { |
2486 | ++ ovn_flow_log(&d->flow, "adding installed"); |
2487 | ++ installed_flow_add(&d->flow, msgs); |
2488 | ++ |
2489 | ++ /* Copy 'd' from 'flow_table' to installed_flows. */ |
2490 | ++ i = installed_flow_dup(d); |
2491 | ++ hmap_insert(&installed_flows, &i->match_hmap_node, i->flow.hash); |
2492 | ++ } |
2493 | ++ link_installed_to_desired(i, d); |
2494 | ++ } |
2495 | ++} |
2496 | ++ |
2497 | ++static void |
2498 | ++update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, |
2499 | ++ struct ovs_list *msgs) |
2500 | ++{ |
2501 | ++ struct desired_flow *f, *f_next; |
2502 | ++ LIST_FOR_EACH_SAFE (f, f_next, track_list_node, |
2503 | ++ &flow_table->tracked_flows) { |
2504 | ++ ovs_list_remove(&f->track_list_node); |
2505 | ++ if (f->is_deleted) { |
2506 | ++ /* The desired flow was deleted */ |
2507 | ++ if (f->installed_flow) { |
2508 | ++ struct installed_flow *i = f->installed_flow; |
2509 | ++ unlink_installed_to_desired(i, f); |
2510 | ++ |
2511 | ++ if (!i->desired_flow) { |
2512 | ++ installed_flow_del(&i->flow, msgs); |
2513 | ++ ovn_flow_log(&i->flow, "removing installed (tracked)"); |
2514 | ++ |
2515 | ++ hmap_remove(&installed_flows, &i->match_hmap_node); |
2516 | ++ installed_flow_destroy(i); |
2517 | ++ } else { |
2518 | ++ /* There are other desired flow(s) referencing this |
2519 | ++ * installed flow, so update the OVS flow for the new |
2520 | ++ * active flow (at least the cookie will be different, |
2521 | ++ * even if the actions are the same). */ |
2522 | ++ struct desired_flow *d = i->desired_flow; |
2523 | ++ ovn_flow_log(&i->flow, "updating installed (tracked)"); |
2524 | ++ installed_flow_mod(&i->flow, &d->flow, msgs); |
2525 | ++ } |
2526 | ++ } |
2527 | ++ desired_flow_destroy(f); |
2528 | ++ } else { |
2529 | ++ /* The desired flow was added or modified. */ |
2530 | ++ struct installed_flow *i = installed_flow_lookup(&f->flow); |
2531 | ++ if (!i) { |
2532 | ++ /* Adding a new flow. */ |
2533 | ++ installed_flow_add(&f->flow, msgs); |
2534 | ++ ovn_flow_log(&f->flow, "adding installed (tracked)"); |
2535 | ++ |
2536 | ++ /* Copy 'f' from 'flow_table' to installed_flows. */ |
2537 | ++ struct installed_flow *new_node = installed_flow_dup(f); |
2538 | ++ hmap_insert(&installed_flows, &new_node->match_hmap_node, |
2539 | ++ new_node->flow.hash); |
2540 | ++ link_installed_to_desired(new_node, f); |
2541 | ++ } else if (i->desired_flow == f) { |
2542 | ++ /* The installed flow is installed for f, but f has change |
2543 | ++ * tracked, so it must have been modified. */ |
2544 | ++ ovn_flow_log(&i->flow, "updating installed (tracked)"); |
2545 | ++ installed_flow_mod(&i->flow, &f->flow, msgs); |
2546 | ++ } else { |
2547 | ++ /* Adding a new flow that conflicts with an existing installed |
2548 | ++ * flow, so just add it to the link. */ |
2549 | ++ link_installed_to_desired(i, f); |
2550 | ++ } |
2551 | ++ /* The track_list_node emptyness is used to check if the node is |
2552 | ++ * already added to track list, so initialize it again here. */ |
2553 | ++ ovs_list_init(&f->track_list_node); |
2554 | ++ } |
2555 | ++ } |
2556 | ++} |
2557 | ++ |
2558 | + /* The flow table can be updated if the connection to the switch is up and |
2559 | + * in the correct state and not backlogged with existing flow_mods. (Our |
2560 | + * criteria for being backlogged appear very conservative, but the socket |
2561 | +@@ -1589,48 +1805,10 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, |
2562 | + } |
2563 | + } |
2564 | + |
2565 | +- /* Iterate through all of the installed flows. If any of them are no |
2566 | +- * longer desired, delete them; if any of them should have different |
2567 | +- * actions, update them. */ |
2568 | +- struct installed_flow *i, *next; |
2569 | +- HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, &installed_flows) { |
2570 | +- unlink_all_refs_for_installed_flow(i); |
2571 | +- struct desired_flow *d = desired_flow_lookup(flow_table, &i->flow, |
2572 | +- NULL); |
2573 | +- if (!d) { |
2574 | +- /* Installed flow is no longer desirable. Delete it from the |
2575 | +- * switch and from installed_flows. */ |
2576 | +- installed_flow_del(&i->flow, &msgs); |
2577 | +- ovn_flow_log(&i->flow, "removing installed"); |
2578 | +- hmap_remove(&installed_flows, &i->match_hmap_node); |
2579 | +- installed_flow_destroy(i); |
2580 | +- } else { |
2581 | +- if (!ofpacts_equal(i->flow.ofpacts, i->flow.ofpacts_len, |
2582 | +- d->flow.ofpacts, d->flow.ofpacts_len) || |
2583 | +- i->flow.cookie != d->flow.cookie) { |
2584 | +- ovn_flow_log(&i->flow, "updating installed"); |
2585 | +- installed_flow_mod(&i->flow, &d->flow, &msgs); |
2586 | +- } |
2587 | +- link_installed_to_desired(i, d); |
2588 | +- |
2589 | +- } |
2590 | +- } |
2591 | +- |
2592 | +- /* Iterate through the desired flows and add those that aren't found |
2593 | +- * in the installed flow table. */ |
2594 | +- struct desired_flow *d; |
2595 | +- HMAP_FOR_EACH (d, match_hmap_node, &flow_table->match_flow_table) { |
2596 | +- i = installed_flow_lookup(&d->flow); |
2597 | +- if (!i) { |
2598 | +- installed_flow_add(&d->flow, &msgs); |
2599 | +- ovn_flow_log(&d->flow, "adding installed"); |
2600 | +- |
2601 | +- /* Copy 'd' from 'flow_table' to installed_flows. */ |
2602 | +- i = installed_flow_dup(d); |
2603 | +- hmap_insert(&installed_flows, &i->match_hmap_node, |
2604 | +- i->flow.hash); |
2605 | +- } |
2606 | +- link_installed_to_desired(i, d); |
2607 | ++ if (flow_table->change_tracked) { |
2608 | ++ update_installed_flows_by_track(flow_table, &msgs); |
2609 | ++ } else { |
2610 | ++ update_installed_flows_by_compare(flow_table, &msgs); |
2611 | + } |
2612 | + |
2613 | + /* Iterate through the installed groups from previous runs. If they |
2614 | +@@ -1744,6 +1922,9 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, |
2615 | + /* We were completely up-to-date before and still are. */ |
2616 | + cur_cfg = nb_cfg; |
2617 | + } |
2618 | ++ |
2619 | ++ flow_table->change_tracked = true; |
2620 | ++ ovs_assert(ovs_list_is_empty(&flow_table->tracked_flows)); |
2621 | + } |
2622 | + |
2623 | + /* Looks up the logical port with the name 'port_name' in 'br_int_'. If |
2624 | +diff --git a/controller/ofctrl.h b/controller/ofctrl.h |
2625 | +index 8789ce490..64b0ea5dd 100644 |
2626 | +--- a/controller/ofctrl.h |
2627 | ++++ b/controller/ofctrl.h |
2628 | +@@ -38,6 +38,11 @@ struct ovn_desired_flow_table { |
2629 | + /* SB uuid index for the cross reference nodes that link to the nodes in |
2630 | + * match_flow_table.*/ |
2631 | + struct hmap uuid_flow_table; |
2632 | ++ |
2633 | ++ /* Is flow changes tracked. */ |
2634 | ++ bool change_tracked; |
2635 | ++ /* Tracked flow changes. */ |
2636 | ++ struct ovs_list tracked_flows; |
2637 | + }; |
2638 | + |
2639 | + /* Interface for OVN main loop. */ |
2640 | +@@ -99,7 +104,6 @@ void ofctrl_flood_remove_flows(struct ovn_desired_flow_table *, |
2641 | + struct hmap *flood_remove_nodes); |
2642 | + void ofctrl_flood_remove_add_node(struct hmap *flood_remove_nodes, |
2643 | + const struct uuid *sb_uuid); |
2644 | +- |
2645 | + void ovn_desired_flow_table_init(struct ovn_desired_flow_table *); |
2646 | + void ovn_desired_flow_table_clear(struct ovn_desired_flow_table *); |
2647 | + void ovn_desired_flow_table_destroy(struct ovn_desired_flow_table *); |
2648 | +-- |
2649 | +2.29.2 |
2650 | + |
2651 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-06.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-06.patch |
2652 | new file mode 100644 |
2653 | index 0000000..2cac47a |
2654 | --- /dev/null |
2655 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-06.patch |
2656 | @@ -0,0 +1,112 @@ |
2657 | +Description: Cherry-pick fix applied to master and branch-20.12 |
2658 | +Origin: https://github.com/ovn-org/ovn/commit/f4e508dd7a6cfbfc2e3250a8c11a8d0fdc1dfdd0 |
2659 | +Applied-Upstream: commit: f4e508dd7a6cfbfc2e3250a8c11a8d0fdc1dfdd0 |
2660 | + |
2661 | +From c9d0a9e69cbbbb43512a10c1775b21e3461e57f3 Mon Sep 17 00:00:00 2001 |
2662 | +From: Han Zhou <hzhou@ovn.org> |
2663 | +Date: Thu, 20 Aug 2020 17:04:04 -0700 |
2664 | +Subject: [PATCH 06/15] ofctrl.c: Merge opposite changes of tracked flows |
2665 | + before installing. |
2666 | + |
2667 | +This patch optimizes the previous patch that incrementally processes |
2668 | +flow installation by merging the "add-after-delete" flow changes as |
2669 | +much as possible to avoid unnecessary OpenFlow updates. |
2670 | + |
2671 | +Acked-by: Mark Michelson <mmichels@redhat.com> |
2672 | +Signed-off-by: Han Zhou <hzhou@ovn.org> |
2673 | +--- |
2674 | + controller/ofctrl.c | 74 +++++++++++++++++++++++++++++++++++++++++++++ |
2675 | + 1 file changed, 74 insertions(+) |
2676 | + |
2677 | +diff --git a/controller/ofctrl.c b/controller/ofctrl.c |
2678 | +index 75b05f122..904b0cf66 100644 |
2679 | +--- a/controller/ofctrl.c |
2680 | ++++ b/controller/ofctrl.c |
2681 | +@@ -1630,10 +1630,84 @@ update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table, |
2682 | + } |
2683 | + } |
2684 | + |
2685 | ++/* Finds and returns a desired_flow in 'deleted_flows' that is exactly the |
2686 | ++ * same as 'target', including cookie and actions. |
2687 | ++ */ |
2688 | ++static struct desired_flow * |
2689 | ++deleted_flow_lookup(struct hmap *deleted_flows, struct ovn_flow *target) |
2690 | ++{ |
2691 | ++ struct desired_flow *d; |
2692 | ++ HMAP_FOR_EACH_WITH_HASH (d, match_hmap_node, target->hash, |
2693 | ++ deleted_flows) { |
2694 | ++ struct ovn_flow *f = &d->flow; |
2695 | ++ if (f->table_id == target->table_id |
2696 | ++ && f->priority == target->priority |
2697 | ++ && minimatch_equal(&f->match, &target->match) |
2698 | ++ && f->cookie == target->cookie |
2699 | ++ && ofpacts_equal(f->ofpacts, f->ofpacts_len, target->ofpacts, |
2700 | ++ target->ofpacts_len)) { |
2701 | ++ return d; |
2702 | ++ } |
2703 | ++ } |
2704 | ++ return NULL; |
2705 | ++} |
2706 | ++ |
2707 | ++/* This function scans the tracked flow changes in the order and merges "add" |
2708 | ++ * or "update" after "deleted" operations for exactly same flow (priority, |
2709 | ++ * table, match, action and cookie), to avoid unnecessary OF messages being |
2710 | ++ * sent to OVS. */ |
2711 | ++static void |
2712 | ++merge_tracked_flows(struct ovn_desired_flow_table *flow_table) |
2713 | ++{ |
2714 | ++ struct hmap deleted_flows = HMAP_INITIALIZER(&deleted_flows); |
2715 | ++ struct desired_flow *f, *next; |
2716 | ++ LIST_FOR_EACH_SAFE (f, next, track_list_node, |
2717 | ++ &flow_table->tracked_flows) { |
2718 | ++ if (f->is_deleted) { |
2719 | ++ /* reuse f->match_hmap_node field since it is already removed from |
2720 | ++ * the desired flow table's match index. */ |
2721 | ++ hmap_insert(&deleted_flows, &f->match_hmap_node, |
2722 | ++ f->flow.hash); |
2723 | ++ } else { |
2724 | ++ struct desired_flow *del_f = deleted_flow_lookup(&deleted_flows, |
2725 | ++ &f->flow); |
2726 | ++ if (!del_f) { |
2727 | ++ continue; |
2728 | ++ } |
2729 | ++ |
2730 | ++ /* del_f must have been installed, otherwise it should have been |
2731 | ++ * removed during track_flow_add_or_modify. */ |
2732 | ++ ovs_assert(del_f->installed_flow); |
2733 | ++ if (!f->installed_flow) { |
2734 | ++ /* f is not installed yet. */ |
2735 | ++ struct installed_flow *i = del_f->installed_flow; |
2736 | ++ unlink_installed_to_desired(i, del_f); |
2737 | ++ link_installed_to_desired(i, f); |
2738 | ++ } else { |
2739 | ++ /* f has been installed before, and now was updated to exact |
2740 | ++ * the same flow as del_f. */ |
2741 | ++ ovs_assert(f->installed_flow == del_f->installed_flow); |
2742 | ++ unlink_installed_to_desired(del_f->installed_flow, del_f); |
2743 | ++ } |
2744 | ++ hmap_remove(&deleted_flows, &del_f->match_hmap_node); |
2745 | ++ ovs_list_remove(&del_f->track_list_node); |
2746 | ++ desired_flow_destroy(del_f); |
2747 | ++ |
2748 | ++ ovs_list_remove(&f->track_list_node); |
2749 | ++ ovs_list_init(&f->track_list_node); |
2750 | ++ } |
2751 | ++ } |
2752 | ++ HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, &deleted_flows) { |
2753 | ++ hmap_remove(&deleted_flows, &f->match_hmap_node); |
2754 | ++ } |
2755 | ++ hmap_destroy(&deleted_flows); |
2756 | ++} |
2757 | ++ |
2758 | + static void |
2759 | + update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, |
2760 | + struct ovs_list *msgs) |
2761 | + { |
2762 | ++ merge_tracked_flows(flow_table); |
2763 | + struct desired_flow *f, *f_next; |
2764 | + LIST_FOR_EACH_SAFE (f, f_next, track_list_node, |
2765 | + &flow_table->tracked_flows) { |
2766 | +-- |
2767 | +2.29.2 |
2768 | + |
2769 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-07.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-07.patch |
2770 | new file mode 100644 |
2771 | index 0000000..c3193c8 |
2772 | --- /dev/null |
2773 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-07.patch |
2774 | @@ -0,0 +1,236 @@ |
2775 | +Description: Cherry-pick fix applied to master and branch-20.12 |
2776 | +Origin: https://github.com/ovn-org/ovn/commit/9d2e8d32fb9865513b70408a665184a67564390d |
2777 | +Applied-Upstream: commit: 9d2e8d32fb9865513b70408a665184a67564390d |
2778 | + |
2779 | +From 6e6ba2742f673caaba3c0fa868e0090ea5ed8466 Mon Sep 17 00:00:00 2001 |
2780 | +From: Han Zhou <hzhou@ovn.org> |
2781 | +Date: Thu, 1 Oct 2020 22:49:04 -0700 |
2782 | +Subject: [PATCH 07/15] ofctrl.c: Fix duplicated flow handling in I-P while |
2783 | + merging opposite changes. |
2784 | + |
2785 | +In a scenario in I-P when a desired flow is removed and an exactly same flow is |
2786 | +added back in the same iteration, the merge_tracked_flows() function will merge |
2787 | +the operation without firing any OpenFlow action. However, if there are |
2788 | +multiple desired flows (e.g. F1 and F2) matching the same installed flow, and |
2789 | +if the one being re-added (say, F1) is the one currently installed in OVS, the |
2790 | +current implementation will update the currently installed flow to F2 in the |
2791 | +data structure while the actual OVS flow is not updated (still F1). So far |
2792 | +there is still no problem, but the member desired_flow of the installed flow |
2793 | +is pointing to the wrong desired flow. |
2794 | + |
2795 | +Now there is only one place where the desired_flow member is consulted, in |
2796 | +update_installed_flows_by_track() for handling a tracked *modified* flow. In |
2797 | +reality flow modification happens only when conjunction flows need to be |
2798 | +combined, which would never happen together with the above scenario of |
2799 | +merge_tracked_flows(). So this wrong desired_flow pointer doesn't cause any |
2800 | +real problem yet. |
2801 | + |
2802 | +However, there is a place that can already utilize this active desired_flow |
2803 | +information, which is when handling a tracked flow deletion in |
2804 | +update_installed_flows_by_track(): we can check if the flow being deleted is |
2805 | +the currently active one installed in OVS. If not, we don't need to send and |
2806 | +OpenFlow modify to OVS. This optimization is added in this patch, apart from |
2807 | +fixing the problem of merge_tracked_flows(). Without the fix, this optimization |
2808 | +would cause the test case added in this patch fail. |
2809 | + |
2810 | +Fixes: f4e508dd7 ("ofctrl.c: Merge opposite changes of tracked flows before installing.") |
2811 | +Acked-by: Dumitru Ceara <dceara@redhat.com> |
2812 | +Signed-off-by: Han Zhou <hzhou@ovn.org> |
2813 | +--- |
2814 | + controller/ofctrl.c | 28 +++++++++++- |
2815 | + tests/ovn.at | 108 ++++++++++++++++++++++++++++++++++++++++++++ |
2816 | + 2 files changed, 134 insertions(+), 2 deletions(-) |
2817 | + |
2818 | +diff --git a/controller/ofctrl.c b/controller/ofctrl.c |
2819 | +index 904b0cf66..df2b2d6dc 100644 |
2820 | +--- a/controller/ofctrl.c |
2821 | ++++ b/controller/ofctrl.c |
2822 | +@@ -788,6 +788,24 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype type) |
2823 | + } |
2824 | + } |
2825 | + |
2826 | |
2827 | ++/* Returns true if a desired flow is active (the one currently installed |
2828 | ++ * among the list of desired flows that are linked to the same installed |
2829 | ++ * flow). */ |
2830 | ++static inline bool |
2831 | ++desired_flow_is_active(struct desired_flow *d) |
2832 | ++{ |
2833 | ++ return (d->installed_flow && d->installed_flow->desired_flow == d); |
2834 | ++} |
2835 | ++ |
2836 | ++/* Set a desired flow as the active one among the list of desired flows |
2837 | ++ * that are linked to the same installed flow. */ |
2838 | ++static inline void |
2839 | ++desired_flow_set_active(struct desired_flow *d) |
2840 | ++{ |
2841 | ++ ovs_assert(d->installed_flow); |
2842 | ++ d->installed_flow->desired_flow = d; |
2843 | ++} |
2844 | ++ |
2845 | + static void |
2846 | + link_installed_to_desired(struct installed_flow *i, struct desired_flow *d) |
2847 | + { |
2848 | +@@ -1678,6 +1696,8 @@ merge_tracked_flows(struct ovn_desired_flow_table *flow_table) |
2849 | + /* del_f must have been installed, otherwise it should have been |
2850 | + * removed during track_flow_add_or_modify. */ |
2851 | + ovs_assert(del_f->installed_flow); |
2852 | ++ |
2853 | ++ bool del_f_was_active = desired_flow_is_active(del_f); |
2854 | + if (!f->installed_flow) { |
2855 | + /* f is not installed yet. */ |
2856 | + struct installed_flow *i = del_f->installed_flow; |
2857 | +@@ -1689,6 +1709,9 @@ merge_tracked_flows(struct ovn_desired_flow_table *flow_table) |
2858 | + ovs_assert(f->installed_flow == del_f->installed_flow); |
2859 | + unlink_installed_to_desired(del_f->installed_flow, del_f); |
2860 | + } |
2861 | ++ if (del_f_was_active) { |
2862 | ++ desired_flow_set_active(f); |
2863 | ++ } |
2864 | + hmap_remove(&deleted_flows, &del_f->match_hmap_node); |
2865 | + ovs_list_remove(&del_f->track_list_node); |
2866 | + desired_flow_destroy(del_f); |
2867 | +@@ -1716,6 +1739,7 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, |
2868 | + /* The desired flow was deleted */ |
2869 | + if (f->installed_flow) { |
2870 | + struct installed_flow *i = f->installed_flow; |
2871 | ++ bool was_active = desired_flow_is_active(f); |
2872 | + unlink_installed_to_desired(i, f); |
2873 | + |
2874 | + if (!i->desired_flow) { |
2875 | +@@ -1724,7 +1748,7 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, |
2876 | + |
2877 | + hmap_remove(&installed_flows, &i->match_hmap_node); |
2878 | + installed_flow_destroy(i); |
2879 | +- } else { |
2880 | ++ } else if (was_active) { |
2881 | + /* There are other desired flow(s) referencing this |
2882 | + * installed flow, so update the OVS flow for the new |
2883 | + * active flow (at least the cookie will be different, |
2884 | +@@ -1748,7 +1772,7 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, |
2885 | + hmap_insert(&installed_flows, &new_node->match_hmap_node, |
2886 | + new_node->flow.hash); |
2887 | + link_installed_to_desired(new_node, f); |
2888 | +- } else if (i->desired_flow == f) { |
2889 | ++ } else if (desired_flow_is_active(f)) { |
2890 | + /* The installed flow is installed for f, but f has change |
2891 | + * tracked, so it must have been modified. */ |
2892 | + ovn_flow_log(&i->flow, "updating installed (tracked)"); |
2893 | +diff --git a/tests/ovn.at b/tests/ovn.at |
2894 | +index 62cec0871..a197eee5c 100644 |
2895 | +--- a/tests/ovn.at |
2896 | ++++ b/tests/ovn.at |
2897 | +@@ -19033,3 +19033,111 @@ OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [expected]) |
2898 | + |
2899 | + OVN_CLEANUP([hv1]) |
2900 | + AT_CLEANUP |
2901 | ++ |
2902 | ++# This test cases tests a scenario of ACL confliction with address set update. |
2903 | ++# It is to cover a corner case when flows are re-processed in the I-P |
2904 | ++# iteration, combined with the scenario of conflicting ACLs. |
2905 | ++AT_SETUP([ovn -- conflict ACLs with address set]) |
2906 | ++ovn_start |
2907 | ++ |
2908 | ++ovn-nbctl ls-add ls1 |
2909 | ++ |
2910 | ++ovn-nbctl lsp-add ls1 lsp1 \ |
2911 | ++-- lsp-set-addresses lsp1 "f0:00:00:00:00:01 10.0.0.1" |
2912 | ++ |
2913 | ++ovn-nbctl lsp-add ls1 lsp2 \ |
2914 | ++-- lsp-set-addresses lsp2 "f0:00:00:00:00:02 10.0.0.2" |
2915 | ++ |
2916 | ++net_add n1 |
2917 | ++sim_add hv1 |
2918 | ++ |
2919 | ++as hv1 |
2920 | ++ovs-vsctl add-br br-phys |
2921 | ++ovn_attach n1 br-phys 192.168.0.1 |
2922 | ++ovs-vsctl -- add-port br-int hv1-vif1 -- \ |
2923 | ++ set interface hv1-vif1 external-ids:iface-id=lsp1 \ |
2924 | ++ options:tx_pcap=hv1/vif1-tx.pcap \ |
2925 | ++ options:rxq_pcap=hv1/vif1-rx.pcap \ |
2926 | ++ ofport-request=1 |
2927 | ++ |
2928 | ++ovs-vsctl -- add-port br-int hv1-vif2 -- \ |
2929 | ++ set interface hv1-vif2 external-ids:iface-id=lsp2 \ |
2930 | ++ options:tx_pcap=hv1/vif2-tx.pcap \ |
2931 | ++ options:rxq_pcap=hv1/vif2-rx.pcap \ |
2932 | ++ ofport-request=2 |
2933 | ++ |
2934 | ++# Default drop |
2935 | ++ovn-nbctl acl-add ls1 to-lport 1000 \ |
2936 | ++'outport == "lsp1" && ip4' drop |
2937 | ++ |
2938 | ++# test_ip INPORT SRC_MAC DST_MAC SRC_IP DST_IP OUTPORT... |
2939 | ++# |
2940 | ++# This shell function causes an ip packet to be received on INPORT. |
2941 | ++# The packet's content has Ethernet destination DST and source SRC |
2942 | ++# (each exactly 12 hex digits) and Ethernet type ETHTYPE (4 hex digits). |
2943 | ++# The OUTPORTs (zero or more) list the VIFs on which the packet should |
2944 | ++# be received. INPORT and the OUTPORTs are specified as logical switch |
2945 | ++# port numbers, e.g. 11 for vif11. |
2946 | ++test_ip() { |
2947 | ++ # This packet has bad checksums but logical L3 routing doesn't check. |
2948 | ++ local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5 |
2949 | ++ local packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}\ |
2950 | ++${dst_ip}0035111100080000 |
2951 | ++ shift; shift; shift; shift; shift |
2952 | ++ as hv1 ovs-appctl netdev-dummy/receive hv1-vif$inport $packet |
2953 | ++ for outport; do |
2954 | ++ echo $packet >> $outport.expected |
2955 | ++ done |
2956 | ++} |
2957 | ++ |
2958 | ++ip_to_hex() { |
2959 | ++ printf "%02x%02x%02x%02x" "$@" |
2960 | ++} |
2961 | ++ |
2962 | ++reset_pcap_file() { |
2963 | ++ local iface=$1 |
2964 | ++ local pcap_file=$2 |
2965 | ++ ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \ |
2966 | ++options:rxq_pcap=dummy-rx.pcap |
2967 | ++ rm -f ${pcap_file}*.pcap |
2968 | ++ ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \ |
2969 | ++options:rxq_pcap=${pcap_file}-rx.pcap |
2970 | ++} |
2971 | ++ |
2972 | ++# Create an address set |
2973 | ++ovn-nbctl create Address_Set name=as1 \ |
2974 | ++addresses=\"10.0.0.2\",\"10.0.0.3\" |
2975 | ++ |
2976 | ++# Create overlapping ACLs resulting in conflict desired OVS flows |
2977 | ++# Add ACL1 uses the address set |
2978 | ++ovn-nbctl --wait=hv acl-add ls1 to-lport 1001 \ |
2979 | ++'outport == "lsp1" && ip4 && ip4.src == $as1' allow |
2980 | ++ |
2981 | ++# Add ACL2 which uses a single IP, which shouldn't take effect because |
2982 | ++# when it is added incrementally there is already a conflict one installed. |
2983 | ++ovn-nbctl --wait=hv acl-add ls1 to-lport 1001 \ |
2984 | ++'outport == "lsp1" && ip4 && ip4.src == 10.0.0.2' drop |
2985 | ++ |
2986 | ++ |
2987 | ++sip=`ip_to_hex 10 0 0 2` |
2988 | ++dip=`ip_to_hex 10 0 0 1` |
2989 | ++test_ip 2 f00000000002 f00000000001 $sip $dip 1 |
2990 | ++OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [1.expected]) |
2991 | ++ |
2992 | ++# Update the address set which causes flow reprocessing but the OVS flow |
2993 | ++# for allowing 10.0.0.2 should keep unchanged |
2994 | ++ovn-nbctl --wait=hv set Address_Set as1 addresses=\"10.0.0.2\" |
2995 | ++ |
2996 | ++test_ip 2 f00000000002 f00000000001 $sip $dip 1 |
2997 | ++OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [1.expected]) |
2998 | ++ |
2999 | ++# Delete the ACL1 that has "allow" action |
3000 | ++ovn-nbctl acl-del ls1 to-lport 1001 \ |
3001 | ++'outport == "lsp1" && ip4 && ip4.src == $as1' |
3002 | ++ |
3003 | ++# ACL2 should take effect and packet should be dropped |
3004 | ++test_ip 2 f00000000002 f00000000001 $sip $dip |
3005 | ++OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [1.expected]) |
3006 | ++ |
3007 | ++OVN_CLEANUP([hv1]) |
3008 | ++AT_CLEANUP |
3009 | +-- |
3010 | +2.29.2 |
3011 | + |
3012 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-08.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-08.patch |
3013 | new file mode 100644 |
3014 | index 0000000..dd2c3a5 |
3015 | --- /dev/null |
3016 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-08.patch |
3017 | @@ -0,0 +1,93 @@ |
3018 | +Description: Cherry-pick fix applied to master and branch-20.12 |
3019 | +Origin: https://github.com/ovn-org/ovn/commit/7cab7bd1268ba67429954da4f73de91090acf779 |
3020 | +Applied-Upstream: commit: 7cab7bd1268ba67429954da4f73de91090acf779 |
3021 | + |
3022 | +From c48afa3159c614442c0b08a9edaf3a24bab3b515 Mon Sep 17 00:00:00 2001 |
3023 | +From: Han Zhou <hzhou@ovn.org> |
3024 | +Date: Fri, 2 Oct 2020 12:47:52 -0700 |
3025 | +Subject: [PATCH 08/15] ofctrl.c: Avoid repeatedly linking an installed flow |
3026 | + and a desired flow. |
3027 | + |
3028 | +In update_installed_flows_by_compare() there are two loops. The first |
3029 | +loop iterates the installed flows and find its peer in desired flows to: |
3030 | + |
3031 | +1. uninstall flows that are not needed anymore |
3032 | + |
3033 | +2. update flows if needed |
3034 | + |
3035 | +At the same time, it links the desired flow found for the installed flow |
3036 | +which also set the desired flow as the current active installed flow. |
3037 | + |
3038 | +The second loop iterates the desired flows and find its peer in installed |
3039 | +flows to install missing flows. At the same time it will detect if there |
3040 | +are conflict desired flows matching same installed flow then just link |
3041 | +them. |
3042 | + |
3043 | +However, currently in the second loop, it blindly link the desired flows to the |
3044 | +installed flows, without checking if it is already linked in the first loop. |
3045 | +Lucky enough, this won't cause any real problem so far, because when there are |
3046 | +conflict flows, the one found in the first loop will be set as active in the |
3047 | +installed_flow, and in the function link_installed_to_desired() checks if it is |
3048 | +already the active desired flow it just does nothing but return. However, the |
3049 | +check in the link_installed_to_desired() is confusing because a desired_flow |
3050 | +may be linked to the installed_flow already but not the active flow, and the |
3051 | +check is insufficient. It should be rather an assertion and let the caller |
3052 | +ensure that a pair of desired_flow and installed_flow is never linked twice. |
3053 | + |
3054 | +For the above reason, this patch does the following changes: |
3055 | + |
3056 | +1. Removes the check in link_installed_to_desired() and convert it to an assert. |
3057 | + |
3058 | +2. Before calling link_installed_to_desired() in the above mentioned loop, |
3059 | + check if the desired flow is already installed. |
3060 | + |
3061 | +Acked-by: Dumitru Ceara <dceara@redhat.com> |
3062 | +Signed-off-by: Han Zhou <hzhou@ovn.org> |
3063 | +--- |
3064 | + controller/ofctrl.c | 19 ++++++++++++++----- |
3065 | + 1 file changed, 14 insertions(+), 5 deletions(-) |
3066 | + |
3067 | +diff --git a/controller/ofctrl.c b/controller/ofctrl.c |
3068 | +index df2b2d6dc..6192bad9b 100644 |
3069 | +--- a/controller/ofctrl.c |
3070 | ++++ b/controller/ofctrl.c |
3071 | +@@ -806,13 +806,18 @@ desired_flow_set_active(struct desired_flow *d) |
3072 | + d->installed_flow->desired_flow = d; |
3073 | + } |
3074 | + |
3075 | ++/* Adds the desired flow to the list of desired flows that have same match |
3076 | ++ * conditions as the installed flow. |
3077 | ++ * |
3078 | ++ * If the newly added desired flow is the first one in the list, it is also set |
3079 | ++ * as the active one. |
3080 | ++ * |
3081 | ++ * It is caller's responsibility to make sure the link between the pair didn't |
3082 | ++ * exist before. */ |
3083 | + static void |
3084 | + link_installed_to_desired(struct installed_flow *i, struct desired_flow *d) |
3085 | + { |
3086 | +- if (i->desired_flow == d) { |
3087 | +- return; |
3088 | +- } |
3089 | +- |
3090 | ++ ovs_assert(i->desired_flow != d); |
3091 | + if (ovs_list_is_empty(&i->desired_refs)) { |
3092 | + ovs_assert(!i->desired_flow); |
3093 | + i->desired_flow = d; |
3094 | +@@ -1643,8 +1648,12 @@ update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table, |
3095 | + /* Copy 'd' from 'flow_table' to installed_flows. */ |
3096 | + i = installed_flow_dup(d); |
3097 | + hmap_insert(&installed_flows, &i->match_hmap_node, i->flow.hash); |
3098 | ++ link_installed_to_desired(i, d); |
3099 | ++ } else if (!d->installed_flow) { |
3100 | ++ /* This is a desired_flow that conflicts with one installed |
3101 | ++ * previously but not linked yet. */ |
3102 | ++ link_installed_to_desired(i, d); |
3103 | + } |
3104 | +- link_installed_to_desired(i, d); |
3105 | + } |
3106 | + } |
3107 | + |
3108 | +-- |
3109 | +2.29.2 |
3110 | + |
3111 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-09.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-09.patch |
3112 | new file mode 100644 |
3113 | index 0000000..eb1a604 |
3114 | --- /dev/null |
3115 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-09.patch |
3116 | @@ -0,0 +1,220 @@ |
3117 | +Description: Cherry-pick fix applied to master and branch-20.12 |
3118 | +Origin: https://github.com/ovn-org/ovn/commit/dadae4f800ccb1f2759378f0bd804dd002e31605 |
3119 | +Applied-Upstream: commit: dadae4f800ccb1f2759378f0bd804dd002e31605 |
3120 | + |
3121 | +From 89773054ea87ca8477e8a4ae6daea1258cf28a6a Mon Sep 17 00:00:00 2001 |
3122 | +From: Dumitru Ceara <dceara@redhat.com> |
3123 | +Date: Sun, 11 Oct 2020 14:05:31 +0200 |
3124 | +Subject: [PATCH 09/15] ofctrl.c: Only merge actions for conjunctive flows. |
3125 | + |
3126 | +In ofctrl_add_or_append_flow() when merging flow actions make sure we only |
3127 | +do that for conjunctive flows. All other actions can not be merged with |
3128 | +action "conjunction". |
3129 | + |
3130 | +CC: Mark Michelson <mmichels@redhat.com> |
3131 | +Fixes: e659bab31a91 ("Combine conjunctions with identical matches into one flow.") |
3132 | +Signed-off-by: Dumitru Ceara <dceara@redhat.com> |
3133 | +Signed-off-by: Han Zhou <hzhou@ovn.org> |
3134 | +--- |
3135 | + controller/ofctrl.c | 124 +++++++++++++++++++++++++++++++++++--------- |
3136 | + 1 file changed, 99 insertions(+), 25 deletions(-) |
3137 | + |
3138 | +diff --git a/controller/ofctrl.c b/controller/ofctrl.c |
3139 | +index 6192bad9b..359cc6a01 100644 |
3140 | +--- a/controller/ofctrl.c |
3141 | ++++ b/controller/ofctrl.c |
3142 | +@@ -206,6 +206,9 @@ struct installed_flow { |
3143 | + struct desired_flow *desired_flow; |
3144 | + }; |
3145 | + |
3146 | ++typedef bool |
3147 | ++(*desired_flow_match_cb)(const struct desired_flow *candidate, |
3148 | ++ const void *arg); |
3149 | + static struct desired_flow *desired_flow_alloc( |
3150 | + uint8_t table_id, |
3151 | + uint16_t priority, |
3152 | +@@ -213,9 +216,15 @@ static struct desired_flow *desired_flow_alloc( |
3153 | + const struct match *match, |
3154 | + const struct ofpbuf *actions); |
3155 | + static struct desired_flow *desired_flow_lookup( |
3156 | ++ struct ovn_desired_flow_table *, |
3157 | ++ const struct ovn_flow *target); |
3158 | ++static struct desired_flow *desired_flow_lookup_check_uuid( |
3159 | + struct ovn_desired_flow_table *, |
3160 | + const struct ovn_flow *target, |
3161 | +- const struct uuid *sb_uuid); |
3162 | ++ const struct uuid *); |
3163 | ++static struct desired_flow *desired_flow_lookup_conjunctive( |
3164 | ++ struct ovn_desired_flow_table *, |
3165 | ++ const struct ovn_flow *target); |
3166 | + static void desired_flow_destroy(struct desired_flow *); |
3167 | + |
3168 | + static struct installed_flow *installed_flow_lookup( |
3169 | +@@ -806,6 +815,19 @@ desired_flow_set_active(struct desired_flow *d) |
3170 | + d->installed_flow->desired_flow = d; |
3171 | + } |
3172 | + |
3173 | ++static bool |
3174 | ++flow_action_has_conj(const struct ovn_flow *f) |
3175 | ++{ |
3176 | ++ const struct ofpact *a = NULL; |
3177 | ++ |
3178 | ++ OFPACT_FOR_EACH (a, f->ofpacts, f->ofpacts_len) { |
3179 | ++ if (a->type == OFPACT_CONJUNCTION) { |
3180 | ++ return true; |
3181 | ++ } |
3182 | ++ } |
3183 | ++ return false; |
3184 | ++} |
3185 | ++ |
3186 | + /* Adds the desired flow to the list of desired flows that have same match |
3187 | + * conditions as the installed flow. |
3188 | + * |
3189 | +@@ -962,7 +984,7 @@ ofctrl_check_and_add_flow(struct ovn_desired_flow_table *flow_table, |
3190 | + struct desired_flow *f = desired_flow_alloc(table_id, priority, cookie, |
3191 | + match, actions); |
3192 | + |
3193 | +- if (desired_flow_lookup(flow_table, &f->flow, sb_uuid)) { |
3194 | ++ if (desired_flow_lookup_check_uuid(flow_table, &f->flow, sb_uuid)) { |
3195 | + if (log_duplicate_flow) { |
3196 | + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); |
3197 | + if (!VLOG_DROP_DBG(&rl)) { |
3198 | +@@ -1002,14 +1024,15 @@ ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, |
3199 | + const struct ofpbuf *actions, |
3200 | + const struct uuid *sb_uuid) |
3201 | + { |
3202 | +- struct desired_flow *f = desired_flow_alloc(table_id, priority, cookie, |
3203 | +- match, actions); |
3204 | +- |
3205 | + struct desired_flow *existing; |
3206 | +- existing = desired_flow_lookup(desired_flows, &f->flow, NULL); |
3207 | ++ struct desired_flow *f; |
3208 | ++ |
3209 | ++ f = desired_flow_alloc(table_id, priority, cookie, match, actions); |
3210 | ++ existing = desired_flow_lookup_conjunctive(desired_flows, &f->flow); |
3211 | + if (existing) { |
3212 | +- /* There's already a flow with this particular match. Append the |
3213 | +- * action to that flow rather than adding a new flow |
3214 | ++ /* There's already a flow with this particular match and action |
3215 | ++ * 'conjunction'. Append the action to that flow rather than |
3216 | ++ * adding a new flow. |
3217 | + */ |
3218 | + uint64_t compound_stub[64 / 8]; |
3219 | + struct ofpbuf compound; |
3220 | +@@ -1248,15 +1271,11 @@ installed_flow_dup(struct desired_flow *src) |
3221 | + return dst; |
3222 | + } |
3223 | + |
3224 | +-/* Finds and returns a desired_flow in 'flow_table' whose key is identical to |
3225 | +- * 'target''s key, or NULL if there is none. |
3226 | +- * |
3227 | +- * If sb_uuid is not NULL, the function will also check if the found flow is |
3228 | +- * referenced by the sb_uuid. */ |
3229 | + static struct desired_flow * |
3230 | +-desired_flow_lookup(struct ovn_desired_flow_table *flow_table, |
3231 | +- const struct ovn_flow *target, |
3232 | +- const struct uuid *sb_uuid) |
3233 | ++desired_flow_lookup__(struct ovn_desired_flow_table *flow_table, |
3234 | ++ const struct ovn_flow *target, |
3235 | ++ desired_flow_match_cb match_cb, |
3236 | ++ const void *arg) |
3237 | + { |
3238 | + struct desired_flow *d; |
3239 | + HMAP_FOR_EACH_WITH_HASH (d, match_hmap_node, target->hash, |
3240 | +@@ -1265,20 +1284,76 @@ desired_flow_lookup(struct ovn_desired_flow_table *flow_table, |
3241 | + if (f->table_id == target->table_id |
3242 | + && f->priority == target->priority |
3243 | + && minimatch_equal(&f->match, &target->match)) { |
3244 | +- if (!sb_uuid) { |
3245 | ++ |
3246 | ++ if (!match_cb || match_cb(d, arg)) { |
3247 | + return d; |
3248 | + } |
3249 | +- struct sb_flow_ref *sfr; |
3250 | +- LIST_FOR_EACH (sfr, sb_list, &d->references) { |
3251 | +- if (uuid_equals(sb_uuid, &sfr->sb_uuid)) { |
3252 | +- return d; |
3253 | +- } |
3254 | +- } |
3255 | + } |
3256 | + } |
3257 | + return NULL; |
3258 | + } |
3259 | + |
3260 | ++/* Finds and returns a desired_flow in 'flow_table' whose key is identical to |
3261 | ++ * 'target''s key, or NULL if there is none. |
3262 | ++ */ |
3263 | ++static struct desired_flow * |
3264 | ++desired_flow_lookup(struct ovn_desired_flow_table *flow_table, |
3265 | ++ const struct ovn_flow *target) |
3266 | ++{ |
3267 | ++ return desired_flow_lookup__(flow_table, target, NULL, NULL); |
3268 | ++} |
3269 | ++ |
3270 | ++static bool |
3271 | ++flow_lookup_match_uuid_cb(const struct desired_flow *candidate, |
3272 | ++ const void *arg) |
3273 | ++{ |
3274 | ++ const struct uuid *sb_uuid = arg; |
3275 | ++ struct sb_flow_ref *sfr; |
3276 | ++ |
3277 | ++ LIST_FOR_EACH (sfr, sb_list, &candidate->references) { |
3278 | ++ if (uuid_equals(sb_uuid, &sfr->sb_uuid)) { |
3279 | ++ return true; |
3280 | ++ } |
3281 | ++ } |
3282 | ++ return false; |
3283 | ++} |
3284 | ++ |
3285 | ++/* Finds and returns a desired_flow in 'flow_table' whose key is identical to |
3286 | ++ * 'target''s key, or NULL if there is none. |
3287 | ++ * |
3288 | ++ * The function will also check if the found flow is referenced by the |
3289 | ++ * 'sb_uuid'. |
3290 | ++ */ |
3291 | ++static struct desired_flow * |
3292 | ++desired_flow_lookup_check_uuid(struct ovn_desired_flow_table *flow_table, |
3293 | ++ const struct ovn_flow *target, |
3294 | ++ const struct uuid *sb_uuid) |
3295 | ++{ |
3296 | ++ return desired_flow_lookup__(flow_table, target, flow_lookup_match_uuid_cb, |
3297 | ++ sb_uuid); |
3298 | ++} |
3299 | ++ |
3300 | ++static bool |
3301 | ++flow_lookup_match_conj_cb(const struct desired_flow *candidate, |
3302 | ++ const void *arg OVS_UNUSED) |
3303 | ++{ |
3304 | ++ return flow_action_has_conj(&candidate->flow); |
3305 | ++} |
3306 | ++ |
3307 | ++/* Finds and returns a desired_flow in 'flow_table' whose key is identical to |
3308 | ++ * 'target''s key, or NULL if there is none. |
3309 | ++ * |
3310 | ++ * The function will only return a matching flow if it contains action |
3311 | ++ * 'conjunction'. |
3312 | ++ */ |
3313 | ++static struct desired_flow * |
3314 | ++desired_flow_lookup_conjunctive(struct ovn_desired_flow_table *flow_table, |
3315 | ++ const struct ovn_flow *target) |
3316 | ++{ |
3317 | ++ return desired_flow_lookup__(flow_table, target, flow_lookup_match_conj_cb, |
3318 | ++ NULL); |
3319 | ++} |
3320 | ++ |
3321 | + /* Finds and returns an installed_flow in installed_flows whose key is |
3322 | + * identical to 'target''s key, or NULL if there is none. */ |
3323 | + static struct installed_flow * |
3324 | +@@ -1614,8 +1689,7 @@ update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table, |
3325 | + struct installed_flow *i, *next; |
3326 | + HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, &installed_flows) { |
3327 | + unlink_all_refs_for_installed_flow(i); |
3328 | +- struct desired_flow *d = |
3329 | +- desired_flow_lookup(flow_table, &i->flow, NULL); |
3330 | ++ struct desired_flow *d = desired_flow_lookup(flow_table, &i->flow); |
3331 | + if (!d) { |
3332 | + /* Installed flow is no longer desirable. Delete it from the |
3333 | + * switch and from installed_flows. */ |
3334 | +-- |
3335 | +2.29.2 |
3336 | + |
3337 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-10.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-10.patch |
3338 | new file mode 100644 |
3339 | index 0000000..6e5fdde |
3340 | --- /dev/null |
3341 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-10.patch |
3342 | @@ -0,0 +1,80 @@ |
3343 | +Description: Cherry-pick fix applied to master and branch-20.12 |
3344 | +Origin: https://github.com/ovn-org/ovn/commit/e49ce9a33f38f29c44e3c30afcc189b5f6a9ef8e |
3345 | +Applied-Upstream: commit: e49ce9a33f38f29c44e3c30afcc189b5f6a9ef8e |
3346 | + |
3347 | +From 5a553b6da28d99d37a27f7e055aa6895184e0520 Mon Sep 17 00:00:00 2001 |
3348 | +From: Dumitru Ceara <dceara@redhat.com> |
3349 | +Date: Sun, 11 Oct 2020 14:05:45 +0200 |
3350 | +Subject: [PATCH 10/15] ofctrl.c: Do not change flow ordering when merging |
3351 | + opposite changes. |
3352 | + |
3353 | +Instead of removing the old desired flow from the list and inserting the new |
3354 | +one at the end we now directly replace the old flow with the new one while |
3355 | +maintaining the same ordering. |
3356 | + |
3357 | +For now order of the flows is not relevant but further commits require |
3358 | +maintaining the order of desired flows. |
3359 | + |
3360 | +Signed-off-by: Dumitru Ceara <dceara@redhat.com> |
3361 | +Signed-off-by: Han Zhou <hzhou@ovn.org> |
3362 | +--- |
3363 | + controller/ofctrl.c | 28 +++++++++++++++++++++------- |
3364 | + 1 file changed, 21 insertions(+), 7 deletions(-) |
3365 | + |
3366 | +diff --git a/controller/ofctrl.c b/controller/ofctrl.c |
3367 | +index 359cc6a01..87e154aff 100644 |
3368 | +--- a/controller/ofctrl.c |
3369 | ++++ b/controller/ofctrl.c |
3370 | +@@ -848,6 +848,26 @@ link_installed_to_desired(struct installed_flow *i, struct desired_flow *d) |
3371 | + d->installed_flow = i; |
3372 | + } |
3373 | + |
3374 | ++/* Replaces 'old_desired' with 'new_desired' in the list of desired flows |
3375 | ++ * that have same match conditions as the installed flow. |
3376 | ++ * |
3377 | ++ * If 'old_desired' was the active flow, 'new_desired' becomes the active one. |
3378 | ++ */ |
3379 | ++static void |
3380 | ++replace_installed_to_desired(struct installed_flow *i, |
3381 | ++ struct desired_flow *old_desired, |
3382 | ++ struct desired_flow *new_desired) |
3383 | ++{ |
3384 | ++ ovs_assert(old_desired->installed_flow == i); |
3385 | ++ ovs_list_replace(&new_desired->installed_ref_list_node, |
3386 | ++ &old_desired->installed_ref_list_node); |
3387 | ++ old_desired->installed_flow = NULL; |
3388 | ++ new_desired->installed_flow = i; |
3389 | ++ if (i->desired_flow == old_desired) { |
3390 | ++ i->desired_flow = new_desired; |
3391 | ++ } |
3392 | ++} |
3393 | ++ |
3394 | + static void |
3395 | + unlink_installed_to_desired(struct installed_flow *i, struct desired_flow *d) |
3396 | + { |
3397 | +@@ -1780,21 +1800,15 @@ merge_tracked_flows(struct ovn_desired_flow_table *flow_table) |
3398 | + * removed during track_flow_add_or_modify. */ |
3399 | + ovs_assert(del_f->installed_flow); |
3400 | + |
3401 | +- bool del_f_was_active = desired_flow_is_active(del_f); |
3402 | + if (!f->installed_flow) { |
3403 | + /* f is not installed yet. */ |
3404 | +- struct installed_flow *i = del_f->installed_flow; |
3405 | +- unlink_installed_to_desired(i, del_f); |
3406 | +- link_installed_to_desired(i, f); |
3407 | ++ replace_installed_to_desired(del_f->installed_flow, del_f, f); |
3408 | + } else { |
3409 | + /* f has been installed before, and now was updated to exact |
3410 | + * the same flow as del_f. */ |
3411 | + ovs_assert(f->installed_flow == del_f->installed_flow); |
3412 | + unlink_installed_to_desired(del_f->installed_flow, del_f); |
3413 | + } |
3414 | +- if (del_f_was_active) { |
3415 | +- desired_flow_set_active(f); |
3416 | +- } |
3417 | + hmap_remove(&deleted_flows, &del_f->match_hmap_node); |
3418 | + ovs_list_remove(&del_f->track_list_node); |
3419 | + desired_flow_destroy(del_f); |
3420 | +-- |
3421 | +2.29.2 |
3422 | + |
3423 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-11.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-11.patch |
3424 | new file mode 100644 |
3425 | index 0000000..52155e4 |
3426 | --- /dev/null |
3427 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-11.patch |
3428 | @@ -0,0 +1,218 @@ |
3429 | +Description: Cherry-pick fix applied to master and branch-20.12 |
3430 | +Origin: https://github.com/ovn-org/ovn/commit/107bb25029350bd0f7dfeeb0ef3053adbd504e3e |
3431 | +Applied-Upstream: commit: 107bb25029350bd0f7dfeeb0ef3053adbd504e3e |
3432 | + |
3433 | +From f170c35c9de7af2b97764e4fa6f4bb2767f5fed9 Mon Sep 17 00:00:00 2001 |
3434 | +From: Dumitru Ceara <dceara@redhat.com> |
3435 | +Date: Sun, 11 Oct 2020 14:05:59 +0200 |
3436 | +Subject: [PATCH 11/15] ofctrl.c: Simplify active desired flow selection. |
3437 | + |
3438 | +Always consider the first "desired flow" in the list of flows that refer an |
3439 | +"installed flow" as the active flow. This simplifies the logic of the flow |
3440 | +update code and is used in a further commit to implement a partial ordering |
3441 | +of desired flows within installed flows. |
3442 | + |
3443 | +Signed-off-by: Dumitru Ceara <dceara@redhat.com> |
3444 | +Signed-off-by: Han Zhou <hzhou@ovn.org> |
3445 | +--- |
3446 | + controller/ofctrl.c | 91 ++++++++++++++++++--------------------------- |
3447 | + 1 file changed, 36 insertions(+), 55 deletions(-) |
3448 | + |
3449 | +diff --git a/controller/ofctrl.c b/controller/ofctrl.c |
3450 | +index 87e154aff..6e2f55e05 100644 |
3451 | +--- a/controller/ofctrl.c |
3452 | ++++ b/controller/ofctrl.c |
3453 | +@@ -188,6 +188,8 @@ struct sb_flow_ref { |
3454 | + * relationship is 1 to N. A link is added when a flow addition is processed. |
3455 | + * A link is removed when a flow deletion is processed, the desired flow |
3456 | + * table is cleared, or the installed flow table is cleared. |
3457 | ++ * The first desired_flow in the list is the active one, the one that is |
3458 | ++ * actually installed. |
3459 | + */ |
3460 | + struct installed_flow { |
3461 | + struct ovn_flow flow; |
3462 | +@@ -199,11 +201,6 @@ struct installed_flow { |
3463 | + * installed flow, e.g. when there are conflict/duplicated ACLs that |
3464 | + * generates same match conditions). */ |
3465 | + struct ovs_list desired_refs; |
3466 | +- |
3467 | +- /* The corresponding flow in desired table. It must be one of the flows in |
3468 | +- * desired_refs list. If there are more than one flows in references list, |
3469 | +- * this is the one that is actually installed. */ |
3470 | +- struct desired_flow *desired_flow; |
3471 | + }; |
3472 | + |
3473 | + typedef bool |
3474 | +@@ -231,6 +228,7 @@ static struct installed_flow *installed_flow_lookup( |
3475 | + const struct ovn_flow *target); |
3476 | + static void installed_flow_destroy(struct installed_flow *); |
3477 | + static struct installed_flow *installed_flow_dup(struct desired_flow *); |
3478 | ++static struct desired_flow *installed_flow_get_active(struct installed_flow *); |
3479 | + |
3480 | + static uint32_t ovn_flow_match_hash(const struct ovn_flow *); |
3481 | + static char *ovn_flow_to_string(const struct ovn_flow *); |
3482 | +@@ -796,24 +794,6 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype type) |
3483 | + log_openflow_rl(&rl, VLL_DBG, oh, "OpenFlow packet ignored"); |
3484 | + } |
3485 | + } |
3486 | +- |
3487 | |
3488 | +-/* Returns true if a desired flow is active (the one currently installed |
3489 | +- * among the list of desired flows that are linked to the same installed |
3490 | +- * flow). */ |
3491 | +-static inline bool |
3492 | +-desired_flow_is_active(struct desired_flow *d) |
3493 | +-{ |
3494 | +- return (d->installed_flow && d->installed_flow->desired_flow == d); |
3495 | +-} |
3496 | +- |
3497 | +-/* Set a desired flow as the active one among the list of desired flows |
3498 | +- * that are linked to the same installed flow. */ |
3499 | +-static inline void |
3500 | +-desired_flow_set_active(struct desired_flow *d) |
3501 | +-{ |
3502 | +- ovs_assert(d->installed_flow); |
3503 | +- d->installed_flow->desired_flow = d; |
3504 | +-} |
3505 | + |
3506 | + static bool |
3507 | + flow_action_has_conj(const struct ovn_flow *f) |
3508 | +@@ -831,27 +811,22 @@ flow_action_has_conj(const struct ovn_flow *f) |
3509 | + /* Adds the desired flow to the list of desired flows that have same match |
3510 | + * conditions as the installed flow. |
3511 | + * |
3512 | +- * If the newly added desired flow is the first one in the list, it is also set |
3513 | +- * as the active one. |
3514 | +- * |
3515 | + * It is caller's responsibility to make sure the link between the pair didn't |
3516 | +- * exist before. */ |
3517 | +-static void |
3518 | ++ * exist before. |
3519 | ++ * |
3520 | ++ * Returns true if the newly added desired flow is selected to be the active |
3521 | ++ * one. |
3522 | ++ */ |
3523 | ++static bool |
3524 | + link_installed_to_desired(struct installed_flow *i, struct desired_flow *d) |
3525 | + { |
3526 | +- ovs_assert(i->desired_flow != d); |
3527 | +- if (ovs_list_is_empty(&i->desired_refs)) { |
3528 | +- ovs_assert(!i->desired_flow); |
3529 | +- i->desired_flow = d; |
3530 | +- } |
3531 | +- ovs_list_insert(&i->desired_refs, &d->installed_ref_list_node); |
3532 | + d->installed_flow = i; |
3533 | ++ ovs_list_push_back(&i->desired_refs, &d->installed_ref_list_node); |
3534 | ++ return installed_flow_get_active(i) == d; |
3535 | + } |
3536 | + |
3537 | + /* Replaces 'old_desired' with 'new_desired' in the list of desired flows |
3538 | + * that have same match conditions as the installed flow. |
3539 | +- * |
3540 | +- * If 'old_desired' was the active flow, 'new_desired' becomes the active one. |
3541 | + */ |
3542 | + static void |
3543 | + replace_installed_to_desired(struct installed_flow *i, |
3544 | +@@ -863,24 +838,22 @@ replace_installed_to_desired(struct installed_flow *i, |
3545 | + &old_desired->installed_ref_list_node); |
3546 | + old_desired->installed_flow = NULL; |
3547 | + new_desired->installed_flow = i; |
3548 | +- if (i->desired_flow == old_desired) { |
3549 | +- i->desired_flow = new_desired; |
3550 | +- } |
3551 | + } |
3552 | + |
3553 | +-static void |
3554 | ++/* Removes the desired flow from the list of desired flows that have the same |
3555 | ++ * match conditions as the installed flow. |
3556 | ++ * |
3557 | ++ * Returns true if the desired flow was the previously active flow. |
3558 | ++ */ |
3559 | ++static bool |
3560 | + unlink_installed_to_desired(struct installed_flow *i, struct desired_flow *d) |
3561 | + { |
3562 | +- ovs_assert(i && i->desired_flow && !ovs_list_is_empty(&i->desired_refs)); |
3563 | ++ struct desired_flow *old_active = installed_flow_get_active(i); |
3564 | ++ |
3565 | + ovs_assert(d && d->installed_flow == i); |
3566 | + ovs_list_remove(&d->installed_ref_list_node); |
3567 | + d->installed_flow = NULL; |
3568 | +- if (i->desired_flow == d) { |
3569 | +- i->desired_flow = ovs_list_is_empty(&i->desired_refs) ? NULL : |
3570 | +- CONTAINER_OF(ovs_list_front(&i->desired_refs), |
3571 | +- struct desired_flow, |
3572 | +- installed_ref_list_node); |
3573 | +- } |
3574 | ++ return old_active == d; |
3575 | + } |
3576 | + |
3577 | + static void |
3578 | +@@ -1280,7 +1253,6 @@ installed_flow_dup(struct desired_flow *src) |
3579 | + { |
3580 | + struct installed_flow *dst = xmalloc(sizeof *dst); |
3581 | + ovs_list_init(&dst->desired_refs); |
3582 | +- dst->desired_flow = NULL; |
3583 | + dst->flow.table_id = src->flow.table_id; |
3584 | + dst->flow.priority = src->flow.priority; |
3585 | + minimatch_clone(&dst->flow.match, &src->flow.match); |
3586 | +@@ -1291,6 +1263,17 @@ installed_flow_dup(struct desired_flow *src) |
3587 | + return dst; |
3588 | + } |
3589 | + |
3590 | ++static struct desired_flow * |
3591 | ++installed_flow_get_active(struct installed_flow *f) |
3592 | ++{ |
3593 | ++ if (!ovs_list_is_empty(&f->desired_refs)) { |
3594 | ++ return CONTAINER_OF(ovs_list_front(&f->desired_refs), |
3595 | ++ struct desired_flow, |
3596 | ++ installed_ref_list_node); |
3597 | ++ } |
3598 | ++ return NULL; |
3599 | ++} |
3600 | ++ |
3601 | + static struct desired_flow * |
3602 | + desired_flow_lookup__(struct ovn_desired_flow_table *flow_table, |
3603 | + const struct ovn_flow *target, |
3604 | +@@ -1439,8 +1422,7 @@ static void |
3605 | + installed_flow_destroy(struct installed_flow *f) |
3606 | + { |
3607 | + if (f) { |
3608 | +- ovs_assert(ovs_list_is_empty(&f->desired_refs)); |
3609 | +- ovs_assert(!f->desired_flow); |
3610 | ++ ovs_assert(!installed_flow_get_active(f)); |
3611 | + ovn_flow_uninit(&f->flow); |
3612 | + free(f); |
3613 | + } |
3614 | +@@ -1836,10 +1818,10 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, |
3615 | + /* The desired flow was deleted */ |
3616 | + if (f->installed_flow) { |
3617 | + struct installed_flow *i = f->installed_flow; |
3618 | +- bool was_active = desired_flow_is_active(f); |
3619 | +- unlink_installed_to_desired(i, f); |
3620 | ++ bool was_active = unlink_installed_to_desired(i, f); |
3621 | ++ struct desired_flow *d = installed_flow_get_active(i); |
3622 | + |
3623 | +- if (!i->desired_flow) { |
3624 | ++ if (!d) { |
3625 | + installed_flow_del(&i->flow, msgs); |
3626 | + ovn_flow_log(&i->flow, "removing installed (tracked)"); |
3627 | + |
3628 | +@@ -1850,7 +1832,6 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, |
3629 | + * installed flow, so update the OVS flow for the new |
3630 | + * active flow (at least the cookie will be different, |
3631 | + * even if the actions are the same). */ |
3632 | +- struct desired_flow *d = i->desired_flow; |
3633 | + ovn_flow_log(&i->flow, "updating installed (tracked)"); |
3634 | + installed_flow_mod(&i->flow, &d->flow, msgs); |
3635 | + } |
3636 | +@@ -1869,7 +1850,7 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, |
3637 | + hmap_insert(&installed_flows, &new_node->match_hmap_node, |
3638 | + new_node->flow.hash); |
3639 | + link_installed_to_desired(new_node, f); |
3640 | +- } else if (desired_flow_is_active(f)) { |
3641 | ++ } else if (installed_flow_get_active(i) == f) { |
3642 | + /* The installed flow is installed for f, but f has change |
3643 | + * tracked, so it must have been modified. */ |
3644 | + ovn_flow_log(&i->flow, "updating installed (tracked)"); |
3645 | +-- |
3646 | +2.29.2 |
3647 | + |
3648 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-12.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-12.patch |
3649 | new file mode 100644 |
3650 | index 0000000..ad07fab |
3651 | --- /dev/null |
3652 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-12.patch |
3653 | @@ -0,0 +1,43 @@ |
3654 | +Description: Cherry-pick fix applied to master and branch-20.12 |
3655 | +Origin: https://github.com/ovn-org/ovn/commit/33c15c145988daa6172928dc870f3a0225515f50 |
3656 | +Applied-Upstream: commit: 33c15c145988daa6172928dc870f3a0225515f50 |
3657 | + |
3658 | +From 063c1fc0858c462719b2d3b33b3fcfdef18bb9a5 Mon Sep 17 00:00:00 2001 |
3659 | +From: Dumitru Ceara <dceara@redhat.com> |
3660 | +Date: Tue, 13 Oct 2020 11:04:03 +0200 |
3661 | +Subject: [PATCH 12/15] ofctrl.c: Always log the most recent flow changes. |
3662 | + |
3663 | +Fixes: 6f0b1e02d9ab ("ofctrl: Incremental processing for flow installation by tracking.") |
3664 | +Signed-off-by: Dumitru Ceara <dceara@redhat.com> |
3665 | +Signed-off-by: Han Zhou <hzhou@ovn.org> |
3666 | +--- |
3667 | + controller/ofctrl.c | 4 ++-- |
3668 | + 1 file changed, 2 insertions(+), 2 deletions(-) |
3669 | + |
3670 | +diff --git a/controller/ofctrl.c b/controller/ofctrl.c |
3671 | +index 6e2f55e05..05a5644ac 100644 |
3672 | +--- a/controller/ofctrl.c |
3673 | ++++ b/controller/ofctrl.c |
3674 | +@@ -1832,8 +1832,8 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, |
3675 | + * installed flow, so update the OVS flow for the new |
3676 | + * active flow (at least the cookie will be different, |
3677 | + * even if the actions are the same). */ |
3678 | +- ovn_flow_log(&i->flow, "updating installed (tracked)"); |
3679 | + installed_flow_mod(&i->flow, &d->flow, msgs); |
3680 | ++ ovn_flow_log(&i->flow, "updating installed (tracked)"); |
3681 | + } |
3682 | + } |
3683 | + desired_flow_destroy(f); |
3684 | +@@ -1853,8 +1853,8 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, |
3685 | + } else if (installed_flow_get_active(i) == f) { |
3686 | + /* The installed flow is installed for f, but f has change |
3687 | + * tracked, so it must have been modified. */ |
3688 | +- ovn_flow_log(&i->flow, "updating installed (tracked)"); |
3689 | + installed_flow_mod(&i->flow, &f->flow, msgs); |
3690 | ++ ovn_flow_log(&i->flow, "updating installed (tracked)"); |
3691 | + } else { |
3692 | + /* Adding a new flow that conflicts with an existing installed |
3693 | + * flow, so just add it to the link. */ |
3694 | +-- |
3695 | +2.29.2 |
3696 | + |
3697 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-13.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-13.patch |
3698 | new file mode 100644 |
3699 | index 0000000..266c5af |
3700 | --- /dev/null |
3701 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-13.patch |
3702 | @@ -0,0 +1,419 @@ |
3703 | +Description: Cherry-pick fix applied to master and branch-20.12 |
3704 | +Origin: https://github.com/ovn-org/ovn/commit/986b3d5e4ad6f05245d021ba699c957246294a22 |
3705 | +Applied-Upstream: commit: 986b3d5e4ad6f05245d021ba699c957246294a22 |
3706 | + |
3707 | +From 7f27cf96b8d16c5dd960ceb620a7269a66bc9cc4 Mon Sep 17 00:00:00 2001 |
3708 | +From: Dumitru Ceara <dceara@redhat.com> |
3709 | +Date: Thu, 15 Oct 2020 11:12:30 +0200 |
3710 | +Subject: [PATCH 13/15] ofctrl.c: Add a predictable resolution for conflicting |
3711 | + flow actions. |
3712 | + |
3713 | +Until now, in case the ACL configuration generates openflows that have |
3714 | +the same match but different actions, ovn-controller was using the |
3715 | +following approach: |
3716 | +1. If the flow being added contains conjunctive actions, merge its |
3717 | + actions with the already existing flow. |
3718 | +2. Otherwise, if the flow is being added incrementally |
3719 | + (update_installed_flows_by_track), don't install the new flow but |
3720 | + instead keep the old one. |
3721 | +3. Otherwise, (update_installed_flows_by_compare), don't install the |
3722 | + new flow but instead keep the old one. |
3723 | + |
3724 | +Even though one can argue that having an ACL with a match that includes |
3725 | +the match of another ACL is a misconfiguration, it can happen that the |
3726 | +users provision OVN like this. Depending on the order of reading and |
3727 | +installing the logical flows, the above operations can yield |
3728 | +unpredictable results, e.g., allow specific traffic but then after |
3729 | +ovn-controller is restarted (or a recompute happens) that specific |
3730 | +traffic starts being dropped. |
3731 | + |
3732 | +A simple example of ACL configuration is: |
3733 | +ovn-nbctl acl-add ls to-lport 3 '(ip4.src==10.0.0.1 || |
3734 | +ip4.src==10.0.0.2) && (ip4.dst == 10.0.0.3 || ip4.dst == 10.0.0.4)' allow |
3735 | +ovn-nbctl acl-add ls to-lport 3 'ip4.src==10.0.0.1' allow |
3736 | +ovn-nbctl acl-add ls to-lport 2 'arp' allow |
3737 | +ovn-nbctl acl-add ls to-lport 1 'ip4' drop |
3738 | + |
3739 | +This is a pattern used by most CMSs: |
3740 | +- define a default deny policy. |
3741 | +- punch holes in the default deny policy based on user specific |
3742 | + configurations. |
3743 | + |
3744 | +Without this commit the behavior for traffic from 10.0.0.1 to 10.0.0.5 |
3745 | +is unpredictable. Depending on the order of operations traffic might be |
3746 | +dropped or allowed. |
3747 | + |
3748 | +It's also quite hard to force the CMS to ensure that such match overlaps |
3749 | +never occur. |
3750 | + |
3751 | +To address this issue we now ensure that all desired flows refering the |
3752 | +same installed flow are partially sorted in the following way: |
3753 | +- first all flows with action "allow". |
3754 | +- then all flows with action "drop". |
3755 | +- then a single flow with action "conjunction" (resulting from merging |
3756 | + all flows with the same match and action conjunction). |
3757 | + |
3758 | +This ensures that "allow" flows have precedence over "drop" flows which |
3759 | +in turn have precedence over "conjunction" flows. Essentially less |
3760 | +restrictive flows are always preferred over more restrictive flows whenever a match |
3761 | +conflict happens. |
3762 | + |
3763 | +CC: Daniel Alvarez <dalvarez@redhat.com> |
3764 | +Reported-at: https://bugzilla.redhat.com/1871931 |
3765 | +Signed-off-by: Dumitru Ceara <dceara@redhat.com> |
3766 | +Acked-by: Mark Gray <mark.d.gray@redhat.com> |
3767 | +Signed-off-by: Han Zhou <hzhou@ovn.org> |
3768 | +--- |
3769 | + controller/ofctrl.c | 74 +++++++++++++-- |
3770 | + tests/ovn.at | 214 ++++++++++++++++++++++++++++++++++++++++++++ |
3771 | + 2 files changed, 283 insertions(+), 5 deletions(-) |
3772 | + |
3773 | +diff --git a/controller/ofctrl.c b/controller/ofctrl.c |
3774 | +index 05a5644ac..48c05b2cc 100644 |
3775 | +--- a/controller/ofctrl.c |
3776 | ++++ b/controller/ofctrl.c |
3777 | +@@ -188,6 +188,14 @@ struct sb_flow_ref { |
3778 | + * relationship is 1 to N. A link is added when a flow addition is processed. |
3779 | + * A link is removed when a flow deletion is processed, the desired flow |
3780 | + * table is cleared, or the installed flow table is cleared. |
3781 | ++ * |
3782 | ++ * To ensure predictable behavior, the list of desired flows is maintained |
3783 | ++ * partially sorted in the following way (from least restrictive to most |
3784 | ++ * restrictive wrt. match): |
3785 | ++ * - allow flows without action conjunction. |
3786 | ++ * - drop flows without action conjunction. |
3787 | ++ * - a single flow with action conjunction. |
3788 | ++ * |
3789 | + * The first desired_flow in the list is the active one, the one that is |
3790 | + * actually installed. |
3791 | + */ |
3792 | +@@ -795,6 +803,12 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype type) |
3793 | + } |
3794 | + } |
3795 | + |
3796 | ++static bool |
3797 | ++flow_action_has_drop(const struct ovn_flow *f) |
3798 | ++{ |
3799 | ++ return f->ofpacts_len == 0; |
3800 | ++} |
3801 | ++ |
3802 | + static bool |
3803 | + flow_action_has_conj(const struct ovn_flow *f) |
3804 | + { |
3805 | +@@ -808,6 +822,33 @@ flow_action_has_conj(const struct ovn_flow *f) |
3806 | + return false; |
3807 | + } |
3808 | + |
3809 | ++static bool |
3810 | ++flow_action_has_allow(const struct ovn_flow *f) |
3811 | ++{ |
3812 | ++ return !flow_action_has_drop(f) && !flow_action_has_conj(f); |
3813 | ++} |
3814 | ++ |
3815 | ++/* Returns true if flow 'a' is preferred over flow 'b'. */ |
3816 | ++static bool |
3817 | ++flow_is_preferred(const struct ovn_flow *a, const struct ovn_flow *b) |
3818 | ++{ |
3819 | ++ if (flow_action_has_allow(b)) { |
3820 | ++ return false; |
3821 | ++ } |
3822 | ++ if (flow_action_has_allow(a)) { |
3823 | ++ return true; |
3824 | ++ } |
3825 | ++ if (flow_action_has_drop(b)) { |
3826 | ++ return false; |
3827 | ++ } |
3828 | ++ if (flow_action_has_drop(a)) { |
3829 | ++ return true; |
3830 | ++ } |
3831 | ++ |
3832 | ++ /* Flows 'a' and 'b' should never both have action conjunction. */ |
3833 | ++ OVS_NOT_REACHED(); |
3834 | ++} |
3835 | ++ |
3836 | + /* Adds the desired flow to the list of desired flows that have same match |
3837 | + * conditions as the installed flow. |
3838 | + * |
3839 | +@@ -820,8 +861,18 @@ flow_action_has_conj(const struct ovn_flow *f) |
3840 | + static bool |
3841 | + link_installed_to_desired(struct installed_flow *i, struct desired_flow *d) |
3842 | + { |
3843 | ++ struct desired_flow *f; |
3844 | ++ |
3845 | ++ /* Find first 'f' such that 'd' is preferred over 'f'. If no such desired |
3846 | ++ * flow exists then 'f' will point after the last element of the list. |
3847 | ++ */ |
3848 | ++ LIST_FOR_EACH (f, installed_ref_list_node, &i->desired_refs) { |
3849 | ++ if (flow_is_preferred(&d->flow, &f->flow)) { |
3850 | ++ break; |
3851 | ++ } |
3852 | ++ } |
3853 | ++ ovs_list_insert(&f->installed_ref_list_node, &d->installed_ref_list_node); |
3854 | + d->installed_flow = i; |
3855 | +- ovs_list_push_back(&i->desired_refs, &d->installed_ref_list_node); |
3856 | + return installed_flow_get_active(i) == d; |
3857 | + } |
3858 | + |
3859 | +@@ -1727,8 +1778,14 @@ update_installed_flows_by_compare(struct ovn_desired_flow_table *flow_table, |
3860 | + link_installed_to_desired(i, d); |
3861 | + } else if (!d->installed_flow) { |
3862 | + /* This is a desired_flow that conflicts with one installed |
3863 | +- * previously but not linked yet. */ |
3864 | +- link_installed_to_desired(i, d); |
3865 | ++ * previously but not linked yet. However, if this flow becomes |
3866 | ++ * active, e.g., it is less restrictive than the previous active |
3867 | ++ * flow then modify the installed flow. |
3868 | ++ */ |
3869 | ++ if (link_installed_to_desired(i, d)) { |
3870 | ++ installed_flow_mod(&i->flow, &d->flow, msgs); |
3871 | ++ ovn_flow_log(&i->flow, "updating installed (conflict)"); |
3872 | ++ } |
3873 | + } |
3874 | + } |
3875 | + } |
3876 | +@@ -1857,8 +1914,15 @@ update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, |
3877 | + ovn_flow_log(&i->flow, "updating installed (tracked)"); |
3878 | + } else { |
3879 | + /* Adding a new flow that conflicts with an existing installed |
3880 | +- * flow, so just add it to the link. */ |
3881 | +- link_installed_to_desired(i, f); |
3882 | ++ * flow, so add it to the link. If this flow becomes active, |
3883 | ++ * e.g., it is less restrictive than the previous active flow |
3884 | ++ * then modify the installed flow. |
3885 | ++ */ |
3886 | ++ if (link_installed_to_desired(i, f)) { |
3887 | ++ installed_flow_mod(&i->flow, &f->flow, msgs); |
3888 | ++ ovn_flow_log(&i->flow, |
3889 | ++ "updating installed (tracked conflict)"); |
3890 | ++ } |
3891 | + } |
3892 | + /* The track_list_node emptyness is used to check if the node is |
3893 | + * already added to track list, so initialize it again here. */ |
3894 | +diff --git a/tests/ovn.at b/tests/ovn.at |
3895 | +index a197eee5c..6dadf5348 100644 |
3896 | +--- a/tests/ovn.at |
3897 | ++++ b/tests/ovn.at |
3898 | +@@ -12656,6 +12656,220 @@ grep conjunction.*conjunction.*conjunction | wc -l`]) |
3899 | + OVN_CLEANUP([hv1]) |
3900 | + AT_CLEANUP |
3901 | + |
3902 | ++AT_SETUP([ovn -- Superseeding ACLs with conjunction]) |
3903 | ++ovn_start |
3904 | ++ |
3905 | ++ovn-nbctl ls-add ls1 |
3906 | ++ |
3907 | ++ovn-nbctl lsp-add ls1 ls1-lp1 \ |
3908 | ++-- lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01" |
3909 | ++ |
3910 | ++ovn-nbctl lsp-add ls1 ls1-lp2 \ |
3911 | ++-- lsp-set-addresses ls1-lp2 "f0:00:00:00:00:02" |
3912 | ++ |
3913 | ++net_add n1 |
3914 | ++sim_add hv1 |
3915 | ++ |
3916 | ++as hv1 |
3917 | ++ovs-vsctl add-br br-phys |
3918 | ++ovn_attach n1 br-phys 192.168.0.1 |
3919 | ++ovs-vsctl -- add-port br-int hv1-vif1 -- \ |
3920 | ++ set interface hv1-vif1 external-ids:iface-id=ls1-lp1 \ |
3921 | ++ options:tx_pcap=hv1/vif1-tx.pcap \ |
3922 | ++ options:rxq_pcap=hv1/vif1-rx.pcap \ |
3923 | ++ ofport-request=1 |
3924 | ++ |
3925 | ++ovs-vsctl -- add-port br-int hv1-vif2 -- \ |
3926 | ++ set interface hv1-vif2 external-ids:iface-id=ls1-lp2 \ |
3927 | ++ options:tx_pcap=hv1/vif2-tx.pcap \ |
3928 | ++ options:rxq_pcap=hv1/vif2-rx.pcap \ |
3929 | ++ ofport-request=2 |
3930 | ++ |
3931 | ++# test_ip INPORT SRC_MAC DST_MAC SRC_IP DST_IP OUTPORT... |
3932 | ++# |
3933 | ++# This shell function causes an ip packet to be received on INPORT. |
3934 | ++# The packet's content has Ethernet destination DST and source SRC |
3935 | ++# (each exactly 12 hex digits) and Ethernet type ETHTYPE (4 hex digits). |
3936 | ++# The OUTPORTs (zero or more) list the VIFs on which the packet should |
3937 | ++# be received. INPORT and the OUTPORTs are specified as logical switch |
3938 | ++# port numbers, e.g. 11 for vif11. |
3939 | ++test_ip() { |
3940 | ++ # This packet has bad checksums but logical L3 routing doesn't check. |
3941 | ++ local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5 |
3942 | ++ local packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}\ |
3943 | ++${dst_ip}0035111100080000 |
3944 | ++ shift; shift; shift; shift; shift |
3945 | ++ as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet |
3946 | ++ for outport; do |
3947 | ++ echo $packet >> $outport.expected |
3948 | ++ done |
3949 | ++} |
3950 | ++ |
3951 | ++ip_to_hex() { |
3952 | ++ printf "%02x%02x%02x%02x" "$@" |
3953 | ++} |
3954 | ++ |
3955 | ++reset_pcap_file() { |
3956 | ++ local iface=$1 |
3957 | ++ local pcap_file=$2 |
3958 | ++ ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \ |
3959 | ++options:rxq_pcap=dummy-rx.pcap |
3960 | ++ rm -f ${pcap_file}*.pcap |
3961 | ++ ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \ |
3962 | ++options:rxq_pcap=${pcap_file}-rx.pcap |
3963 | ++} |
3964 | ++ |
3965 | ++# Add a default deny ACL and an allow ACL for specific IP traffic. |
3966 | ++ovn-nbctl acl-add ls1 to-lport 2 'arp' allow |
3967 | ++ovn-nbctl acl-add ls1 to-lport 1 'ip4' drop |
3968 | ++ovn-nbctl acl-add ls1 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 |
3969 | ++ovn-nbctl acl-add ls1 to-lport 3 '(ip4.src==10.0.0.1 || ip4.src==10.0.0.42) && (ip4.dst == 10.0.0.3 || ip4.dst == 10.0.0.4)' allow |
3970 | ++ovn-nbctl --wait=hv sync |
3971 | ++ |
3972 | ++# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. |
3973 | ++for src in `seq 1 2`; do |
3974 | ++ for dst in `seq 3 4`; do |
3975 | ++ sip=`ip_to_hex 10 0 0 $src` |
3976 | ++ dip=`ip_to_hex 10 0 0 $dst` |
3977 | ++ |
3978 | ++ test_ip 1 f00000000001 f00000000002 $sip $dip 2 |
3979 | ++ done |
3980 | ++done |
3981 | ++ |
3982 | ++# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.5 should be dropped. |
3983 | ++dip=`ip_to_hex 10 0 0 5` |
3984 | ++for src in `seq 1 2`; do |
3985 | ++ sip=`ip_to_hex 10 0 0 $src` |
3986 | ++ |
3987 | ++ test_ip 1 f00000000001 f00000000002 $sip $dip |
3988 | ++done |
3989 | ++ |
3990 | ++cat 2.expected > expout |
3991 | ++$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets |
3992 | ++AT_CHECK([cat 2.packets], [0], [expout]) |
3993 | ++reset_pcap_file hv1-vif2 hv1/vif2 |
3994 | ++rm -f 2.packets |
3995 | ++> 2.expected |
3996 | ++ |
3997 | ++# Add two less restrictive allow ACLs for src IP 10.0.0.1. |
3998 | ++ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1 || ip4.src==10.0.0.1' allow |
3999 | ++ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1' allow |
4000 | ++ovn-nbctl --wait=hv sync |
4001 | ++ |
4002 | ++# Check OVS flows, the less restrictive flows should have been installed. |
4003 | ++AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ |
4004 | ++ grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl |
4005 | ++priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4006 | ++priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4007 | ++priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2) |
4008 | ++priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2),conjunction(3,1/2) |
4009 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) |
4010 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) |
4011 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2) |
4012 | ++]) |
4013 | ++ |
4014 | ++# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. |
4015 | ++for src in `seq 1 2`; do |
4016 | ++ for dst in `seq 3 4`; do |
4017 | ++ sip=`ip_to_hex 10 0 0 $src` |
4018 | ++ dip=`ip_to_hex 10 0 0 $dst` |
4019 | ++ |
4020 | ++ test_ip 1 f00000000001 f00000000002 $sip $dip 2 |
4021 | ++ done |
4022 | ++done |
4023 | ++ |
4024 | ++# Traffic 10.0.0.2 -> 10.0.0.5 should be dropped. |
4025 | ++sip=`ip_to_hex 10 0 0 2` |
4026 | ++dip=`ip_to_hex 10 0 0 5` |
4027 | ++test_ip 1 f00000000001 f00000000002 $sip $dip |
4028 | ++ |
4029 | ++# Traffic 10.0.0.1 -> 10.0.0.5 should be allowed. |
4030 | ++sip=`ip_to_hex 10 0 0 1` |
4031 | ++dip=`ip_to_hex 10 0 0 5` |
4032 | ++test_ip 1 f00000000001 f00000000002 $sip $dip 2 |
4033 | ++ |
4034 | ++cat 2.expected > expout |
4035 | ++$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets |
4036 | ++AT_CHECK([cat 2.packets], [0], [expout]) |
4037 | ++reset_pcap_file hv1-vif2 hv1/vif2 |
4038 | ++rm -f 2.packets |
4039 | ++> 2.expected |
4040 | ++ |
4041 | ++#sleep infinity |
4042 | ++ |
4043 | ++# Remove the first less restrictive allow ACL. |
4044 | ++ovn-nbctl acl-del ls1 to-lport 3 'ip4.src==10.0.0.1 || ip4.src==10.0.0.1' |
4045 | ++ovn-nbctl --wait=hv sync |
4046 | ++ |
4047 | ++# Check OVS flows, the second less restrictive allow ACL should have been installed. |
4048 | ++AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ |
4049 | ++ grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl |
4050 | ++priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4051 | ++priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4052 | ++priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2) |
4053 | ++priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2),conjunction(3,1/2) |
4054 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) |
4055 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) |
4056 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2) |
4057 | ++]) |
4058 | ++ |
4059 | ++# Remove the less restrictive allow ACL. |
4060 | ++ovn-nbctl acl-del ls1 to-lport 3 'ip4.src==10.0.0.1' |
4061 | ++ovn-nbctl --wait=hv sync |
4062 | ++ |
4063 | ++# Check OVS flows, the 10.0.0.1 conjunction should have been reinstalled. |
4064 | ++AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ |
4065 | ++ grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl |
4066 | ++priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4067 | ++priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4068 | ++priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2) |
4069 | ++priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2),conjunction(3,1/2) |
4070 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=conjunction(2,2/2),conjunction(3,2/2) |
4071 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) |
4072 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2) |
4073 | ++]) |
4074 | ++ |
4075 | ++# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. |
4076 | ++for src in `seq 1 2`; do |
4077 | ++ for dst in `seq 3 4`; do |
4078 | ++ sip=`ip_to_hex 10 0 0 $src` |
4079 | ++ dip=`ip_to_hex 10 0 0 $dst` |
4080 | ++ |
4081 | ++ test_ip 1 f00000000001 f00000000002 $sip $dip 2 |
4082 | ++ done |
4083 | ++done |
4084 | ++ |
4085 | ++# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.5 should be dropped. |
4086 | ++dip=`ip_to_hex 10 0 0 5` |
4087 | ++for src in `seq 1 2`; do |
4088 | ++ sip=`ip_to_hex 10 0 0 $src` |
4089 | ++ |
4090 | ++ test_ip 1 f00000000001 f00000000002 $sip $dip |
4091 | ++done |
4092 | ++ |
4093 | ++cat 2.expected > expout |
4094 | ++$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets |
4095 | ++AT_CHECK([cat 2.packets], [0], [expout]) |
4096 | ++ |
4097 | ++# Re-add the less restrictive allow ACL for src IP 10.0.0.1 |
4098 | ++ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1' allow |
4099 | ++ovn-nbctl --wait=hv sync |
4100 | ++ |
4101 | ++# Check OVS flows, the less restrictive flows should have been installed. |
4102 | ++AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ |
4103 | ++ grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl |
4104 | ++priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4105 | ++priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4106 | ++priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2) |
4107 | ++priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2),conjunction(3,1/2) |
4108 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) |
4109 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) |
4110 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2) |
4111 | ++]) |
4112 | ++ |
4113 | ++OVN_CLEANUP([hv1]) |
4114 | ++AT_CLEANUP |
4115 | ++ |
4116 | + # 3 hypervisors, one logical switch, 3 logical ports per hypervisor |
4117 | + AT_SETUP([ovn -- L2 Drop and Allow ACL w/ Stateful ACL]) |
4118 | + ovn_start |
4119 | +-- |
4120 | +2.29.2 |
4121 | + |
4122 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-14.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-14.patch |
4123 | new file mode 100644 |
4124 | index 0000000..939f963 |
4125 | --- /dev/null |
4126 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-14.patch |
4127 | @@ -0,0 +1,126 @@ |
4128 | +Description: Cherry-pick fix applied to master and branch-20.12 |
4129 | +Origin: https://github.com/ovn-org/ovn/commit/d6594a4695084e1b0f4fb3170547942876d3a81a |
4130 | +Applied-Upstream: commit: d6594a4695084e1b0f4fb3170547942876d3a81a |
4131 | + |
4132 | +From 0389d5deadba2df04a33178d2b6ef55f615dba79 Mon Sep 17 00:00:00 2001 |
4133 | +From: Dumitru Ceara <dceara@redhat.com> |
4134 | +Date: Tue, 10 Nov 2020 10:37:34 +0100 |
4135 | +Subject: [PATCH 14/15] ovn.at: Make some of the tests more predictable. |
4136 | + |
4137 | +Fix some of the race conditions that are present in the current OVN test |
4138 | +suite. |
4139 | + |
4140 | +Signed-off-by: Dumitru Ceara <dceara@redhat.com> |
4141 | +Signed-off-by: Ben Pfaff <blp@ovn.org> |
4142 | +--- |
4143 | + tests/ovn.at | 48 +++++++++++++++++++++++++++--------------------- |
4144 | + 1 file changed, 27 insertions(+), 21 deletions(-) |
4145 | + |
4146 | +diff --git a/tests/ovn.at b/tests/ovn.at |
4147 | +index 6dadf5348..cc200f8a3 100644 |
4148 | +--- a/tests/ovn.at |
4149 | ++++ b/tests/ovn.at |
4150 | +@@ -3429,6 +3429,7 @@ for i in 1 2 3; do |
4151 | + ovn-nbctl lsp-set-addresses lp$i$j "f0:00:00:00:00:$i$j 192.168.0.$i$j" "$extra_addr" |
4152 | + ovn-nbctl lsp-set-port-security lp$i$j "f0:00:00:00:00:$i$j 192.168.0.$i$j" "$extra_addr" |
4153 | + fi |
4154 | ++ OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp$i$j` = xup]) |
4155 | + done |
4156 | + done |
4157 | + |
4158 | +@@ -12758,14 +12759,15 @@ ovn-nbctl --wait=hv sync |
4159 | + |
4160 | + # Check OVS flows, the less restrictive flows should have been installed. |
4161 | + AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ |
4162 | +- grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl |
4163 | ++ grep "priority=1003" | awk '{print $7 " " $8}' | \ |
4164 | ++ sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl |
4165 | + priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4166 | + priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4167 | +-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2) |
4168 | +-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2),conjunction(3,1/2) |
4169 | ++priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4170 | ++priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4171 | + priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) |
4172 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) |
4173 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2) |
4174 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4175 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4176 | + ]) |
4177 | + |
4178 | + # Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. |
4179 | +@@ -12803,14 +12805,15 @@ ovn-nbctl --wait=hv sync |
4180 | + |
4181 | + # Check OVS flows, the second less restrictive allow ACL should have been installed. |
4182 | + AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ |
4183 | +- grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl |
4184 | ++ grep "priority=1003" | awk '{print $7 " " $8}' | \ |
4185 | ++ sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl |
4186 | + priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4187 | + priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4188 | +-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2) |
4189 | +-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2),conjunction(3,1/2) |
4190 | ++priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4191 | ++priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4192 | + priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) |
4193 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) |
4194 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2) |
4195 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4196 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4197 | + ]) |
4198 | + |
4199 | + # Remove the less restrictive allow ACL. |
4200 | +@@ -12819,14 +12822,15 @@ ovn-nbctl --wait=hv sync |
4201 | + |
4202 | + # Check OVS flows, the 10.0.0.1 conjunction should have been reinstalled. |
4203 | + AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ |
4204 | +- grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl |
4205 | ++ grep "priority=1003" | awk '{print $7 " " $8}' | \ |
4206 | ++ sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl |
4207 | + priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4208 | + priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4209 | +-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2) |
4210 | +-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2),conjunction(3,1/2) |
4211 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=conjunction(2,2/2),conjunction(3,2/2) |
4212 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) |
4213 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2) |
4214 | ++priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4215 | ++priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4216 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=conjunction(),conjunction() |
4217 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4218 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4219 | + ]) |
4220 | + |
4221 | + # Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. |
4222 | +@@ -12857,14 +12861,15 @@ ovn-nbctl --wait=hv sync |
4223 | + |
4224 | + # Check OVS flows, the less restrictive flows should have been installed. |
4225 | + AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ |
4226 | +- grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl |
4227 | ++ grep "priority=1003" | awk '{print $7 " " $8}' | \ |
4228 | ++ sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl |
4229 | + priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4230 | + priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4231 | +-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(2,1/2),conjunction(3,1/2) |
4232 | +-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(2,1/2),conjunction(3,1/2) |
4233 | ++priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4234 | ++priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4235 | + priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) |
4236 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2) |
4237 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2) |
4238 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4239 | ++priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4240 | + ]) |
4241 | + |
4242 | + OVN_CLEANUP([hv1]) |
4243 | +@@ -13462,6 +13467,7 @@ ovn-nbctl set Logical_Switch_Port ls1-lr0 type=router \ |
4244 | + # Create HA chassis group |
4245 | + ovn-nbctl ha-chassis-group-add hagrp1 |
4246 | + ovn-nbctl ha-chassis-group-add-chassis hagrp1 hv1 30 |
4247 | ++ovn-nbctl --wait=sb sync |
4248 | + |
4249 | + hagrp1_uuid=`ovn-nbctl --bare --columns _uuid find ha_chassis_group name="hagrp1"` |
4250 | + |
4251 | +-- |
4252 | +2.29.2 |
4253 | + |
4254 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-15.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-15.patch |
4255 | new file mode 100644 |
4256 | index 0000000..cb13bb7 |
4257 | --- /dev/null |
4258 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-15.patch |
4259 | @@ -0,0 +1,66 @@ |
4260 | +Description: Cherry-pick fix applied to master |
4261 | +Origin: https://github.com/ovn-org/ovn/commit/0879a0aa9f332bdc689769a1bc2f156f1c3149a5 |
4262 | +Applied-Upstream: commit: 0879a0aa9f332bdc689769a1bc2f156f1c3149a5 |
4263 | + |
4264 | +From 0879a0aa9f332bdc689769a1bc2f156f1c3149a5 Mon Sep 17 00:00:00 2001 |
4265 | +From: Dumitru Ceara <dceara@redhat.com> |
4266 | +Date: Mon, 7 Dec 2020 21:30:52 +0100 |
4267 | +Subject: [PATCH] tests: Add ofctl_strip_all() to filter OVS flow outputs. |
4268 | + |
4269 | +Extend the already existing ofctl_strip() to also ignore n_packets, |
4270 | +n_bytes, cookie. Use some helper functions from |
4271 | +tests/system-userspace-packet-type-aware.at, which will be removed |
4272 | +by a future commit. |
4273 | + |
4274 | +Signed-off-by: Dumitru Ceara <dceara@redhat.com> |
4275 | +Signed-off-by: Numan Siddique <numans@ovn.org> |
4276 | +--- |
4277 | + tests/ofproto-macros.at | 23 ++++++++++++++++++++++- |
4278 | + 1 file changed, 22 insertions(+), 1 deletion(-) |
4279 | + |
4280 | +diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at |
4281 | +index a6e89a951..dd5d3848d 100644 |
4282 | +--- a/tests/ofproto-macros.at |
4283 | ++++ b/tests/ofproto-macros.at |
4284 | +@@ -1,12 +1,27 @@ |
4285 | + m4_divert_push([PREPARE_TESTS]) |
4286 | + [ |
4287 | ++# Strips 'n_packets=...' from ovs-ofctl output. |
4288 | ++strip_n_packets () { |
4289 | ++ sed 's/ n_packets=[0-9]*,//' |
4290 | ++} |
4291 | ++ |
4292 | ++# Strips 'n_bytes=...' from ovs-ofctl output. |
4293 | ++strip_n_bytes () { |
4294 | ++ sed 's/ n_bytes=[0-9]*,//' |
4295 | ++} |
4296 | ++ |
4297 | ++# Strips 'cookie=...' from ovs-ofctl output. |
4298 | ++strip_cookie () { |
4299 | ++ sed 's/ cookie=0x[0-9a-fA-F]*,//' |
4300 | ++} |
4301 | ++ |
4302 | + # Strips out uninteresting parts of ovs-ofctl output, as well as parts |
4303 | + # that vary from one run to another. |
4304 | + ofctl_strip () { |
4305 | + sed ' |
4306 | + s/ (xid=0x[0-9a-fA-F]*)// |
4307 | + s/ duration=[0-9.]*s,// |
4308 | +-s/ cookie=0x0,// |
4309 | ++s/ cookie=0,// |
4310 | + s/ table=0,// |
4311 | + s/ n_packets=0,// |
4312 | + s/ n_bytes=0,// |
4313 | +@@ -19,6 +34,12 @@ s/dir\/[0-9]*\/br0.mgmt/dir\/XXXX\/br0.mgmt/ |
4314 | + ' |
4315 | + } |
4316 | + |
4317 | ++# Strips out uninteresting parts of ovs-ofctl output, including n_packets=.. |
4318 | ++# n_bytes=.. |
4319 | ++ofctl_strip_all () { |
4320 | ++ ofctl_strip | strip_n_packets | strip_n_bytes | strip_cookie |
4321 | ++} |
4322 | ++ |
4323 | + # Filter (multiline) vconn debug messages from ovs-vswitchd.log. |
4324 | + # Use with vconn_sub() and ofctl_strip() |
4325 | + print_vconn_debug () { awk -F\| < ovs-vswitchd.log ' |
4326 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-16.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-16.patch |
4327 | new file mode 100644 |
4328 | index 0000000..c1c9806 |
4329 | --- /dev/null |
4330 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-16.patch |
4331 | @@ -0,0 +1,135 @@ |
4332 | +Description: Cherry-pick fix applied to master and branch-20.12 |
4333 | +Origin: https://github.com/ovn-org/ovn/commit/6e3d69e6b5153e26c869a03ec7dd1aaa40488005 |
4334 | +Applied-Upstream: commit: 6e3d69e6b5153e26c869a03ec7dd1aaa40488005 |
4335 | + |
4336 | +From f144e0e33af8ca8e1748546c379f0b22b86576b7 Mon Sep 17 00:00:00 2001 |
4337 | +From: Dumitru Ceara <dceara@redhat.com> |
4338 | +Date: Mon, 7 Dec 2020 21:31:25 +0100 |
4339 | +Subject: [PATCH 15/15] tests: Fix test "ovn -- Superseding ACLs with |
4340 | + conjunction". |
4341 | + |
4342 | +The test was checking the output of "ovs-ofctl dump-flows" without |
4343 | +taking into account that some fields don't have predictable values, |
4344 | +e.g., hard_age. |
4345 | + |
4346 | +Instead, use ofctl_strip_all(). |
4347 | + |
4348 | +Fixes: 986b3d5e4ad6 ("ofctrl.c: Add a predictable resolution for conflicting flow actions.") |
4349 | +Reported-by: Numan Siddique <numans@ovn.org> |
4350 | +Signed-off-by: Dumitru Ceara <dceara@redhat.com> |
4351 | +Signed-off-by: Numan Siddique <numans@ovn.org> |
4352 | +--- |
4353 | + tests/ovn.at | 72 ++++++++++++++++++++++++++-------------------------- |
4354 | + 1 file changed, 36 insertions(+), 36 deletions(-) |
4355 | + |
4356 | +diff --git a/tests/ovn.at b/tests/ovn.at |
4357 | +index cc200f8a3..549f15ea7 100644 |
4358 | +--- a/tests/ovn.at |
4359 | ++++ b/tests/ovn.at |
4360 | +@@ -12758,16 +12758,16 @@ ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1' allow |
4361 | + ovn-nbctl --wait=hv sync |
4362 | + |
4363 | + # Check OVS flows, the less restrictive flows should have been installed. |
4364 | +-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ |
4365 | +- grep "priority=1003" | awk '{print $7 " " $8}' | \ |
4366 | ++AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | \ |
4367 | ++ grep "priority=1003" | \ |
4368 | + sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl |
4369 | +-priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4370 | +-priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4371 | +-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4372 | +-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4373 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) |
4374 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4375 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4376 | ++ table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4377 | ++ table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4378 | ++ table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4379 | ++ table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4380 | ++ table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) |
4381 | ++ table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4382 | ++ table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4383 | + ]) |
4384 | + |
4385 | + # Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. |
4386 | +@@ -12804,16 +12804,16 @@ ovn-nbctl acl-del ls1 to-lport 3 'ip4.src==10.0.0.1 || ip4.src==10.0.0.1' |
4387 | + ovn-nbctl --wait=hv sync |
4388 | + |
4389 | + # Check OVS flows, the second less restrictive allow ACL should have been installed. |
4390 | +-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ |
4391 | +- grep "priority=1003" | awk '{print $7 " " $8}' | \ |
4392 | ++AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | \ |
4393 | ++ grep "priority=1003" | \ |
4394 | + sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl |
4395 | +-priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4396 | +-priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4397 | +-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4398 | +-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4399 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) |
4400 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4401 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4402 | ++ table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4403 | ++ table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4404 | ++ table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4405 | ++ table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4406 | ++ table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) |
4407 | ++ table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4408 | ++ table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4409 | + ]) |
4410 | + |
4411 | + # Remove the less restrictive allow ACL. |
4412 | +@@ -12821,16 +12821,16 @@ ovn-nbctl acl-del ls1 to-lport 3 'ip4.src==10.0.0.1' |
4413 | + ovn-nbctl --wait=hv sync |
4414 | + |
4415 | + # Check OVS flows, the 10.0.0.1 conjunction should have been reinstalled. |
4416 | +-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ |
4417 | +- grep "priority=1003" | awk '{print $7 " " $8}' | \ |
4418 | ++AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | \ |
4419 | ++ grep "priority=1003" | \ |
4420 | + sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl |
4421 | +-priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4422 | +-priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4423 | +-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4424 | +-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4425 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=conjunction(),conjunction() |
4426 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4427 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4428 | ++ table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4429 | ++ table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4430 | ++ table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4431 | ++ table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4432 | ++ table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=conjunction(),conjunction() |
4433 | ++ table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4434 | ++ table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4435 | + ]) |
4436 | + |
4437 | + # Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. |
4438 | +@@ -12860,16 +12860,16 @@ ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1' allow |
4439 | + ovn-nbctl --wait=hv sync |
4440 | + |
4441 | + # Check OVS flows, the less restrictive flows should have been installed. |
4442 | +-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \ |
4443 | +- grep "priority=1003" | awk '{print $7 " " $8}' | \ |
4444 | ++AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | \ |
4445 | ++ grep "priority=1003" | \ |
4446 | + sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl |
4447 | +-priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4448 | +-priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4449 | +-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4450 | +-priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4451 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) |
4452 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4453 | +-priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4454 | ++ table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4455 | ++ table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4456 | ++ table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4457 | ++ table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4458 | ++ table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) |
4459 | ++ table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4460 | ++ table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4461 | + ]) |
4462 | + |
4463 | + OVN_CLEANUP([hv1]) |
4464 | +-- |
4465 | +2.29.2 |
4466 | + |
4467 | diff --git a/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-17.patch b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-17.patch |
4468 | new file mode 100644 |
4469 | index 0000000..5444fac |
4470 | --- /dev/null |
4471 | +++ b/debian/patches/ovn-ofctrl-predictable-resolution-conflicting-flow-actions-17.patch |
4472 | @@ -0,0 +1,106 @@ |
4473 | +Description: Adapt test suite to 20.03 |
4474 | +Author: Frode Nordahl <frode.nordahl@canonical.com> |
4475 | +Forwarded: not-needed |
4476 | + |
4477 | +--- a/tests/ovn.at 2020-12-11 11:04:20.143110765 +0000 |
4478 | ++++ b/tests/ovn.at 2020-12-11 11:05:49.170781292 +0000 |
4479 | +@@ -12758,16 +12758,16 @@ |
4480 | + ovn-nbctl --wait=hv sync |
4481 | + |
4482 | + # Check OVS flows, the less restrictive flows should have been installed. |
4483 | +-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | \ |
4484 | ++AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=44 | ofctl_strip_all | \ |
4485 | + grep "priority=1003" | \ |
4486 | + sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl |
4487 | +- table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4488 | +- table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4489 | +- table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4490 | +- table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4491 | +- table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) |
4492 | +- table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4493 | +- table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4494 | ++ table=44, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,45) |
4495 | ++ table=44, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,45) |
4496 | ++ table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4497 | ++ table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4498 | ++ table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,45) |
4499 | ++ table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4500 | ++ table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4501 | + ]) |
4502 | + |
4503 | + # Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. |
4504 | +@@ -12804,16 +12804,16 @@ |
4505 | + ovn-nbctl --wait=hv sync |
4506 | + |
4507 | + # Check OVS flows, the second less restrictive allow ACL should have been installed. |
4508 | +-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | \ |
4509 | ++AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=44 | ofctl_strip_all | \ |
4510 | + grep "priority=1003" | \ |
4511 | + sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl |
4512 | +- table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4513 | +- table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4514 | +- table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4515 | +- table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4516 | +- table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) |
4517 | +- table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4518 | +- table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4519 | ++ table=44, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,45) |
4520 | ++ table=44, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,45) |
4521 | ++ table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4522 | ++ table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4523 | ++ table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,45) |
4524 | ++ table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4525 | ++ table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4526 | + ]) |
4527 | + |
4528 | + # Remove the less restrictive allow ACL. |
4529 | +@@ -12821,16 +12821,16 @@ |
4530 | + ovn-nbctl --wait=hv sync |
4531 | + |
4532 | + # Check OVS flows, the 10.0.0.1 conjunction should have been reinstalled. |
4533 | +-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | \ |
4534 | ++AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=44 | ofctl_strip_all | \ |
4535 | + grep "priority=1003" | \ |
4536 | + sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl |
4537 | +- table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4538 | +- table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4539 | +- table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4540 | +- table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4541 | +- table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=conjunction(),conjunction() |
4542 | +- table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4543 | +- table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4544 | ++ table=44, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,45) |
4545 | ++ table=44, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,45) |
4546 | ++ table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4547 | ++ table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4548 | ++ table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=conjunction(),conjunction() |
4549 | ++ table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4550 | ++ table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4551 | + ]) |
4552 | + |
4553 | + # Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed. |
4554 | +@@ -12860,16 +12860,16 @@ |
4555 | + ovn-nbctl --wait=hv sync |
4556 | + |
4557 | + # Check OVS flows, the less restrictive flows should have been installed. |
4558 | +-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | \ |
4559 | ++AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=44 | ofctl_strip_all | \ |
4560 | + grep "priority=1003" | \ |
4561 | + sed 's/conjunction([[^)]]*)/conjunction()/g' | sort], [0], [dnl |
4562 | +- table=45, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46) |
4563 | +- table=45, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46) |
4564 | +- table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4565 | +- table=45, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4566 | +- table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46) |
4567 | +- table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4568 | +- table=45, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4569 | ++ table=44, priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,45) |
4570 | ++ table=44, priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,45) |
4571 | ++ table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3 actions=conjunction(),conjunction() |
4572 | ++ table=44, priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4 actions=conjunction(),conjunction() |
4573 | ++ table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,45) |
4574 | ++ table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction() |
4575 | ++ table=44, priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction() |
4576 | + ]) |
4577 | + |
4578 | + OVN_CLEANUP([hv1]) |
4579 | diff --git a/debian/patches/series b/debian/patches/series |
4580 | index d55d9eb..c571f35 100644 |
4581 | --- a/debian/patches/series |
4582 | +++ b/debian/patches/series |
4583 | @@ -1 +1,20 @@ |
4584 | ovn-controller-ofctrl-probe-interval.patch |
4585 | +ovn-northd-revert-manage-arp-process-locally-dvr.patch |
4586 | +ovn-ctl-cluster-db-upgrades.patch |
4587 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-01.patch |
4588 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-02.patch |
4589 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-03.patch |
4590 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-04.patch |
4591 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-05.patch |
4592 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-06.patch |
4593 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-07.patch |
4594 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-08.patch |
4595 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-09.patch |
4596 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-10.patch |
4597 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-11.patch |
4598 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-12.patch |
4599 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-13.patch |
4600 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-14.patch |
4601 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-15.patch |
4602 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-16.patch |
4603 | +ovn-ofctrl-predictable-resolution-conflicting-flow-actions-17.patch |