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/src/quic_sock.c b/src/quic_sock.c
index b50c228..a2be525 100644
--- a/src/quic_sock.c
+++ b/src/quic_sock.c
@@ -364,13 +364,14 @@
/* 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 @@
(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);
- /* 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();
+ 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;
}