RepoMirrors/haproxy
1
0
Fork 0
mirror of http://git.haproxy.org/git/haproxy.git/ synced 2025-02-18 11:47:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

MINOR: quic: explicitely ignore sendto error

Browse Source 
qc_snd_buf() returns an error if sendto has failed. On standard
conditions, we should check for EAGAIN/EWOULDBLOCK errno and if so,
register the file-descriptor in the poller to retry the operation later.

However, quic_conn uses directly the listener fd which is shared for all
QUIC connections of this listener on several threads. Thus, it's
complicated to implement fd supversion via the poller : there is no
mechanism to easily wakeup quic_conn or MUX after a sendto failure.

A quick and simple solution for the moment is to considered a datagram
as properly emitted even on sendto error. In the end, this will trigger
the quic_conn retransmission timer as data will be considered lost on
the network and the send operation will be retried. This solution will
be replaced when fd management for quic_conn is reworked.

In fact, this quick hack was already in use in the current code, albeit
not voluntarily. This is due to a bug caused by an API mismatch on the
return type of qc_snd_buf() which never emits a negative error code
despite its documentation. Thus, all its invocation were considered as a
success. If this bug was fixed, the sending would would have been
interrupted by a break which could cause the transfer to freeze.

qc_snd_buf() invocation is clean up : the break statement is removed.
Send operation is now always explicitely conducted entirely even on
error and buffer data is purged.

A simple optimization has been added to skip over sendto when looping
over several datagrams at the first sendto error. However, to properly
function, it requires a fix on the return type of qc_snd_buf() which is
provided in another patch.

As the behavior before and after this patch seems identical, it is not
labelled as a BUG. However, it should be backported for cleaning
purpose. It may also have an impact on github issue #1808.
This commit is contained in:
Amaury Denoyelle 2022-08-05 15:22:28 +02:00
parent e7df68a219
commit 906b058954
1 changed files with 17 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/xprt_quic.c
View File

@ -3030,6 +3030,7 @@ int qc_send_ppkts(struct qring *qr, struct ssl_sock_ctx *ctx)
{ {
struct quic_conn *qc; struct quic_conn *qc;
struct cbuf *cbuf; struct cbuf *cbuf;
char skip_sendto = 0;
qc = ctx->qc; qc = ctx->qc;
cbuf = qr->cbuf; cbuf = qr->cbuf;
@ -3063,9 +3064,23 @@ int qc_send_ppkts(struct qring *qr, struct ssl_sock_ctx *ctx)
tmpbuf.area = (char *)pos; tmpbuf.area = (char *)pos;
tmpbuf.size = tmpbuf.data = dglen; tmpbuf.size = tmpbuf.data = dglen;
/* If sendto is on error just skip the call to it for the rest
* of the loop but continue to purge the buffer. Data will be
* transmitted when QUIC packets are detected as lost on our
* side.
*
* TODO use fd-monitoring to detect when send operation can be
* retry. This should improve the bandwidth without relying on
* retransmission timer. However, it requires a major rework on
* quic-conn fd management.
*/
TRACE_PROTO("to send", QUIC_EV_CONN_SPPKTS, qc); TRACE_PROTO("to send", QUIC_EV_CONN_SPPKTS, qc);
if(qc_snd_buf(qc, &tmpbuf, tmpbuf.data, 0) <= 0) if (!skip_sendto) {
break; if (qc_snd_buf(qc, &tmpbuf, tmpbuf.data, 0)) {
skip_sendto = 1;
TRACE_PROTO("sendto error, simulate sending for the rest of data", QUIC_EV_CONN_SPPKTS, qc);
}
}
cb_del(cbuf, dglen + headlen); cb_del(cbuf, dglen + headlen);
qc->tx.bytes += tmpbuf.data; qc->tx.bytes += tmpbuf.data;