MEDIUM: quic: improve fatal error handling on send

Send is conducted through qc_send_ppkts() for a QUIC connection. There
is two types of error which can be encountered on sendto() or affiliated
syscalls :
* transient error. In this case, sending is simulated with the remaining
  data and retransmission process is used to have the opportunity to
  retry emission
* fatal error. If this happens, the connection should be closed as soon
  as possible. This is done via qc_kill_conn() function. Until this
  patch, only ECONNREFUSED errno was considered as fatal.

Modify the QUIC send API to be able to differentiate transient and fatal
errors more easily. This is done by fixing the return value of the
sendto() wrapper qc_snd_buf() :
* on fatal error, a negative error code is returned. This is now the
  case for every errno except EAGAIN, EWOULDBLOCK, ENOTCONN, EINPROGRESS
  and EBADF.
* on a transient error, 0 is returned. This is the case for the listed
  errno values above and also if a partial send has been conducted by
  the kernel.
* on success, the return value of sendto() syscall is returned.

This commit will be useful to be able to handle transient error with a
quic-conn owned socket. In this case, the socket should be subscribed to
the poller and no simulated send will be conducted.

This commit allows errno management to be confined in the quic-sock
module which is a nice cleanup.

On a final note, EBADF should be considered as fatal. This will be the
subject of a next commit.

This should be backported up to 2.7.

diff --git a/include/haproxy/quic_sock.h b/include/haproxy/quic_sock.h
index 073850103144812e54ba4ee3ee9495cfbd9b6ba6..55ea170a18c06b321fe1ce0895a796bda23678e9 100644 (file)
--- a/include/haproxy/quic_sock.h
+++ b/include/haproxy/quic_sock.h
@@ -44,7 +44,7 @@ struct connection *quic_sock_accept_conn(struct listener *l, int *status);
 struct task *quic_lstnr_dghdlr(struct task *t, void *ctx, unsigned int state);
 void quic_lstnr_sock_fd_iocb(int fd);
 int qc_snd_buf(struct quic_conn *qc, const struct buffer *buf, size_t count,
-               int flags, int *syscall_errno);
+               int flags);
 int qc_rcv_buf(struct quic_conn *qc);
 
 /* Set default value for <qc> socket as uninitialized. */

diff --git a/src/quic_conn.c b/src/quic_conn.c
index 290df7179592bdc544770a17a19530a2b646907e..6e6b6b172e7e07cdadefcb7c0d811538e6a17888 100644 (file)
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -15,7 +15,6 @@
 #include <haproxy/quic_conn.h>
 
 #define _GNU_SOURCE
-#include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
 
