Many places deal with buffer realignment after data removal. The method
is always the same : if the buffer is empty, set its pointer to the origin.
Let's have a function for this so that we have less code to change with the
new API.
Add a new function that lets you set the amount of input in a buffer.
For now it extends/truncates b->i except if the total length is
below b->o in which case it clears i and adjusts o.
Instead of doing b->i -= directly, introduce b_sub(), that does the job, to
make it easier to switch to the future API.
Also add b_add(), that increases b->i, instead of using it directly, and
bo_add(), that does increase b->o.
Here's the list of newly introduced functions :
- b_data(), returning the total amount of data in the buffer (currently i+o)
- b_orig(), returning the origin of the storage area, that is, the place of
position 0.
- b_wrap(), pointer to wrapping point (currently data+size)
- b_size(), returning the size of the buffer
- b_room(), returning the amount of bytes left available
- b_full(), returning true if the buffer is full, otherwise false
- b_stop(), pointer to end of data mark (currently p+i), used to compute
distances or a stop pointer for a loop.
- b_peek(), this one will help make the transition to the new buffer model.
It returns a pointer to a position in the buffer known from an offest
relative to the beginning of the data in the buffer. Thus, we can replace
the following occurrences :
bo_ptr(b) => b_peek(b, 0);
bo_end(b) => b_peek(b, b->o);
bi_ptr(b) => b_peek(b, b->o);
bi_end(b) => b_peek(b, b->i + b->o);
b_ptr(b, ofs) => b_peek(b, b->o + ofs);
- b_head(), pointer to the beginning of data (currently bo_ptr())
- b_tail(), pointer to first free place (currently bi_ptr())
- b_next() / b_next_ofs(), pointer to the next byte, taking wrapping
into account.
- b_dist(), returning the distance between two pointers belonging to a buffer
- b_reset(), which resets the buffer
- b_space_wraps(), indicating if the free space wraps around the buffer
- b_almost_full(), indicating if 3/4 or more of the buffer are used
Some of these are provided with the unchecked variants using the "__"
prefix, or with the "_ofs" suffix indicating they return a relative
position to the buffer's origin instead of a pointer.
Cc: Olivier Houchard <ohouchard@haproxy.com>
Passing unsigned ints everywhere is painful, and will cause some headache
later when we'll want to integrate better with struct ist which already
uses size_t. Let's switch buffers to use size_t instead.
The buffer code currently depends on pools and other stuff and is not
really autonomous anymore. The rewrite of the new API is an opportunity
to clean this up. This patch creates a new file (buf.h) which does not
depend on other elements and which will only contain what is needed to
perform the most basic buffer operations. The new API will be introduced
in this file and the conversion will be finished once buffer.h is empty.
The definition of struct buffer was moved to this new file, using more
explicity stdint types for the sizes and offsets.
Most new functions will be implemented in two variants :
__b_something() : unchecked variant, no wrapping is expected
b_something() : wrapping-checked variant
This way callers will be able to select which one to use depending on
the use cases.
The behavior of sigprocmask in an multithreaded environment is
undefined.
The new macro ha_sigmask() calls either pthreads_sigmask() or
sigprocmask() if haproxy was built with thread support or not.
This should be backported to 1.8.
A few users reported that building without threads was accidently broken
after commit 6b96f72 ("BUG/MEDIUM: pollers: Use a global list for fd
shared between threads.") due to all_threads_mask not being defined.
It's OK to set it to zero as other code parts do when threads are
enabled but only one thread is used.
This needs to be backported to 1.8.
With the old model, any fd shared by multiple threads, such as listeners
or dns sockets, would only be updated on one threads, so that could lead
to missed event, or spurious wakeups.
To avoid this, add a global list for fd that are shared, using the same
implementation as the fd cache, and only remove entries from this list
when every thread as updated its poller.
[wt: this will need to be backported to 1.8 but differently so this patch
must not be backported as-is]
We'll need this in order to support uploading chunks. The h2 to h1
converter checks for the presence of the content-length header field
as well as the CONNECT method and returns these information to the
caller. The caller indicates whether or not a body is detected for
the message (presence of END_STREAM or not). No transfer-encoding
header is emitted yet.
With gcc < 4.7, when HAProxy is built with threads, the macros
HA_ATOMIC_CAS/XCHG/STORE relies on the legacy __sync builtins. These macros
are slightly complicated than the versions relying on the '_atomic'
builtins. Internally, some local variables are defined, prefixed with '__' to
avoid name clashes with the caller.
On the other hand, the macros HA_ATOMIC_UPDATE_MIN/MAX call HA_ATOMIC_CAS. Some
local variables are also definied in these macros, following the same naming
rule as below. The problem is that '__new' variable is used in
HA_ATOMIC_MIN/_MAX and in HA_ATOMIC_CAS. Obviously, the behaviour is undefined
because '__new' in HA_ATOMIC_CAS is left uninitialized. Unfortunatly gcc fails
to detect this error.
To fix the problem, all internal variables to macros are now suffixed with name
of the macros to avoid clashes (for instance, '__new_cas' in HA_ATOMIC_CAS).
This patch must be backported in 1.8.
The management of the servers and the proxies queues was not thread-safe at
all. First, the accesses to <strm>->pend_pos were not protected. So it was
possible to release it on a thread (for instance because the stream is released)
and to use it in same time on another one (because we redispatch pending
connections for a server). Then, the accesses to stream's information (flags and
target) from anywhere is forbidden. To be safe, The stream's state must always
be updated in the context of process_stream.
So to fix these issues, the queue module has been refactored. A lock has been
added in the pendconn structure. And now, when we try to dequeue a pending
connection, we start by unlinking it from the server/proxy queue and we wake up
the stream. Then, it is the stream reponsibility to really dequeue it (or
release it). This way, we are sure that only the stream can create and release
its <pend_pos> field.
However, be careful. This new implementation should be thread-safe
(hopefully...). But it is not optimal and in some situations, it could be really
slower in multi-threaded mode than in single-threaded one. The problem is that,
when we try to dequeue pending connections, we process it from the older one to
the newer one independently to the thread's affinity. So we need to wait the
other threads' wakeup to really process them. If threads are blocked in the
poller, this will add a significant latency. This problem happens when maxconn
values are very low.
This patch must be backported in 1.8.
When the block of data need to be split to support the wrapping, the start of
the second block of data was wrong. We must be sure to skup data copied during
the first memcpy.
This patch must be backported to 1.8.
When the block of data need to be split to support the wrapping, the start of
the second block of data was wrong. We must be sure to skip data copied during
the first memcpy.
This patch must be backported to 1.8, 1.7, 1.6 and 1.5.
Since we use padding before the allocated page, it's trivial to place
the allocated address there and see if it gets mangled once we release
it.
This may be backported to stable releases already using DEBUG_UAF.
Commit 158fa75 ("MINOR: pools: implement DEBUG_UAF to detect use after free")
implemented pool use-after-free detection, but the mmap() return value isn't
properly checked, preventing the call to pool_alloc_area() from returning
NULL. So on out-of-memory a mangled pointer is returned, causing a crash on
the pool_alloc() site instead of forcing a GC. It doesn't affect regular
operations however, just complicates complex bug investigations.
This fix should be backported to 1.8 and to 1.7.
Since commit cf975d4 ("MINOR: pools/threads: Implement lockless memory
pools."), we support lockless pools. However the parts dedicated to
detecting use-after-free are not present in this part, making DEBUG_UAF
useless in this situation.
The present patch sets a new define CONFIG_HAP_LOCKLESS_POOLS when such
a compatible architecture is detected, and when pool debugging is not
requested, then makes use of this everywhere in pools and buffers
functions. This way enabling DEBUG_UAF will automatically disable the
lockless version.
No backport is needed as this is purely 1.9-dev.
This removes the end label from memory.h.
The labels are unused as of cf975d46bc
which is unreleased (and incidentally the first commit containing
those labels, thus they never have been used).
A TLS ticket keys file can be updated on the CLI and used in same time. So we
need to protect it to be sure all accesses are thread-safe. Because updates are
infrequent, a R/W lock has been used.
This patch must be backported in 1.8
Commit f61f0cb ("MINOR: threads: Introduce double-width CAS on x86_64
and arm.") introduced the double CAS. But the ARMv7 version is bogus,
it uses the value of the pointers instead of dereferencing them. When
lucky, it simply doesn't build due to impossible registers combinations.
Otherwise it will immediately crash at run time when facing traffic.
No backport is needed, this bug was introduced in 1.9-dev.
Create a local, per-thread, fdcache, for file descriptors that only belongs
to one thread, and make the global fd cache mostly lockless, as we can get
a lot of contention on the fd cache lock.
Recent changes to the enum were not synchronized with the lock debugging
code. Now we use a switch/case instead of an array so that the compiler
throws a warning if there is any inconsistency.
To be backported to 1.8 (at least to add the START entry).
Marc Fournier reported an interesting case when using threads with the
master-worker mode : sometimes, a listener would have its FD closed
during startup. Sometimes it could even be health checks seeing this.
What happens is that after the threads are created, and the pollers
enabled on each threads, the master-worker pipe is registered, and at
the same time a close() is performed on the write side of this pipe
since the children must not use it.
But since this is replicated in every thread, what happens is that the
first thread closes the pipe, thus releases the FD, and the next thread
starting a listener in parallel gets this FD reassigned. Then another
thread closes the FD again, which this time corresponds to the listener.
It can also happen with the health check sockets if they're started
early enough.
This patch splits the mworker_pipe_register() function in two, so that
the close() of the write side of the FD is performed very early after the
fork() and long before threads are created (we don't need to delay it
anyway). Only the pipe registration is done in the threaded code since
it is important that the pollers are properly allocated for this.
The mworker_pipe_register() function now takes care of registering the
pipe only once, and this is guaranteed by a new surrounding lock.
The call to protocol_enable_all() looks fragile in theory since it
scans the list of proxies and their listeners, though in practice
all threads scan the same list and take the same locks for each
listener so it's not possible that any of them escapes the process
and finishes before all listeners are started. And the operation is
idempotent.
This fix must be backported to 1.8. Thanks to Marc for providing very
detailed traces clearly showing the problem.
This one allows not to inflate some structures when threads are
disabled. Now struct global is 1.4 kB instead of 33 kB.
Should be backported to 1.8 for ease of backporting of upcoming
patches.
Till now the use of __atomic_* gcc builtins required gcc >= 4.7. Since
some supported and quite common operating systems like CentOS 6 still
come with older versions (4.4) and the mapping to the older builtins
is reasonably simple, let's implement it.
This code is only used for gcc < 4.7. It has been quickly tested on a
machine using gcc 4.4.4 and provided expected results.
This patch should be backported to 1.8.
In hpack_dht_make_room(), we try to fulfill this rule form RFC7541#4.4 :
"It is not an error to attempt to add an entry that is larger than the
maximum size; an attempt to add an entry larger than the maximum size
causes the table to be emptied of all existing entries and results in
an empty table."
Unfortunately it is not consistent with the way it's used in
hpack_dht_insert() as this last one will consider a success as a
confirmation it can copy the header into the table, and a failure as
an indexing error. This results in the two following issues :
- if a client sends too large a header into an empty table, this
header may overflow the table. Fortunately, most clients send
small headers like :authority first, and never mark headers that
don't fit into the table as indexable since it is counter-productive ;
- if a client sends too large a header into a populated table, the
operation fails after the table is totally flushed and the request
is not processed.
This patch fixes the two issues at once :
- a header not fitting into an empty table is always a sign that it
will never fit ;
- not fitting into the table is not an error
Thanks to Yves Lafon for reporting detailed traces demonstrating this
issue. This fix must be backported to 1.8.
If the hpack decoder sees an invalid header index, it emits value
"### ERR ###" that was used during debugging instead of rejecting the
block. This is harmless, and was detected by h2spec.
To backport to 1.8.
Released version 1.9-dev0 with the following main changes :
- BUG/MEDIUM: stream: don't automatically forward connect nor close
- BUG/MAJOR: stream: ensure analysers are always called upon close
- BUG/MINOR: stream-int: don't try to read again when CF_READ_DONTWAIT is set
- MEDIUM: mworker: Add systemd `Type=notify` support
- BUG/MEDIUM: cache: free callback to remove from tree
- CLEANUP: cache: remove unused struct
- MEDIUM: cache: enable the HTTP analysers
- CLEANUP: cache: remove wrong comment
- MINOR: threads/atomic: rename local variables in macros to avoid conflicts
- MINOR: threads/plock: rename local variables in macros to avoid conflicts
- MINOR: threads/atomic: implement pl_mb() in asm on x86
- MINOR: threads/atomic: implement pl_bts() on non-x86
- MINOR: threads/build: atomic: replace the few inlines with macros
- BUILD: threads/plock: fix a build issue on Clang without optimization
- BUILD: ebtree: don't redefine types u32/s32 in scope-aware trees
- BUILD: compiler: add a new type modifier __maybe_unused
- BUILD: h2: mark some inlined functions "unused"
- BUILD: server: check->desc always exists
- BUG/MEDIUM: h2: properly report connection errors in headers and data handlers
- MEDIUM: h2: add a function to emit an HTTP/1 request from a headers list
- MEDIUM: h2: change hpack_decode_headers() to only provide a list of headers
- BUG/MEDIUM: h2: always reassemble the Cookie request header field
- BUG/MINOR: systemd: ignore daemon mode
- CONTRIB: spoa_example: allow to compile outside HAProxy.
- CONTRIB: spoa_example: remove bref, wordlist, cond_wordlist
- CONTRIB: spoa_example: remove last dependencies on type "sample"
- CONTRIB: spoa_example: remove SPOE enums that are useless for clients
- CLEANUP: cache: reorder includes
- MEDIUM: shctx: use unsigned int for len and block_count
- MEDIUM: cache: "show cache" on the cli
- BUG/MEDIUM: cache: use key=0 as a condition for freeing
- BUG/MEDIUM: cache: refcount forbids to free the objects
- BUG/MEDIUM: cache fix cli_kws structure
- BUG/MEDIUM: deinit: correctly deinitialize the proxy and global listener tasks
- BUG/MINOR: ssl: Always start the handshake if we can't send early data.
- MINOR: ssl: Don't disable early data handling if we could not write.
- MINOR: pools: prepare functions to override malloc/free in pools
- MINOR: pools: implement DEBUG_UAF to detect use after free
- BUG/MEDIUM: threads/time: fix time drift correction
- BUG/MEDIUM: threads/time: maintain a common time reference between all threads
- MINOR: sample: Add "thread" sample fetch
- BUG/MINOR: Use crt_base instead of ca_base when crt is parsed on a server line
- BUG/MINOR: stream: fix tv_request calculation for applets
- BUG/MAJOR: h2: always remove a stream from the send list before freeing it
- BUG/MAJOR: threads/task: dequeue expired tasks under the WQ lock
- MINOR: ssl: Handle reading early data after writing better.
- MINOR: mux: Make sure every string is woken up after the handshake.
- MEDIUM: cache: store sha1 for hashing the cache key
- MINOR: http: implement the "http-request reject" rule
- MINOR: h2: send RST_STREAM before GOAWAY on reject
- MEDIUM: h2: don't gracefully close the connection anymore on Connection: close
- MINOR: h2: make use of client-fin timeout after GOAWAY
- MEDIUM: config: ensure that tune.bufsize is at least 16384 when using HTTP/2
- MINOR: ssl: Handle early data with BoringSSL
- BUG/MEDIUM: stream: always release the stream-interface on abort
- BUG/MEDIUM: cache: free ressources in chn_end_analyze
- MINOR: cache: move the refcount decrease in the applet release
- BUG/MINOR: listener: Allow multiple "process" options on "bind" lines
- MINOR: config: Support a range to specify processes in "cpu-map" parameter
- MINOR: config: Slightly change how parse_process_number works
- MINOR: config: Export parse_process_number and use it wherever it's applicable
- MINOR: standard: Add my_ffsl function to get the position of the bit set to one
- MINOR: config: Add auto-increment feature for cpu-map
- MINOR: config: Support partial ranges in cpu-map directive
- MINOR:: config: Remove thread-map directive
- MINOR: config: Add the threads support in cpu-map directive
- MINOR: config: Add threads support for "process" option on "bind" lines
- MEDIUM: listener: Bind listeners on a thread subset if specified
- CLEANUP: debug: Use DPRINTF instead of fprintf into #ifdef DEBUG_FULL/#endif
- CLEANUP: log: Rename Alert/Warning in ha_alert/ha_warning
- MINOR/CLEANUP: proxy: rename "proxy" to "proxies_list"
- CLEANUP: pools: rename all pool functions and pointers to remove this "2"
- DOC: update the roadmap file with the latest changes merged in 1.8
- DOC: fix mangled version in peers protocol documentation
- DOC: add initial peers protovol v2.0 documentation.
- DOC: mention William as maintainer of the cache and master-worker
- DOC: add Christopher and Emeric as maintainers of the threads
- MINOR: cache: replace a fprint() by an abort()
- MEDIUM: cache: max-age configuration keyword
- DOC: explain HTTP2 timeout behavior
- DOC: cache: configuration and management
- MAJOR: mworker: exits the master on failure
- BUG/MINOR: threads: don't drop "extern" on the lock in include files
- MINOR: task: keep a pointer to the currently running task
- MINOR: task: align the rq and wq locks
- MINOR: fd: cache-align fdtab and fdcache locks
- MINOR: buffers: cache-align buffer_wq_lock
- CLEANUP: server: reorder some fields in struct server to save 40 bytes
- CLEANUP: proxy: slightly reorder the struct proxy to reduce holes
- CLEANUP: checks: remove 16 bytes of holes in struct check
- CLEANUP: cache: more efficiently pack the struct cache
- CLEANUP: fd: place the lock at the beginning of struct fdtab
- CLEANUP: pools: align pools on a cache line
- DOC: config: add a few bits about how to configure HTTP/2
- BUG/MAJOR: threads/queue: avoid recursive locking in pendconn_get_next_strm()
- BUILD: Makefile: reorder object files by size
There are just a few pools, and they're stressed a lot, so it makes
sense to dedicate them a cache line to avoid contention and to place
the lock at the beginning.
Commit 9dcf9b6 ("MINOR: threads: Use __decl_hathreads to declare locks")
accidently lost a few "extern" in certain lock declarations, possibly
causing certain entries to be declared at multiple places. Apparently
it hasn't caused any harm though.
The offending ones were :
- fdtab_lock
- fdcache_lock
- poll_lock
- buffer_wq_lock
During the migration to the second version of the pools, the new
functions and pool pointers were all called "pool_something2()" and
"pool2_something". Now there's no more pool v1 code and it's a real
pain to still have to deal with this. Let's clean this up now by
removing the "2" everywhere, and by renaming the pool heads
"pool_head_something".
