BUG/MINOR: server/dns: use server_set_inetaddr() to unset srv addr from DNS

As seen before, server's addr and svc_port should not be updated directly
during runtime, because even if the update is performed under the lock,
some competing threads might be reading ->addr and ->svc_port without
the lock because they simply cannot afford it.

To prevent races with such competing threads, server's addr and port
should only be updated using server_set_inetaddr() function or similar.

This patch depends on:
 - "MINOR: server: ensure connection cleanup on server addr changes"
 - "CLEANUP: server/event_hdl: remove purge_conn hint in INETADDR event"
 - "MEDIUM: server: merge srv_update_addr() and srv_update_addr_port() logic"
 - "MEDIUM: server: make server_set_inetaddr() updater serializable"
 - "MINOR: server/event_hdl: expose updater info through INETADDR event"
 - "MINOR: server: add dns hint in server_inetaddr_updater struct"
 - "MEDIUM: server/dns: clear RMAINT when addr resolves again"

While it could be backported in 2.9 with cd994407a ("BUG/MAJOR:
server/addr: fix a race during server addr:svc_port updates") to ensure
addr and svc_port reset performed by resolver's code comply with the
API taking care of pushing the update (and thus avoid any race), some
patch dependencies are quite sensitive so it's probably best to avoid
backporting for no good reason, or at least wait for it to be considered
stable to prevent any breakeages.

diff --git a/src/resolvers.c b/src/resolvers.c
index e02c8a5612c5415d7b6f2fef5304d66baaf55f9a..ab9541467e2859ffee082a270e39ee31229eddb8 100644 (file)
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -659,14 +659,17 @@ static void leave_resolver_code()
  */
 static void resolv_srvrq_cleanup_srv(struct server *srv)
 {
+       struct server_inetaddr srv_addr;
+
        _resolv_unlink_resolution(srv->resolv_requester);
        HA_SPIN_LOCK(SERVER_LOCK, &srv->lock);
        srvrq_set_srv_down(srv);
        ha_free(&srv->hostname);
        ha_free(&srv->hostname_dn);
        srv->hostname_dn_len = 0;
-       memset(&srv->addr, 0, sizeof(srv->addr));
-       srv->svc_port = 0;
+       memset(&srv_addr, 0, sizeof(srv_addr));
+       /* unset server's addr */
+       server_set_inetaddr(srv, &srv_addr, SERVER_INETADDR_UPDATER_NONE, NULL);
        srv->flags |= SRV_F_NO_RESOLUTION;
 
        ebpt_delete(&srv->host_dn);

diff --git a/src/server.c b/src/server.c
index c3ddcb2a67de177d0ab4bda143afbc6355a7d8f3..8718adc91c4195a40e38a821b5a26c2950d91d8b 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -4448,8 +4448,13 @@ int snr_resolution_cb(struct resolv_requester *requester, struct dns_counters *c
                srv_update_addr(s, firstip, firstip_sin_family, SERVER_INETADDR_UPDATER_DNS_CACHE);
 
  update_status:
-       if (!snr_set_srv_down(s, has_no_ip) && has_no_ip)
-               memset(&s->addr, 0, sizeof(s->addr));
+       if (!snr_set_srv_down(s, has_no_ip) && has_no_ip) {
+               struct server_inetaddr s_addr;
+
+               memset(&s_addr, 0, sizeof(s_addr));
+               /* unset server's addr */
+               server_set_inetaddr(s, &s_addr, SERVER_INETADDR_UPDATER_NONE, NULL);
+       }
        return 1;
 
  invalid:
@@ -4457,8 +4462,13 @@ int snr_resolution_cb(struct resolv_requester *requester, struct dns_counters *c
                counters->app.resolver.invalid++;
                goto update_status;
        }
-       if (!snr_set_srv_down(s, has_no_ip) && has_no_ip)
-               memset(&s->addr, 0, sizeof(s->addr));
+       if (!snr_set_srv_down(s, has_no_ip) && has_no_ip) {
+               struct server_inetaddr s_addr;
+
+               memset(&s_addr, 0, sizeof(s_addr));
+               /* unset server's addr */
+               server_set_inetaddr(s, &s_addr, SERVER_INETADDR_UPDATER_NONE, NULL);
+       }
        return 0;
 }
 
@@ -4539,7 +4549,11 @@ int snr_resolution_error_cb(struct resolv_requester *requester, int error_code)
 
        HA_SPIN_LOCK(SERVER_LOCK, &s->lock);
        if (!snr_set_srv_down(s, 1)) {
-               memset(&s->addr, 0, sizeof(s->addr));
+               struct server_inetaddr s_addr;
+
+               memset(&s_addr, 0, sizeof(s_addr));
+               /* unset server's addr */
+               server_set_inetaddr(s, &s_addr, SERVER_INETADDR_UPDATER_NONE, NULL);
                HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock);
                resolv_detach_from_resolution_answer_items(requester->resolution, requester);
                return 0;