[PATCH v3] Implement DNS round robin (with generate_random_buffer())

[Garming Sam]

Hi,

Thanks for the patch, it seems to work as I would expect. Unfortunately, 
the current tests are not built with this randomization in mind and 
running 'make test' causes a number of failures. The parameter would 
have to be turned off for our normal testing and a different environment 
required for testing this feature in particular. At the very least, it 
means that we can't simply rush this feature in right now.


Thanks,

Garming Sam

On 20/07/15 09:17, Robin McCorkell wrote:
> Could we get this feature merged before feature freeze on Tuesday? Kai
> said it looked good, and it improves feature parity with Microsoft
> domain controllers (which DNS shuffle by default).
>
> On 07/07/2015 3:33 PM, Robin McCorkell wrote:
>> Controlled by a default-on parameter in smb.conf, DNS responses will be
>> rotated by an inplace O(n) rotation, with rotation amount decided by
>> random number.
>>
>> Signed-off-by: Robin McCorkell <rmccorkell at karoshi.org.uk>
>> ---
>>   docs-xml/smbdotconf/domain/dnsroundrobin.xml | 11 +++++++++
>>   lib/param/loadparm.c                         |  5 ++--
>>   lib/param/param_table.c                      |  8 +++++++
>>   source4/dns_server/dns_query.c               | 34 ++++++++++++++++++++++++++++
>>   4 files changed, 56 insertions(+), 2 deletions(-)
>>   create mode 100644 docs-xml/smbdotconf/domain/dnsroundrobin.xml
>>
>> diff --git a/docs-xml/smbdotconf/domain/dnsroundrobin.xml b/docs-xml/smbdotconf/domain/dnsroundrobin.xml
>> new file mode 100644
>> index 0000000..1095533
>> --- /dev/null
>> +++ b/docs-xml/smbdotconf/domain/dnsroundrobin.xml
>> @@ -0,0 +1,11 @@
>> +<samba:parameter name="dns round robin"
>> +                 context="G"
>> +                 type="boolean"
>> +                 xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
>> +<description>
>> +	<para>If set to <constant>yes</constant>, Samba will rotate responses to
>> +	DNS queries, to provide round robin load balancing.</para>
>> +</description>
>> +
>> +<value type="default">yes</value>
>> +</samba:parameter>
>> diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
>> index bb215b2..b381e3e 100644
>> --- a/lib/param/loadparm.c
>> +++ b/lib/param/loadparm.c
>> @@ -2546,8 +2546,9 @@ struct loadparm_context *loadparm_init(TALLOC_CTX *mem_ctx)
>>   	lpcfg_do_global_parameter(lp_ctx, "rndc command", "/usr/sbin/rndc");
>>   	lpcfg_do_global_parameter(lp_ctx, "nsupdate command", "/usr/bin/nsupdate -g");
>>   
>> -        lpcfg_do_global_parameter(lp_ctx, "allow dns updates", "secure only");
>> -        lpcfg_do_global_parameter(lp_ctx, "dns forwarder", "");
>> +	lpcfg_do_global_parameter(lp_ctx, "dns round robin", "True");
>> +	lpcfg_do_global_parameter(lp_ctx, "allow dns updates", "secure only");
>> +	lpcfg_do_global_parameter(lp_ctx, "dns forwarder", "");
>>   
>>   	lpcfg_do_global_parameter(lp_ctx, "algorithmic rid base", "1000");
>>   
>> diff --git a/lib/param/param_table.c b/lib/param/param_table.c
>> index 287839f..4cd342b 100644
>> --- a/lib/param/param_table.c
>> +++ b/lib/param/param_table.c
>> @@ -3829,6 +3829,14 @@ struct parm_struct parm_table[] = {
>>   		.enum_list	= NULL,
>>   	},
>>   	{
>> +		.label          = "dns round robin",
>> +		.type           = P_BOOL,
>> +		.p_class        = P_GLOBAL,
>> +		.offset         = GLOBAL_VAR(dns_round_robin),
>> +		.special        = NULL,
>> +		.enum_list      = NULL,
>> +	},
>> +	{
>>   		.label		= "allow dns updates",
>>   		.type		= P_ENUM,
>>   		.p_class	= P_GLOBAL,
>> diff --git a/source4/dns_server/dns_query.c b/source4/dns_server/dns_query.c
>> index 4e3c6cc..32c4e45 100644
>> --- a/source4/dns_server/dns_query.c
>> +++ b/source4/dns_server/dns_query.c
>> @@ -4,6 +4,7 @@
>>      DNS server handler for queries
>>   
>>      Copyright (C) 2010 Kai Blin  <kai at samba.org>
>> +   Copyright (C) 2015 Robin McCorkell <rmccorkell at karoshi.org.uk>
>>   
>>      This program is free software; you can redistribute it and/or modify
>>      it under the terms of the GNU General Public License as published by
>> @@ -252,6 +253,35 @@ static WERROR ask_forwarder_recv(
>>   	return WERR_OK;
>>   }
>>   
>> +static void round_robin_rotate(struct dnsp_DnssrvRpcRecord *recs,
>> +			       uint16_t num_records)
>> +{
>> +	struct dnsp_DnssrvRpcRecord tmp;
>> +	uint16_t first, n_first, next, last;
>> +	uint8_t random;
>> +
>> +	/* see http://en.cppreference.com/w/cpp/algorithm/rotate for algorithm */
>> +	first = 0;
>> +	last = num_records;
>> +
>> +	generate_random_buffer(&random, 1);
>> +	next = n_first = random % num_records;
>> +
>> +	while (first != next) {
>> +		tmp = recs[next];
>> +		recs[next] = recs[first];
>> +		recs[first] = tmp;
>> +		++next;
>> +		++first;
>> +
>> +		if (next == last) {
>> +			next = n_first;
>> +		} else if (first == n_first) {
>> +			n_first = next;
>> +		}
>> +	}
>> +}
>> +
>>   static WERROR handle_question(struct dns_server *dns,
>>   			      TALLOC_CTX *mem_ctx,
>>   			      const struct dns_name_question *question,
>> @@ -270,6 +300,10 @@ static WERROR handle_question(struct dns_server *dns,
>>   	werror = dns_lookup_records(dns, mem_ctx, dn, &recs, &rec_count);
>>   	W_ERROR_NOT_OK_RETURN(werror);
>>   
>> +	if (lpcfg_dns_round_robin(dns->task->lp_ctx)) {
>> +		round_robin_rotate(mem_ctx, recs, rec_count);
>> +	}
>> +
>>   	ans = talloc_realloc(mem_ctx, ans, struct dns_res_rec, rec_count + ai);
>>   	if (ans == NULL) {
>>   		return WERR_NOMEM;
>