BUG/MEDIUM: server/dns: perform svc_port updates atomically from SRV records

This was the last missing bit from cd994407a ("BUG/MAJOR: server/addr:
fix a race during server addr:svc_port updates")

Indeed, despite the fix, svc_port updates from resolvers were still
directly performed on the server's struct.

Now they make proper use of the server_set_inetaddr() function so the port
change (+ optional addr change with AR) will be propagated atomically.

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 updates 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
This commit is contained in:
Aurelien DARRAGON 2023-12-12 19:47:56 +01:00 committed by Christopher Faulet
parent 64c9c8ef39
commit c5cace3100

View File

@ -832,12 +832,16 @@ static void resolv_check_response(struct resolv_resolution *res)
srv_found: srv_found:
/* And update this server, if found (srv is locked here) */ /* And update this server, if found (srv is locked here) */
if (srv) { if (srv) {
struct server_inetaddr srv_addr;
uint8_t ip_change = 0;
/* re-enable DNS resolution for this server by default */ /* re-enable DNS resolution for this server by default */
srv->flags &= ~SRV_F_NO_RESOLUTION; srv->flags &= ~SRV_F_NO_RESOLUTION;
srv->srvrq_check->expire = TICK_ETERNITY; srv->srvrq_check->expire = TICK_ETERNITY;
srv->svc_port = item->port; server_get_inetaddr(srv, &srv_addr);
srv->flags &= ~SRV_F_MAPPORTS; srv_addr.port.svc = item->port;
srv_addr.port.map = 0;
/* Check if an Additional Record is associated to this SRV record. /* Check if an Additional Record is associated to this SRV record.
* Perform some sanity checks too to ensure the record can be used. * Perform some sanity checks too to ensure the record can be used.
@ -850,10 +854,12 @@ static void resolv_check_response(struct resolv_resolution *res)
switch (item->ar_item->type) { switch (item->ar_item->type) {
case DNS_RTYPE_A: case DNS_RTYPE_A:
srv_update_addr(srv, &item->ar_item->data.in4.sin_addr, AF_INET, SERVER_INETADDR_UPDATER_DNS_AR); srv_addr.family = AF_INET;
srv_addr.addr.v4 = item->ar_item->data.in4.sin_addr;
break; break;
case DNS_RTYPE_AAAA: case DNS_RTYPE_AAAA:
srv_update_addr(srv, &item->ar_item->data.in6.sin6_addr, AF_INET6, SERVER_INETADDR_UPDATER_DNS_AR); srv_addr.family = AF_INET6;
srv_addr.addr.v6 = item->ar_item->data.in6.sin6_addr;
break; break;
} }
@ -863,8 +869,15 @@ static void resolv_check_response(struct resolv_resolution *res)
* It is usless to perform an extra resolution * It is usless to perform an extra resolution
*/ */
_resolv_unlink_resolution(srv->resolv_requester); _resolv_unlink_resolution(srv->resolv_requester);
ip_change = 1;
} }
if (ip_change)
server_set_inetaddr_warn(srv, &srv_addr, SERVER_INETADDR_UPDATER_DNS_AR);
else
server_set_inetaddr(srv, &srv_addr, SERVER_INETADDR_UPDATER_NONE, NULL);
if (!srv->hostname_dn) { if (!srv->hostname_dn) {
const char *msg = NULL; const char *msg = NULL;
char hostname[DNS_MAX_NAME_SIZE+1]; char hostname[DNS_MAX_NAME_SIZE+1];