http: Skip req and beresp trailers
This is the first step of the following plan:
- process but drop incoming trailers to preserve HTTP framing
- forward verbatim trailers for pass transactions
- guarded by an
experimental::pass_trailersflag
- guarded by an
- manipulate trailers in VCL
The first step, this pull request, should allow applications producing trailers to work with Varnish when the trailers are not strictly required.
The second step should allow protocols like gRPC to pass through Varnish, with the caveats that body filters could break the meaning of trailers (for example checksums), hence the experimental flag.
It will require internal changes, and from the look of it we may not be able to directly use the existing data structures and we may need to a dedicated OA_TRAILERS object attribute.
The last step will likely require changes to the VCL state machines in addition to new symbols.
Most of the work on trailers was done by @walid-git under my supervision.
This pull request includes work from other pull requests too:
- 4 commits of mine taken from #3798, turning
BS_CACHEDinto a request flag - 4 commits from @nigoroll taken from #3809 to better delimit HTTP/1 chunks
- I disregarded @mbgrydeland's concern from #3811 that can be implemented later
- 2 commits of mine generalizing pipelining in a workspace
- the rest is trailer support from @walid-git
- new
BS_TRAILERSbody status for coordination - new
gettrls()callback for directors (missing docs update) - process incoming trailers in HTTP/1 code
- move trailers outside of the HTTP/1 VFP code, they are not part of the body (as suggested in https://github.com/varnishcache/varnish-cache/pull/3809#issuecomment-1181577982)
- process
beresptrailers - process
reqtrailers - process incoming trailers in h2 code
- new
This is a lot of commits, but they should all have a reasonable size for reviewers. I already reviewed Walid's work, and already addressed my own review items while Walid is away. I added Walid or myself as a co-author when I made significant changes (at the scale of individual commits).
All failing platforms appear to overflow the workspace used in the h2 test case, except the sanitizers job that fails on something else.
I will have a closer look later.
As it turned out, the failing tests were exposing a race revolving around the request body status. Transitioning to BS_TAKEN would compete against the new transition to BS_TRAILERS (happening in different threads in the h2 case).
I took care of that and added a couple patches next to the ones picked from #3798 at the beginning of this patch series. I'm no longer able to stress test cases and cause failures and I believe the race is gone.
I have not yet studied the remaining CI failure, triggering a panic from the workspace emulator. I suspect either a logic error in the workspace pipelining (expanding from req keep-alive only to req and beresp trailers) or an error in how it translates in the workspace emulator.
I also started an effort to improve feature parity between HTTP/1 and h2 txreq in varnishtest but this is not ready.
As I suspected, the emulated WS_Pipeline() was incorrect. There was a missing workspace release too.
bugwash:
The following commits can be merged: https://github.com/varnishcache/varnish-cache/compare/23ec1fc3fb1198486350e1aedceb5b363d6e429c...98752585d7e086a685816a2934860c72ac0a92c6
The workspace refactoring commits can be submitted independently.
I'm fine with you commiting those first six commits while we wait for @nigoroll
As per bugwash:
- I pushed ce3e446f4e...f0e9df843d to trunk
- I submitted workspace changes independently (#4130)
- they are still present at the beginning of this patch series
- I squashed outstanding commits
Force-pushed to rebase against trunk after merging #4130.
~~I am slightly worried about the concern from https://github.com/varnishcache/varnish-cache/pull/3811#pullrequestreview-1074005690 being unresolved and pulled into VCP with this PR.~~ This was posted on the wrong PR. Please disregard.