Address sets may contain duplicate values such as "::1" and "::01" or
"1.1.1.1" and "1.1.1.1/32", which produce the same desired flow.
When adding such a duplicate to an existing address set, an lflow using
the address set that is updated incrementally will produce the same
desired flow that it produced for the original value, find that this
desired flow already exist and link the lflow to the desired flow again.
This currently assumes that the existing desired flow was produced by
another lflow, which is not the case here and results in a second
reference from the lflow to the desired flow.
This then leads to a crash later on when removing or recomputing the
lflow and going through remove_flows_from_sb_to_flow by tripping over
the ovs_assert(ovs_list_is_empty(&f->list_node)) as the code there does
not anticipate seeing the same desired flow twice for one lflow.
Fix this by detecting and preventing the duplicate reference. Also log
a warning as this likely indicates a problem somewhere else.
Fixes: 7c0b538762f6 ("ovn-controller: Handle addresses addition in address set
incrementally.")
Signed-off-by: Felix Moebius <[email protected]>
---
v2:
- addressed Ales' comments: added test and rate limit
---
controller/ofctrl.c | 13 ++++++++++-
tests/ovn-controller.at | 52 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 64 insertions(+), 1 deletion(-)
diff --git a/controller/ofctrl.c b/controller/ofctrl.c
index 2e34514b6..9f3ce0368 100644
--- a/controller/ofctrl.c
+++ b/controller/ofctrl.c
@@ -1402,11 +1402,22 @@ ofctrl_add_or_append_conj_flow(struct
ovn_desired_flow_table *desired_flows,
* actions properly when handle addrset ip deletion, instead of simply
* delete the flow. */
struct sb_flow_ref *sfr;
+ bool duplicate = false;
LIST_FOR_EACH (sfr, sb_list, &f->references) {
+ duplicate |= uuid_equals(&sfr->sb_uuid, sb_uuid);
ovs_list_remove(&sfr->as_ip_flow_list);
ovs_list_init(&sfr->as_ip_flow_list);
}
- link_flow_to_sb(desired_flows, f, sb_uuid, NULL);
+
+ if (duplicate) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+ char *s = ovn_flow_to_string(&f->flow);
+ VLOG_WARN_RL(&rl, "lflow "UUID_FMT" already references desired "
+ "flow: %s", UUID_ARGS(sb_uuid), s);
+ free(s);
+ } else {
+ link_flow_to_sb(desired_flows, f, sb_uuid, NULL);
+ }
} else {
actions = create_conjunction_actions(conjunctions, NULL);
f = desired_flow_alloc(table_id, priority, 0, match, actions,
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index 6ecf50358..df244f30e 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -2088,6 +2088,58 @@ AT_CHECK([echo $(($reprocess_count_new -
$reprocess_count_old))], [0], [2
OVN_CLEANUP([hv1])
AT_CLEANUP
+AT_SETUP([ovn-controller - I-P for address set update: handle duplicate
addresses])
+AT_KEYWORDS([as-i-p])
+
+ovn_start
+
+net_add n1
+sim_add hv1
+as hv1
+check ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+check ovs-vsctl -- add-port br-int hv1-vif1 -- \
+ set interface hv1-vif1 external-ids:iface-id=ls1-lp1
+
+check ovn-nbctl ls-add ls1
+
+check ovn-nbctl lsp-add ls1 ls1-lp1 \
+-- lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01"
+
+wait_for_ports_up
+ovn-appctl -t ovn-controller vlog/set file:dbg
+
+# Initial state:
+# 2 ASes, each has 3 IPs, no duplicates.
+check_uuid ovn-nbctl create address_set name=as1
addresses=10.0.0.1,10.0.0.2,10.0.0.3
+check_uuid ovn-nbctl create address_set name=as2
addresses='["::1","::2","::3"]'
+check ovn-nbctl acl-add ls1 to-lport 100 'outport == "ls1-lp1" && ip4.src ==
$as1 && tcp && tcp.dst == {101, 102}' drop
+check ovn-nbctl acl-add ls1 to-lport 100 'outport == "ls1-lp1" && ip6.src ==
$as2 && tcp && tcp.dst == {201, 202}' drop
+
+check ovn-nbctl --wait=hv sync
+
+# Add duplicate IP, then remove it again.
+# This should generate a warning, but should not crash.
+check ovn-nbctl add address_set as1 addresses 10.0.0.1/32
+check ovn-nbctl --wait=hv sync
+check ovn-nbctl remove address_set as1 addresses 10.0.0.1/32
+check ovn-nbctl --wait=hv sync
+
+AT_CHECK([grep "already references desired flow" hv1/ovn-controller.log | grep
-q "nw_src=10.0.0.1"])
+
+# Same for duplicate IPv6 addresses.
+check ovn-nbctl add address_set as2 addresses '["::01"]'
+check ovn-nbctl --wait=hv sync
+check ovn-nbctl remove address_set as2 addresses '["::01"]'
+check ovn-nbctl --wait=hv sync
+
+AT_CHECK([grep "already references desired flow" hv1/ovn-controller.log | grep
-q "ipv6_src=::1"])
+
+OVN_CLEANUP([hv1
+/already references desired flow/d
+])
+AT_CLEANUP
+
AT_SETUP([ovn-controller - I-P for address set update: mac])
AT_KEYWORDS([as-i-p])
--
2.51.1
Diese E Mail enthält möglicherweise vertrauliche Inhalte und ist nur für die
Verwertung durch den vorgesehenen Empfänger bestimmt.
Sollten Sie nicht der vorgesehene Empfänger sein, setzen Sie den Absender bitte
unverzüglich in Kenntnis und löschen diese E Mail.
Unsere Datenschutzhinweise finden Sie
hier
.
This e-mail may contain confidential content and is intended only for the
specified recipient/s.
If you are not the intended recipient, please inform the sender immediately and
delete this e-mail.
Our privacy policy can be found here.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
