internal/router: make router.Cleanup idempotent

On routers where we want to wait for NTP by checking nvram key. Before
waiting, we clean up the router to ensure it's restored to original
state. However, router.Cleanup is not idempotent, causing dnsmasq
restarted. On tomato/ddwrt, restarting have no delay, and spawning new
dnsmasq process immediately. On merlin, somehow it takes time to spawn
new dnsmasq process, causing ctrld wrongly think there's no one
listening on port 53.

Fixing this by ensuring router.Cleanup is idempotent. While at it, also
adding "ntp_done" to nvram key, which is now using on latest ddwrt.
This commit is contained in:
Cuong Manh Le
2023-08-11 16:11:04 +00:00
committed by Cuong Manh Le
parent ab8f072388
commit 0c096d5f07
5 changed files with 49 additions and 23 deletions

View File

@@ -24,6 +24,8 @@ import (
"github.com/Control-D-Inc/ctrld/internal/resolvconffile" "github.com/Control-D-Inc/ctrld/internal/resolvconffile"
) )
const resolvConfBackupFailedMsg = "open /etc/resolv.pre-ctrld-backup.conf: read-only file system"
// allocate loopback ip // allocate loopback ip
// sudo ip a add 127.0.0.2/24 dev lo // sudo ip a add 127.0.0.2/24 dev lo
func allocateIP(ip string) error { func allocateIP(ip string) error {
@@ -73,6 +75,14 @@ func setDNS(iface *net.Interface, nameservers []string) error {
trySystemdResolve = true trySystemdResolve = true
break break
} }
// This error happens on read-only file system, which causes ctrld failed to create backup
// for /etc/resolv.conf file. It is ok, because the DNS is still set anyway, and restore
// DNS will fallback to use DHCP if there's no backup /etc/resolv.conf file.
// The error format is controlled by us, so checking for error string is fine.
// See: ../../internal/dns/direct.go:L278
if r.Mode() == "direct" && strings.Contains(err.Error(), resolvConfBackupFailedMsg) {
return nil
}
return err return err
} }
currentNS := currentDNS(iface) currentNS := currentDNS(iface)

View File

@@ -87,12 +87,14 @@ func (d *Ddwrt) Cleanup() error {
if d.cfg.FirstListener().IsDirectDnsListener() { if d.cfg.FirstListener().IsDirectDnsListener() {
return nil return nil
} }
if val, _ := nvram.Run("get", nvram.CtrldSetupKey); val == "1" { if val, _ := nvram.Run("get", nvram.CtrldSetupKey); val != "1" {
nvramKvMap["dnsmasq_options"] = "" return nil // was restored, nothing to do.
// Restore old configs. }
if err := nvram.Restore(nvramKvMap, nvram.CtrldSetupKey); err != nil {
return err nvramKvMap["dnsmasq_options"] = ""
} // Restore old configs.
if err := nvram.Restore(nvramKvMap, nvram.CtrldSetupKey); err != nil {
return err
} }
// Restart dnsmasq service. // Restart dnsmasq service.

View File

@@ -52,6 +52,13 @@ func (m *Merlin) Setup() error {
if m.cfg.FirstListener().IsDirectDnsListener() { if m.cfg.FirstListener().IsDirectDnsListener() {
return nil return nil
} }
// Already setup.
if val, _ := nvram.Run("get", nvram.CtrldSetupKey); val == "1" {
return nil
}
if _, err := nvram.Run("set", nvram.CtrldSetupKey+"=1"); err != nil {
return err
}
buf, err := os.ReadFile(dnsmasq.MerlinPostConfPath) buf, err := os.ReadFile(dnsmasq.MerlinPostConfPath)
// Already setup. // Already setup.
if bytes.Contains(buf, []byte(dnsmasq.MerlinPostConfMarker)) { if bytes.Contains(buf, []byte(dnsmasq.MerlinPostConfMarker)) {
@@ -92,11 +99,13 @@ func (m *Merlin) Cleanup() error {
if m.cfg.FirstListener().IsDirectDnsListener() { if m.cfg.FirstListener().IsDirectDnsListener() {
return nil return nil
} }
if val, _ := nvram.Run("get", nvram.CtrldSetupKey); val == "1" { if val, _ := nvram.Run("get", nvram.CtrldSetupKey); val != "1" {
// Restore old configs. return nil // was restored, nothing to do.
if err := nvram.Restore(nvramKvMap, nvram.CtrldSetupKey); err != nil { }
return err
} // Restore old configs.
if err := nvram.Restore(nvramKvMap, nvram.CtrldSetupKey); err != nil {
return err
} }
buf, err := os.ReadFile(dnsmasq.MerlinPostConfPath) buf, err := os.ReadFile(dnsmasq.MerlinPostConfPath)

View File

@@ -18,12 +18,15 @@ func WaitNvram() error {
// Wait until `ntp_ready=1` set. // Wait until `ntp_ready=1` set.
b := backoff.NewBackoff("ntp.Wait", func(format string, args ...any) {}, 10*time.Second) b := backoff.NewBackoff("ntp.Wait", func(format string, args ...any) {}, 10*time.Second)
for { for {
out, err := nvram.Run("get", "ntp_ready") // ddwrt use "ntp_done": https://github.com/mirror/dd-wrt/blob/a08c693527ab3204bf7bebd408a7c9a83b6ede47/src/router/rc/ntp.c#L100
if err != nil { for _, key := range []string{"ntp_ready", "ntp_done"} {
return fmt.Errorf("PreStart: nvram: %w", err) out, err := nvram.Run("get", key)
} if err != nil {
if out == "1" { return fmt.Errorf("PreStart: nvram: %w", err)
return nil }
if out == "1" {
return nil
}
} }
b.BackOff(context.Background(), errors.New("ntp not ready")) b.BackOff(context.Background(), errors.New("ntp not ready"))
} }

View File

@@ -89,12 +89,14 @@ func (f *FreshTomato) Cleanup() error {
if f.cfg.FirstListener().IsDirectDnsListener() { if f.cfg.FirstListener().IsDirectDnsListener() {
return nil return nil
} }
if val, _ := nvram.Run("get", nvram.CtrldSetupKey); val == "1" { if val, _ := nvram.Run("get", nvram.CtrldSetupKey); val != "1" {
nvramKvMap["dnsmasq_custom"] = "" return nil // was restored, nothing to do.
// Restore old configs. }
if err := nvram.Restore(nvramKvMap, nvram.CtrldSetupKey); err != nil {
return err nvramKvMap["dnsmasq_custom"] = ""
} // Restore old configs.
if err := nvram.Restore(nvramKvMap, nvram.CtrldSetupKey); err != nil {
return err
} }
// Restart dnscrypt-proxy service. // Restart dnscrypt-proxy service.