On Wed, Jun 26, 2024 at 8:20 AM Naveen Yerramneni < [email protected]> wrote:
> This change reduces the probability of conflicts when > multiple nodes tries to add the same FDB entry to SB at the > same time. When conflict occurs, OVN controller does full > recompute which is heavy weight on the scale setup. > > Signed-off-by: Naveen Yerramneni <[email protected]> > Suggested-by: Dumitru Ceara <[email protected]> > --- > controller/mac-cache.c | 4 +++- > controller/mac-cache.h | 4 +++- > controller/ovn-controller.c | 2 +- > controller/pinctrl.c | 28 ++++++++++++++++++++-------- > tests/ovn.at | 6 +++--- > 5 files changed, 30 insertions(+), 14 deletions(-) > > diff --git a/controller/mac-cache.c b/controller/mac-cache.c > index d8c4e2aed..de45d7a6a 100644 > --- a/controller/mac-cache.c > +++ b/controller/mac-cache.c > @@ -245,7 +245,8 @@ mac_binding_lookup(struct ovsdb_idl_index > *sbrec_mac_binding_by_lport_ip, > > /* FDB. */ > struct fdb * > -fdb_add(struct hmap *map, struct fdb_data fdb_data) { > +fdb_add(struct hmap *map, struct fdb_data fdb_data, long long timestamp) > +{ > struct fdb *fdb = fdb_find(map, &fdb_data); > > if (!fdb) { > @@ -255,6 +256,7 @@ fdb_add(struct hmap *map, struct fdb_data fdb_data) { > } > > fdb->data = fdb_data; > + fdb->timestamp = timestamp; > > return fdb; > } > diff --git a/controller/mac-cache.h b/controller/mac-cache.h > index 3c78f9440..b94e7a1cf 100644 > --- a/controller/mac-cache.h > +++ b/controller/mac-cache.h > @@ -83,6 +83,7 @@ struct fdb { > struct fdb_data data; > /* Reference to the SB FDB record. */ > const struct sbrec_fdb *sbrec_fdb; > + long long timestamp; > }; > > struct bp_packet_data { > @@ -149,7 +150,8 @@ mac_binding_lookup(struct ovsdb_idl_index > *sbrec_mac_binding_by_lport_ip, > const char *logical_port, const char *ip); > > /* FDB. */ > -struct fdb *fdb_add(struct hmap *map, struct fdb_data fdb_data); > +struct fdb *fdb_add(struct hmap *map, struct fdb_data fdb_data, > + long long timestamp); > > void fdb_remove(struct hmap *map, struct fdb *fdb); > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 6874f99a3..e99bdb3ef 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -3322,7 +3322,7 @@ fdb_add_sb(struct mac_cache_data *data, const struct > sbrec_fdb *sfdb) > return; > } > > - struct fdb *fdb = fdb_add(&data->fdbs, fdb_data); > + struct fdb *fdb = fdb_add(&data->fdbs, fdb_data, 0); > > fdb->sbrec_fdb = sfdb; > } > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index f2e382a44..e3ded793b 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -4739,6 +4739,7 @@ pinctrl_destroy(void) > /* Buffered "put_mac_binding" operation. */ > > #define MAX_MAC_BINDING_DELAY_MSEC 50 > +#define MAX_FDB_DELAY_MSEC 50 > #define MAX_MAC_BINDINGS 1000 > > /* Contains "struct mac_binding"s. */ > @@ -8961,21 +8962,30 @@ run_put_fdbs(struct ovsdb_idl_txn *ovnsb_idl_txn, > return; > } > > + long long now = time_msec(); > const struct fdb *fdb; > - HMAP_FOR_EACH (fdb, hmap_node, &put_fdbs) { > - run_put_fdb(ovnsb_idl_txn, sbrec_fdb_by_dp_key_mac, > - sbrec_port_binding_by_key, > - sbrec_datapath_binding_by_key, fdb); > + HMAP_FOR_EACH_SAFE (fdb, hmap_node, &put_fdbs) { > + if (now >= fdb->timestamp) { > + run_put_fdb(ovnsb_idl_txn, sbrec_fdb_by_dp_key_mac, > + sbrec_port_binding_by_key, > + sbrec_datapath_binding_by_key, fdb); > + fdb_remove(&put_fdbs, fdb); > + } > } > - fdbs_clear(&put_fdbs); > } > > > static void > wait_put_fdbs(struct ovsdb_idl_txn *ovnsb_idl_txn) > + OVS_REQUIRES(pinctrl_mutex) > { > - if (ovnsb_idl_txn && !hmap_is_empty(&put_fdbs)) { > - poll_immediate_wake(); > + if (!ovnsb_idl_txn) { > + return; > + } > + > + struct fdb *fdb; > + HMAP_FOR_EACH (fdb, hmap_node, &put_fdbs) { > + poll_timer_wait_until(fdb->timestamp); > } > } > > @@ -8995,6 +9005,8 @@ pinctrl_handle_put_fdb(const struct flow *md, const > struct flow *headers) > .mac = headers->dl_src, > }; > > - fdb_add(&put_fdbs, fdb_data); > + uint32_t delay = random_range(MAX_FDB_DELAY_MSEC) + 1; > + long long timestamp = time_msec() + delay; > + fdb_add(&put_fdbs, fdb_data, timestamp); > notify_pinctrl_main(); > } > diff --git a/tests/ovn.at b/tests/ovn.at > index af0eaeed3..7b199d3f5 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -34382,21 +34382,21 @@ AT_CHECK([ovn-nbctl --wait=hv sync]) > # Learning is disabled, the table should be empty > send_packet 20 > AT_CHECK([ovn-nbctl --wait=hv sync]) > -AT_CHECK([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:20") = 0]) > +OVS_WAIT_UNTIL([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:20") > = 0]) > > # Enable learning on localnet port > AT_CHECK([ovn-nbctl set logical_switch_port ln_port > options:localnet_learn_fdb=true]) > AT_CHECK([ovn-nbctl --wait=hv sync]) > send_packet 20 > AT_CHECK([ovn-nbctl --wait=hv sync]) > -AT_CHECK([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:20") = 1]) > +OVS_WAIT_UNTIL([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:20") > = 1]) > > # Disable learning on localnet port > AT_CHECK([ovn-nbctl set logical_switch_port ln_port > options:localnet_learn_fdb=false]) > AT_CHECK([ovn-nbctl --wait=hv sync]) > send_packet 30 > AT_CHECK([ovn-nbctl --wait=hv sync]) > -AT_CHECK([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:30") = 0]) > +OVS_WAIT_UNTIL([test $(ovn-sbctl list fdb | grep -c "00:00:00:00:10:30") > = 0]) > > OVN_CLEANUP([hv1]) > AT_CLEANUP > -- > 2.36.6 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Looks good to me, thanks. Acked-by: Ales Musil <[email protected]> -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
