Hi Hans,
Please see my reaction/question on your remark about #53 and the main instance.
Thanks for reviewing,
Paul
> Op 8 jun. 2017, om 16:34 heeft Hans Dedecker <dedec...@gmail.com> het
> volgende geschreven:
>
> On Wed, Jun 7, 2017 at 12:07 PM, Paul Oranje <p...@xs4all.nl> wrote:
>> With this patch the dnsmasq init script manages resolv.conf if and only if
>> when dnsmasq will listen on 127.0.0.1#53 (is main resolver instance).
>> Also, resolvfile is now set irrespective of the value of noresolv.
>>
>> Fixes (partially) FS#785
>>
>> Signed-off-by: Paul Oranje <p...@xs4all.nl>
>> ---
>> The intended invariant is that resolv.conf is managed whenever a resolver
>> listens on 127.0.0.1#53. Besides dnsmasq, unbound can take that role as well.
>> When no instance of dnsmasq has been configured to listen on 127.0.0.1#53
>> then
>> resolv.conf is not touched.
>>
>> Currently unbound handles resolv.conf also, but leaves it to dnsmasq whenever
>> that will run, even when no dnsmasq instance will listen on localhost:53. So
>> for unbound PR#4454a has been submitted to make sure it always manages
>> resov.conf when it owns localhost:domain.
>>
>> Background:
>> This patch has been discussed with Hans Dedecker and Eric Luerhsen and
>> replaces
>> an earlier patch that is hereby retracted:
>> dnsmasq: write resolv.conf also when noresolv = 1
>>
>> Paul
>>
>> ---
>> .../network/services/dnsmasq/files/dnsmasq.init | 60
>> +++++++++++++---------
>> 1 file changed, 35 insertions(+), 25 deletions(-)
>>
>> diff --git a/package/network/services/dnsmasq/files/dnsmasq.init
>> b/package/network/services/dnsmasq/files/dnsmasq.init
>> index 62a3169c67..cab3d2843c 100644
>> --- a/package/network/services/dnsmasq/files/dnsmasq.init
>> +++ b/package/network/services/dnsmasq/files/dnsmasq.init
>> @@ -695,6 +695,21 @@ dhcp_relay_add() {
>> fi
>> }
>>
>> +dnsmasq_ismain()
>> +{
>> + local cfg="$1"
>> + local port notinterfaces
>> +
>> + config_get port "$cfg" port "53"
>> + [ $port = "53" ] || return 1
> Any reason why a dnsmasq instance has to be bound on port 53 to be
> considered as main instance ? If port is not 0 it can be considered as
> a dnsmasq main instance as a dnat rule can rewrite the udp destination
> port
The "main instance" is with respect to /etc/resolv.conf and its format does not
support setting a port for the nameserver [1].
A dnat rule that rewrites the destination port absolutely did not cross my mind
- would one rewrite a local port ?
The management of resolv.conf by the init script of dnsmasq (and of unbound) is
about making **itself** the nameserver for the processes running on the same
host. When it cannot be assumed that 127.0.0.1#53 is listened on by a local
daemon, how can one pick the "right" instance when multiple instances resolver
are possible ?
If rewriting of a local port to another local port must be coped with, then a
separate config value that explicitly denotes the instance that will run as the
nameserver for the local processes might be needed.
[1] man 5 resolv.conf
>> +
>> + config_get notinterfaces "$cfg" notinterface ""
>> + [ -n $notinterfaces -a list_contains $notinterfaces "loopback" ] ||
>> return 1
> Has this been tested ?
> Following error is thrown "sh: loopback: unknown operand" when doing
> dnsmasq restart
Yes tested, evidently not enough, not with dnsmasq owning #53, so the test did
not cover this code path.
Will take care of that (and test with dnsmasq owning #53).
A grep on this erroneous syntax revealed it being the only one.
>> +
>> + # dnsmasq instance is designated to listen on 127.0.0.1#53.
>> + return 0
>> +}
>> +
>> dnsmasq_start()
>> {
>> local cfg="$1" disabled resolvfile user_dhcpscript
>> @@ -839,14 +854,10 @@ dnsmasq_start()
>> [ -n "$leasefile" -a \! -e "$leasefile" ] && touch "$leasefile"
>> config_get_bool cachelocal "$cfg" cachelocal 1
>>
>> - config_get_bool noresolv "$cfg" noresolv 0
>> - if [ "$noresolv" != "1" ]; then
>> - config_get resolvfile "$cfg" resolvfile
>> "/tmp/resolv.conf.auto"
>> - # So jail doesn't complain if file missing
>> - [ -n "$resolvfile" -a \! -e "$resolvfile" ] && touch
>> "$resolvfile"
>> - fi
>> -
>> - [ -n "$resolvfile" ] && xappend "--resolv-file=$resolvfile"
>> + config_get resolvfile "$cfg" resolvfile "/tmp/resolv.conf.auto"
>> + xappend "--resolv-file=$resolvfile"
>> + # So jail doesn't complain if file missing
>> + [ \! -e "$resolvfile" ] && touch "$resolvfile"
>>
>> config_get hostsfile "$cfg" dhcphostsfile
>> [ -e "$hostsfile" ] && xappend "--dhcp-hostsfile=$hostsfile"
>> @@ -959,16 +970,6 @@ dnsmasq_start()
>> echo >> $CONFIGFILE_TMP
>> mv -f $CONFIGFILE_TMP $CONFIGFILE
>>
>> - [ "$resolvfile" = "/tmp/resolv.conf.auto" ] && {
>> - rm -f /tmp/resolv.conf
>> - [ $ADD_LOCAL_DOMAIN -eq 1 ] && [ -n "$DOMAIN" ] && {
>> - echo "search $DOMAIN" >> /tmp/resolv.conf
>> - }
>> - DNS_SERVERS="$DNS_SERVERS 127.0.0.1"
>> - for DNS_SERVER in $DNS_SERVERS ; do
>> - echo "nameserver $DNS_SERVER" >> /tmp/resolv.conf
>> - done
>> - }
>>
>> procd_open_instance $cfg
>> procd_set_param command $PROG -C $CONFIGFILE -k -x
>> /var/run/dnsmasq/dnsmasq."${cfg}".pid
>> @@ -986,20 +987,29 @@ dnsmasq_start()
>> procd_add_jail_mount_rw /var/run/dnsmasq/ $leasefile
>>
>> procd_close_instance
>> +
>> +
>> + # write /tmp/resolve.conf only for main instance
>> + dnsmasq_ismain $cfg && {
>> + rm -f /tmp/resolv.conf
>> + [ $ADD_LOCAL_DOMAIN -eq 1 ] && [ -n "$DOMAIN" ] && {
>> + echo "search $DOMAIN" >> /tmp/resolv.conf
>> + }
>> + DNS_SERVERS="$DNS_SERVERS 127.0.0.1"
>> + for DNS_SERVER in $DNS_SERVERS ; do
>> + echo "nameserver $DNS_SERVER" >> /tmp/resolv.conf
>> + done
>> + }
>> }
>>
>> dnsmasq_stop()
>> {
>> local cfg="$1"
>>
>> - config_get resolvfile "$cfg" "resolvfile"
>> -
>> #relink /tmp/resolve.conf only for main instance
>> - [ "$resolvfile" = "/tmp/resolv.conf.auto" ] && {
>> - [ -f /tmp/resolv.conf ] && {
>> - rm -f /tmp/resolv.conf
>> - ln -s "$resolvfile" /tmp/resolv.conf
>> - }
>> + dnsmasq_ismain $cfg && {
>> + [ -f /tmp/resolv.conf ] && rm -f /tmp/resolv.conf
>> + ln -s /tmp/resolv.conf.auto /tmp/resolv.conf
>> }
>>
>> rm -f ${BASEDHCPSTAMPFILE}.${cfg}.*.dhcp
>> --
>> 2.13.0
>>
> Increase PKG_RELEASE as well in the package Makefile
Will do that.
>
> Hans
>
> _______________________________________________
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev
_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
