When generating SB records that have tunnel_keys (e.g., Datapath, Port_Binding, Multicast_Group) ovn-northd tries to reuse the tunnel_key value from the original SB record, if any available.
However, there's no check for tunnel_keys that would conflict with newly allocated keys for new records. In order to avoid that, we first check that the tunnel_key value in the SB record doesn't match a key already allocated by northd in the current run. If there's a conflict, ovn-north will generate a new key for the SB record. One way to reproduce the issue is: $ ovn-nbctl ls-add ls1 $ ovn-nbctl ls-add ls2 $ ovn-nbctl lsp-add ls1 lsp1 $ ovn-nbctl lsp-add ls2 lsp2 $ ovn-nbctl --wait=sb sync $ ovn-nbctl lsp-del lsp2 -- lsp-add ls1 lsp2 Reported-by: Dan Williams <d...@redhat.com> Reported-at: https://bugzilla.redhat.com/1828637 Signed-off-by: Dumitru Ceara <dce...@redhat.com> --- northd/ovn-northd.c | 27 +++++++++++++++++++++++---- tests/ovn-northd.at | 30 ++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index de59452..dad6a45 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -1068,7 +1068,13 @@ build_datapaths(struct northd_context *ctx, struct hmap *datapaths, struct ovn_datapath *od, *next; if (!ovs_list_is_empty(&nb_only) || !ovs_list_is_empty(&both)) { LIST_FOR_EACH (od, list, &both) { - ovn_add_tnlid(&dp_tnlids, od->sb->tunnel_key); + if (!ovn_tnlid_in_use(&dp_tnlids, od->sb->tunnel_key)) { + ovn_add_tnlid(&dp_tnlids, od->sb->tunnel_key); + } else { + uint32_t dp_key = ovn_datapath_allocate_key(&dp_tnlids); + + sbrec_datapath_binding_set_tunnel_key(od->sb, dp_key); + } } } @@ -3465,7 +3471,12 @@ build_ports(struct northd_context *ctx, /* For logical ports that are in both databases, index the in-use * tunnel_keys. */ LIST_FOR_EACH (op, list, &both) { - ovn_add_tnlid(&op->od->port_tnlids, op->sb->tunnel_key); + if (!ovn_tnlid_in_use(&op->od->port_tnlids, op->sb->tunnel_key)) { + ovn_add_tnlid(&op->od->port_tnlids, op->sb->tunnel_key); + } else { + sbrec_port_binding_set_tunnel_key(op->sb, + ovn_port_allocate_key(op->od)); + } if (op->sb->tunnel_key > op->od->port_key_hint) { op->od->port_key_hint = op->sb->tunnel_key; } @@ -3727,8 +3738,16 @@ ovn_igmp_group_add(struct northd_context *ctx, struct hmap *igmp_groups, igmp_group->address = *address; if (mcgroup) { igmp_group->mcgroup.key = mcgroup->tunnel_key; - ovn_add_tnlid(&datapath->mcast_info.group_tnlids, - mcgroup->tunnel_key); + if (!ovn_tnlid_in_use(&datapath->mcast_info.group_tnlids, + mcgroup->tunnel_key)) { + ovn_add_tnlid(&datapath->mcast_info.group_tnlids, + mcgroup->tunnel_key); + } else { + uint32_t group_key = + ovn_mcast_group_allocate_key(&datapath->mcast_info); + + sbrec_multicast_group_set_tunnel_key(mcgroup, group_key); + } } else { igmp_group->mcgroup.key = 0; } diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index a4469c7..4823d14 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -1414,3 +1414,33 @@ lr_uuid=$(ovn-nbctl --columns _uuid list Logical_Router .) AT_CHECK[test ${nb_uuid} = ${lr_uuid}] AT_CLEANUP + +AT_SETUP([ovn -- check reconcile stale tunnel keys]) +ovn_start + +ovn-nbctl ls-add ls1 +ovn-nbctl ls-add ls2 +ovn-nbctl lsp-add ls1 lsp1 +ovn-nbctl lsp-add ls2 lsp2 +AT_CHECK([ovn-nbctl --wait=sb sync], [0]) + +# Ports are bound on different datapaths so it's expected that they both +# get tunnel_key == 1. +AT_CHECK([test 1 = $(ovn-sbctl --bare --columns tunnel_key find \ +port_binding logical_port=lsp1)]) +AT_CHECK([test 1 = $(ovn-sbctl --bare --columns tunnel_key find \ +port_binding logical_port=lsp2)]) + +ovn-nbctl lsp-del lsp2 -- lsp-add ls1 lsp2 +AT_CHECK([ovn-nbctl --wait=sb sync], [0]) + +pb1_key=$(ovn-sbctl --bare --columns tunnel_key find port_binding \ +logical_port=lsp1) +pb2_key=$(ovn-sbctl --bare --columns tunnel_key find port_binding \ +logical_port=lsp2) + +# ovn-northd should allocate a new tunnel_key for lsp1 or lsp2 to maintain +# unique DB indices. +AT_CHECK([test ${pb1_key} != ${pb2_key}]) + +AT_CLEANUP _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
