master
Commits on May 19, 2022
-
CLEANUP: quic: adjust comment/coding style for TPs init
Fix typo in comment and adjust code alignment for better readability.
-
BUG/MEDIUM: quic: fix initialization for local/remote TPs
The local and remote TPs were both processed through the same function quic_transport_params_init(). This caused the remote TPs to be overwritten with values configured for our local usage. Change this by reserving quic_transport_params_init() only for our local TPs. Remote TPs are simply initialized via quic_dflt_transport_params_cpy(). This bug could result in a connection closed in error by the client due to a violation of its TPs. For example, curl client closed the connection after receiving too many CONNECTION_ID due to an invalid active_connection_id value used.
-
MINOR: quic: detect EBADF on sendto()
EBADF can be encountered during process termination after fd listener has been reset to -1.
-
MINOR: quic: abort on unlisted errno on sendto()
If an unlisted errno is reported, abort the process. If a crash is reported on this condition, we must determine if the error code is a bug, should interrupt emission on the fd or if we can retry the syscall.
-
BUG/MINOR: quic: break for error on sendto
If sendto returns an error, we should not retry the call and break from the sending loop. An exception is made for EINTR which allows to retry immediately the syscall. This bug caused an infinite loop reproduced when the process is in the closing state by SIGUSR1 but there is still QUIC data emission left.
-
MEDIUM: check: Use the CS to handle subscriptions for read/write events
Instead of using the health-check to subscribe to read/write events, we now rely on the conn-stream. Indeed, on the server side, the conn-stream's endpoint is a multiplexer. Thus it seems appropriate to handle subscriptions for read/write events the same way than for the streams. Of course, the I/O callback function is not the same. We use srv_chk_io_cb() instead of cs_conn_io_cb().
-
REORG: check: Rename and export I/O callback function
event_srv_chk_io() function is renamed srv_chk_io_cb() to be consistant with the I/O callback function of connections. In addition, this function is exported. It will be required to use the conn-stream's subscriptions.
-
MEDIUM: check: No longer shutdown the connection in .wake callback fu…
…nction The connection is already closed by the health-check itself. Thus there is now reason to duplicate this part in the .wake callback function. It is enough to wake the health-check and wait.
-
BUG/MINOR: check: Reinit the buffer wait list at the end of a check
The buffer wait list is used to deal with buffer allocation failure. But at the end of health-check, it must be reinitialized. There is no reason to reason to get a buffer between two health-check runs. And in fact, the associated flags, CHK_ST_IN_ALLOC and CHK_ST_OUT_ALLOC, are already cleared at the end of a health-check. This patch must be backported as far as 2.2. On the 2.2, MT_LIST_ADDED and MT_LIST_DEL must be used instead of LIST_INLIST and LIST_DEL_INIT.
-
BUG/MEDIUM: config: Reset outline buffer size on realloc error in rea…
…dcfgfile() When the line parsing failed because outline buffer must be reallocated, if my_realloc2() call fails, the buffer size must be reset. Indeed, in this case the current line is skipped, a fatal error is reported and we jump to the next line. At this stage the outline buffer is NULL. If the buffer size is not reset, the next call to parse_line() crashes because we try to write in the buffer. We fail to detect the outline buffer is too small to copy any character. To fix the issue, outlinesize variable must be set to 0 when outline allocation failed. This patch should fix the issue #1563. It must be backported as far as 2.2.
Commits on May 18, 2022
-
MINOR: mux-quic: free RX buf if empty
Release the QCS RX buffer if emptied afer qcs_consume(). This improves memory usage and avoids a QCS to keep an allocated buffer, particularly when no data is received anymore. Buffer is automatically reallocated if needed via qc_get_ncbuf().
-
BUG/MINOR: mux-quic: support nul buffer with qc_free_ncbuf()
qc_free_ncbuf() may now be used with a NCBUF_NULL buffer as parameter. This is useful when using this function on a QCS with no allocated buffer. This case was not reproduced for the moment, but it will soon become more present as buffers will be released if emptied. Also a call to offer_buffers() is added to conform with the dynamic buffer management of haproxy.
-
MINOR: mux-quic: implement MAX_DATA emission
This commit is similar to the previous one but deals with MAX_DATA for connection-level data flow control. It uses the same function qcc_consume_qcs() to update flow control level and generate a MAX_DATA frame if needed.
-
MINOR: mux-quic: implement MAX_STREAM_DATA emission
Send MAX_STREAM_DATA frames when at least half of the allocated flow-control has been demuxed, frame and cleared. This is necessary to support QUIC STREAM with received data greater than a buffer. Transcoders must use the new function qcc_consume_qcs() to empty the QCS buffer. This will allow to monitor current flow-control level and generate a MAX_STREAM_DATA frame if required. This frame will be emitted via qc_io_cb().
-
MINOR: mux-quic: reorganize flow-control frames emission
Adjust the mechanism for MAX_STREAMS_BIDI emission. When a bidirectional stream is removed, current flow-control level is checked. If needed, a MAX_STREAMS_BIDI frame is generated and inserted in a new list in the QCS instance. The new frames will be emitted at the start of qc_send(). This has no impact on the current MAX_STREAMS_BIDI behavior. However, this mechanism is more flexible and will allow to implement quickly MAX_STREAM_DATA/MAX_DATA emission.
-
MINOR: mux-quic: remove qcc_decode_qcs() call in XPRT
Slightly change the interface for qcc_recv() between MUX and XPRT. The MUX is now responsible to call qcc_decode_qcs(). This is cleaner as now the XPRT does not have to deal with an extra QCS parameter and the MUX will call qcc_decode_qcs() only if really needed. This change is possible since there is no extra buffering for out-of-order STREAM frames and the XPRT does not have to handle buffered frames.
-
MEDIUM: mux-quic: implement recv on io-cb
Previously, qc_io_cb() of mux-quic only dealt with TX. Add support for RX in it. This is done through a new function qc_recv(qcc). It loops over all QCS instances and call qcc_decode_qcs(qcs). This has no impact from the quic-conn layer as qcc_decode_qcs(qcs) is called directly. However, this allows to have a resume point when demux is blocked on the upper layer HTX full buffer. Note that for the moment, only RX for bidirectional streams is managed in qc_io_cb(). Unidirectional streams use their own mechanism for both TX/RX. It should be unified in the near future in a refactoring.
-
MINOR: h3: flag demux as full on HTX full
Flag QCS if HTX buffer is full on demux. This will block all future operations on QCS demux and should limit unnecessary decode_qcs() calls. The flag is cleared on rcv_buf operation called by conn-stream.
-
MINOR: h3: do not wait a complete frame for demuxing
Previously, H3 demuxer refused to proceed the payload if the frame was not entirely received and the QCS buffer is not full. This code was duplicated from the H2 demuxer. In H2, this is a justified optimization as only one frame at a time can be demuxed. However, this is not the case in H3 with interleaved frames in the lower layer QUIC STREAM frames. This condition is now removed. H3 demuxer will proceed payload as soon as possible. An exception is kept for HEADERS frame as the code is not able to deal with partial HEADERS. With this change, H3 demuxer should consume less memory. To ensure that we never received a HEADER bigger than the RX buffer, we should use the H3 SETTINGS_MAX_FIELD_SECTION_SIZE.
-
BUG/MINOR: mux-quic: update session's idle delay before stream creation
This commit is an adaptation from the following patch : commit d0de677 Author: Willy Tarreau <w@1wt.eu> Date: Fri Feb 4 09:05:37 2022 +0100 BUG/MINOR: mux-h2: update the session's idle delay before creating the stream This should fix the incorrect timeouts present in httplog format for QUIC requests.
-
MINOR: ncbuf: refactor ncb_advance()
First adjusted some typos in comments inside the function. Second, change the naming of some variable to reduce confusion. A special case has been inserted when advance is done inside a GAP block and this block is the last of the buffer. In this case, the whole buffer will be emptied, equivalent to a ncb_init() operation.
-
BUG/MINOR: ncbuf: fix ncb_is_empty()
ncb_is_empty() was plainly incorrect as it directly dereferences the memory to read offset blocks instead of ncb_read_off(). The result is undefined. Also, BUG_ON() statement is wrong when the buffer starts with a data block. In this case, ncb_head() is not the first gap offset but instead just random data. The calculated sum in BUG_ON() statement has thus no meaning and may cause an abort. Adjust this by reorganizing the whole function. Only the first data block size is read. If and only if not nul, the first gap size is then checked. ncb_is_full() has been rewritten to share the same model as ncb_is_empty().
-
OPTIM: quic: realign empty Rx buffer
quic_rx_pkts_del() function removes packets from QUIC RX buffer. In most cases, the buffer will be emptied after it. In this case, it's useful to realign it. This will avoid future data wrapping and use of an unnecessary junk to fill a too small contiguous space.
-
BUG/MEDIUM: quic: fix Rx buffering
The quic-conn manages a buffer to store received QUIC packets. When the buffer wraps, the gap is filled until the end with junk and packets can be inserted at the start of the buffer. On the other end, deletion is implemented via quic_rx_pkts_del(). Packets are removed one by one if their refcount is nul. If junk is found, the buffer is emptied until its wrap. This seems to work in most cases but a bug was found in a particular case : on insertion if buffer gap is not at the end of the buffer. In this case, the gap was filled, which is useless as now the buffer is full and the packet cannot be inserted. Worst, on deletion, when junk is removed there is a risk to removed new packets. This can happens in the following case : 1. buffer contig space is too small, junk is inserted in the middle of it 2. on quic_rx_pkts_del() invocation, a packet is removed, but not the next one because its refcount is still positive. When a new packet is received, it will be stored after the junk. 3. on next quic_rx_pkts_del(), when junk is removed, all contig data is cleared, with newer packets data too. This will cause a transfer between a client and haproxy to be stalled. This can be reproduced with big enough POST requests. I triggered it with ngtcp2 and 10M of posted data. Hopefully, the solution of this bug is simple. If contig space is not big enough to store a packet, but the space is not at the end of the buffer, no junk is inserted and the packet is dropped as we cannot buffered it. This ensures that junk is only present at the end of the buffer and when removed no packets data is purged with it.
-
CLEANUP: httpclient: Remove useless test on ss_dst in httpclient_appl…
…et_init() In httpclient_applet_init() function, ss_dst variable is always defined before the call to sockaddr_alloc(). There is no reason to test it. This patch should fix the issue #1706.
-
CLEANUP: peers: Remove unreachable code in peer_session_create()
An error label is now unreachable in peer_session_create(). This patch should fix the issue #1704.
-
BUG/MINOR: spoe: Fix error handling in spoe_init_appctx()
labels used in goto statement was not called in the right order. Thus if there is an error during the appctx startup, it is possible to leak a task. This patch should fix the issue #1703. No backport needed.
-
CLEANUP: http_ana: Make use of the return value of stream_generate_un…
…ique_id() Even if `unique_id` and `s->unique_id` are identical it is a bit odd to `isttest()` `unique_id` and then use `s->unique_id` in the call to `http_add_header()`. This "issue" was introduced in a17e662, because before that commit the function returned the length of the ID, as it was not an ist.
Commits on May 17, 2022
-
MINOR: ssl: Add 'ssl-provider-path' global option
When loading providers with 'ssl-provider' global options, this ssl-provider-path option can be used to set the search path that is to be used by openssl. It behaves the same way as the OPENSSL_MODULES environment variable.
-
REGTESTS: abortonclose: Fix some race conditions
Depending on the timing, the second client that should be reported as a client abort during connection attempt ("CC--" termination state) is sometime logged as a server close ("SC--" termination state) instead. It happens because sometime the connection failure to the server s1 is detected by haproxy before the client c2 aborts. There is no retries and the connection timeout is set to 100ms. So, to work, the client abort must be performed and detected by haproxy in less than 100ms. To fix the issue, the c2 client is now routed to a backend with a connection timeout set to 1 second and 10 retries. It should be large enough to detect the client aborts (~10s) In addition, there is another race condition when the script is started. sometime, server s1 is not stopped when the first client sends its request. So a barrier was added to be sure it is stopped before starting to send requests. And we wait to be sure the server is detected as DOWN to unblock the barrier. It is performed by a dedicated backend with an healthcheck on the server s1. This patch should solve issue #1664. -
CLEANUP: proxy: Remove dead code when parsing "http-restrict-req-hdr-…
…names" option negation or default modifiers are not supported for this option. However, this was already tested earlier in cfg_parse_listen() function. Thus, when "http-restrict-req-hdr-names" option is parsed, the keyword modifier is always equal to KWM_STD. It is useless to test it again at this place. This patch should solve the issue #1702.
-
MINOR: conn-stream/applet: Stop setting appctx as the endpoint context
The appctx is already the endpoint target. It is confusing to also use it to set the endpoint context. So, never set the endpoint ctx when an appctx is created or attached to an existing conn-stream.
-
MEDIUM: peers: Balance applets across threads
When creating a new applet for peer outgoing connection, we check the load on each thread. Threads with least applet count are preferred. With this solution we avoid a situation when many outgoing connections run on the same thread causing significant load on single CPU core.
-
MINOR: peers: Track number of applets run by thread
Maintain number of peers applets run on all threads. It will be used in next patch for least loaded thread selection.
-
MEDIUM: applet: Add support for async appctx startup on a thread subset
It is now possible to start an appctx on a thread subset. Some controls were added here and there. It is forbidden to start a backend appctx on another thread than the local one. If a frontend appctx is started on another thread or a thread subset, the applet .init callback function must be defined. This callback function is responsible to finalize the appctx startup. It can be performed synchornously. In this case, the appctx is started on the local thread. It is not really useful but it is valid. Or it can be performed asynchronously. In this case, .init callback function is called when the appctx is woken up for the first time. When this happens, the appctx affinity is set to the current thread to be able to start the session and the stream.