+++ /dev/null
-From 1962e6128a4d86a7c54977577e1e4224cadbb5f7 Mon Sep 17 00:00:00 2001
-From: Alexander Scheel <ascheel@redhat.com>
-Date: Wed, 2 Aug 2017 15:11:49 -0400
-Subject: [PATCH] [client] Switch to non-blocking sockets
-
-Switch the gssproxy client library to non-blocking sockets, allowing
-for timeout and retry operations. The client will automatically retry
-both send() and recv() operations three times on ETIMEDOUT. If the
-combined send() and recv() hit the three time limit, ETIMEDOUT will be
-exposed to the caller in the minor status.
-
-Signed-off-by: Alexander Scheel <ascheel@redhat.com>
-Reviewed-by: Simo Sorce <simo@redhat.com>
-[rharwood@redhat.com: commit message cleanups, rebased]
-Reviewed-by: Robbie Harwood <rharwood@redhat.com>
-(cherry picked from commit d035646c8feb0b78f0c157580ca02c46cd00dd7e)
----
- proxy/src/client/gpm_common.c | 317 +++++++++++++++++++++++++++++++---
- 1 file changed, 295 insertions(+), 22 deletions(-)
-
-diff --git a/proxy/src/client/gpm_common.c b/proxy/src/client/gpm_common.c
-index 2133618..dba23a6 100644
---- a/proxy/src/client/gpm_common.c
-+++ b/proxy/src/client/gpm_common.c
-@@ -7,9 +7,15 @@
- #include <stdlib.h>
- #include <time.h>
- #include <pthread.h>
-+#include <sys/epoll.h>
-+#include <fcntl.h>
-+#include <sys/timerfd.h>
-
- #define FRAGMENT_BIT (1 << 31)
-
-+#define RESPONSE_TIMEOUT 15
-+#define MAX_TIMEOUT_RETRY 3
-+
- struct gpm_ctx {
- pthread_mutex_t lock;
- int fd;
-@@ -20,6 +26,9 @@ struct gpm_ctx {
- gid_t gid;
-
- int next_xid;
-+
-+ int epollfd;
-+ int timerfd;
- };
-
- /* a single global struct is not particularly efficient,
-@@ -39,6 +48,8 @@ static void gpm_init_once(void)
- pthread_mutex_init(&gpm_global_ctx.lock, &attr);
-
- gpm_global_ctx.fd = -1;
-+ gpm_global_ctx.epollfd = -1;
-+ gpm_global_ctx.timerfd = -1;
-
- seedp = time(NULL) + getpid() + pthread_self();
- gpm_global_ctx.next_xid = rand_r(&seedp);
-@@ -69,6 +80,7 @@ static int gpm_open_socket(struct gpm_ctx *gpmctx)
- struct sockaddr_un addr = {0};
- char name[PATH_MAX];
- int ret;
-+ unsigned flags;
- int fd = -1;
-
- ret = get_pipe_name(name);
-@@ -86,6 +98,18 @@ static int gpm_open_socket(struct gpm_ctx *gpmctx)
- goto done;
- }
-
-+ ret = fcntl(fd, F_GETFD, &flags);
-+ if (ret != 0) {
-+ ret = errno;
-+ goto done;
-+ }
-+
-+ ret = fcntl(fd, F_SETFD, flags | O_NONBLOCK);
-+ if (ret != 0) {
-+ ret = errno;
-+ goto done;
-+ }
-+
- ret = connect(fd, (struct sockaddr *)&addr, sizeof(addr));
- if (ret == -1) {
- ret = errno;
-@@ -163,6 +187,158 @@ static int gpm_release_sock(struct gpm_ctx *gpmctx)
- return pthread_mutex_unlock(&gpmctx->lock);
- }
-
-+static void gpm_timer_close(struct gpm_ctx *gpmctx) {
-+ if (gpmctx->timerfd < 0) {
-+ return;
-+ }
-+
-+ close(gpmctx->timerfd);
-+ gpmctx->timerfd = -1;
-+}
-+
-+static int gpm_timer_setup(struct gpm_ctx *gpmctx, int timeout_seconds) {
-+ int ret;
-+ struct itimerspec its;
-+
-+ if (gpmctx->timerfd >= 0) {
-+ gpm_timer_close(gpmctx);
-+ }
-+
-+ gpmctx->timerfd = timerfd_create(CLOCK_MONOTONIC, TFD_NONBLOCK | TFD_CLOEXEC);
-+ if (gpmctx->timerfd < 0) {
-+ return errno;
-+ }
-+
-+ its.it_interval.tv_sec = timeout_seconds;
-+ its.it_interval.tv_nsec = 0;
-+ its.it_value.tv_sec = timeout_seconds;
-+ its.it_value.tv_nsec = 0;
-+
-+ ret = timerfd_settime(gpmctx->timerfd, 0, &its, NULL);
-+ if (ret) {
-+ ret = errno;
-+ gpm_timer_close(gpmctx);
-+ return ret;
-+ }
-+
-+ return 0;
-+}
-+
-+static void gpm_epoll_close(struct gpm_ctx *gpmctx) {
-+ if (gpmctx->epollfd < 0) {
-+ return;
-+ }
-+
-+ close(gpmctx->epollfd);
-+ gpmctx->epollfd = -1;
-+}
-+
-+static int gpm_epoll_setup(struct gpm_ctx *gpmctx) {
-+ struct epoll_event ev;
-+ int ret;
-+
-+ if (gpmctx->epollfd >= 0) {
-+ gpm_epoll_close(gpmctx);
-+ }
-+
-+ gpmctx->epollfd = epoll_create1(EPOLL_CLOEXEC);
-+ if (gpmctx->epollfd == -1) {
-+ return errno;
-+ }
-+
-+ /* Add timer */
-+ ev.events = EPOLLIN;
-+ ev.data.fd = gpmctx->timerfd;
-+ ret = epoll_ctl(gpmctx->epollfd, EPOLL_CTL_ADD, gpmctx->timerfd, &ev);
-+ if (ret == -1) {
-+ ret = errno;
-+ gpm_epoll_close(gpmctx);
-+ return ret;
-+ }
-+
-+ return ret;
-+}
-+
-+static int gpm_epoll_wait(struct gpm_ctx *gpmctx, uint32_t event_flags) {
-+ int ret;
-+ int epoll_ret;
-+ struct epoll_event ev;
-+ struct epoll_event events[2];
-+ uint64_t timer_read;
-+
-+ if (gpmctx->epollfd < 0) {
-+ ret = gpm_epoll_setup(gpmctx);
-+ if (ret)
-+ return ret;
-+ }
-+
-+ ev.events = event_flags;
-+ ev.data.fd = gpmctx->fd;
-+ epoll_ret = epoll_ctl(gpmctx->epollfd, EPOLL_CTL_ADD, gpmctx->fd, &ev);
-+ if (epoll_ret == -1) {
-+ ret = errno;
-+ gpm_epoll_close(gpmctx);
-+ return ret;
-+ }
-+
-+ do {
-+ epoll_ret = epoll_wait(gpmctx->epollfd, events, 2, -1);
-+ } while (epoll_ret < 0 && errno == EINTR);
-+
-+ if (epoll_ret < 0) {
-+ /* Error while waiting that isn't EINTR */
-+ ret = errno;
-+ gpm_epoll_close(gpmctx);
-+ } else if (epoll_ret == 0) {
-+ /* Shouldn't happen as timeout == -1; treat it like a timeout
-+ * occurred. */
-+ ret = ETIMEDOUT;
-+ gpm_epoll_close(gpmctx);
-+ } else if (epoll_ret == 1 && events[0].data.fd == gpmctx->timerfd) {
-+ /* Got an event which is only our timer */
-+ ret = read(gpmctx->timerfd, &timer_read, sizeof(uint64_t));
-+ if (ret == -1 && errno != EAGAIN && errno != EWOULDBLOCK) {
-+ /* In the case when reading from the timer failed, don't hide the
-+ * timer error behind ETIMEDOUT such that it isn't retried */
-+ ret = errno;
-+ } else {
-+ /* If ret == 0, then we definitely timed out. Else, if ret == -1
-+ * and errno == EAGAIN or errno == EWOULDBLOCK, we're in a weird
-+ * edge case where epoll thinks the timer can be read, but it
-+ * is blocking more; treat it like a TIMEOUT and retry, as
-+ * nothing around us would handle EAGAIN from timer and retry
-+ * it. */
-+ ret = ETIMEDOUT;
-+ }
-+ gpm_epoll_close(gpmctx);
-+ } else {
-+ /* If ret == 2, then we ignore the timerfd; that way if the next
-+ * operation cannot be performed immediately, we timeout and retry.
-+ * If ret == 1 and data.fd == gpmctx->fd, return 0. */
-+ ret = 0;
-+ }
-+
-+ epoll_ret = epoll_ctl(gpmctx->epollfd, EPOLL_CTL_DEL, gpmctx->fd, NULL);
-+ if (epoll_ret == -1) {
-+ /* If we previously had an error, expose that error instead of
-+ * clobbering it with errno; else if no error, then assume it is
-+ * better to notify of the error deleting the event than it is
-+ * to continue. */
-+ if (ret == 0)
-+ ret = errno;
-+ gpm_epoll_close(gpmctx);
-+ }
-+
-+ return ret;
-+}
-+
-+static int gpm_retry_socket(struct gpm_ctx *gpmctx)
-+{
-+ gpm_epoll_close(gpmctx);
-+ gpm_close_socket(gpmctx);
-+ return gpm_open_socket(gpmctx);
-+}
-+
- /* must be called after the lock has been grabbed */
- static int gpm_send_buffer(struct gpm_ctx *gpmctx,
- char *buffer, uint32_t length)
-@@ -183,8 +359,13 @@ static int gpm_send_buffer(struct gpm_ctx *gpmctx,
- retry = false;
- do {
- do {
-+ ret = gpm_epoll_wait(gpmctx, EPOLLOUT);
-+ if (ret != 0) {
-+ goto done;
-+ }
-+
- ret = 0;
-- wn = send(gpmctx->fd, &size, sizeof(uint32_t), MSG_NOSIGNAL);
-+ wn = write(gpmctx->fd, &size, sizeof(uint32_t));
- if (wn == -1) {
- ret = errno;
- }
-@@ -192,8 +373,7 @@ static int gpm_send_buffer(struct gpm_ctx *gpmctx,
- if (wn != 4) {
- /* reopen and retry once */
- if (retry == false) {
-- gpm_close_socket(gpmctx);
-- ret = gpm_open_socket(gpmctx);
-+ ret = gpm_retry_socket(gpmctx);
- if (ret == 0) {
- retry = true;
- continue;
-@@ -208,9 +388,14 @@ static int gpm_send_buffer(struct gpm_ctx *gpmctx,
-
- pos = 0;
- while (length > pos) {
-- wn = send(gpmctx->fd, buffer + pos, length - pos, MSG_NOSIGNAL);
-+ ret = gpm_epoll_wait(gpmctx, EPOLLOUT);
-+ if (ret) {
-+ goto done;
-+ }
-+
-+ wn = write(gpmctx->fd, buffer + pos, length - pos);
- if (wn == -1) {
-- if (errno == EINTR) {
-+ if (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK) {
- continue;
- }
- ret = errno;
-@@ -231,7 +416,7 @@ done:
-
- /* must be called after the lock has been grabbed */
- static int gpm_recv_buffer(struct gpm_ctx *gpmctx,
-- char *buffer, uint32_t *length)
-+ char **buffer, uint32_t *length)
- {
- uint32_t size;
- ssize_t rn;
-@@ -239,6 +424,11 @@ static int gpm_recv_buffer(struct gpm_ctx *gpmctx,
- int ret;
-
- do {
-+ ret = gpm_epoll_wait(gpmctx, EPOLLIN);
-+ if (ret) {
-+ goto done;
-+ }
-+
- ret = 0;
- rn = read(gpmctx->fd, &size, sizeof(uint32_t));
- if (rn == -1) {
-@@ -258,11 +448,22 @@ static int gpm_recv_buffer(struct gpm_ctx *gpmctx,
- goto done;
- }
-
-+ *buffer = malloc(*length);
-+ if (*buffer == NULL) {
-+ ret = ENOMEM;
-+ goto done;
-+ }
-+
- pos = 0;
- while (*length > pos) {
-- rn = read(gpmctx->fd, buffer + pos, *length - pos);
-+ ret = gpm_epoll_wait(gpmctx, EPOLLIN);
-+ if (ret) {
-+ goto done;
-+ }
-+
-+ rn = read(gpmctx->fd, *buffer + pos, *length - pos);
- if (rn == -1) {
-- if (errno == EINTR) {
-+ if (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK) {
- continue;
- }
- ret = errno;
-@@ -281,6 +482,7 @@ done:
- if (ret) {
- /* on errors we can only close the fd and return */
- gpm_close_socket(gpmctx);
-+ gpm_epoll_close(gpmctx);
- }
- return ret;
- }
-@@ -309,6 +511,63 @@ static struct gpm_ctx *gpm_get_ctx(void)
- return &gpm_global_ctx;
- }
-
-+static int gpm_send_recv_loop(struct gpm_ctx *gpmctx, char *send_buffer,
-+ uint32_t send_length, char** recv_buffer,
-+ uint32_t *recv_length)
-+{
-+ int ret;
-+ int retry_count;
-+
-+ /* setup timer */
-+ ret = gpm_timer_setup(gpmctx, RESPONSE_TIMEOUT);
-+ if (ret)
-+ return ret;
-+
-+ for (retry_count = 0; retry_count < MAX_TIMEOUT_RETRY; retry_count++) {
-+ /* send to proxy */
-+ ret = gpm_send_buffer(gpmctx, send_buffer, send_length);
-+
-+ if (ret == 0) {
-+ /* No error, continue to recv */
-+ } else if (ret == ETIMEDOUT) {
-+ /* Close and reopen socket before trying again */
-+ ret = gpm_retry_socket(gpmctx);
-+ if (ret != 0)
-+ return ret;
-+ ret = ETIMEDOUT;
-+
-+ /* RETRY entire send */
-+ continue;
-+ } else {
-+ /* Other error */
-+ return ret;
-+ }
-+
-+ /* receive answer */
-+ ret = gpm_recv_buffer(gpmctx, recv_buffer, recv_length);
-+ if (ret == 0) {
-+ /* No error */
-+ break;
-+ } else if (ret == ETIMEDOUT) {
-+ /* Close and reopen socket before trying again */
-+ ret = gpm_retry_socket(gpmctx);
-+
-+ /* Free buffer and set it to NULL to prevent free(xdr_reply_ctx) */
-+ free(recv_buffer);
-+ recv_buffer = NULL;
-+
-+ if (ret != 0)
-+ return ret;
-+ ret = ETIMEDOUT;
-+ } else {
-+ /* Other error */
-+ return ret;
-+ }
-+ }
-+
-+ return ret;
-+}
-+
- OM_uint32 gpm_release_buffer(OM_uint32 *minor_status,
- gss_buffer_t buffer)
- {
-@@ -399,15 +658,20 @@ int gpm_make_call(int proc, union gp_rpc_arg *arg, union gp_rpc_res *res)
- gp_rpc_msg msg;
- XDR xdr_call_ctx;
- XDR xdr_reply_ctx;
-- char buffer[MAX_RPC_SIZE];
-- uint32_t length;
-+ char *send_buffer = NULL;
-+ char *recv_buffer = NULL;
-+ uint32_t send_length;
-+ uint32_t recv_length;
- uint32_t xid;
- bool xdrok;
- bool sockgrab = false;
- int ret;
-
-- xdrmem_create(&xdr_call_ctx, buffer, MAX_RPC_SIZE, XDR_ENCODE);
-- xdrmem_create(&xdr_reply_ctx, buffer, MAX_RPC_SIZE, XDR_DECODE);
-+ send_buffer = malloc(MAX_RPC_SIZE);
-+ if (send_buffer == NULL)
-+ return ENOMEM;
-+
-+ xdrmem_create(&xdr_call_ctx, send_buffer, MAX_RPC_SIZE, XDR_ENCODE);
-
- memset(&msg, 0, sizeof(gp_rpc_msg));
- msg.header.type = GP_RPC_CALL;
-@@ -450,22 +714,22 @@ int gpm_make_call(int proc, union gp_rpc_arg *arg, union gp_rpc_res *res)
- goto done;
- }
-
-- /* send to proxy */
-- ret = gpm_send_buffer(gpmctx, buffer, xdr_getpos(&xdr_call_ctx));
-- if (ret) {
-- goto done;
-- }
-+ /* set send_length */
-+ send_length = xdr_getpos(&xdr_call_ctx);
-
-- /* receive answer */
-- ret = gpm_recv_buffer(gpmctx, buffer, &length);
-- if (ret) {
-+ /* Send request, receive response with timeout */
-+ ret = gpm_send_recv_loop(gpmctx, send_buffer, send_length, &recv_buffer,
-+ &recv_length);
-+ if (ret)
- goto done;
-- }
-
- /* release the lock */
- gpm_release_sock(gpmctx);
- sockgrab = false;
-
-+ /* Create the reply context */
-+ xdrmem_create(&xdr_reply_ctx, recv_buffer, recv_length, XDR_DECODE);
-+
- /* decode header */
- memset(&msg, 0, sizeof(gp_rpc_msg));
- xdrok = xdr_gp_rpc_msg(&xdr_reply_ctx, &msg);
-@@ -489,12 +753,21 @@ int gpm_make_call(int proc, union gp_rpc_arg *arg, union gp_rpc_res *res)
- }
-
- done:
-+ gpm_timer_close(gpmctx);
-+ gpm_epoll_close(gpmctx);
-+
- if (sockgrab) {
- gpm_release_sock(gpmctx);
- }
- xdr_free((xdrproc_t)xdr_gp_rpc_msg, (char *)&msg);
- xdr_destroy(&xdr_call_ctx);
-- xdr_destroy(&xdr_reply_ctx);
-+
-+ if (recv_buffer != NULL)
-+ xdr_destroy(&xdr_reply_ctx);
-+
-+ free(send_buffer);
-+ free(recv_buffer);
-+
- return ret;
- }
-
Code Review / pti / rtp.git / blobdiff