[LEDE-DEV] [PATCH] dnsmasq: write resolv.conf also when noresolv = 1
Paul Oranje
por at xs4all.nl
Sat Jun 3 06:33:07 PDT 2017
Thanks, please see a few quick reactions of mine inline ...
Paul
> Op 3 jun. 2017, om 14:18 heeft Hans Dedecker <dedeckeh at gmail.com> het volgende geschreven:
>
> On Thu, Jun 1, 2017 at 12:00 PM, Paul Oranje <por at xs4all.nl> wrote:
>> Hello Hans,
>>
>> A new version of this small patch is worked on -overlooked your previous comment and have lately been busy with other stuff-, but after studying the code a little more I’ve a few questions. The intended patch changes code that was added with commit a35f9bbc43c3da06eed042f80dc09e8c1da681b4 (dnsmasq: Multiple dnsmasq instances support) that was authored by you.
>>
>>
>> The routines dnsmasq_start() and dnsmasq_stop() contain a guard on writing and resetting /tmp/resolv.conf:
>> [ "$resolvfile" = "/tmp/resolv.conf.auto” ] &&
>>
>> As explained in FS#785, the guard test fails when $noresolv is true and $resolvfile has not been explicitly configured to its default "/tmp/resolv.conf.auto", causing /tmp/resolv.conf to remain a soft link to /tmp/resolv.conf.auto (the WAN its DNS).
>>
>> The routine dnsmasq_stop() the guard is commented with
>> #relink /tmp/resolve.conf only for main instance
>>
>> This suggests that only for the main (first ?) instance /tmp/resolv.conf would be handled, which makes sense, but the test to accomplish that seems wrong.
>>
>> Q1:
>> What is the supposed relation between dnsmasq instance and the value of the value/setting of resolvfile ?
> The DNS servers learned on the wan interface(s) by netifd are written
> in /tmp/resolv.conf.auto. By default a dnsmasq instance uses
> /tmp/resolv.conf.auto as resolver file unless configured otherwise.
Indeed and whether dnsmasq uses the (whatever) resolvfile is governed by the noresolv parameter; these two parameters are orthogonal.
The init script though skips setting resolvfile when noresolv is true.
> Using /tmp/rsolv.conf.auto as resolver file triggers logic to rewrite
> /tmp/resolv.conf
Why would the question whether local DNS resolution is handled by the DNS available on WAN interface be determined by this specific **value** of $resolvfile (and irrespective of the actual existence of that file) ?
Also the above question (Q1) is about the relation with the dnsmasq **instance**.
Could you please share your thoughts on the relation behind first instance and the value of $resolvfile ?
>> A fix I considered is to omit this guard altogether (so different from the patch previously submitted), so that the local DNS service will allways be used for local (router) DNS resolution when dnsmasq is started; at stop of dnsmasq the local resolution would return to use $resolvfile. Obviously doing so in dnsmasq_start() and dnsmasq_stop() routines that start or stop an instance would be the wrong place to do so, since that code will be called for each instance. These routines ar spawned from the start_service() and stop_service() routines that do iterate on the instances.
>>
>> Q2:
>> Would placement of the /tmp/resolv.conf handling not be better placed in the start_ and stop_service() routines or, be guarded by a better test in the dnsmasq_start() and _stop() routines ? In other words: what would be a correct test ?
> we could move rewriting of /tmp/resolv.conf to the start routine; but
> this will not be trivial as for every dnsmasq instance DNS_SERVERS and
> DOMAIN state will need to be kept
The objective is to rewrite /tmp/resolv.conf to use local resolution whenever a local resolver runs (i.e. dnsmasq, unbound, ... listening on localhost:53). As multiple instances may be started, triggering on the first/main one seems to make sense (for lack of a better criterium).
So the question is: what do you think is a (practical) guard for that (and in what routines would those be placed best) ?
>> In the stop routine the file /tmp/resolv.conf is (re)set to a soft link to $resolvfile, which currently because of the test always is "/tmp/resolv.conf.auto”, irrespective whether that file exists or not.
>>
>> Q3:
>> What would be the desired state of /tmp/resolv.conf after dnsmasq has been stopped ?
> In case dnsmasq is stopped /tmp/resolv.conf needs to use only the wan
> DNS servers; which are written by netifd in /tmp/resolv.conf.auto; and
> not anymore 127.0.0.1.
Okay, so that should never be any self user defined resolvfile ?
> Hans
>>
>>
>> Bye,
>> Paul
>>
>>
>>
>>> Op 20 mei 2017, om 20:58 heeft Hans Dedecker <dedeckeh at gmail.com> het volgende geschreven:
>>>
>>> On Sat, May 20, 2017 at 12:41 AM, Paul Oranje <por at xs4all.nl> wrote:
>>>> When UCI dhcp.dnsmasq.noresolv is true, dnsmasq ignores the (wan) resolv.conf
>>>> for upstream name resolution and the dnsmasq init script ialso skips writing
>>>> /tmp/resolv.conf (/etc/resolv.conf soft links that file).
>>>>
>>>> Not using the local running DNS service when noresolv is true does not make
>>>> sence; the semantics of that config value do not imply this. With this patch
>>>> the init script also writes /tmp/resolv.conf when noresolv is true.
>>>>
>>>> fixes FS#785
>>>>
>>>> Signed-off-by: Paul Oranje <por at xs4all.nl>
>>>> ---
>>>> This patch replaces an earlier patch with subject
>>>> dnsmasq: also write /tmp/resolv.conf when UCI dhcp.dnsmasq.noresolv is '1'
>>>> which is obsoleted because that subject was required to be shorter.
>>>> ---
>>>> package/network/services/dnsmasq/files/dnsmasq.init | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/package/network/services/dnsmasq/files/dnsmasq.init b/package/network/services/dnsmasq/files/dnsmasq.init
>>>> index 30fec7a4ee..197aae9de1 100644
>>>> --- a/package/network/services/dnsmasq/files/dnsmasq.init
>>>> +++ b/package/network/services/dnsmasq/files/dnsmasq.init
>>>> @@ -947,7 +947,7 @@ dnsmasq_start()
>>>> echo >> $CONFIGFILE_TMP
>>>> mv -f $CONFIGFILE_TMP $CONFIGFILE
>>>>
>>>> - [ "$resolvfile" = "/tmp/resolv.conf.auto" ] && {
>>>> + [ "$noresolv" = "1" -o "$resolvfile" = "/tmp/resolv.conf.auto" ] && {
>>>> rm -f /tmp/resolv.conf
>>>> [ $ADD_LOCAL_DOMAIN -eq 1 ] && [ -n "$DOMAIN" ] && {
>>>> echo "search $DOMAIN" >> /tmp/resolv.conf
>>>> @@ -982,7 +982,7 @@ dnsmasq_stop()
>>>> config_get resolvfile "$cfg" "resolvfile"
>>>>
>>>> #relink /tmp/resolve.conf only for main instance
>>>> - [ "$resolvfile" = "/tmp/resolv.conf.auto" ] && {
>>>> + [ "$noresolv" = "1" -o "$resolvfile" = "/tmp/resolv.conf.auto" ] && {
>>> As mentioned in the previous code review noresolv value must be read
>>> from config (similar to resolvfile) otherwise this will not work
>>>> [ -f /tmp/resolv.conf ] && {
>>>> rm -f /tmp/resolv.conf
>>>> ln -s "$resolvfile" /tmp/resolv.conf
>>> As mentioned in the previous code review resolvfile can now be empty
>>> which will make ln -s produce an error
>>>
>>> Hans
>>>> --
>>>>
>>>>
>>>> _______________________________________________
>>>> Lede-dev mailing list
>>>> Lede-dev at lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/lede-dev
>>>
>>> _______________________________________________
>>> Lede-dev mailing list
>>> Lede-dev at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/lede-dev
>>
>
> _______________________________________________
> Lede-dev mailing list
> Lede-dev at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev
More information about the Lede-dev
mailing list