Skip to content

Commit

Permalink
DNS: allow defining records that owned by OVN only
Browse files Browse the repository at this point in the history
Currently OVN allows users to create DNS records
and define domains within these records.

These domains can be associated with IPV4 or IPv6
or both, when the user creates a domain with both
IPv4 and IPv6 ovn will answer each query for this
domain immediately and everything works as expected.

But if the user only creates a domain with only IPv4
or IPv6 this will cause the DNS queries respond take
longer than the usual since OVN will forward query and
user will keep waiting for an answer until someone else
replies for this query or timeout occur.

The above behavior is a bit problematic if the user knows
that this domain is only configured in OVN we should not
forward queries for this domain to the outside.

This patch adds an option:ovn-owned for the DNS table
that can be set to "true" when creating a DNS.

When setting this option to "true" all the domains within
this table will be treated as local domains only,
and queries for domains within this table that don't have
an accurate IP will be refused immediately to save the time
of waiting for timeout.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1946662
Signed-off-by: Mohammad Heib <mheib@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
  • Loading branch information
mohammadheib authored and dceara committed Nov 20, 2023
1 parent 8e71bee commit 1622526
Show file tree
Hide file tree
Showing 8 changed files with 160 additions and 5 deletions.
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
Post v23.09.0
-------------
- DNS now have an "options" column for configuration of extra options.
- A new DNS option "ovn-owned" has been added to allow defining domains
that are owned only by ovn, queries for that domain will not be processed
externally.
- Disable OpenFlow inactivity probing between ovn-controller and OVS.
OF connection is established over unix socket, which is a reliable
connection method and doesn't require additional probing.
Expand Down
43 changes: 42 additions & 1 deletion controller/pinctrl.c
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ struct pinctrl {
struct latch pinctrl_thread_exit;
bool mac_binding_can_timestamp;
bool fdb_can_timestamp;
bool dns_supports_ovn_owned;
};

static struct pinctrl pinctrl;
Expand Down Expand Up @@ -2713,6 +2714,7 @@ struct dns_data {
uint64_t *dps;
size_t n_dps;
struct smap records;
struct smap options;
bool delete;
};

Expand Down Expand Up @@ -2741,6 +2743,7 @@ sync_dns_cache(const struct sbrec_dns_table *dns_table)
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;
Expand All @@ -2755,6 +2758,12 @@ sync_dns_cache(const struct sbrec_dns_table *dns_table)
smap_clone(&dns_data->records, &sbrec_dns->records);
}

if (pinctrl.dns_supports_ovn_owned
&& !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++) {
Expand All @@ -2767,6 +2776,7 @@ sync_dns_cache(const struct sbrec_dns_table *dns_table)
if (d->delete) {
shash_delete(&dns_cache, iter);
smap_destroy(&d->records);
smap_destroy(&d->options);
free(d->dps);
free(d);
}
Expand All @@ -2781,6 +2791,7 @@ destroy_dns_cache(void)
struct dns_data *d = iter->data;
shash_delete(&dns_cache, iter);
smap_destroy(&d->records);
smap_destroy(&d->options);
free(d->dps);
free(d);
}
Expand Down Expand Up @@ -2854,6 +2865,8 @@ dns_build_ptr_answer(
free(encoded);
}

#define DNS_RCODE_SERVER_REFUSE 0x5

