Message ID | 20170828155604.24229-1-jer@airfi.aero |
---|---|
State | Superseded |
Headers | show |
Series | pppd: Do not overwrite /etc/resolv.conf | expand |
On 28-08-17 17:56, Jeroen Roovers wrote: > With option usepeerdns, pppd by default writes to /etc/ppp/resolv.conf, > which is not very useful and might be impossible. However, when more > than one interface can be used for DNS resolution, pppd will overwrite > any existing entries when /etc/resolv.conf is used instead, which is > bad. I agree that this is bad. > Fix this by setting the path to a place we assume is always > writeable, /tmp/ppp-resolv.conf, and which does not interfere with other > mechanisms that write DNS resolver configurations. However, this is a regression. People who currently have ppp as their only interface, or who never have ppp and ethernet running at the same time, currently have a fully working solution. With this change, DNS doesn't work anymore over ppp unless you add some script to merge /tmp/ppp-resolv.conf with /tmp/resolv.conf So I think this patch should be combined with a /etc/ppp/ip-up (and ip-down) script that updates /etc/resolv.conf, similar like how udhcpc.script does it. I guess you must have something like that anyway, otherwise you would never have DNS over ppp, right? Regards, Arnout > --- > package/pppd/pppd.mk | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/package/pppd/pppd.mk b/package/pppd/pppd.mk > index 6139c5b085..47937730d5 100644 > --- a/package/pppd/pppd.mk > +++ b/package/pppd/pppd.mk > @@ -36,10 +36,10 @@ PPPD_POST_EXTRACT_HOOKS += PPPD_DROP_INTERNAL_IF_PPOL2TP_H > > # pppd defaults to /etc/ppp/resolv.conf, which not be writable and is > # definitely not useful since the C library only uses > -# /etc/resolv.conf. Therefore, we change pppd to use /etc/resolv.conf > +# /etc/resolv.conf. Therefore, we change pppd to use /tmp/ppp-resolv.conf > # instead. > define PPPD_SET_RESOLV_CONF > - $(SED) 's,ppp/resolv.conf,resolv.conf,' $(@D)/pppd/pathnames.h > + $(SED) 's,/etc/ppp/resolv.conf,/tmp/ppp-resolv.conf,' $(@D)/pppd/pathnames.h > endef > PPPD_POST_EXTRACT_HOOKS += PPPD_SET_RESOLV_CONF > >
On 28 August 2017 at 22:28, Arnout Vandecappelle <arnout@mind.be> wrote: >> pppd will overwrite any existing entries when /etc/resolv.conf is used instead, which is >> bad. > > I agree that this is bad. Thanks. > However, this is a regression. 1. dnsmasq(8) mentions using /etc/dhcp/resolv.conf as well as /etc/ppp/resolv.conf. 2. On the greater Internet you can easily find plenty of examples of people getting confused because /etc/resolv.conf does not reflect what usepeerdns has written to the wrong file in /etc/ppp/ 3. usepeerdns is not enable by default. > People who currently have ppp as their only > interface, or who never have ppp and ethernet running at the same time So it's a regression for what I can only assume must be a tiny minority of system builders who don't configure their own systems and are powerless but to rely on a second-guessed set of defaults that you would like to support. Additionally, the current pppd.mk suggests /etc/ppp/resolv.conf is not writeable (in some configurations) but you say adding an ip-up script would solve the issue, when that ip-up script is then supposed to write to /etc/resolv.conf, which supposedly *is* writeable. Please enlighten me. Kind regards, jer
On 11-09-17 09:58, Jeroen Roovers wrote: > On 28 August 2017 at 22:28, Arnout Vandecappelle <arnout@mind.be> wrote: > >>> pppd will overwrite any existing entries when /etc/resolv.conf is used instead, which is >>> bad. >> >> I agree that this is bad. > > Thanks. > >> However, this is a regression. > > 1. dnsmasq(8) mentions using /etc/dhcp/resolv.conf as well as > /etc/ppp/resolv.conf. > 2. On the greater Internet you can easily find plenty of examples of > people getting confused because /etc/resolv.conf does not reflect what > usepeerdns has written to the wrong file in /etc/ppp/ > 3. usepeerdns is not enable by default. > >> People who currently have ppp as their only >> interface, or who never have ppp and ethernet running at the same time > > So it's a regression for what I can only assume must be a tiny > minority of system builders who don't configure their own systems and > are powerless but to rely on a second-guessed set of defaults that you > would like to support. My point is: people who currently use pppd and another network interface simultaneously either have something that doesn't really work (unlikely) or have fixed it locally (more likely). However, people who currently have a working ppp-only setup would no longer have a working setup after updating Buildroot. That's something we prefer to avoid. If there is no way to avoid breaking existing configs, we probably indeed should still accept this patch because as you say, it does fix things for a (probably) more common use case. Well, except it doesn't really fix things, because you still need an additional script to actually use the DNS information. Your point 1. is interesting. It suggests that a better solution would be to not change pathnames.h at all, but instead symlink /etc/ppp/resolv.conf to either /etc/resolv.conf (keeping the current behaviour) or /tmp/ppp-resolv.conf (providing the behaviour you want). We recently added a "Migrating" section in the Buildroot manual. So what we could do is: 1. ln -s ../../tmp/ppp-resolv.conf $(TARGET_DIR)/etc/ppp/resolv.conf 2. Add a paragraph to the Migrating section explaining that a configuration that was using pppd with usepeerdns needs to either enable dnsmasq or to symlink $(TARGET_DIR)/etc/ppp/resolv.conf to /tmp/resolv.conf or to create a /etc/ppp/ip-up script. Does that sound reasonable? > > Additionally, the current pppd.mk suggests /etc/ppp/resolv.conf is not > writeable (in some configurations) but you say adding an ip-up script > would solve the issue, when that ip-up script is then supposed to > write to /etc/resolv.conf, which supposedly *is* writeable. Indeed, /etc/resolv.conf is a symlink to /tmp/resolv.conf. Regards, Arnout > > Please enlighten me. > > > Kind regards, > jer >
Hi Jeroen, On 12-09-17 01:04, Arnout Vandecappelle wrote: > > > On 11-09-17 09:58, Jeroen Roovers wrote: >> On 28 August 2017 at 22:28, Arnout Vandecappelle <arnout@mind.be> wrote: > My point is: people who currently use pppd and another network interface > simultaneously either have something that doesn't really work (unlikely) or have > fixed it locally (more likely). However, people who currently have a working > ppp-only setup would no longer have a working setup after updating Buildroot. > That's something we prefer to avoid. > > If there is no way to avoid breaking existing configs, we probably indeed > should still accept this patch because as you say, it does fix things for a > (probably) more common use case. Well, except it doesn't really fix things, > because you still need an additional script to actually use the DNS information. > > Your point 1. is interesting. It suggests that a better solution would be to > not change pathnames.h at all, but instead symlink /etc/ppp/resolv.conf to > either /etc/resolv.conf (keeping the current behaviour) or /tmp/ppp-resolv.conf > (providing the behaviour you want). > > > We recently added a "Migrating" section in the Buildroot manual. So what we > could do is: > 1. ln -s ../../tmp/ppp-resolv.conf $(TARGET_DIR)/etc/ppp/resolv.conf > 2. Add a paragraph to the Migrating section explaining that a configuration that > was using pppd with usepeerdns needs to either enable dnsmasq or to symlink > $(TARGET_DIR)/etc/ppp/resolv.conf to /tmp/resolv.conf or to create a > /etc/ppp/ip-up script. > > Does that sound reasonable? I imagine that my proposed solution is way too complicated. So instead, I applied the patch from Maksim Salau [1]. It is also very simple, like yours, but in addition it keeps backward compatibility. A more extended solution can still be added in the future. Regards, Arnout [1] http://patchwork.ozlabs.org/patch/826889/
diff --git a/package/pppd/pppd.mk b/package/pppd/pppd.mk index 6139c5b085..47937730d5 100644 --- a/package/pppd/pppd.mk +++ b/package/pppd/pppd.mk @@ -36,10 +36,10 @@ PPPD_POST_EXTRACT_HOOKS += PPPD_DROP_INTERNAL_IF_PPOL2TP_H # pppd defaults to /etc/ppp/resolv.conf, which not be writable and is # definitely not useful since the C library only uses -# /etc/resolv.conf. Therefore, we change pppd to use /etc/resolv.conf +# /etc/resolv.conf. Therefore, we change pppd to use /tmp/ppp-resolv.conf # instead. define PPPD_SET_RESOLV_CONF - $(SED) 's,ppp/resolv.conf,resolv.conf,' $(@D)/pppd/pathnames.h + $(SED) 's,/etc/ppp/resolv.conf,/tmp/ppp-resolv.conf,' $(@D)/pppd/pathnames.h endef PPPD_POST_EXTRACT_HOOKS += PPPD_SET_RESOLV_CONF