[SSSD] [PATCHES] ad srv: prefer servers that are in the same domain as client

Pavel Březina pbrezina at redhat.com
Thu Aug 8 11:24:17 UTC 2013


On 08/08/2013 03:32 AM, Simo Sorce wrote:
> On Wed, 2013-08-07 at 13:56 +0200, Lukas Slebodnik wrote:
> [..]
> nitpicking some more given Lucas nacked.
>
>>> +static errno_t ad_sort_servers(struct ares_srv_reply **list, void *pvt)
>>> +{
>>> +    struct ad_srv_plugin_state *state = NULL;
>>> +    struct ares_srv_reply dummy[3] = {{NULL, NULL, 0, 0, 0}};
>
> I think this can be done by simply writing:
>
> 	struct ares_srv_reply dummy[3] = { 0 };
>
>>> +    struct ares_srv_reply *head = &dummy[0];
>>> +    struct ares_srv_reply *tail = &dummy[0];
>>> +    struct ares_srv_reply *in_head = &dummy[1];
>>> +    struct ares_srv_reply *in_tail = &dummy[1];
>>> +    struct ares_srv_reply *out_head = &dummy[2];
>>> +    struct ares_srv_reply *out_tail = &dummy[2];
>>> +    struct ares_srv_reply *cur = NULL;
>>> +    struct ares_srv_reply *next = NULL;
>>> +    const char *domain = NULL;
>>> +    errno_t ret;
>>> +
>>> +    if (list == NULL) {
>>> +        ret = EINVAL;
>>> +        goto done;
>>> +    }
>>> +
>>> +    if (*list == NULL || (*list)->next == NULL) {
>>> +        ret = EOK;
>>> +        goto done;
>>> +    }
>>> +
>>> +    state = talloc_get_type(pvt, struct ad_srv_plugin_state);
>>> +    domain = state->discovery_domain;
>>> +
>>> +    /* first sort by rfc2782 */
>>> +    ret = resolv_sort_srv_reply(list);
>>> +    if (ret != EOK) {
>>> +        DEBUG(SSSDBG_CRIT_FAILURE, ("resolv_sort_srv_reply() failed "
>>> +                                    "[%d]: %s\n", ret, strerror(ret)));
>>> +        goto done;
>>> +    }
>>> +
>>> +    /* when several servers share priority, prefer the one that is located
>>> +     * in the same domain as client (e.g. child domain instead of forest
>>> +     * root) */
>>> +
>>> +    next = *list;
>>> +    do {
>>> +        cur = next;
>>> +        next = cur->next;
>>> +
>>> +        if (is_host_in_domain(cur->host, domain)) {
>>> +            in_tail->next = cur;
>>> +            in_tail = in_tail->next;
>>> +            in_tail->next = NULL;
>>> +        } else {
>>> +            out_tail->next = cur;
>>> +            out_tail = out_tail->next;
>>> +            out_tail->next = NULL;
>>> +        }
>>> +
>>> +        if (next == NULL || cur->priority != next->priority) {
>>> +            /* priority has changed or we have reached the end of the srv list,
>>> +             * we will merge the lists */
>>> +            APPEND_ARES_LIST(tail, in_head, in_tail);
>>> +            APPEND_ARES_LIST(tail, out_head, out_tail);
>>> +        }
>>> +    } while (next != NULL);
>>> +
>>> +    *list = head->next;
>>> +
>>> +    ret = EOK;
>>> +
>>> +done:
>>> +    return ret;
>>> +}
>>
>> I am sorry, but this linked list sorting is not very readable.
>> There is a lot of "struct ares_srv_reply" pointers:
>>      head, tail, in_head, in_tail, out_head, out_tail, cur, next
>> I tried to visualize this sorting with pencil and paper, but
>> it was a real mess.
>>
>> I would prefer to:
>>    either: use similar approach like in reply_priority_sort
>>            from file src/resolv/async_resolv.c (merger-sort)
>>    or: don't change this function, but unit test should be written for this
>>        function
>
> Why not simply make this a doubly linked list and use the functions in
> src/util/dlinklist.h ?

I'm working with struct ares_srv_reply that comes from ares api.

> Or if you really have to use single linked, maybe add a slinklist.h file
> in util/ that does things as much as possible like dlinklist.h so that
> we use the same style.

Done.



-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-resolv_sort_srv_reply-remove-unnecessary-mem_ctx.patch
Type: text/x-patch
Size: 4827 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130808/12ead07d/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-fo_discover_srv_send-allow-custom-ordering-function.patch
Type: text/x-patch
Size: 9375 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130808/12ead07d/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Add-is_host_in_domain-util-function.patch
Type: text/x-patch
Size: 3236 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130808/12ead07d/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Add-macros-to-manipulate-with-single-linked-list.patch
Type: text/x-patch
Size: 6899 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130808/12ead07d/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-ad-srv-prefer-servers-that-are-in-the-same-domain-as.patch
Type: text/x-patch
Size: 3883 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130808/12ead07d/attachment-0004.bin>


More information about the sssd-devel mailing list