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 :
  906b058954
  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.
This commit is contained in:
Amaury Denoyelle 2022-08-05 11:56:36 +02:00
parent 906b058954
commit 6715cbf97f
2 changed files with 34 additions and 31 deletions

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

View File

@ -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);
/* 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;
}