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 e754d9e92531d6c08a7da667aa97aa039cd79380..367a9a533be782768af4de5fde96c9910dd2ceb4 100644 (file)
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -3030,6 +3030,7 @@ int qc_send_ppkts(struct qring *qr, struct ssl_sock_ctx *ctx)
 {
        struct quic_conn *qc;
        struct cbuf *cbuf;
+       char skip_sendto = 0;
 
        qc = ctx->qc;
        cbuf = qr->cbuf;
@@ -3063,9 +3064,23 @@ int qc_send_ppkts(struct qring *qr, struct ssl_sock_ctx *ctx)
                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;