/* Called with in the pinctrl_handler thread context. */
static void
pinctrl_handle_dns_lookup(
Expand All @@ -2867,6 +2880,7 @@ pinctrl_handle_dns_lookup(
enum ofputil_protocol proto = ofputil_protocol_from_ofp_version(version);
struct dp_packet *pkt_out_ptr = NULL;
uint32_t success = 0;
bool send_refuse = false;

/* Parse result field. */
const struct mf_field *f;
Expand Down Expand Up @@ -2966,9 +2980,11 @@ pinctrl_handle_dns_lookup(

uint64_t dp_key = ntohll(pin->flow_metadata.flow.metadata);
const char *answer_data = NULL;
bool ovn_owned = false;
struct shash_node *iter;
SHASH_FOR_EACH (iter, &dns_cache) {
struct dns_data *d = iter->data;
ovn_owned = smap_get_bool(&d->options, "ovn-owned", false);
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
Expand Down Expand Up @@ -3024,10 +3040,22 @@ pinctrl_handle_dns_lookup(
ancount++;
}
}

/* DNS is configured with a record for this domain with
* an IPv4/IPV6 only, so instead of ignoring this A/AAAA query,
* we can reply with RCODE = 5 (server refuses) and that
* will speed up the DNS process by not letting the customer
* wait for a timeout.
*/
if (ovn_owned && (query_type == DNS_QUERY_TYPE_AAAA ||
query_type == DNS_QUERY_TYPE_A) && !ancount) {
send_refuse = true;
}

destroy_lport_addresses(&ip_addrs);
}

if (!ancount) {
if (!ancount && !send_refuse) {
ofpbuf_uninit(&dns_answer);
goto exit;
}
Expand Down Expand Up @@ -3062,6 +3090,10 @@ pinctrl_handle_dns_lookup(

/* Set the answer RRs. */
out_dns_header->ancount = htons(ancount);
if (send_refuse) {
/* set RCODE = 5 (server refuses). */
out_dns_header->hi_flag |= DNS_RCODE_SERVER_REFUSE;
}
out_dns_header->arcount = 0;

/* Copy the Query section. */
Expand Down Expand Up @@ -3530,6 +3562,15 @@ pinctrl_update(const struct ovsdb_idl *idl, const char *br_int_name)
notify_pinctrl_handler();
}

bool dns_supports_ovn_owned = sbrec_server_has_dns_table_col_options(idl);
if (dns_supports_ovn_owned != pinctrl.dns_supports_ovn_owned) {
pinctrl.dns_supports_ovn_owned = dns_supports_ovn_owned;

/* Notify pinctrl_handler that fdb timestamp column
* availability has changed. */
notify_pinctrl_handler();
}

ovs_mutex_unlock(&pinctrl_mutex);
}

Expand Down
13 changes: 13 additions & 0 deletions northd/northd.c
Original file line number Diff line number Diff line change
Expand Up @@ -17301,6 +17301,19 @@ sync_dns_entries(struct ovsdb_idl_txn *ovnsb_txn,
free(dns_id);
}

/* Copy DNS options to SB*/
struct smap options = SMAP_INITIALIZER(&options);
if (!smap_is_empty(&dns_info->sb_dns->options)) {
smap_clone(&options, &dns_info->sb_dns->options);
}

bool ovn_owned = smap_get_bool(&dns_info->nb_dns->options,
"ovn-owned", false);
smap_replace(&options, "ovn-owned",
ovn_owned? "true" : "false");
sbrec_dns_set_options(dns_info->sb_dns, &options);
smap_destroy(&options);

/* Set the datapaths and records. If nothing has changed, then
* this will be a no-op.
*/
Expand Down
9 changes: 7 additions & 2 deletions ovn-nb.ovsschema
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "OVN_Northbound",
"version": "7.1.0",
"cksum": "217362582 33949",
"version": "7.2.0",
"cksum": "1069338687 34162",
"tables": {
"NB_Global": {
"columns": {
Expand Down Expand Up @@ -560,6 +560,11 @@
"value": "string",
"min": 0,
"max": "unlimited"}},
"options": {
"type": {"key": "string",
"value": "string",
"min": 0,
"max": "unlimited"}},
"external_ids": {"type": {"key": "string",
"value": "string",
"min": 0,
Expand Down
16 changes: 16 additions & 0 deletions ovn-nb.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4542,6 +4542,22 @@ or
<p><b>Example: </b> "4.0.0.10.in-addr.arpa" = "vm1.ovn.org"</p>
</column>

<column name="options" key="ovn-owned">
If set to true, then the OVN will be the main responsible for
<code>DNS Records</code> within this row.

<p>
A <code>DNS</code> row with this option set to <code>true</code>
can be created for domains that the user needs to configure
locally and don't care about IPv6 only interested in IPv4 or
vice versa.

This will let ovn send IPv4 DNS reply and reject/ignore IPv6
queries to save the waiting for a timeout on those uninteresting
queries.
</p>
</column>

<column name="external_ids">
See <em>External IDs</em> at the beginning of this document.
</column>
Expand Down
9 changes: 7 additions & 2 deletions ovn-sb.ovsschema
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "OVN_Southbound",
"version": "20.29.0",
"cksum": "3967183656 30959",
"version": "20.30.0",
"cksum": "2972392849 31172",
"tables": {
"SB_Global": {
"columns": {
Expand Down Expand Up @@ -363,6 +363,11 @@
"refTable": "Datapath_Binding"},
"min": 1,
"max": "unlimited"}},
"options": {
"type": {"key": "string",
"value": "string",
"min": 0,
"max": "unlimited"}},
"external_ids": {"type": {"key": "string",
"value": "string",
"min": 0,
Expand Down
6 changes: 6 additions & 0 deletions ovn-sb.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4500,6 +4500,12 @@ tcp.flags = RST;
in this column.
</column>

<column name="options" key="ovn-owned">
This column indicates that all the <code>Domains</code> in this table
are owned by OVN, and all <code>DNS queries</code> for those domains
will be answered locally by either an IP address or
<code>DNS rejection</code>.
</column>
<group title="Common Columns">
<column name="external_ids">
See <em>External IDs</em> at the beginning of this document.
Expand Down
65 changes: 65 additions & 0 deletions tests/ovn.at
Original file line number Diff line number Diff line change
Expand Up @@ -11283,6 +11283,71 @@ reset_pcap_file hv1-vif2 hv1/vif2
rm -f 1.expected
rm -f 2.expected

# send AAAA query for a server known domain that don't have
# any IPV6 address associated with this domain, and expected
# server refused DNS reply to save the sender time of waiting for timeout.
AS_BOX([Test IPv6 (AAAA records) NO timeout.])
# Add back the DNS options for ls1-lp1 without ipv6.
check ovn-nbctl remove DNS $DNS1 records vm1.ovn.org
check ovn-nbctl set DNS $DNS1 records:vm1.ovn.org="10.0.0.4"
check ovn-nbctl --wait=hv set DNS $DNS1 options:ovn-owned=true
ovn-sbctl list DNS > dns6
AT_CAPTURE_FILE([dns6])
ovn-sbctl dump-flows > sbflows6
AT_CAPTURE_FILE([sbflows6])

set_dns_params vm1_ipv6_only
src_ip=`ip_to_hex 10 0 0 6`
dst_ip=`ip_to_hex 10 0 0 1`
dns_reply=1
test_dns 2 f00000000002 f000000000f0 $src_ip $dst_ip $dns_reply $dns_req_data $dns_resp_data

# NXT_RESUMEs should be 13.
OVS_WAIT_UNTIL([test 13 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])

$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets
# dns hdr with server refuse RCODE
echo "01028125" > expout
# only check for the DNS HDR flags since we are not getting any DNS answer
AT_CHECK([cat 2.packets | cut -c -92 | cut -c 85-], [0], [expout])

reset_pcap_file hv1-vif1 hv1/vif1
reset_pcap_file hv1-vif2 hv1/vif2
rm -f 1.expected
rm -f 2.expected

# send A query for a server known domain that don't have
# any IPv4 address associated with this domain, and expected
# server refused DNS reply to save the sender time of waiting for timeout.
AS_BOX([Test IPv4 (A records) NO timeout.])
# Add back the DNS options for ls1-lp1 without ipv4.
check ovn-nbctl remove DNS $DNS1 records vm1.ovn.org
check ovn-nbctl set DNS $DNS1 records:vm1.ovn.org="aef0::4"
check ovn-nbctl --wait=hv set DNS $DNS1 options:ovn-owned=true
ovn-sbctl list DNS > dns7
AT_CAPTURE_FILE([dns7])
ovn-sbctl dump-flows > sbflows7
AT_CAPTURE_FILE([sbflows7])

set_dns_params vm1
src_ip=`ip_to_hex 10 0 0 6`
dst_ip=`ip_to_hex 10 0 0 1`
dns_reply=1
test_dns 2 f00000000002 f000000000f0 $src_ip $dst_ip $dns_reply $dns_req_data $dns_resp_data

# NXT_RESUMEs should be 14.
OVS_WAIT_UNTIL([test 14 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])

$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets
# dns hdr with server refuse RCODE
echo "01028125" > expout
# only check for the DNS HDR flags since we are not getting any DNS answer
AT_CHECK([cat 2.packets | cut -c -92 | cut -c 85-], [0], [expout])

reset_pcap_file hv1-vif1 hv1/vif1
reset_pcap_file hv1-vif2 hv1/vif2
rm -f 1.expected
rm -f 2.expected
OVN_CLEANUP([hv1])

AT_CLEANUP
Expand Down

0 comments on commit 1622526

Please sign in to comment.