MINOR: quic: explicitely ignore sendto error
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.
diff --git a/src/xprt_quic.c b/src/xprt_quic.c
index e754d9e..367a9a5 100644
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -3030,6 +3030,7 @@
{
struct quic_conn *qc;
struct cbuf *cbuf;
+ char skip_sendto = 0;
qc = ctx->qc;
cbuf = qr->cbuf;
@@ -3063,9 +3064,23 @@
tmpbuf.area = (char *)pos;
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);
- if(qc_snd_buf(qc, &tmpbuf, tmpbuf.data, 0) <= 0)
- break;
+ if (!skip_sendto) {
+ 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);
qc->tx.bytes += tmpbuf.data;