On 09/04/2013 10:01 AM, Jakub Hrozek wrote:
On Mon, Aug 26, 2013 at 04:26:09PM +0200, Pavel Březina wrote:
Yes, I don't have a problem with that. As long as priority and weight
are honored.
I have done it AD only after all. Enabling it for all providers
required more changes than expected. I don't think those changes
should be done in this state of development...
I like the new approach.
From 9c03180cadcc8bde2da5037156034ba4591a6c15 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Fri, 2 Aug 2013 23:07:53 +0200
Subject: [PATCH 1/4] resolv_sort_srv_reply: remove unnecessary mem_ctx
ACK. This function is also unit-tested and the tests include leak
checks.
From 093a7292376593927b4bbaea29ccc208e83ff1a6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Mon, 26 Aug 2013 11:52:42 +0200
Subject: [PATCH 2/4] fo srv: add priority and weight to fo_server_info
This will give SRV plugins all information needed for additional
sorting.
I don't see the weight used anywhere except the place where they are
assigned.
From dd634784178fad7a2c98d06a5c2316f546a9727a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Sat, 3 Aug 2013 01:02:02 +0200
Subject: [PATCH 3/4] utils: add is_host_in_domain()
ACK
From 36610e77be90227e5e8c02fd1ea2192e3307059b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Mon, 26 Aug 2013 16:02:51 +0200
Subject: [PATCH 4/4] ad srv: prefer servers that are in the same domain as
client
https://fedorahosted.org/sssd/ticket/2001
ACK
Thanks for the review. New weightless patches are attached.
From 28b2d9dd2f1c00f5c0367c46c9e46ca8865b8889 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Fri, 2 Aug 2013 23:07:53 +0200
Subject: [PATCH 1/4] resolv_sort_srv_reply: remove unnecessary mem_ctx
---
src/providers/fail_over_srv.c | 2 +-
src/resolv/async_resolv.c | 18 +++++++++++-------
src/resolv/async_resolv.h | 2 +-
src/tests/resolv-tests.c | 4 ++--
4 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/src/providers/fail_over_srv.c b/src/providers/fail_over_srv.c
index 189abf14b87bb9475db2fb2049edcb48fb2f47bc..76f6d4689e0fba2d2bcf7d1b6a4ece3b4d382fac 100644
--- a/src/providers/fail_over_srv.c
+++ b/src/providers/fail_over_srv.c
@@ -98,7 +98,7 @@ static void fo_discover_srv_done(struct tevent_req *subreq)
DEBUG(SSSDBG_TRACE_FUNC, ("Got answer. Processing...\n"));
/* sort and store the answer */
- ret = resolv_sort_srv_reply(state, &reply_list);
+ ret = resolv_sort_srv_reply(&reply_list);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, ("Could not sort the answers from DNS "
"[%d]: %s\n", ret, strerror(ret)));
diff --git a/src/resolv/async_resolv.c b/src/resolv/async_resolv.c
index 0a2c5189f8ebf2f10e326e133718714e06de1b8a..dda288575f62fde9f3553402bf70382d27c535a3 100644
--- a/src/resolv/async_resolv.c
+++ b/src/resolv/async_resolv.c
@@ -2048,8 +2048,7 @@ static struct ares_srv_reply *reply_priority_sort(struct ares_srv_reply *list)
return list;
}
-static int reply_weight_rearrange(TALLOC_CTX *mem_ctx,
- int len,
+static int reply_weight_rearrange(int len,
struct ares_srv_reply **start,
struct ares_srv_reply **end)
{
@@ -2059,12 +2058,13 @@ static int reply_weight_rearrange(TALLOC_CTX *mem_ctx,
struct ares_srv_reply *r, *prev, *tmp;
struct ares_srv_reply *new_start = NULL;
struct ares_srv_reply *new_end = NULL;
+ int ret;
if (len <= 1) {
return EOK;
}
- totals = talloc_array(mem_ctx, int, len);
+ totals = talloc_array(NULL, int, len);
if (!totals) {
return ENOMEM;
}
@@ -2122,7 +2122,8 @@ static int reply_weight_rearrange(TALLOC_CTX *mem_ctx,
if (r == NULL || totals[i] == -1) {
DEBUG(1, ("Bug: did not select any server!\n"));
- return EIO;
+ ret = EIO;
+ goto done;
}
/* remove r from the old list */
@@ -2146,12 +2147,15 @@ static int reply_weight_rearrange(TALLOC_CTX *mem_ctx,
/* return the rearranged list */
*start = new_start;
*end = new_end;
+ ret = EOK;
+
+done:
talloc_free(totals);
- return EOK;
+ return ret;
}
int
-resolv_sort_srv_reply(TALLOC_CTX *mem_ctx, struct ares_srv_reply **reply)
+resolv_sort_srv_reply(struct ares_srv_reply **reply)
{
int ret;
struct ares_srv_reply *pri_start, *pri_end, *next, *prev_end;
@@ -2184,7 +2188,7 @@ resolv_sort_srv_reply(TALLOC_CTX *mem_ctx, struct ares_srv_reply **reply)
/* rearrange each priority level according to the weight field */
next = pri_end->next;
pri_end->next = NULL;
- ret = reply_weight_rearrange(mem_ctx, len, &pri_start, &pri_end);
+ ret = reply_weight_rearrange(len, &pri_start, &pri_end);
if (ret) {
DEBUG(1, ("Error rearranging priority level [%d]: %s\n",
ret, strerror(ret)));
diff --git a/src/resolv/async_resolv.h b/src/resolv/async_resolv.h
index 2895753b4b43573cc935c112eb5518edbedf00b0..919ba370b87304735bff279d1502386d2ac28735 100644
--- a/src/resolv/async_resolv.h
+++ b/src/resolv/async_resolv.h
@@ -148,7 +148,7 @@ int resolv_getsrv_recv(TALLOC_CTX *mem_ctx,
/* This is an implementation of section "Usage rules" of RFC 2782 */
int
-resolv_sort_srv_reply(TALLOC_CTX *mem_ctx, struct ares_srv_reply **reply);
+resolv_sort_srv_reply(struct ares_srv_reply **reply);
/** Get TXT record **/
struct tevent_req *resolv_gettxt_send(TALLOC_CTX *mem_ctx,
diff --git a/src/tests/resolv-tests.c b/src/tests/resolv-tests.c
index a8e4c4120cee286890dfdd67f3176fdf0463c162..8fde675f4040e47d1387858d1512c310a85369f6 100644
--- a/src/tests/resolv-tests.c
+++ b/src/tests/resolv-tests.c
@@ -722,7 +722,7 @@ START_TEST(test_resolv_sort_srv_reply)
}
/* do the sort */
- ret = resolv_sort_srv_reply(test_ctx, &replies);
+ ret = resolv_sort_srv_reply(&replies);
fail_if(ret != EOK);
/* check if the list is sorted */
@@ -757,7 +757,7 @@ START_TEST(test_resolv_sort_srv_reply)
}
/* do the sort */
- ret = resolv_sort_srv_reply(test_ctx, &replies);
+ ret = resolv_sort_srv_reply(&replies);
fail_if(ret != EOK);
/* clean up */
--
1.7.11.7
From 8ad552610b19edc3ce6e337de88d32ac87f90c2d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Mon, 26 Aug 2013 11:52:42 +0200
Subject: [PATCH 2/4] fo srv: add priority to fo_server_info
This will give SRV plugins all information needed for additional
sorting.
---
src/providers/fail_over_srv.c | 1 +
src/providers/fail_over_srv.h | 2 ++
2 files changed, 3 insertions(+)
diff --git a/src/providers/fail_over_srv.c b/src/providers/fail_over_srv.c
index 76f6d4689e0fba2d2bcf7d1b6a4ece3b4d382fac..de3b7f9ce92000252e6ae036febaa5a614ca244f 100644
--- a/src/providers/fail_over_srv.c
+++ b/src/providers/fail_over_srv.c
@@ -124,6 +124,7 @@ static void fo_discover_srv_done(struct tevent_req *subreq)
record = record->next, i++) {
state->servers[i].host = talloc_steal(state->servers, record->host);
state->servers[i].port = record->port;
+ state->servers[i].priority = record->priority;
}
talloc_zfree(reply_list);
diff --git a/src/providers/fail_over_srv.h b/src/providers/fail_over_srv.h
index 568350d1ecb6f473fa2072874ebb64dd1161d50a..8d193cc3c3e96df5acf0cd5231c44c383290bbe2 100644
--- a/src/providers/fail_over_srv.h
+++ b/src/providers/fail_over_srv.h
@@ -31,6 +31,8 @@
struct fo_server_info {
char *host;
int port;
+ unsigned short priority;
+ unsigned short weight;
};
/*
--
1.7.11.7
From 0bb2cc20ef6f181d32f76e814f5edad415a65b76 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Sat, 3 Aug 2013 01:02:02 +0200
Subject: [PATCH 3/4] utils: add is_host_in_domain()
---
src/tests/util-tests.c | 28 ++++++++++++++++++++++++++++
src/util/util.c | 15 +++++++++++++++
src/util/util.h | 2 ++
3 files changed, 45 insertions(+)
diff --git a/src/tests/util-tests.c b/src/tests/util-tests.c
index 9af12081dd64d0b94cae7a190bf7915181e3d57b..fd3dfb9bbbfd0a5cd6ee3a97e13630404f1c03bd 100644
--- a/src/tests/util-tests.c
+++ b/src/tests/util-tests.c
@@ -808,6 +808,33 @@ START_TEST(test_split_on_separator)
}
END_TEST
+START_TEST(test_is_host_in_domain)
+{
+ struct {
+ const char *host;
+ const char *domain;
+ bool expected;
+ } data[] = {{"example.com", "example.com", true},
+ {"client.example.com", "example.com", true},
+ {"client.child.example.com", "example.com", true},
+ {"example.com", "child.example.com", false},
+ {"client.example.com", "child.example.com", false},
+ {"client.child.example.com", "child.example.com", true},
+ {"my.com", "example.com", false},
+ {"myexample.com", "example.com", false},
+ {NULL, NULL, false}};
+ bool ret;
+ int i;
+
+ for (i = 0; data[i].host != NULL; i++) {
+ ret = is_host_in_domain(data[i].host, data[i].domain);
+ fail_if(ret != data[i].expected, "Host: %s, Domain: %s, Expected: %d, "
+ "Got: %d\n", data[i].host, data[i].domain,
+ data[i].expected, ret);
+ }
+}
+END_TEST
+
Suite *util_suite(void)
{
Suite *s = suite_create("util");
@@ -824,6 +851,7 @@ Suite *util_suite(void)
tcase_add_test (tc_util, test_add_string_to_list);
tcase_add_test (tc_util, test_string_in_list);
tcase_add_test (tc_util, test_split_on_separator);
+ tcase_add_test (tc_util, test_is_host_in_domain);
tcase_set_timeout(tc_util, 60);
TCase *tc_utf8 = tcase_create("utf8");
diff --git a/src/util/util.c b/src/util/util.c
index 4802967663cb1409d1ccad98a3841f7342e4d925..fb3bed146e2c634375a1133ef512673dee16718a 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -724,3 +724,18 @@ int domain_to_basedn(TALLOC_CTX *memctx, const char *domain, char **basedn)
*basedn = dn;
return EOK;
}
+
+bool is_host_in_domain(const char *host, const char *domain)
+{
+ int diff = strlen(host) - strlen(domain);
+
+ if (diff == 0 && strcmp(host, domain) == 0) {
+ return true;
+ }
+
+ if (diff > 0 && strcmp(host + diff, domain) == 0 && host[diff - 1] == '.') {
+ return true;
+ }
+
+ return false;
+}
diff --git a/src/util/util.h b/src/util/util.h
index 516edc81cab5792a30062b2908a58cc3fc21cef4..c2c31f5d50e86315a3e751daf49cf594550bc7c6 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -506,6 +506,8 @@ void safezero(void *data, size_t size);
int domain_to_basedn(TALLOC_CTX *memctx, const char *domain, char **basedn);
+bool is_host_in_domain(const char *host, const char *domain);
+
/* from nscd.c */
enum nscd_db {
NSCD_DB_PASSWD,
--
1.7.11.7
From 210cd5852a865e389caf83f220e976114bf2f030 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Mon, 26 Aug 2013 16:02:51 +0200
Subject: [PATCH 4/4] ad srv: prefer servers that are in the same domain as
client
https://fedorahosted.org/sssd/ticket/2001
---
src/providers/ad/ad_srv.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 89 insertions(+)
diff --git a/src/providers/ad/ad_srv.c b/src/providers/ad/ad_srv.c
index 4ae91f07bfbaafcb12caf82d3e3847f111cdf9fc..af25c10728ea047655013ee474934ed280236aa6 100644
--- a/src/providers/ad/ad_srv.c
+++ b/src/providers/ad/ad_srv.c
@@ -38,6 +38,77 @@
#define AD_AT_NT_VERSION "NtVer"
#define AD_AT_NETLOGON "netlogon"
+static errno_t ad_sort_servers_by_dns(TALLOC_CTX *mem_ctx,
+ const char *domain,
+ struct fo_server_info **_srv,
+ size_t num)
+{
+ struct fo_server_info *out = NULL;
+ struct fo_server_info *srv = NULL;
+ struct fo_server_info in_domain[num];
+ struct fo_server_info out_domain[num];
+ size_t srv_index = 0;
+ size_t in_index = 0;
+ size_t out_index = 0;
+ size_t i, j;
+
+ if (_srv == NULL) {
+ return EINVAL;
+ }
+
+ srv = *_srv;
+
+ if (num <= 1) {
+ return EOK;
+ }
+
+ out = talloc_zero_array(mem_ctx, struct fo_server_info, num);
+ if (out == NULL) {
+ return ENOMEM;
+ }
+
+ /* When several servers share priority, we will prefer the one that
+ * is located in the same domain as client (e.g. child domain instead
+ * of forest root) but obey their weight. We will use the fact that
+ * the servers are already sorted by priority. */
+
+ for (i = 0; i < num; i++) {
+ if (is_host_in_domain(srv[i].host, domain)) {
+ /* this is a preferred server, push it to the in domain list */
+ in_domain[in_index] = srv[i];
+ in_index++;
+ } else {
+ /* this is a normal server, push it to the out domain list */
+ out_domain[out_index] = srv[i];
+ out_index++;
+ }
+
+ if (i + 1 == num || srv[i].priority != srv[i + 1].priority) {
+ /* priority has changed or we have reached the end of the srv list,
+ * we will merge the list into final list and start over with
+ * next priority */
+ for (j = 0; j < in_index; j++) {
+ out[srv_index] = in_domain[j];
+ talloc_steal(out, out[srv_index].host);
+ srv_index++;
+ }
+
+ for (j = 0; j < out_index; j++) {
+ out[srv_index] = out_domain[j];
+ talloc_steal(out, out[srv_index].host);
+ srv_index++;
+ }
+
+ in_index = 0;
+ out_index = 0;
+ }
+ }
+
+ talloc_free(*_srv);
+ *_srv = out;
+ return EOK;
+}
+
struct ad_get_dc_servers_state {
struct fo_server_info *servers;
size_t num_servers;
@@ -762,6 +833,24 @@ static void ad_srv_plugin_servers_done(struct tevent_req *subreq)
DEBUG(SSSDBG_TRACE_FUNC, ("Got %lu primary and %lu backup servers\n",
state->num_primary_servers, state->num_backup_servers));
+ ret = ad_sort_servers_by_dns(state, state->discovery_domain,
+ &state->primary_servers,
+ state->num_primary_servers);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE, ("Unable to sort primary servers by DNS"
+ "[%d]: %s\n", ret, sss_strerror(ret)));
+ /* continue */
+ }
+
+ ret = ad_sort_servers_by_dns(state, state->discovery_domain,
+ &state->backup_servers,
+ state->num_backup_servers);
+ if (ret != EOK) {
+ DEBUG(SSSDBG_MINOR_FAILURE, ("Unable to sort backup servers by DNS"
+ "[%d]: %s\n", ret, sss_strerror(ret)));
+ /* continue */
+ }
+
tevent_req_done(req);
}
--
1.7.11.7
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
