On 11/13/23 13:34, Mohammad Heib wrote:
> 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 <[email protected]>
> ---
With a couple of minor changes (see below) I applied this to the main
branch. Thanks, Mohammad!
Regards,
Dumitru
> NEWS | 4 +++
> controller/pinctrl.c | 43 ++++++++++++++++++++++++++++-
> northd/northd.c | 13 +++++++++
> ovn-nb.ovsschema | 9 ++++--
> ovn-nb.xml | 14 ++++++++++
> ovn-sb.ovsschema | 9 ++++--
> ovn-sb.xml | 6 ++++
> tests/ovn.at | 65 ++++++++++++++++++++++++++++++++++++++++++++
> 8 files changed, 158 insertions(+), 5 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 30f6edb28..e10fb79dd 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -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.
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index d88e951a6..cf48089b6 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -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;
> @@ -2713,6 +2714,7 @@ struct dns_data {
> uint64_t *dps;
> size_t n_dps;
> struct smap records;
> + struct smap options;
> bool delete;
> };
>
> @@ -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;
> @@ -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++) {
> @@ -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);
> }
> @@ -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);
> }
> @@ -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(
> @@ -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;
> @@ -2966,6 +2980,7 @@ 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;
> @@ -2978,6 +2993,7 @@ pinctrl_handle_dns_lookup(
> 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);
I moved this out of the inner loop.
> break;
> }
> }
> @@ -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;
> }
> @@ -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. */
> @@ -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);
> }
>
> diff --git a/northd/northd.c b/northd/northd.c
> index f8b046d83..e93d0c8f8 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -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.
> */
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> index e103360ec..b2e0993e0 100644
> --- a/ovn-nb.ovsschema
> +++ b/ovn-nb.ovsschema
> @@ -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": {
> @@ -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,
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 1de0c3041..4aa8107a5 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -4542,6 +4542,20 @@ 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 Table.
> +
> + <p> A <code>DNS Table</code> 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>
I fixed up indentation here.
> + </column>
> +
> <column name="external_ids">
> See <em>External IDs</em> at the beginning of this document.
> </column>
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index a512e00e5..72e230b75 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -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": {
> @@ -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,
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index 8d0bed94d..e581d6965 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -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.
> diff --git a/tests/ovn.at b/tests/ovn.at
> index b8c61f87f..543eca9d0 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -11281,6 +11281,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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
