BUG/MINOR: quic: adjust errno handling on sendto

qc_snd_buf returned a size_t which means that it was never negative
despite its documentation. Thus the caller who checked for this was
never informed of a sendto error.

Clean this by changing the return value of qc_snd_buf() to an integer.
A 0 is returned on success. Every other values are considered as an
error.

This commit should be backported up to 2.6. Note that to not cause
malfunctions, it must be backported after the previous patch :
  906b0589546b700b532472ede019e5c5a8ac1f38
  MINOR: quic: explicitely ignore sendto error
This is to ensure that a sendto error does not cause send to be
interrupted which may cause a stalled transfer without a proper retry
mechanism.

The impact of this bug seems null as caller explicitely ignores sendto
error. However this part of code seems to be subject to strange issues
and it may fix them in part. It may be of interest for github issue #1808.

diff --git a/include/haproxy/quic_sock.h b/include/haproxy/quic_sock.h
index 3e3edd26707d0212702f6e2eb5872281deb20449..b69b98474e622563377da823a6f5a0951dce5ba6 100644 (file)
--- a/include/haproxy/quic_sock.h
+++ b/include/haproxy/quic_sock.h
@@ -40,8 +40,8 @@ int quic_sock_get_dst(struct connection *conn, struct sockaddr *addr, socklen_t
 int quic_sock_accepting_conn(const struct receiver *rx);
 struct connection *quic_sock_accept_conn(struct listener *l, int *status);
 void quic_sock_fd_iocb(int fd);
-size_t qc_snd_buf(struct quic_conn *qc, const struct buffer *buf, size_t count,
-                  int flags);
+int qc_snd_buf(struct quic_conn *qc, const struct buffer *buf, size_t count,
+               int flags);
 
 void quic_accept_push_qc(struct quic_conn *qc);
 

diff --git a/src/quic_sock.c b/src/quic_sock.c
index b50c22803b2914e0634f9ee0b68b84dadea25a8c..a2be5255f8543cbcb5ab8346c4e6c9857b478726 100644 (file)
--- a/src/quic_sock.c
+++ b/src/quic_sock.c
@@ -364,13 +364,14 @@ void quic_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.
- * Return the size of this datagram if succeeded, 0 if truncated and -1 in case of
- * any error.
+ *
+ * Returns 0 on success else non-zero.
+ *
  * 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.
  */
-size_t qc_snd_buf(struct quic_conn *qc, const struct buffer *buf, size_t sz,
-                  int flags)
+int qc_snd_buf(struct quic_conn *qc, const struct buffer *buf, size_t sz,
+               int flags)
 {
        ssize_t ret;
 
@@ -380,34 +381,36 @@ size_t qc_snd_buf(struct quic_conn *qc, const struct buffer *buf, size_t sz,
                             (struct sockaddr *)&qc->peer_addr, get_addr_len(&qc->peer_addr));
        } while (ret < 0 && errno == EINTR);
 
-       if (ret > 0) {
-               if (ret != sz)
-                       return 0;
+       if (ret < 0 || ret != sz) {
+               /* TODO adjust errno for UDP context. */
+               if (errno == EAGAIN || errno == EWOULDBLOCK ||
+                   errno == ENOTCONN || errno == EINPROGRESS || errno == EBADF) {
+                       struct proxy *prx = qc->li->bind_conf->frontend;
+                       struct quic_counters *prx_counters =
+                         EXTRA_COUNTERS_GET(prx->extra_counters_fe,
+                                            &quic_stats_module);
+
+                       if (errno == EAGAIN || errno == EWOULDBLOCK)
+                               HA_ATOMIC_INC(&prx_counters->socket_full);
+                       else
+                               HA_ATOMIC_INC(&prx_counters->sendto_err);
+               }
+               else if (errno) {
+                       /* TODO unlisted errno : handle it explicitely. */
+                       ABORT_NOW();
+               }
 
-               /* 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 that it's not enough per second.
-                */
-               _HA_ATOMIC_ADD(&global.out_bytes, ret);
-               update_freq_ctr(&global.out_32bps, (ret + 16) / 32);
-       }
-       else if (ret == 0 || errno == EAGAIN || errno == EWOULDBLOCK ||
-                errno == ENOTCONN || errno == EINPROGRESS || errno == EBADF) {
-               struct proxy *prx = qc->li->bind_conf->frontend;
-               struct quic_counters *prx_counters =
-                       EXTRA_COUNTERS_GET(prx->extra_counters_fe, &quic_stats_module);
-               /* TODO must be handle properly. It is justified for UDP ? */
-               if (errno == EAGAIN || errno == EWOULDBLOCK)
-                       HA_ATOMIC_INC(&prx_counters->socket_full);
-               else
-                       HA_ATOMIC_INC(&prx_counters->sendto_err);
-       }
-       else if (errno) {
-               /* TODO unlisted errno : handle it explicitely. */
-               ABORT_NOW();
+               return 1;
        }
 
-       return ret;
+       /* 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
+        * that it's not enough per second.
+        */
+       _HA_ATOMIC_ADD(&global.out_bytes, ret);
+       update_freq_ctr(&global.out_32bps, (ret + 16) / 32);
+
+       return 0;
 }