The incremental processing is broken for addresses
that have mask which could "erase" portion of the address
itself e.g. 10.16.0.47/4, after applying the mask with token
parser the address becomes 0.0.0.0/4, which is fine for
logical flows. However, for the deletion/addition to database
we need the original string representation which cannot be
built from the parsed token.
Use svec instead for the comparison which allows us to keep the
original strings.
The change to svec shouldn't have any significant performance
impact, see the numbers below show. The setup was created by
the scale script from Han [0].
Numbers with expr:
Time spent on processing nb_cfg 1:
ovn-northd delay before processing: 21ms
ovn-northd completion: 544ms
ovn-controller(s) completion: 544ms
Time spent on processing nb_cfg 2:
ovn-northd delay before processing: 17ms
ovn-northd completion: 537ms
ovn-controller(s) completion: 535ms
Time spent on processing nb_cfg 3:
ovn-northd delay before processing: 20ms
ovn-northd completion: 552ms
ovn-controller(s) completion: 550ms
Time spent on processing nb_cfg 4:
ovn-northd delay before processing: 16ms
ovn-northd completion: 529ms
ovn-controller(s) completion: 528ms
Numbers with svec:
Time spent on processing nb_cfg 1:
ovn-northd delay before processing: 19ms
ovn-northd completion: 548ms
ovn-controller(s) completion: 548ms
Time spent on processing nb_cfg 2:
ovn-northd delay before processing: 19ms
ovn-northd completion: 537ms
ovn-controller(s) completion: 537ms
Time spent on processing nb_cfg 3:
ovn-northd delay before processing: 15ms
ovn-northd completion: 522ms
ovn-controller(s) completion: 521ms
Time spent on processing nb_cfg 4:
ovn-northd delay before processing: 17ms
ovn-northd completion: 522ms
ovn-controller(s) completion: 520ms
[0]
https://github.com/hzhou8/ovn-test-script/blob/e1149318201309db6d96ae8d5a33fcfbbe1872a3/test_ovn_controller_scale.sh
Fixes: 0d5e0a6ced49 ("northd: Add I-P for syncing SB address sets.")
Reported-at: https://bugzilla.redhat.com/2196885
Signed-off-by: Ales Musil <[email protected]>
---
northd/en-sync-sb.c | 66 +++++++++++++++++++++------------------------
1 file changed, 30 insertions(+), 36 deletions(-)
diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c
index 758f00bfd..4bd77b168 100644
--- a/northd/en-sync-sb.c
+++ b/northd/en-sync-sb.c
@@ -22,7 +22,6 @@
#include "openvswitch/util.h"
#include "en-sync-sb.h"
-#include "include/ovn/expr.h"
#include "lib/inc-proc-eng.h"
#include "lib/lb.h"
#include "lib/ovn-nb-idl.h"
@@ -325,45 +324,40 @@ static void
update_sb_addr_set(const char **nb_addresses, size_t n_addresses,
const struct sbrec_address_set *sb_as)
{
- struct expr_constant_set *cs_nb_as =
- expr_constant_set_create_integers(
- (const char *const *) nb_addresses, n_addresses);
- struct expr_constant_set *cs_sb_as =
- expr_constant_set_create_integers(
- (const char *const *) sb_as->addresses, sb_as->n_addresses);
-
- struct expr_constant_set *addr_added = NULL;
- struct expr_constant_set *addr_deleted = NULL;
- expr_constant_set_integers_diff(cs_sb_as, cs_nb_as, &addr_added,
- &addr_deleted);
-
- struct ds ds = DS_EMPTY_INITIALIZER;
- if (addr_added && addr_added->n_values) {
- for (size_t i = 0; i < addr_added->n_values; i++) {
- ds_clear(&ds);
- expr_constant_format(&addr_added->values[i], EXPR_C_INTEGER, &ds);
- sbrec_address_set_update_addresses_addvalue(sb_as, ds_cstr(&ds));
- }
+ size_t i;
+ char *ip;
+
+ struct svec svec_nb_as = SVEC_EMPTY_INITIALIZER;
+ struct svec svec_sb_as = SVEC_EMPTY_INITIALIZER;
+
+ for (i = 0; i < n_addresses; i++) {
+ svec_add(&svec_nb_as, nb_addresses[i]);
}
- if (addr_deleted && addr_deleted->n_values) {
- for (size_t i = 0; i < addr_deleted->n_values; i++) {
- ds_clear(&ds);
- expr_constant_format(&addr_deleted->values[i],
- EXPR_C_INTEGER, &ds);
- sbrec_address_set_update_addresses_delvalue(sb_as, ds_cstr(&ds));
- }
+
+ for (i = 0; i < sb_as->n_addresses; i++) {
+ svec_add(&svec_sb_as, sb_as->addresses[i]);
+ }
+
+ struct svec addr_added;
+ struct svec addr_deleted;
+
+ svec_sort(&svec_nb_as);
+ svec_sort(&svec_sb_as);
+ svec_diff(&svec_nb_as, &svec_sb_as, &addr_added, NULL, &addr_deleted);
+
+ SVEC_FOR_EACH (i, ip, &addr_added) {
+ sbrec_address_set_update_addresses_addvalue(sb_as, ip);
+ }
+
+ SVEC_FOR_EACH (i, ip, &addr_deleted) {
+ sbrec_address_set_update_addresses_delvalue(sb_as, ip);
}
- ds_destroy(&ds);
- expr_constant_set_destroy(cs_nb_as);
- free(cs_nb_as);
- expr_constant_set_destroy(cs_sb_as);
- free(cs_sb_as);
- expr_constant_set_destroy(addr_added);
- free(addr_added);
- expr_constant_set_destroy(addr_deleted);
- free(addr_deleted);
+ svec_destroy(&svec_nb_as);
+ svec_destroy(&svec_sb_as);
+ svec_destroy(&addr_added);
+ svec_destroy(&addr_deleted);
}
static void
--
2.40.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
