Hi Mark,
Of all the patches in this series, this is the one I am least sure about.
It seems simple enough, but it's not clear to me
1) Why would the number of nat entries on an ovn_datapath differ from
what's in the database? Since this series doesn't introduce any "real"
incremental processing, we should be recreating the ovn_datapath on
every iteration. And on each iteration, we should have a fresh view of
the database, so why would the values differ?
2) Why is this only an issue in destroy_nat_entries()? There are several
other places in the code where we iterate over the number of nat entries
using od->nbr->n_nat. Why are those not affected?
3) How does this patch address that issue? The patch just sets the
ovn_datapath's n_nat_entries to be the same as the database's count,
which according to the commit message, is not always accurate.
Can you provide some clarity here? Maybe tell of a situation where
od->nbr->n_nat is incorrect?
On 9/3/21 8:21 AM, Mark Gray wrote:
destroy_nat_entries() iterates over nat_entries using the
number of NAT entries from the NB database. When using I-P,
this behaviour is incorrect as the number of NAT entries in
the NBDB may differ from that stored in 'struct ovn_datapath'
causing unexpected behaviour.
Signed-off-by: Mark Gray <mark.d.g...@redhat.com>
---
northd/northd.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/northd/northd.c b/northd/northd.c
index 724baa3994d5..829c4479f14b 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -605,6 +605,8 @@ struct ovn_datapath {
/* NAT entries configured on the router. */
struct ovn_nat *nat_entries;
+ size_t n_nat_entries;
+
bool has_distributed_nat;
/* Set of nat external ips on the router. */
@@ -789,6 +791,7 @@ init_nat_entries(struct ovn_datapath *od)
od->has_distributed_nat = true;
}
}
+ od->n_nat_entries = od->nbr->n_nat;
}
static void
@@ -802,7 +805,7 @@ destroy_nat_entries(struct ovn_datapath *od)
destroy_lport_addresses(&od->dnat_force_snat_addrs);
destroy_lport_addresses(&od->lb_force_snat_addrs);
- for (size_t i = 0; i < od->nbr->n_nat; i++) {
+ for (size_t i = 0; i < od->n_nat_entries; i++) {
destroy_lport_addresses(&od->nat_entries[i].ext_addrs);
}
}
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