@@ -3446,11 +3445,12 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf,
 
 /* Send datagrams stored in <buf>.
  *
- * This function returns 1 for success. Even if sendto() syscall failed,
- * buffer is drained and packets are considered as emitted and this function returns 1
- * There is a unique exception when sendto() fails with ECONNREFUSED as errno,
- * this function returns 0.
- * QUIC loss detection mechanism is used as a back door way to retry sending.
+ * This function returns 1 for success. On error, there is several behavior
+ * depending on underlying sendto() error :
+ * - for a fatal error, 0 is returned and connection is killed.
+ * - a transient error is assimilated to a success case with 1 returned.
+ *   Remaining data are purged from the buffer and will eventually be detected
+ *   as lost which gives the opportunity to retry sending.
  */
 int qc_send_ppkts(struct buffer *buf, struct ssl_sock_ctx *ctx)
 {
@@ -3490,18 +3490,13 @@ int qc_send_ppkts(struct buffer *buf, struct ssl_sock_ctx *ctx)
                 * quic-conn fd management.
                 */
                if (!skip_sendto) {
-                       int syscall_errno;
-
-                       syscall_errno = 0;
-                       if (qc_snd_buf(qc, &tmpbuf, tmpbuf.data, 0, &syscall_errno)) {
-                               if (syscall_errno == ECONNREFUSED) {
-                                       /* Let's kill this connection asap. */
-                                       TRACE_PROTO("UDP port unreachable", QUIC_EV_CONN_SPPKTS, qc);
-                                       qc_kill_conn(qc);
-                                       b_del(buf, buf->data);
-                                       goto leave;
-                               }
-
+                       int ret = qc_snd_buf(qc, &tmpbuf, tmpbuf.data, 0);
+                       if (ret < 0) {
+                               qc_kill_conn(qc);
+                               b_del(buf, buf->data);
+                               goto leave;
+                       }
+                       else if (!ret) {
                                skip_sendto = 1;
                                TRACE_ERROR("sendto error, simulate sending for the rest of data", QUIC_EV_CONN_SPPKTS, qc);
                        }

diff --git a/src/quic_sock.c b/src/quic_sock.c
index b5ef5a0400e8bb3f15d232e72ce4770309e2122b..95f771135ea595e804379f3ee5457fd0ea644e5f 100644 (file)
--- a/src/quic_sock.c
+++ b/src/quic_sock.c
@@ -501,17 +501,16 @@ static void quic_conn_sock_fd_iocb(int fd)
 /* Send a datagram stored into <buf> buffer with <sz> as size.
  * The caller must ensure there is at least <sz> bytes in this buffer.
  *
- * Returns 0 on success else non-zero. When failed, this function also
- * sets <*syscall_errno> to the errno only when the send*() syscall failed.
- * As the C library will never set errno to 0, the caller must set
- * <*syscall_errno> to 0 before calling this function to be sure to get
- * the correct errno in case a send*() syscall failure.
+ * Returns the total bytes sent over the socket. 0 is returned if a transient
+ * error is encountered which allows send to be retry later. A negative value
+ * is used for a fatal error which guarantee that all future send operation for
+ * this connection will fail.
  *
  * TODO standardize this function for a generic UDP sendto wrapper. This can be
  * done by removing the <qc> arg and replace it with address/port.
  */
 int qc_snd_buf(struct quic_conn *qc, const struct buffer *buf, size_t sz,
-               int flags, int *syscall_errno)
+               int flags)
 {
        ssize_t ret;
 
@@ -619,31 +618,29 @@ int qc_snd_buf(struct quic_conn *qc, const struct buffer *buf, size_t sz,
                  EXTRA_COUNTERS_GET(prx->extra_counters_fe,
                                     &quic_stats_module);
 
-               *syscall_errno = errno;
-               /* TODO adjust errno for UDP context. */
                if (errno == EAGAIN || errno == EWOULDBLOCK ||
                    errno == ENOTCONN || errno == EINPROGRESS || errno == EBADF) {
                        if (errno == EAGAIN || errno == EWOULDBLOCK)
                                HA_ATOMIC_INC(&prx_counters->socket_full);
                        else
                                HA_ATOMIC_INC(&prx_counters->sendto_err);
+
+                       /* transient error */
+                       TRACE_PRINTF(TRACE_LEVEL_USER, QUIC_EV_CONN_SPPKTS, qc, 0, 0, 0,
+                                    "UDP send failure errno=%d (%s)", errno, strerror(errno));
+                       return 0;
                }
-               else if (errno) {
-                       /* TODO unlisted errno : handle it explicitly.
-                        * ECONNRESET may be encounter on quic-conn socket.
-                        */
+               else {
+                       /* unrecoverable error */
                        HA_ATOMIC_INC(&prx_counters->sendto_err_unknown);
+                       TRACE_PRINTF(TRACE_LEVEL_USER, QUIC_EV_CONN_SPPKTS, qc, 0, 0, 0,
+                                    "UDP send failure errno=%d (%s)", errno, strerror(errno));
+                       return -1;
                }
-
-               /* Note that one must not consider that this macro will not modify errno. */
-               TRACE_PRINTF(TRACE_LEVEL_DEVELOPER, QUIC_EV_CONN_LPKT, qc, 0, 0, 0,
-                            "syscall error (errno=%d)", *syscall_errno);
-
-               return 1;
        }
 
        if (ret != sz)
-               return 1;
+               return 0;
 
        /* we count the total bytes sent, and the send rate for 32-byte blocks.
         * The reason for the latter is that freq_ctr are limited to 4GB and
@@ -652,7 +649,7 @@ int qc_snd_buf(struct quic_conn *qc, const struct buffer *buf, size_t sz,
        _HA_ATOMIC_ADD(&th_ctx->out_bytes, ret);
        update_freq_ctr(&th_ctx->out_32bps, (ret + 16) / 32);
 
-       return 0;
+       return ret;
 }
 
 /* Receive datagram on <qc> FD-owned socket.