| Message ID | 20190221035228.5145-1-yszhou4tech@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [OpenWrt-Devel] dnsmasq: prefer localuse over resolvfile guesswork | expand |
> Op 21 feb. 2019, om 04:52 heeft Yousong Zhou <yszhou4tech@gmail.com> het volgende geschreven: > > This makes it clear that localuse when explicitly specified in the > config will have its final say on whether or not the initscript should > touch /etc/resolv.conf, no matter whatever the result of previous > guesswork would be > > Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com> > --- > package/network/services/dnsmasq/Makefile | 2 +- > package/network/services/dnsmasq/files/dnsmasq.init | 8 ++++---- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/package/network/services/dnsmasq/Makefile b/package/network/services/dnsmasq/Makefile > index d874296ea7..8c8427f6c1 100644 > --- a/package/network/services/dnsmasq/Makefile > +++ b/package/network/services/dnsmasq/Makefile > @@ -10,7 +10,7 @@ include $(TOPDIR)/rules.mk > PKG_NAME:=dnsmasq > PKG_UPSTREAM_VERSION:=2.80 > PKG_VERSION:=$(subst test,~~test,$(subst rc,~rc,$(PKG_UPSTREAM_VERSION))) > -PKG_RELEASE:=9 > +PKG_RELEASE:=10 > > PKG_SOURCE:=$(PKG_NAME)-$(PKG_UPSTREAM_VERSION).tar.xz > PKG_SOURCE_URL:=http://thekelleys.org.uk/dnsmasq > diff --git a/package/network/services/dnsmasq/files/dnsmasq.init b/package/network/services/dnsmasq/files/dnsmasq.init > index f65736e268..4ae71e438b 100644 > --- a/package/network/services/dnsmasq/files/dnsmasq.init > +++ b/package/network/services/dnsmasq/files/dnsmasq.init > @@ -733,7 +733,7 @@ dnsmasq_start() > { > local cfg="$1" > local disabled user_dhcpscript > - local resolvfile localuse > + local resolvfile localuse=0 > > config_get_bool disabled "$cfg" disabled 0 > [ "$disabled" -gt 0 ] && return 0 > @@ -884,13 +884,13 @@ dnsmasq_start() > config_get_bool cachelocal "$cfg" cachelocal 1 > > config_get_bool noresolv "$cfg" noresolv 0 > - config_get_bool localuse "$cfg" localuse 0 > if [ "$noresolv" != "1" ]; then > config_get resolvfile "$cfg" resolvfile /tmp/resolv.conf.auto > [ -n "$resolvfile" -a ! -e "$resolvfile" ] && touch "$resolvfile" > xappend "--resolv-file=$resolvfile" > [ "$resolvfile" = "/tmp/resolv.conf.auto" ] && localuse=1 > fi > + config_get_bool localuse "$cfg" localuse "$localuse" > > config_get hostsfile "$cfg" dhcphostsfile > [ -e "$hostsfile" ] && xappend "--dhcp-hostsfile=$hostsfile" > @@ -1040,13 +1040,13 @@ dnsmasq_start() > dnsmasq_stop() > { > local cfg="$1" > - local noresolv resolvfile localuse > + local noresolv resolvfile localuse=0 > > config_get_bool noresolv "$cfg" noresolv 0 > - config_get_bool localuse "$cfg" localuse 0 > config_get resolvfile "$cfg" "resolvfile" > > [ "$noresolv" = 0 -a "$resolvfile" = "/tmp/resolv.conf.auto" ] && localuse=1 > + config_get_bool localuse "$cfg" localuse "$localuse" > [ "$localuse" -gt 0 ] && ln -sf "/tmp/resolv.conf.auto" /tmp/resolv.conf > > rm -f ${BASEDHCPSTAMPFILE}.${cfg}.*.dhcp > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel Tested this patch (8f16fba501) and the previous patch (ec2a2a2aea) on top of 18.06.2. The patches apply cleanly and the patched file works as expected: - localuse=1 and noresolv and resolvfile not set => dnsmasq writes resolv.conf - localuse and noresolv and resolvfile not set => dnsmasq writes resolv.conf - localuse=0 and noresolv and resolvfile not set => dnsmasq leaves resolv.conf untouched - localuse not set and noresolv=1 and resolvfile='/tmp/resolv.conf.auto' => dnsmasq leaves resolv.conf untouched. The latter two scenario's work well with Unbound (where dnsmasq handles DHCP and DNS of lan host names). Thanks. Tested-by: Paul Oranje <por@oranjevos.nl> Sitenotes: Would also writing "nameserver ::1" to resolv.conf be a good idea ? (Unbound does) When dnsmasq does not listen on port 53, then it cannot serve as the nameserver for the localhost (resolv.conf does not allow specifying a port) unless some clever DNAT rule is in place.
On Thu, 21 Feb 2019 at 22:14, Paul Oranje <por@oranjevos.nl> wrote: > > > > > Op 21 feb. 2019, om 04:52 heeft Yousong Zhou <yszhou4tech@gmail.com> het volgende geschreven: > > > > This makes it clear that localuse when explicitly specified in the > > config will have its final say on whether or not the initscript should > > touch /etc/resolv.conf, no matter whatever the result of previous > > guesswork would be > > > > Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com> <snipped> > > Tested this patch (8f16fba501) and the previous patch (ec2a2a2aea) on top of 18.06.2. > > The patches apply cleanly and the patched file works as expected: > > - localuse=1 and noresolv and resolvfile not set => dnsmasq writes resolv.conf > - localuse and noresolv and resolvfile not set => dnsmasq writes resolv.conf > - localuse=0 and noresolv and resolvfile not set => dnsmasq leaves resolv.conf untouched > - localuse not set and noresolv=1 and resolvfile='/tmp/resolv.conf.auto' => dnsmasq leaves resolv.conf untouched. > > The latter two scenario's work well with Unbound (where dnsmasq handles DHCP and DNS of lan host names). > Thanks. > > Tested-by: Paul Oranje <por@oranjevos.nl> Thanks ;) > > Sitenotes: > Would also writing "nameserver ::1" to resolv.conf be a good idea ? (Unbound does) The 127.0.0.1 address always works so there is no need for ::1 plus the possibility that people may use a build without ipv6. > When dnsmasq does not listen on port 53, then it cannot serve as the nameserver for the localhost (resolv.conf does not allow specifying a port) unless some clever DNAT rule is in place. Yes, I got the same conclusion last time I tried to find ways to specify port in resolv.conf ;) yousong
On Thu, Feb 21, 2019 at 4:52 AM Yousong Zhou <yszhou4tech@gmail.com> wrote: > > This makes it clear that localuse when explicitly specified in the > config will have its final say on whether or not the initscript should > touch /etc/resolv.conf, no matter whatever the result of previous > guesswork would be > > Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com> Acked by: Hans Dedecker <dedeckeh@gmail.com> > --- > package/network/services/dnsmasq/Makefile | 2 +- > package/network/services/dnsmasq/files/dnsmasq.init | 8 ++++---- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/package/network/services/dnsmasq/Makefile b/package/network/services/dnsmasq/Makefile > index d874296ea7..8c8427f6c1 100644 > --- a/package/network/services/dnsmasq/Makefile > +++ b/package/network/services/dnsmasq/Makefile > @@ -10,7 +10,7 @@ include $(TOPDIR)/rules.mk > PKG_NAME:=dnsmasq > PKG_UPSTREAM_VERSION:=2.80 > PKG_VERSION:=$(subst test,~~test,$(subst rc,~rc,$(PKG_UPSTREAM_VERSION))) > -PKG_RELEASE:=9 > +PKG_RELEASE:=10 > > PKG_SOURCE:=$(PKG_NAME)-$(PKG_UPSTREAM_VERSION).tar.xz > PKG_SOURCE_URL:=http://thekelleys.org.uk/dnsmasq > diff --git a/package/network/services/dnsmasq/files/dnsmasq.init b/package/network/services/dnsmasq/files/dnsmasq.init > index f65736e268..4ae71e438b 100644 > --- a/package/network/services/dnsmasq/files/dnsmasq.init > +++ b/package/network/services/dnsmasq/files/dnsmasq.init > @@ -733,7 +733,7 @@ dnsmasq_start() > { > local cfg="$1" > local disabled user_dhcpscript > - local resolvfile localuse > + local resolvfile localuse=0 > > config_get_bool disabled "$cfg" disabled 0 > [ "$disabled" -gt 0 ] && return 0 > @@ -884,13 +884,13 @@ dnsmasq_start() > config_get_bool cachelocal "$cfg" cachelocal 1 > > config_get_bool noresolv "$cfg" noresolv 0 > - config_get_bool localuse "$cfg" localuse 0 > if [ "$noresolv" != "1" ]; then > config_get resolvfile "$cfg" resolvfile /tmp/resolv.conf.auto > [ -n "$resolvfile" -a ! -e "$resolvfile" ] && touch "$resolvfile" > xappend "--resolv-file=$resolvfile" > [ "$resolvfile" = "/tmp/resolv.conf.auto" ] && localuse=1 > fi > + config_get_bool localuse "$cfg" localuse "$localuse" > > config_get hostsfile "$cfg" dhcphostsfile > [ -e "$hostsfile" ] && xappend "--dhcp-hostsfile=$hostsfile" > @@ -1040,13 +1040,13 @@ dnsmasq_start() > dnsmasq_stop() > { > local cfg="$1" > - local noresolv resolvfile localuse > + local noresolv resolvfile localuse=0 > > config_get_bool noresolv "$cfg" noresolv 0 > - config_get_bool localuse "$cfg" localuse 0 > config_get resolvfile "$cfg" "resolvfile" > > [ "$noresolv" = 0 -a "$resolvfile" = "/tmp/resolv.conf.auto" ] && localuse=1 > + config_get_bool localuse "$cfg" localuse "$localuse" > [ "$localuse" -gt 0 ] && ln -sf "/tmp/resolv.conf.auto" /tmp/resolv.conf > > rm -f ${BASEDHCPSTAMPFILE}.${cfg}.*.dhcp
> Op 22 feb. 2019, om 02:54 heeft Yousong Zhou <yszhou4tech@gmail.com> het volgende geschreven: > > On Thu, 21 Feb 2019 at 22:14, Paul Oranje <por@oranjevos.nl> wrote: >> >> >> >>> Op 21 feb. 2019, om 04:52 heeft Yousong Zhou <yszhou4tech@gmail.com> het volgende geschreven: >>> >>> This makes it clear that localuse when explicitly specified in the >>> config will have its final say on whether or not the initscript should >>> touch /etc/resolv.conf, no matter whatever the result of previous >>> guesswork would be >>> >>> Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com> > > <snipped> > >> >> Tested this patch (8f16fba501) and the previous patch (ec2a2a2aea) on top of 18.06.2. >> >> The patches apply cleanly and the patched file works as expected: >> >> - localuse=1 and noresolv and resolvfile not set => dnsmasq writes resolv.conf >> - localuse and noresolv and resolvfile not set => dnsmasq writes resolv.conf >> - localuse=0 and noresolv and resolvfile not set => dnsmasq leaves resolv.conf untouched >> - localuse not set and noresolv=1 and resolvfile='/tmp/resolv.conf.auto' => dnsmasq leaves resolv.conf untouched. >> >> The latter two scenario's work well with Unbound (where dnsmasq handles DHCP and DNS of lan host names). >> Thanks. >> >> Tested-by: Paul Oranje <por@oranjevos.nl> > > Thanks ;) Can these patches also be applied against the openwrt-18.06 branch or must they be submitted separately for that end ? > >> >> Sitenotes: >> Would also writing "nameserver ::1" to resolv.conf be a good idea ? (Unbound does) > > The 127.0.0.1 address always works so there is no need for ::1 plus > the possibility that people may use a build without ipv6. > >> When dnsmasq does not listen on port 53, then it cannot serve as the nameserver for the localhost (resolv.conf does not allow specifying a port) unless some clever DNAT rule is in place. > > Yes, I got the same conclusion last time I tried to find ways to > specify port in resolv.conf ;) > > yousong
On Fri, 22 Feb 2019 at 20:13, Paul Oranje <por@oranjevos.nl> wrote: > > > > Op 22 feb. 2019, om 02:54 heeft Yousong Zhou <yszhou4tech@gmail.com> het volgende geschreven: > > > > On Thu, 21 Feb 2019 at 22:14, Paul Oranje <por@oranjevos.nl> wrote: > >> > >> > >> > >>> Op 21 feb. 2019, om 04:52 heeft Yousong Zhou <yszhou4tech@gmail.com> het volgende geschreven: > >>> > >>> This makes it clear that localuse when explicitly specified in the > >>> config will have its final say on whether or not the initscript should > >>> touch /etc/resolv.conf, no matter whatever the result of previous > >>> guesswork would be > >>> > >>> Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com> > > > > <snipped> > > > >> > >> Tested this patch (8f16fba501) and the previous patch (ec2a2a2aea) on top of 18.06.2. > >> > >> The patches apply cleanly and the patched file works as expected: > >> > >> - localuse=1 and noresolv and resolvfile not set => dnsmasq writes resolv.conf > >> - localuse and noresolv and resolvfile not set => dnsmasq writes resolv.conf > >> - localuse=0 and noresolv and resolvfile not set => dnsmasq leaves resolv.conf untouched > >> - localuse not set and noresolv=1 and resolvfile='/tmp/resolv.conf.auto' => dnsmasq leaves resolv.conf untouched. > >> > >> The latter two scenario's work well with Unbound (where dnsmasq handles DHCP and DNS of lan host names). > >> Thanks. > >> > >> Tested-by: Paul Oranje <por@oranjevos.nl> > > > > Thanks ;) > Can these patches also be applied against the openwrt-18.06 branch or must they be submitted separately for that end ? I just did the cherry-pick and sent the result to the devel list. Skimmed through past history of release branch, I think this acked-by process is needed as these patches are with new options for feature changes, not just fixes. Regards, yousong
diff --git a/package/network/services/dnsmasq/Makefile b/package/network/services/dnsmasq/Makefile index d874296ea7..8c8427f6c1 100644 --- a/package/network/services/dnsmasq/Makefile +++ b/package/network/services/dnsmasq/Makefile @@ -10,7 +10,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=dnsmasq PKG_UPSTREAM_VERSION:=2.80 PKG_VERSION:=$(subst test,~~test,$(subst rc,~rc,$(PKG_UPSTREAM_VERSION))) -PKG_RELEASE:=9 +PKG_RELEASE:=10 PKG_SOURCE:=$(PKG_NAME)-$(PKG_UPSTREAM_VERSION).tar.xz PKG_SOURCE_URL:=http://thekelleys.org.uk/dnsmasq diff --git a/package/network/services/dnsmasq/files/dnsmasq.init b/package/network/services/dnsmasq/files/dnsmasq.init index f65736e268..4ae71e438b 100644 --- a/package/network/services/dnsmasq/files/dnsmasq.init +++ b/package/network/services/dnsmasq/files/dnsmasq.init @@ -733,7 +733,7 @@ dnsmasq_start() { local cfg="$1" local disabled user_dhcpscript - local resolvfile localuse + local resolvfile localuse=0 config_get_bool disabled "$cfg" disabled 0 [ "$disabled" -gt 0 ] && return 0 @@ -884,13 +884,13 @@ dnsmasq_start() config_get_bool cachelocal "$cfg" cachelocal 1 config_get_bool noresolv "$cfg" noresolv 0 - config_get_bool localuse "$cfg" localuse 0 if [ "$noresolv" != "1" ]; then config_get resolvfile "$cfg" resolvfile /tmp/resolv.conf.auto [ -n "$resolvfile" -a ! -e "$resolvfile" ] && touch "$resolvfile" xappend "--resolv-file=$resolvfile" [ "$resolvfile" = "/tmp/resolv.conf.auto" ] && localuse=1 fi + config_get_bool localuse "$cfg" localuse "$localuse" config_get hostsfile "$cfg" dhcphostsfile [ -e "$hostsfile" ] && xappend "--dhcp-hostsfile=$hostsfile" @@ -1040,13 +1040,13 @@ dnsmasq_start() dnsmasq_stop() { local cfg="$1" - local noresolv resolvfile localuse + local noresolv resolvfile localuse=0 config_get_bool noresolv "$cfg" noresolv 0 - config_get_bool localuse "$cfg" localuse 0 config_get resolvfile "$cfg" "resolvfile" [ "$noresolv" = 0 -a "$resolvfile" = "/tmp/resolv.conf.auto" ] && localuse=1 + config_get_bool localuse "$cfg" localuse "$localuse" [ "$localuse" -gt 0 ] && ln -sf "/tmp/resolv.conf.auto" /tmp/resolv.conf rm -f ${BASEDHCPSTAMPFILE}.${cfg}.*.dhcp
This makes it clear that localuse when explicitly specified in the config will have its final say on whether or not the initscript should touch /etc/resolv.conf, no matter whatever the result of previous guesswork would be Signed-off-by: Yousong Zhou <yszhou4tech@gmail.com> --- package/network/services/dnsmasq/Makefile | 2 +- package/network/services/dnsmasq/files/dnsmasq.init | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-)