[ovs-dev] [PATCH v2 ovn 3/3] ovn-controller: Minimize SB DB port_binding lookups.
Dumitru Ceara
dceara at redhat.com
Thu Sep 12 14:17:01 UTC 2019
On Wed, Aug 28, 2019 at 2:36 PM Numan Siddique <nusiddiq at redhat.com> wrote:
>
>
>
> On Fri, Aug 23, 2019 at 4:55 PM Dumitru Ceara <dceara at redhat.com> wrote:
>>
>> Instead of storing only peer_ports in struct local_datapath, store both
>> local-remote mappings for patch ports. Also, it's useful to directly
>> store sbrec_port_binding pointers for all datapath ports as we avoid
>> doing costly sbrec_port_binding index lookups by port name.
>>
>> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
>
>
> This patch needs a rebase.
>
> Thanks
> Numan
Done, I sent v3:
https://patchwork.ozlabs.org/project/openvswitch/list/?series=130413
Thanks,
Dumitru
>
>>
>> ---
>> controller/binding.c | 19 ++++++++++++---
>> controller/ovn-controller.h | 11 ++++++++-
>> controller/physical.c | 4 ++-
>> controller/pinctrl.c | 53 +++++++++++--------------------------------
>> 4 files changed, 41 insertions(+), 46 deletions(-)
>>
>> diff --git a/controller/binding.c b/controller/binding.c
>> index 47d4fea..242163d 100644
>> --- a/controller/binding.c
>> +++ b/controller/binding.c
>> @@ -160,13 +160,24 @@ add_local_datapath__(struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
>> peer->datapath, false,
>> depth + 1, local_datapaths);
>> ld->n_peer_ports++;
>> - ld->peer_ports = xrealloc(ld->peer_ports,
>> - ld->n_peer_ports *
>> - sizeof *ld->peer_ports);
>> - ld->peer_ports[ld->n_peer_ports - 1] = peer;
>> + if (ld->n_peer_ports > ld->n_allocated_peer_ports) {
>> + ld->peer_ports =
>> + x2nrealloc(ld->peer_ports,
>> + &ld->n_allocated_peer_ports,
>> + sizeof *ld->peer_ports);
>> + }
>> + ld->peer_ports[ld->n_peer_ports - 1].local = pb;
>> + ld->peer_ports[ld->n_peer_ports - 1].remote = peer;
>> }
>> }
>> }
>> +
>> + ld->n_ports++;
>> + if (ld->n_ports > ld->n_allocated_ports) {
>> + ld->ports = x2nrealloc(ld->ports, &ld->n_allocated_ports,
>> + sizeof *ld->ports);
>> + }
>> + ld->ports[ld->n_ports - 1] = pb;
>> }
>> sbrec_port_binding_index_destroy_row(target);
>> }
>> diff --git a/controller/ovn-controller.h b/controller/ovn-controller.h
>> index 41feec3..86b300e 100644
>> --- a/controller/ovn-controller.h
>> +++ b/controller/ovn-controller.h
>> @@ -60,8 +60,17 @@ struct local_datapath {
>> * hypervisor. */
>> bool has_local_l3gateway;
>>
>> - const struct sbrec_port_binding **peer_ports;
>> + const struct sbrec_port_binding **ports;
>> + size_t n_ports;
>> + size_t n_allocated_ports;
>> +
>> + struct {
>> + const struct sbrec_port_binding *local;
>> + const struct sbrec_port_binding *remote;
>> + } *peer_ports;
>> +
>> size_t n_peer_ports;
>> + size_t n_allocated_peer_ports;
>> };
>>
>> struct local_datapath *get_local_datapath(const struct hmap *,
>> diff --git a/controller/physical.c b/controller/physical.c
>> index a05962b..49bc1a4 100644
>> --- a/controller/physical.c
>> +++ b/controller/physical.c
>> @@ -266,11 +266,13 @@ put_replace_router_port_mac_flows(const struct
>> }
>>
>> for (int i = 0; i < ld->n_peer_ports; i++) {
>> - const struct sbrec_port_binding *rport_binding = ld->peer_ports[i];
>> + const struct sbrec_port_binding *rport_binding;
>> struct eth_addr router_port_mac;
>> struct match match;
>> struct ofpact_mac *replace_mac;
>>
>> + rport_binding = ld->peer_ports[i].remote;
>> +
>> /* Table 65, priority 150.
>> * =======================
>> *
>> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
>> index 21ed75f..a8ff0bb 100644
>> --- a/controller/pinctrl.c
>> +++ b/controller/pinctrl.c
>> @@ -174,8 +174,7 @@ static struct pinctrl pinctrl;
>> static void init_buffered_packets_map(void);
>> static void destroy_buffered_packets_map(void);
>> static void
>> -run_buffered_binding(struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
>> - struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
>> +run_buffered_binding(struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
>> const struct hmap *local_datapaths)
>> OVS_REQUIRES(pinctrl_mutex);
>>
>> @@ -239,10 +238,7 @@ static void wait_controller_event(struct ovsdb_idl_txn *ovnsb_idl_txn);
>> static void init_ipv6_ras(void);
>> static void destroy_ipv6_ras(void);
>> static void ipv6_ra_wait(long long int send_ipv6_ra_time);
>> -static void prepare_ipv6_ras(
>> - struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
>> - struct ovsdb_idl_index *sbrec_port_binding_by_name,
>> - const struct hmap *local_datapaths)
>> +static void prepare_ipv6_ras(const struct hmap *local_datapaths)
>> OVS_REQUIRES(pinctrl_mutex);
>> static void send_ipv6_ras(struct rconn *swconn,
>> long long int *send_ipv6_ra_time)
>> @@ -2166,8 +2162,7 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
>> send_garp_prepare(sbrec_port_binding_by_datapath,
>> sbrec_port_binding_by_name, br_int, chassis,
>> local_datapaths, active_tunnels);
>> - prepare_ipv6_ras(sbrec_port_binding_by_datapath,
>> - sbrec_port_binding_by_name, local_datapaths);
>> + prepare_ipv6_ras(local_datapaths);
>> sync_dns_cache(dns_table);
>> controller_event_run(ovnsb_idl_txn, ce_table, chassis);
>> ip_mcast_sync(ovnsb_idl_txn, chassis, local_datapaths,
>> @@ -2175,8 +2170,7 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
>> sbrec_port_binding_by_key,
>> sbrec_igmp_groups,
>> sbrec_ip_multicast_opts);
>> - run_buffered_binding(sbrec_port_binding_by_datapath,
>> - sbrec_mac_binding_by_lport_ip,
>> + run_buffered_binding(sbrec_mac_binding_by_lport_ip,
>> local_datapaths);
>> ovs_mutex_unlock(&pinctrl_mutex);
>> }
>> @@ -2412,9 +2406,7 @@ send_ipv6_ras(struct rconn *swconn, long long int *send_ipv6_ra_time)
>> /* Called by pinctrl_run(). Runs with in the main ovn-controller
>> * thread context. */
>> static void
>> -prepare_ipv6_ras(struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
>> - struct ovsdb_idl_index *sbrec_port_binding_by_name,
>> - const struct hmap *local_datapaths)
>> +prepare_ipv6_ras(const struct hmap *local_datapaths)
>> OVS_REQUIRES(pinctrl_mutex)
>> {
>> struct shash_node *iter, *iter_next;
>> @@ -2427,25 +2419,12 @@ prepare_ipv6_ras(struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
>> bool changed = false;
>> const struct local_datapath *ld;
>> HMAP_FOR_EACH (ld, hmap_node, local_datapaths) {
>> - struct sbrec_port_binding *target = sbrec_port_binding_index_init_row(
>> - sbrec_port_binding_by_datapath);
>> - sbrec_port_binding_index_set_datapath(target, ld->datapath);
>>
>> - struct sbrec_port_binding *pb;
>> - SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target,
>> - sbrec_port_binding_by_datapath) {
>> - if (!smap_get_bool(&pb->options, "ipv6_ra_send_periodic", false)) {
>> - continue;
>> - }
>> + for (size_t i = 0; i < ld->n_peer_ports; i++) {
>> + const struct sbrec_port_binding *peer = ld->peer_ports[i].remote;
>> + const struct sbrec_port_binding *pb = ld->peer_ports[i].local;
>>
>> - const char *peer_s = smap_get(&pb->options, "peer");
>> - if (!peer_s) {
>> - continue;
>> - }
>> -
>> - const struct sbrec_port_binding *peer
>> - = lport_lookup_by_name(sbrec_port_binding_by_name, peer_s);
>> - if (!peer) {
>> + if (!smap_get_bool(&pb->options, "ipv6_ra_send_periodic", false)) {
>> continue;
>> }
>>
>> @@ -2484,7 +2463,6 @@ prepare_ipv6_ras(struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
>>
>> /* pinctrl_handler thread will send the IPv6 RAs. */
>> }
>> - sbrec_port_binding_index_destroy_row(target);
>> }
>>
>> /* Remove those that are no longer in the SB database */
>> @@ -2729,8 +2707,7 @@ run_put_mac_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn,
>> }
>>
>> static void
>> -run_buffered_binding(struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
>> - struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
>> +run_buffered_binding(struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
>> const struct hmap *local_datapaths)
>> OVS_REQUIRES(pinctrl_mutex)
>> {
>> @@ -2738,13 +2715,10 @@ run_buffered_binding(struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
>> bool notify = false;
>>
>> HMAP_FOR_EACH (ld, hmap_node, local_datapaths) {
>> - struct sbrec_port_binding *target = sbrec_port_binding_index_init_row(
>> - sbrec_port_binding_by_datapath);
>> - sbrec_port_binding_index_set_datapath(target, ld->datapath);
>>
>> - const struct sbrec_port_binding *pb;
>> - SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target,
>> - sbrec_port_binding_by_datapath) {
>> + for (size_t i = 0; i < ld->n_ports; i++) {
>> +
>> + const struct sbrec_port_binding *pb = ld->ports[i];
>> struct buffered_packets *cur_qp, *next_qp;
>> HMAP_FOR_EACH_SAFE (cur_qp, next_qp, hmap_node,
>> &buffered_packets_map) {
>> @@ -2762,7 +2736,6 @@ run_buffered_binding(struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
>> ds_destroy(&ip_s);
>> }
>> }
>> - sbrec_port_binding_index_destroy_row(target);
>> }
>> buffered_packets_map_gc();
>>
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list