sinjuginas/haproxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

BUG/MINOR: quic: adjust errno handling on sendto

Browse Source 
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.

(cherry picked from commit 6715cbf97f5142f18748d1575632082d3b0fbe91)
Signed-off-by: Willy Tarreau <w@1wt.eu>
This commit is contained in:
Amaury Denoyelle 2022-08-05 11:56:36 +02:00 committed by Willy Tarreau
parent 0dfe70e17e
commit 3366213eab
2 changed files with 34 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
include/haproxy/quic_sock.h
View File

@ -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);

61
src/quic_sock.c
View File

@ -366,13 +366,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;
@ -382,34 +383,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);
/* 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;
}