Hello,

On Mon, 2018-05-28 at 09:17 +0300, Tapani Tarvainen wrote:
> Hi all,
> 
> I'm only starting to use IPFire, so apologies if I'm missing some
> conventions here. I'm fairly experienced with Linux and firewalls in
> general, however.

Welcome :)

> I found some operations in IPFire unexpectedly slow, in particular
> Edit Hosts: every operation, whether changing a host name or just
> disabling one, took about five minutes to complete.

Yes, this is not implemented really well. I was a bit under a time constraint
and so far nobody has complained that this was an issue.

The restart script is called and will import every single entry one at a time.

> The machine in question is rather slow but not *that* slow
> (1GHz Via C3, 1GB RAM).
> 
> What's more, during that time DNS is broken: ipfire responds to
> queries but gives wrong answers, specifically NXDOMAIN for hosts that
> do exist. And that was causing problems with my internal mail server.

Exist in the local configuration or where?

> So I looked at the source.
> 
> It turns out that every operation in Edit Hosts triggers
> unbound restart, and that's where the time goes:
> 
> # time /etc/init.d/unbound restart
> [...]
> 
> real    4m7.531s
> user    1m55.320s
> sys     0m7.760s
> 
> Looking at the script, this is where it spends most of the time:
> 
> 
> update_hosts() {
> 	local enabled address hostname domainname
> 
> 	while IFS="," read -r enabled address hostname domainname; do
> 		[ "${enabled}" = "on" ] || continue
> 
> 		# Build FQDN
> 		local fqdn="${hostname}.${domainname}"
> 
> 		unbound-control -q local_data "${fqdn} ${LOCAL_TTL} IN A
> ${address}"
> 
> 		# Skip reverse resolution if the address equals the GREEN
> address
> 		[ "${address}" = "${GREEN_ADDRESS}" ] && continue
> 
> 		# Add RDNS
> 		address=$(ip_address_revptr ${address})
> 		unbound-control -q local_data "${address} ${LOCAL_TTL} IN PTR
> ${fqdn}"
> 	done < /var/ipfire/main/hosts
> }
> 
> 
> I have roughly 150 entries in hosts list, so that ends up calling
> unbound-control about 300 times. And there's a race condition between
> the time unbound is started and when those entries are added (at
> different times).
> 
> I made the following simple change:
> 
> 
> update_hosts() {
> 	local enabled address hostname domainname
> 
> 	while IFS="," read -r enabled address hostname domainname; do
> 		[ "${enabled}" = "on" ] || continue
> 
> 		# Build FQDN
> 		local fqdn="${hostname}.${domainname}"
> 
> 		echo "${fqdn} ${LOCAL_TTL} IN A ${address}"
> 
> 		# Skip reverse resolution if the address equals the GREEN
> address
> 		[ "${address}" = "${GREEN_ADDRESS}" ] && continue
> 
> 		# Add RDNS
> 		address=$(ip_address_revptr ${address})
> 		echo "${address} ${LOCAL_TTL} IN PTR ${fqdn}"
> 	done < /var/ipfire/main/hosts |	unbound-control -q local_datas
> }

It would have helped to send this as a patch, so the changes are easier to spot.

> Result:
> 
> # time /etc/init.d/unbound restart
> [...]
> 
> real    0m15.568s
> user    0m4.827s
> sys     0m1.403s
> 
> 
> So it saves in my case four minutes at every hosts change, every dhcp change,
> every boot. Still not blazingly fast but already tolerably so.
> 
> 
> That is small and obvious enough change that perhaps it could be
> considered for next upgrade (121)?

Yes, please send this as a patch:

  https://wiki.ipfire.org/devel/submit-patches

> 
> I can submit it as a patch if that helps.
> 
> 
> It's still not a good fix though: it doesn't remove the race condition
> window, only shortens it. Rather than using unbound-control to add
> local entries it'd be better to put them to a file and include that in
> unbound.conf (put the file or two in /etc/unbound/local.d/) so they'd
> all take effect immediately when unbound is started. This would be a
> bit bigger change but probably not much: if people think it'd be
> useful I could give it a go.

If you would want to go ahead and implement that solution that wouldn't be a bad
idea either.

> Of course it's not actually necessary to restart unbound for every
> change in hosts or dhcp at all, they could be effected by making
> individual changes with unbound-control, but that would be a much
> bigger change (I haven't looked at that part of the code in detail
> enough to judge how big). But improving the startup time would be
> useful even if that's done at some point.

It would be best just to change the entry that was touched, but it would be
quite complicated to figure out what has changed and since I thought that most
people would only have a few entries here it didn't look like it was worth
implementing this.

Best,
-Michael