Skip to content

Commit

Permalink
ovn-controller: Add a separate dns cache module and I-P for SB DNS.
Browse files Browse the repository at this point in the history
Presently,
  - Any changes to SB DNS, results in full recompute of lflow_output
    engine node (which can be expensive).

  - DNS internal cache is maintained by pinctrl module and because of
    pinctrl mutex, DNS repsonder handling in the pinctrl thread
    can get delayed if the mutex is held by ovn-controller main
    thread and may result in DNS timeouts for the VIFs.

This patch addressed these concerns by

 - Removing the sb_dns as input to lflow_output engine node as it is
   not required at all.

 - maintaining a separate DNS cache in a separate module.  DNS cache is
   now updated incrementally with the new engine node 'en_dns_cache' for
   any changes to the SB DNS table.  A separate mutex - dns_cache_mutex
   is used to protect the DNS cache.

pinctrl_run() no longer maintains the DNS cache and it doesn't need
to iterate all the SB DNS rows anymore.  This patch also removes
the 'dns_supports_ovn_owned' check in pinctrl as ovn-controller
doesn't update the SB DNS options column.

Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2024-October/053332.html
Reported-by: Gavin McKee <gavmckee80@googlemail.com>
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
  • Loading branch information
numansiddique committed Nov 12, 2024
1 parent 0f806cf commit 817d4e5
Show file tree
Hide file tree
Showing 6 changed files with 314 additions and 138 deletions.
4 changes: 3 additions & 1 deletion controller/automake.mk
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ controller_ovn_controller_SOURCES = \
controller/statctrl.h \
controller/statctrl.c \
controller/ct-zone.h \
controller/ct-zone.c
controller/ct-zone.c \
controller/ovn-dns.c \
controller/ovn-dns.h

controller_ovn_controller_LDADD = lib/libovn.la $(OVS_LIBDIR)/libopenvswitch.la
man_MANS += controller/ovn-controller.8
Expand Down
48 changes: 46 additions & 2 deletions controller/ovn-controller.c
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
#include "statctrl.h"
#include "lib/dns-resolve.h"
#include "ct-zone.h"
#include "ovn-dns.h"

VLOG_DEFINE_THIS_MODULE(main);

Expand Down Expand Up @@ -3321,6 +3322,44 @@ en_bfd_chassis_cleanup(void *data OVS_UNUSED){
sset_destroy(&bfd_chassis->bfd_chassis);
}

static void *
en_dns_cache_init(struct engine_node *node OVS_UNUSED,
struct engine_arg *arg OVS_UNUSED)
{
ovn_dns_cache_init();
return NULL;
}

static void
en_dns_cache_run(struct engine_node *node, void *data OVS_UNUSED)
{
const struct sbrec_dns_table *dns_table =
EN_OVSDB_GET(engine_get_input("SB_dns", node));

ovn_dns_sync_cache(dns_table);

engine_set_node_state(node, EN_UPDATED);
}

static bool
dns_cache_sb_dns_handler(struct engine_node *node, void *data OVS_UNUSED)
{
const struct sbrec_dns_table *dns_table =
EN_OVSDB_GET(engine_get_input("SB_dns", node));

ovn_dns_update_cache(dns_table);

engine_set_node_state(node, EN_UPDATED);
return true;
}

static void
en_dns_cache_cleanup(void *data OVS_UNUSED)
{
ovn_dns_cache_destroy();
}


/* Engine node which is used to handle the Non VIF data like
* - OVS patch ports
* - Tunnel ports and the related chassis information.
Expand Down Expand Up @@ -5053,6 +5092,7 @@ main(int argc, char *argv[])
ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lb_data, "lb_data");
ENGINE_NODE(mac_cache, "mac_cache");
ENGINE_NODE(bfd_chassis, "bfd_chassis");
ENGINE_NODE(dns_cache, "dns_cache");

#define SB_NODE(NAME, NAME_STR) ENGINE_NODE_SB(NAME, NAME_STR);
SB_NODES
Expand Down Expand Up @@ -5188,7 +5228,7 @@ main(int argc, char *argv[])
* process all changes. */
engine_add_input(&en_lflow_output, &en_sb_logical_dp_group,
engine_noop_handler);
engine_add_input(&en_lflow_output, &en_sb_dns, NULL);

