From patchwork Fri Jun 3 09:26:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pavel Raiskup X-Patchwork-Id: 12738 Received: (qmail 8063 invoked by alias); 3 Jun 2016 09:26:36 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 8050 invoked by uid 89); 3 Jun 2016 09:26:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=listening, waited, Hx-languages-length:4443, torn X-HELO: mx1.redhat.com From: Pavel Raiskup To: libc-alpha@sourceware.org Cc: fweimer@redhat.com Subject: [PATCH] sunrpc/clnt_udp.c: always respect timeout(s) Date: Fri, 3 Jun 2016 11:26:19 +0200 Message-Id: <1464945979-10503-1-git-send-email-praiskup@redhat.com> Previously, when the port we were listening on was really busy, clntudp_call () ended up processing the UDP queue without properly defined time-limit (clntudp_call could keep processing forever without returning back to caller). The fix incorporates using of clock_gettime () to measure the real timeout, instead of simply counting the number of poll()'s. We should not face some huge slowdown because we call clock_gettime only twice in most cases *and* never in busy loop. * sunrpc/clnt_udp.c (clntudp_call): Count everything related to timeouts in milliseconds. Use clock_gettime (). Still keep checking for timeout in post-poll() time because the for(;;) loop is torn by goto statement that requires checking for timeout anyway. (clntudp_call_timeouted): New helper checking whether we still should re-send the udp request. (timeval_to_ms): New helper macro. --- sunrpc/clnt_udp.c | 66 ++++++++++++++++++++++++++++++++----------------------- 1 file changed, 38 insertions(+), 28 deletions(-) diff --git a/sunrpc/clnt_udp.c b/sunrpc/clnt_udp.c index 6ffa5f2..2cab4c2 100644 --- a/sunrpc/clnt_udp.c +++ b/sunrpc/clnt_udp.c @@ -1,6 +1,8 @@ /* * clnt_udp.c, Implements a UDP/IP based, client side RPC. * + * Copyright (c) 2016, Free Software Foundation, Inc. + * * Copyright (c) 2010, Oracle America, Inc. * * Redistribution and use in source and binary forms, with or without @@ -294,6 +296,24 @@ is_network_up (int sock) return run != NULL; } +static bool_t +clntudp_call_timeouted (const struct timespec *start, int timeout, int *resend_timeout) +{ + struct timespec now; + long remains, waited; + + __clock_gettime (CLOCK_MONOTONIC, &now); + /* count everything in milliseconds */ + waited = (now.tv_sec - start->tv_sec - 1) * 1000 + + (now.tv_nsec - start->tv_nsec + 1000000000) / 1000000; + remains = timeout - waited; + if (*resend_timeout > remains) + *resend_timeout = remains; + return (*resend_timeout <= 0); +} + +#define timeval_to_ms(t) ((t).tv_sec * 1000 + (t).tv_usec / 1000) + static enum clnt_stat clntudp_call (cl, proc, xargs, argsp, xresults, resultsp, utimeout) CLIENT *cl; /* client handle */ @@ -310,28 +330,27 @@ clntudp_call (cl, proc, xargs, argsp, xresults, resultsp, utimeout) int inlen; socklen_t fromlen; struct pollfd fd; - int milliseconds = (cu->cu_wait.tv_sec * 1000) + - (cu->cu_wait.tv_usec / 1000); + int milliseconds = timeval_to_ms (cu->cu_wait); struct sockaddr_in from; struct rpc_msg reply_msg; XDR reply_xdrs; - struct timeval time_waited; bool_t ok; int nrefreshes = 2; /* number of times to refresh cred */ - struct timeval timeout; + int timeout; /* in ms */ int anyup; /* any network interface up */ + struct timespec start; + + __clock_gettime (CLOCK_MONOTONIC, &start); if (cu->cu_total.tv_usec == -1) - { - timeout = utimeout; /* use supplied timeout */ - } + timeout = timeval_to_ms (utimeout); /* use supplied timeout */ else - { - timeout = cu->cu_total; /* use default timeout */ - } + timeout = timeval_to_ms (cu->cu_total); /* use default timeout */ + + if (timeout < milliseconds) + /* probably user's mistake, but no reason to misbehave */ + milliseconds = timeout; - time_waited.tv_sec = 0; - time_waited.tv_usec = 0; call_again: xdrs = &(cu->cu_outxdrs); if (xargs == NULL) @@ -360,7 +379,7 @@ send_again: /* * Hack to provide rpc-based message passing */ - if (timeout.tv_sec == 0 && timeout.tv_usec == 0) + if (timeout == 0) { return (cu->cu_error.re_status = RPC_TIMEDOUT); } @@ -389,29 +408,20 @@ send_again: return (cu->cu_error.re_status = RPC_CANTRECV); } - time_waited.tv_sec += cu->cu_wait.tv_sec; - time_waited.tv_usec += cu->cu_wait.tv_usec; - while (time_waited.tv_usec >= 1000000) - { - time_waited.tv_sec++; - time_waited.tv_usec -= 1000000; - } - if ((time_waited.tv_sec < timeout.tv_sec) || - ((time_waited.tv_sec == timeout.tv_sec) && - (time_waited.tv_usec < timeout.tv_usec))) - goto send_again; + if (!clntudp_call_timeouted (&start, timeout, &milliseconds)) + goto send_again; return (cu->cu_error.re_status = RPC_TIMEDOUT); - /* - * buggy in other cases because time_waited is not being - * updated. - */ case -1: if (errno == EINTR) continue; cu->cu_error.re_errno = errno; return (cu->cu_error.re_status = RPC_CANTRECV); } + + if (clntudp_call_timeouted (&start, timeout, &milliseconds)) + return (cu->cu_error.re_status = RPC_TIMEDOUT); + #ifdef IP_RECVERR if (fd.revents & POLLERR) {