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

Mark Gray mrmarkgray at gmail.com
Tue Nov 9 19:36:03 UTC 2021


From: Mark Gray <mark.d.gray at 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 at ovn.org>
Signed-off-by: Mark Gray <mark.d.gray at redhat.com>
---
 northd/northd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

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);
     }
 }
-- 
2.17.1


-- 
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus



More information about the dev mailing list