diff mbox series

[ovs-dev,v5,2/6] northd: Add n_nat_entries field to 'struct ovn_datapath'

Message ID 20211109193607.31630-3-mrmarkgray@gmail.com
State Accepted
Delegated to: Han Zhou
Headers show
Series northd: Introduce incremental processing framework | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Mark Gray Nov. 9, 2021, 7:36 p.m. UTC 
From: Mark Gray <mark.d.gray@redhat.com>

destroy_nat_entries() iterates over nat_entries using 'n_nat'
as the number of NAT entries from the NB database. This behaviour can be
incorrect as it assumes that there are 'n_nat' 'nat_entries'. 'struct
ovn_datapath' should maintain a count of 'nat_entries' in 'struct
ovn_datapath' rather than read the value from NBDB IDL to properly
account for the number of 'nat_entries'.

This issue becomes apparent when using destroy_nat_entries()
as part of an incremental processing loop:

Consider an example in which we have completed iteration x, and
started iteration x+1. If we add a NAT entry to NBDB between iteration x
and iteration x+1, od->nbr->n_nat will contain the updated value of
n_nat as the northbound record will have been updated. However, if
we do not (re)initialize the nat entries in od, od->n_nat_entries
could be equal to the previous value. This can cause destroy_nat_entries()
to loop over the wrong number of 'nat_entries' causing unexpected
behaviour.

Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Mark Gray <mark.d.gray@redhat.com>
---
 northd/northd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/northd/northd.c b/northd/northd.c
index c4bdfb649215..938dadf5563c 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -603,6 +603,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. */
@@ -787,6 +789,7 @@  init_nat_entries(struct ovn_datapath *od)
             od->has_distributed_nat = true;
         }
     }
+    od->n_nat_entries = od->nbr->n_nat;
 }
 
 static void
@@ -800,7 +803,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);
     }
 }