- 28 Jan, 2021 1 commit
-
-
Nils Goroll authored
In fee476ac VRB_Ignore(), VRB_Cache() and VRB_Free() were rightly out of vmods' sight. But VRB_Iterate() is legitemately required for vmods, and including it in the move seems to be an accident.
-
- 27 Jan, 2021 1 commit
-
-
Nils Goroll authored
Advise from shellcheck taken undogmatically. (I always forget: is $() for `..` posix-sh or not?)
-
- 26 Jan, 2021 2 commits
-
-
Poul-Henning Kamp authored
information to make them easier to clone.
-
Dridi Boukelmoune authored
Salvaged from #3503
-
- 25 Jan, 2021 6 commits
-
-
Dridi Boukelmoune authored
The rationale is that if we add another type of task in the future, for example probe tasks, we won't need to update the default mask for VMOD functions and object methods.
-
Dridi Boukelmoune authored
Refs #3498
-
Dridi Boukelmoune authored
With a default read mask based on the type of symbol. Starting with a default comprehensive mask should then help implement the $Restrict stanza from VIP4, simply by AND-ing the masks.
-
Nils Goroll authored
Simply take an ssize_t and return size_t asserting that the value is greater than or equal to zero. The name might still not be optimimal, as well as choice of *size_t. Addresses Coverity CID 1472401, CID 1472402
-
Dridi Boukelmoune authored
This way every failure can be tested again without selecting a specific subdirectory.
-
Nils Goroll authored
adds a variant of PAN_dump_struct() which does not start another indentation level on a new line.
-
- 23 Jan, 2021 4 commits
-
-
Nils Goroll authored
A copy-pasta in 55dffc35 prevented the top request from being dumped.
-
Nils Goroll authored
55dffc35 introduced a PAN_dump_struct() call, which implies printing '{' and VSB_indent(vsb, 2). Fix missing '}' and wrong indentation. Also remove redundant "priv" output, which now happens in PAN_dump_struct() Diff best viewed ignoring whitespace (-b option).
-
Nils Goroll authored
This test fails constantly now on vtest arm / Freebsd 13.0 for timeout. As vcl compilations are probably the most expensive operations of vtcs, we speed up this test by reducing the number of vcls loaded from 19 to 11. We do this by collapsing all of the client side failure tests into a single vcl. Being at it, we also reduce logexpect synchronization points. The number of vcl loads could be reduced further by also collapsing the backend side, but as this would reduce clarity more than for the client side alone, I first wanted to see if this first step brings down tuntime enough.
-
Nils Goroll authored
Do not overwrite an already latched errno. Note that it does not make any difference at the moment because we only ever use ENOMEM Ref #3502
-
- 22 Jan, 2021 3 commits
-
-
Nils Goroll authored
This is similar to #3502, but the out-of-storage condition happens after the object has been created successfully. Use a logexpect to ensure we test as intended.
-
Nils Goroll authored
We set up fetch filters (VFPs) before initializing a storage object to fetch into. If that fails, we close the filters again. The esi_gzip VFP interacts with the storage object and was missing error handling for the case of a teardown with no storage object. Implementation: We make sure that for the VFP_ERROR case, we do not interact with the storage object (which makes no sense if that is going to be C-4'ed any moment). vfp_vep_callback() keeps its hands off if (struct vef_priv).error is set, so we use that to avoid complications in the code.
-
Nils Goroll authored
-
- 21 Jan, 2021 1 commit
-
-
Nils Goroll authored
... which is not important enough for me to tackle it right now, in particular because I really do not understand enough about how this code works yet, e.g. which of the if() blocks can possibly happen in which order. *cough* fsm? *cough*
-
- 20 Jan, 2021 6 commits
-
-
Dridi Boukelmoune authored
This was already the behavior before #3468, but then the filtering was moved to libvarnishapi to prevent varnishncsa from duplicating the logic behind transaction collection. Unfortunately, despite libvarnishapi's effort to not consider a client or backend transaction without a Begin tag as a candidate, a transaction missing one will get a synthetic Begin prior to dispatch. Regardless, that would have only applied to vxid grouping so even if varnishncsa was presented by default with only relevant transactions, it could still see incomplete transactions in request grouping panic for the same reason. Fixes #3501
-
Nils Goroll authored
do someone a favor and reformat the VPI_vcl_fini() call. Because ifp->fin basically gets output via "\t%s\n", this triggers my OCD in a some other way ;), but I am not going down that rabbit hole at this point...
-
Nils Goroll authored
We do not need to call trunc() to truncate a double into an integert. Mark the intention for flexelint with a cast.
-
Nils Goroll authored
This change is based on the precondition that VRT_fail() should work wherever we have a VRT_CTX and DTRT, see also previous. on vcl_init {} -------------- We already handled a "direct" VRT_fail() via a vmod function correctly. This commit adds a test. Yet we can also VRT_fail() via a PRIV_TASK fini callback and did not handle that case (triggered an assertion in VRT_fail()). Notice that this case has worked in client / backend tasks for long and a test case has been added with 746384b2 . It has now gained relevance with 43d9e5fb on vcl_fini {} -------------- By design, vcl_fini {} must not fail, which is also why the only valid return() value is "ok" (VCL_RET_OK). Consequently, we had if (method && *ctx->handling && *ctx->handling != VCL_RET_OK) { assert(ev == VCL_EVENT_LOAD); in vcl_send_event(). And then came VRT_fail(). If we wanted to sustain an assertion similar to the above, we would need to require vmod authors to never call VRT_fail() from VCL_MET_FINI - which would complicate code, be likely overlooked and receive little to no test coverage. So instead we now ignore errors during vcl_fini {} and emit a VCL_error log line. I considered the alternative to check for VCL_MET_FINI in VRT_fail(), but decided to handle the exception where it applies, which is in vcl_send_event(). I am aware that this part of the change may trigger some controversy and I am open to discussing alternatives. If anything, we now avoid unmotivated assertion failures triggered for the new tests in v00051.vtc for the time being.
-
Nils Goroll authored
Ref 43d9e5fb : PRIV_* fini methods need to leave (struct vrt_ctx).handling alone, except that they might call VRT_fail(), see also 746384b2 Thus we add assertions that handling be either 0 or VCL_RET_FAIL outside the FSM. To be able to do so, we need to change VCL_RET_OK into 0 when vcl_init{} has returned successfully. The vcl_fini{} case is slightly more complicated: By design, only "ok" (VCL_RET_OK) is allowed, but VRT_fail() also added VCL_RET_FAIL, so we de-facto get a "fail" return if any vmod code called VRT_fail(). Because PRIV_* handling happens from VCC generated code via VGC_Discard(), we need to check and change (struct vrt_ctx).handling right after calling vcl_fini{} / VGC_function_vcl_fini() from VGC_Discard(). This is VPI_vcl_fini(). Implementation note: I also considered void VPI_vcl_fini(VRT_CTX, vcl_func_f fini_sub), having VPI_vcl_fini call the fini_sub, but that stirred up includes of VPI where vcl.h is not included.
-
Nils Goroll authored
Though introduced for the purpose of failing upon rollback, they actually just call VRT_fail() from the PRIV_* fini callback and thus should be named accordingly.
-
- 19 Jan, 2021 1 commit
-
-
Poul-Henning Kamp authored
Not sure where things went wrong on this...
-
- 18 Jan, 2021 15 commits
-
-
Dridi Boukelmoune authored
Only if strtoll(3) stumbled upon a VNUM special character. Fixes #3463
-
Nils Goroll authored
forgotten static declaration
-
Dridi Boukelmoune authored
The time_t t variable is not guaranteed to have a specific size, so instead we use a dedicated long l variable. The explicit cast from double is also restored, spotted by flexelint. Refs #3499
-
Martin Blix Grydeland authored
-
Dridi Boukelmoune authored
The option was present in libvarnishapi but never exposed.
-
Dridi Boukelmoune authored
And remove [bcE]_opt handling entirely out of varnishncsa, relying on libvarnishapi to do this filtering. As such we can consolidate the tag management without backend marker fiddling.
-
Dridi Boukelmoune authored
This doesn't change the behavior of varnishncsa that already ignores ESI sub-requests unless the -E option is specified. This is however breaking the default behavior of other VSL processing tools that gained the -E option, aligning with varnishncsa's behavior. Except varnishhist, that gained the E filter in its -P argument instead of the -E option. This allows to also not collect ESI transactions in VXID mode to further reduce churn and reduce the risk for overruns in varnishncsa.
-
Dridi Boukelmoune authored
-
Dridi Boukelmoune authored
-
Dridi Boukelmoune authored
-
Dridi Boukelmoune authored
But paced, because they may repeat in a semi-tight loop.
-
Dridi Boukelmoune authored
-
Dridi Boukelmoune authored
For high-throughput scenarios we might be collecting irrelevant transactions, especially sessions that are never buffered, and drastically increase the risk of overruns with long-running sessions (typically when varnishd is behind a load balancer or a TLS terminator that we can entrust with high client connection reuse). The -b and -c options have an effect on the output, which is way too late for such a setup. These options primarily work with the MSB backend and client markers but that is not enough to rule out sessions since they are marked as client transactions. The same goes for incomplete transactions for which we are lacking a Begin tag, that would still be ruled out at the output stage. We also need to collect irrelevant transactions for the sake of grouping, because even when we do not wish to output them we have to maintain the hierarchy. Therefore, we can identify candidate transactions early and not waste memory footprint and churn keeping track of transactions that would have no effect besides increasing the risk of overruns.
-
Dridi Boukelmoune authored
-
Dridi Boukelmoune authored
Otherwise the combination of -E and -b only outputs backend requests.
-