Fail to send update if first attempt to transmit IP address fails, never recovers!

[redbrain17]

If the first attempt to transmit the own IP Address to DynDNS failes. There are no further tries until the IP Address changes again. DynDNS has at this time no current IP address of the device.

possible patch for this (for discussion)

diff --git a/src/ddns.c b/src/ddns.c
index 8292347..3d49824 100644
--- a/src/ddns.c
+++ b/src/ddns.c
@@ -281,7 +281,7 @@ static int check_alias_update_table(ddns_t *ctx)
  */
            override = time_to_check(ctx, alias);
            if (!alias->ip_has_changed && !override) {
-               alias->update_required = 0;
+           //  alias->update_required = 0;
                continue;
            }

[troglobit]

This looks serious!

Could you tell me a bit more about where and how inadyn fails for you?

• How does your inadyn.conf look?
• Do you have any log messages?

I'd like to narrow down the exact cause for the failure to update so we can
add a better recovery mechanism.

[redbrain17]

here the inadyn.conf we used:

ssl
iface eth0
system default@dyndns.org
username user
password 12345
alias tainy-vpn-hub-tre.dyndns.org
cache-dir /tmp/
pidfile /var/run/inadyn.pid

Sry no further logs,

inadyn tried only one time to report its own address to the DynDNS Server.
It fails on first attempt because in our scenario the packets are routed through the default gateway settings to the remote site of a GRE tunnel and can't reach the DynDNS server on that way.

Then inadyn checks cyclically every two minutes to see if the IP address of the WAN interface has changed.

If this is not the case (and that is in most cases) no repetitions are performed and dyndns has no current IP address of the device

[troglobit]

Thanks for the update, will do a root-cause analysis and see what it could be.
Currently I think this is what's missing:

diff --git a/src/ddns.c b/src/ddns.c
index 1a2cae2..8015d59 100755
--- a/src/ddns.c
+++ b/src/ddns.c
@@ -337,6 +337,9 @@ static int send_update(ddns_t *ctx, ddns_info_t *info, ddns_alias_t *alias, int
              rc == RC_DYNDNS_RSP_RETRY_LATER ? "Temporary" : "Fatal");
        logit(LOG_WARNING, "[%d %s] %s", trans.status, trans.status_desc,
              trans.p_rsp_body != trans.p_rsp ? trans.p_rsp_body : "");
+
+       /* Update failed, force update again in ctx->cmd_check_period seconds */
+       ctx->force_addr_update = 1;
    } else {
        logit(LOG_INFO, "Successful alias table update for %s => new IP# %s",
              alias->name, alias->address);

[troglobit]

This bug should now hopefully be fixed. Thanks for reporting it! 😃 👍

[BulldozerBSG]

Not fixed. If function http_init (in function send_update) fails then the flag will not be set.

Why function http_init failed:

1. Not resolved host name
2. Not connect to host

[troglobit]

OK, let's reopen then 👍

[troglobit]

Please audit 02945ed, when everyone's happy we can go ahead and make another release 😃

[BulldozerBSG]

Looks good.

P.S.
In ddns.c:update_alias_table (line 409) I see comment in code

/* Only reset if send_update() succeeds. */
alias->update_required = 0;

And see part of commit 7e94e41

 @@ -635,14 +628,19 @@ static int check_alias_update_table(ddns_t *ctx)
    for (i = 0; i < ctx->info_count; i++) {
        ddns_info_t *info = &ctx->info[i];

 -      if (!info->ip_has_changed && !override)
 -          continue;
 -
        for (j = 0; j < info->alias_count; j++) {
 -          info->alias[j].update_required = 1;
 +          int override;
 +          ddns_alias_t *alias = &info->alias[j];
 +
 +          override = time_to_check(ctx, alias);
 +          if (!alias->ip_has_changed && !override) {
 +              alias->update_required = 0;
 +              continue;
 +          }
 +
 +          alias->update_required = 1;
            logit(LOG_WARNING, "Update %s for alias %s, new IP# %s",
 -                override ? "forced" : "needed",
 -                info->alias[j].names.name, info->my_ip_address.name);
 +                override ? "forced" : "needed", alias->name, alias->address);
        }
    }

I do not understand the purpose of this line in patch

 +              alias->update_required = 0;

And why the proposed patch does not fit? (in first comment #107 (comment))

[troglobit]

@BulldozerBSG Thanks for the audit!

I don't really understand your other question. The patch in question was not in the correct place. The update_required member is not the same as the force_addr_update, the former is short lived in one lap (iteration) of the loop, whereas the latter is respected over several laps (iterations).