On Sat, Jun 3, 2017 at 3:33 PM, Paul Oranje <p...@xs4all.nl> wrote:
> 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.
Agree the value of noresolv should have no influence on reading he
resolvfile parameter; this behavior has been introduced in commit
2ac21bd793946a214295b760cd652b4150d3dc07
>
>> 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 ?
If resolvfile is set to /tmp/resolv.conf.auto assumption is made this
is the main dnsmasq instance due to a lack of a better criterium. DHCP
configs in use before the introduction of multiple dnsmasq instances
used /tmp/resolv.conf.auto as netifd by default populates the file
/tmp/resolv.conf.auto. That's the only link I see between the main
dnsmasq 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) ?
The dnsmasq instance which listens on 127.0.0.1 (and thus does not
exclude the loopback interface in the notinterface parameter) for dns
requests can be used as trigger to rewrite /tmp/resolv.conf.auto which
means the logic needs to be kept in dnsmasq_start
>
>>> 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 ?
AFAIK this has never been the case before
Hans
>
>> 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
