[Dnsmasq-discuss] [PATCH] Answer queued TCP queries after a stale answer

[Simon Kelley]

This looks sensible.


The motivation for the existing behaviour is that I expected that the 
client would open a new TCP connection when it got the EOF, and this is 
faster, since the second query could proceed in parallel with the 
stale-cache refresh, rather then blocking until after the stale-cache 
refresh completes.

Given the rather dubious glibc behaviour, then this patch is a better 
solution.

patch applied.


Simon.

On 13/10/2026 16:00, Dominik Derigs wrote:
> From: DL6ER <dl6er at dl6er.de>
> 
> When `tcp_request()` answers from stale cache data, it closes the
> connection right away, so that the child process can refresh the name
> and exit. A query the client has already sent on that connection is
> never read, and its answer is lost.
> 
> glibc runs into this: after a truncated UDP reply, it sends the A and
> AAAA queries together on one TCP connection, gets EOF after the first
> answer and fails the lookup without retrying the second query. In a
> test with both answers expired, 7 of 20 such lookups lost the second
> answer.
> 
> Close the connection only if the client has not sent anything else
> yet. Otherwise, refresh the name and go on answering the queries on the
> connection, as for any other forwarded query.
> 
> Signed-off-by: DL6ER <dl6er at dl6er.de>
> ---
> Hi Simon,
> 
> with --use-stale-cache, tcp_request() closes the connection right after
> a stale answer, so a query the client has already queued on it is lost.
> glibc's A+AAAA retry over TCP after a truncated UDP reply runs into this
> and fails the lookup.
> 
> The patch keeps the connection open only when more data is already
> waiting (MSG_PEEK), refreshes the name and then goes on answering. An
> idle connection is still closed right away, as before.
> 
> Reproducer, loopback only: an upstream dnsmasq on port 5454 with --host-
> record=stale.test,192.0.2.1,2001:db8::1 --local-ttl=1, and the dnsmasq
> under test with --server=127.0.0.1#5454 --use-stale-cache. Query A and
> AAAA once over UDP, wait 1.5 s, then send both queries in one write on a
> TCP connection. Both answers arrived in 13 of 20 trials before and in 20
> of 20 with the patch.
> 
> Downstream report: https://github.com/pi-hole/FTL/issues/3167
> 
> Best regards,
> Dominik
> 
>   src/forward.c | 30 +++++++++++++++++++++++-------
>   1 file changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/src/forward.c b/src/forward.c
> index f736c30..dae9a4f 100644
> --- a/src/forward.c
> +++ b/src/forward.c
> @@ -2406,7 +2406,7 @@ void tcp_request(int confd, time_t now, struct iovec *bigbuff,
>     unsigned char *pheader;
>     unsigned int mark = 0;
>     int have_mark = 0;
> -  int first, last, filtered, do_stale = 0;
> +  int first, last, filtered, do_stale = 0, closed = 0;
>     struct iovec out_iov[2];
>     
>     bigbuff->iov_base = NULL;
> @@ -2742,8 +2742,16 @@ void tcp_request(int confd, time_t now, struct iovec *bigbuff,
>   	    }
>   	}
>   
> +      /* Fresh data is in the cache now. Go on with the queries the client
> +	 had already sent, if the connection is still open. */
>         if (do_stale)
> -	break;
> +	{
> +	  if (closed)
> +	    break;
> +	  do_stale = 0;
> +	  daemon->log_source_addr = &peer_addr;
> +	  continue;
> +	}
>         
>         /* In case of local answer or no connections made. */
>         if (m == 0)
> @@ -2792,19 +2800,27 @@ void tcp_request(int confd, time_t now, struct iovec *bigbuff,
>         /* If we answered with stale data, this process will now try and get fresh data into
>   	 the cache and cannot therefore accept new queries. Close the incoming
>   	 connection to signal that to the client. Then set do_stale and loop round
> -	 once more to try and get fresh data, after which we exit. */
> +	 once more to try and get fresh data, after which we exit. A client which
> +	 has already sent its next query is waiting for the answer and would lose
> +	 it, so keep the connection open then and answer it after the refresh. */
>         if (stale)
>   	{
> -	  shutdown(confd, SHUT_RDWR);
> -	  close(confd);
> +	  char c;
> +	
> +	  if (recv(confd, &c, 1, MSG_PEEK | MSG_DONTWAIT) != 1)
> +	    {
> +	      shutdown(confd, SHUT_RDWR);
> +	      close(confd);
> +	      closed = 1;
> +	    }
>   	  do_stale = 1;
>   	  /* Don't mark the query with the source when refreshing stale data. */
>   	  daemon->log_source_addr = NULL;
>   	}
>       }
>     
> -/* If we ran once to get fresh data, confd is already closed. */
> -  if (!do_stale)
> +/* If we ran once to get fresh data, confd may be closed already. */
> +  if (!closed)
>       {
>         shutdown(confd, SHUT_RDWR);
>         close(confd);