Thanks, please see a few quick reactions of mine inline ...
Paul
> Op 3 jun. 2017, om 14:18 heeft Hans Dedecker <dedec...@gmail.com> het
> volgende geschreven:
>
> On Thu, Jun 1, 2017 at 12:00 PM, Paul Oranje <p...@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 <dedec...@gmail.com> het
>>> volgende geschreven:
>>>
>>> On Sat, May 20, 2017 at 12:41 AM, Paul Oranje <p...@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 <p...@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@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
>>
>
> _______________________________________________
> 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