engine_add_input(&en_lflow_output, &en_lb_data,
lflow_output_lb_data_handler);
engine_add_input(&en_lflow_output, &en_sb_fdb,
Expand Down Expand Up @@ -5247,6 +5287,11 @@ main(int argc, char *argv[])
engine_add_input(&en_mac_cache, &en_sb_port_binding,
engine_noop_handler);

engine_add_input(&en_dns_cache, &en_sb_dns,
dns_cache_sb_dns_handler);

engine_add_input(&en_controller_output, &en_dns_cache,
NULL);
engine_add_input(&en_controller_output, &en_lflow_output,
controller_output_lflow_output_handler);
engine_add_input(&en_controller_output, &en_pflow_output,
Expand Down Expand Up @@ -5691,7 +5736,6 @@ main(int argc, char *argv[])
sbrec_igmp_group,
sbrec_ip_multicast,
sbrec_fdb_by_dp_key_mac,
sbrec_dns_table_get(ovnsb_idl_loop.idl),
sbrec_controller_event_table_get(
ovnsb_idl_loop.idl),
sbrec_service_monitor_table_get(
Expand Down
233 changes: 233 additions & 0 deletions controller/ovn-dns.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,233 @@
/* Copyright (c) 2024, Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include <config.h>

/* OVS includes */
#include "include/openvswitch/shash.h"
#include "include/openvswitch/thread.h"
#include "openvswitch/vlog.h"

/* OVN includes. */
#include "lib/ovn-sb-idl.h"
#include "ovn-dns.h"

VLOG_DEFINE_THIS_MODULE(ovndns);

/* Internal DNS cache entry for each SB DNS record. */
struct dns_data {
uint64_t *dps;
size_t n_dps;
struct smap records;
struct smap options;
bool delete;
};

/* shash of 'struct dns_data'. */
static struct shash dns_cache_ = SHASH_INITIALIZER(&dns_cache_);

/* Mutex to protect dns_cache_. */
static struct ovs_mutex dns_cache_mutex = OVS_MUTEX_INITIALIZER;

static void update_cache_with_dns_rec(const struct sbrec_dns *,
struct dns_data *,
const char *dns_id,
struct shash *dns_cache);
void
ovn_dns_cache_init(void)
{
}

void
ovn_dns_cache_destroy(void)
{
ovs_mutex_lock(&dns_cache_mutex);
struct shash_node *iter;
SHASH_FOR_EACH_SAFE (iter, &dns_cache_) {
struct dns_data *d = iter->data;
shash_delete(&dns_cache_, iter);
smap_destroy(&d->records);
smap_destroy(&d->options);
free(d->dps);
free(d);
}
shash_destroy(&dns_cache_);
ovs_mutex_unlock(&dns_cache_mutex);
}

void
ovn_dns_sync_cache(const struct sbrec_dns_table *dns_table)
{
ovs_mutex_lock(&dns_cache_mutex);
struct shash_node *iter;
SHASH_FOR_EACH (iter, &dns_cache_) {
struct dns_data *d = iter->data;
d->delete = true;
}

const struct sbrec_dns *sbrec_dns;
SBREC_DNS_TABLE_FOR_EACH (sbrec_dns, dns_table) {
const char *dns_id = smap_get(&sbrec_dns->external_ids, "dns_id");
if (!dns_id) {
continue;
}

struct dns_data *dns_data = shash_find_data(&dns_cache_, dns_id);
if (!dns_data) {
dns_data = xmalloc(sizeof *dns_data);
smap_init(&dns_data->records);
smap_init(&dns_data->options);
shash_add(&dns_cache_, dns_id, dns_data);
dns_data->n_dps = 0;
dns_data->dps = NULL;
} else {
free(dns_data->dps);
}

dns_data->delete = false;

if (!smap_equal(&dns_data->records, &sbrec_dns->records)) {
smap_destroy(&dns_data->records);
smap_clone(&dns_data->records, &sbrec_dns->records);
}

if (!smap_equal(&dns_data->options, &sbrec_dns->options)) {
smap_destroy(&dns_data->options);
smap_clone(&dns_data->options, &sbrec_dns->options);
}

dns_data->n_dps = sbrec_dns->n_datapaths;
dns_data->dps = xcalloc(dns_data->n_dps, sizeof(uint64_t));
for (size_t i = 0; i < sbrec_dns->n_datapaths; i++) {
dns_data->dps[i] = sbrec_dns->datapaths[i]->tunnel_key;
}
}

SHASH_FOR_EACH_SAFE (iter, &dns_cache_) {
struct dns_data *d = iter->data;
if (d->delete) {
shash_delete(&dns_cache_, iter);
smap_destroy(&d->records);
smap_destroy(&d->options);
free(d->dps);
free(d);
}
}
ovs_mutex_unlock(&dns_cache_mutex);
}

void
ovn_dns_update_cache(const struct sbrec_dns_table *dns_table)
{
ovs_mutex_lock(&dns_cache_mutex);

const struct sbrec_dns *sbrec_dns;
SBREC_DNS_TABLE_FOR_EACH_TRACKED (sbrec_dns, dns_table) {
const char *dns_id = smap_get(&sbrec_dns->external_ids, "dns_id");
if (!dns_id) {
continue;
}

struct shash_node *shash_node = shash_find(&dns_cache_, dns_id);
if (sbrec_dns_is_deleted(sbrec_dns)) {
if (shash_node) {
struct dns_data *dns_data = shash_node->data;
shash_delete(&dns_cache_, shash_node);
smap_destroy(&dns_data->records);
smap_destroy(&dns_data->options);
free(dns_data->dps);
free(dns_data);
}
} else {
update_cache_with_dns_rec(sbrec_dns,
shash_node ? shash_node->data : NULL,
dns_id, &dns_cache_);
}
}

ovs_mutex_unlock(&dns_cache_mutex);
}

const char *
ovn_dns_lookup(const char *query_name, uint64_t dp_key, bool *ovn_owned)
{
ovs_mutex_lock(&dns_cache_mutex);

*ovn_owned = false;
struct shash_node *iter;
const char *answer_data = NULL;
SHASH_FOR_EACH (iter, &dns_cache_) {
struct dns_data *d = iter->data;
for (size_t i = 0; i < d->n_dps; i++) {
if (d->dps[i] == dp_key) {
/* DNS records in SBDB are stored in lowercase. Convert to
* lowercase to perform case insensitive lookup
*/
char *query_name_lower = str_tolower(query_name);
answer_data = smap_get(&d->records, query_name_lower);
free(query_name_lower);
if (answer_data) {
*ovn_owned = smap_get_bool(&d->options, "ovn-owned",
false);
break;
}
}
}

if (answer_data) {
break;
}
}

ovs_mutex_unlock(&dns_cache_mutex);

return answer_data;
}


/* Static functions. */
static void
update_cache_with_dns_rec(const struct sbrec_dns *sbrec_dns,
struct dns_data *dns_data,
const char *dns_id,
struct shash *dns_cache)
{
if (!dns_data) {
dns_data = xmalloc(sizeof *dns_data);
smap_init(&dns_data->records);
smap_init(&dns_data->options);
shash_add(dns_cache, dns_id, dns_data);
dns_data->n_dps = 0;
dns_data->dps = NULL;
} else {
free(dns_data->dps);
}

if (!smap_equal(&dns_data->records, &sbrec_dns->records)) {
smap_destroy(&dns_data->records);
smap_clone(&dns_data->records, &sbrec_dns->records);
}

if (!smap_equal(&dns_data->options, &sbrec_dns->options)) {
smap_destroy(&dns_data->options);
smap_clone(&dns_data->options, &sbrec_dns->options);
}

dns_data->n_dps = sbrec_dns->n_datapaths;
dns_data->dps = xcalloc(dns_data->n_dps, sizeof(uint64_t));
for (size_t i = 0; i < sbrec_dns->n_datapaths; i++) {
dns_data->dps[i] = sbrec_dns->datapaths[i]->tunnel_key;
}
}
29 changes: 29 additions & 0 deletions controller/ovn-dns.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/* Copyright (c) 2024, Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#ifndef OVN_DNS_H
#define OVN_DNS_H

struct shash;
struct sbrec_dns_table;

void ovn_dns_cache_init(void);
void ovn_dns_cache_destroy(void);
void ovn_dns_sync_cache(const struct sbrec_dns_table *);
void ovn_dns_update_cache(const struct sbrec_dns_table *);
const char *ovn_dns_lookup(const char *query_name, uint64_t dp_key,
bool *ovn_owned);

#endif /* OVN_DNS_H */
Loading

0 comments on commit 817d4e5

Please sign in to comment.