Re: Coverity Scan: Analysis completed for varnish

[Dridi Boukelmoune]

On Mon, Nov 20, 2023 at 8:11 AM  wrote:
>
>
> Your request for analysis of varnish has been completed successfully.
> The results are available at 
> https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yrJbcjUxJo9eCHXi2QbgV6m5FSuTtQOxGY1oSL52Ydbrw-3D-3DoMch_WyTzqwss9kUEGhvWd0SG502mTu1yasCtuh9h-2FD3Je4-2B1YrCNgUzvmy9ARK83qQKiZ8s3KpzAY1kug4Y-2B6DtdQ0CUSnTmZa4-2FrTabEi7ESQvN1IAjfusVH6vQzhgxWftrMiC7f-2BVPEqJjIA3g1KVCPV2NrWqo4RKQv8mpaWqHwK7CzBh38ftnSPCGyz6-2FNRit5oaD7HhneOxPbChyQimjpD1kOp-2BLvIu5gRIFlQG02EY-3D
>
> Build ID: 571479
>
> Analysis Summary:
>New defects found: 26
>Defects eliminated: 0

I think Coverity Scan learned new tricks because nothing happened
since the last run:

---8<--8<--8<--8<--8<--8<--8<--8<--8<--8<--8<---
$ git log origin/master --since=2023-11-12 --reverse -p
commit 34c87dd2ce90b7b12e49551834d9c6fa00b4c59a (origin/master, origin/HEAD)
Author: Dag Haavi Finstad 
Date:   Mon Nov 6 14:13:50 2023 +0100

changes.rst: minor language tweak

diff --git a/doc/changes.rst b/doc/changes.rst
index 11737f68ab..786803a7cb 100644
--- a/doc/changes.rst
+++ b/doc/changes.rst
@@ -53,10 +53,10 @@ Varnish Cache NEXT (2024-03-15)
   In particular, this feature is used to reduce resource consumption
   of HTTP/2 "rapid reset" attacks (see below).

-  Note, in particular, that *req_reset* events may lead to client
-  tasks for which no VCL is called ever. Presumably, this is thus the
-  first time that valid `vcl(7)` client transactions may not contain
-  any ``VCL_call`` records.
+  Note that *req_reset* events may lead to client tasks for which no
+  VCL is called ever. Presumably, this is thus the first time that
+  valid `vcl(7)` client transactions may not contain any ``VCL_call``
+  records.

 * Added mitigation options and visibility for HTTP/2 "rapid reset"
   attacks (CVE-2023-44487_, 3996_, 3997_, 3998_, 3999_).
--->8-->8-->8-->8-->8-->8-->8-->8-->8-->8-->8---

Still, worth investigating.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Looking for tips to install varnish-modules

[Dridi Boukelmoune]

On Mon, Aug 21, 2023 at 8:31 PM Kari Cowan  wrote:
>
> Installing varnish-modules
> https://github.com/varnish/varnish-modules
> - has generic install notes
>
> Version for Varnish7.3
> https://github.com/varnish/varnish-modules/releases/tag/0.22.0
> - unloaded and untarred in /home/VarnishAdmin
>
> Trying to follow install notes, but they don't work or my download isn't the 
> right thing to download (?)

Did you install the varnish-devel package?
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: gcc -fanalyze fyi

[Dridi Boukelmoune]

On Fri, Jul 14, 2023 at 4:26 PM Nils Goroll  wrote:
>
> FYI:
>
> Triggered by some promising news *) I built gcc trunk
> (0d2673e995f0dd69f406a34d2e87d2a25cf3c285) and tried -fanalyze.
>
> I looked at the first couple of reports and believe they were all false
> positives. Also it seems gcc does not offer a way to annotate false positives,
> so for now I believe it is still of no use to us.

I tried every major iteration of GCC's -fanalyze with great success on
small mostly-self-contained code bases, but past a certain size I only
get noise.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Varnishtest client user agents

[Dridi Boukelmoune]

On Wed, Jul 12, 2023 at 7:18 AM Poul-Henning Kamp  wrote:
>
> ----
> Dridi Boukelmoune writes:
>
> > > I would prefer you do not overload any Well Known Headers.
> > >
> > > X-VTC: c1
> > >
> > > maybe ?
> >
> > In the spirit of rfc6648 i would rather use a "VTC-" prefix, so
> > something more like
> >
> > VTC-Client: c1
> >
> > But also, since a VTC client mocks a user agent, I would rather use
> > the standard header for this. We can also add a -noua option similar
> > to -nolen, -nohost and -nodate and imply -noua when an explicit `-hdr
> > "User-Agent: other"` is present.
>
> So what about the servers ?

I would be on board with the user agent counterpart:

User-Agent: vtest (c1)
vs
Server: vtest (s1)

I suppose this would be even more useful for the `s0 -dispatch` case,
where the Via header generated by varnishd would not carry this
nuance.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Varnishtest client user agents

[Dridi Boukelmoune]

On Wed, Jul 12, 2023 at 6:59 AM Poul-Henning Kamp  wrote:
>
> ----
> Dridi Boukelmoune writes:
> > On Wed, Jul 12, 2023 at 5:21=E2=80=AFAM Poul-Henning Kamp  > dk> wrote:
> > >
> > > 
> > > Dridi Boukelmoune writes:
> > >
> > > > Would it be OK to have `client cNAME` send a `User-Agent: vtest
> > > > (cNAME)` or similar to reduce some of it?
> > >
> > > I generally just use /c1 /c2 etc in the URL ?
> >
> > That's part of the frustration, whenever I'm troubleshooting a test
> > case I have to jump through more hoops to get something useful. What I
> > would like is something out of the box, such as clients advertising
> > themselves in a User-Agent header.
>
> I would prefer you do not overload any Well Known Headers.
>
> X-VTC: c1
>
> maybe ?

In the spirit of rfc6648 i would rather use a "VTC-" prefix, so
something more like

VTC-Client: c1

But also, since a VTC client mocks a user agent, I would rather use
the standard header for this. We can also add a -noua option similar
to -nolen, -nohost and -nodate and imply -noua when an explicit `-hdr
"User-Agent: other"` is present.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Varnishtest client user agents

[Dridi Boukelmoune]

On Wed, Jul 12, 2023 at 5:21 AM Poul-Henning Kamp  wrote:
>
> ----
> Dridi Boukelmoune writes:
>
> > Would it be OK to have `client cNAME` send a `User-Agent: vtest
> > (cNAME)` or similar to reduce some of it?
>
> I generally just use /c1 /c2 etc in the URL ?

That's part of the frustration, whenever I'm troubleshooting a test
case I have to jump through more hoops to get something useful. What I
would like is something out of the box, such as clients advertising
themselves in a User-Agent header.

I would have considered `Server: vtest (sNAME)` if it weren't for the
Via headers added by Varnish already. In that regard I no longer need
an indirection to find out where the bereq went to when multiple
servers are involved.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Varnishtest client user agents

[Dridi Boukelmoune]

Greetings,

When I write test cases involving multiple requests that don't need to
come from the same client session I try to make one client per
request. When a client reports something wrong, it's usual very
convenient because I don't have to track down too many events.

When I need to jump from a client error to Varnish logs, I find it
increasingly frustrating to actually need to retrace events in the
absence of an X-Varnish header.

Would it be OK to have `client cNAME` send a `User-Agent: vtest
(cNAME)` or similar to reduce some of it?

This should be for the most part transparent, except for test cases
doing request accounting or other header shenanigans (HPACK maybe?)
that should be trivial to adjust.

Thanks,
Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Idea for multi-level CLI access control

[Dridi Boukelmoune]

On Tue, Jun 27, 2023 at 9:24 AM Poul-Henning Kamp  wrote:
>
> ----
> Dridi Boukelmoune writes:
> > On Mon, Jun 26, 2023 at 6:39=E2=80=AFPM Poul-Henning Kamp  > dk> wrote:
> > >
>
> > Regarding the specific suggestion above, I don't think we would be
> > satisfied with this model. In the security barriers diagram [1] we
> > identified the following roles:
> >
> > - ADMIN
> > - OPER
> > - BACKEND
> > - ANON
>
> My idea was not meant as a replacement for any of those roles,
> it just an idea for how to implement CLI connections with
> different access levels - if we want to have that.

Same, my overview keeps the current roles and adds TENANT below ADMIN.

> > For CLI access, we would probably want a new role TENANT, managed by
> > ADMIN. Ideally, everything in the cache (VCL, contents, operations
> > like ban) would be virtually partitioned such that a tenant could not
> > directly affect the resources of other tenants.
>
> I think that is a bit beyond the scope of the current discussion, but
> it is certainly relevant to keep it in mind.
>
> > > * Varnishd should identify itself (-i/-n) in the 107 message so that the
> > >   client can pick which secret file to use if it has access to multiple.
> >
> > If each "account" (admin or tenant) has one dedicated secret,
> > this is probably not needed.
>
> I dont see the admin/tenant split eliminating the potential benefit
> of being able to hand out restricted CLI access secrets.

The whole idea is to partition the cache logically, so each tenant
sees a "restricted" set of resources. Commands like start and stop
would require admin privileges, so a "mere" tenant would not be able
use them. They would however be able to vcl.load and vcl.list, but
only see their VCLs, not other tenants'.

Instead of a tenant or a partition, let's call it a context and use a
familiar -Z option:

# operate as ADMIN, regular CLI `auth ...` under the hood
# secret found from VSM with varnish credentials
# /etc/varnish/secret read with MGT credentials (same as today)
$ varnishadm context.create -S /etc/varnish/example.com/secret example.com

# operate as TENANT, performs `auth -Z example.com ...` under the hood
# secret found from VSM with varnish credentials
# /etc/varnish/example.com/secret read with TENANT credentials
$ varnishadm -Z example.com vcl.load www /etc/varnish/example.com/main.vcl

That's one way you could interpret my previous email.

> As for CLI plain-text:  I would really love to find a good and mostly
> seamless way to use SSH for CLI access.

Another option could be mutual TLS once we bring HTTPS support to the cache.

We will discuss this internally and come back with a proposal.

Best,
Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Idea for multi-level CLI access control

[Dridi Boukelmoune]

On Mon, Jun 26, 2023 at 6:39 PM Poul-Henning Kamp  wrote:
>
> We talked about the overall security model during bugwash today and
> while trimming the hedges I had the following idea:
>
> Today the fundamental authentication to open a CLI port is that
> that you have access to the exact and entire contents of the "secret"
> file and can generate a proof of this.
>
> We keep that, but...
>
> 1.  We allow varnishd to have multiple secret files.
> When a CLI connection attempts to authenticate, varnishd tries
> them all.
>
> 2.  Secret files can be "old style" or "new style", in both
> cases the "proof" uses the entire content of the secret file,
> byte for byte.
>
> 3.  "New style" secret files have the following syntax:
>
> Lines which start with '#' are comments and are ignored.
>
> First line:
>
> "secret: "  NL
>
> Then any number of rules:
>
> ("permit: " | "deny: ")  NL
>
> varnishd always appends a "deny: ." rule at the end of the
> list of rules.
>
> All submitted CLI commands are tested against these rules in
> the order they appear in the secret file, and the search
> terminates when one of them matches.
>
> A trivial example of a secret file could be:
>
> secret: swordfish
> deny: vcl
> deny: stop
> # Note: Do not name a backend "kanban"
> deny: ban

As agreed during bugwash, Varnish Software should come back with a
proposal after our review of the current security model and what we
think we could or should change. My plan is to first summarize the
current model to have a frame of reference for a future model.

Regarding the specific suggestion above, I don't think we would be
satisfied with this model. In the security barriers diagram [1] we
identified the following roles:

- ADMIN
- OPER
- BACKEND
- ANON

For CLI access, we would probably want a new role TENANT, managed by
ADMIN. Ideally, everything in the cache (VCL, contents, operations
like ban) would be virtually partitioned such that a tenant could not
directly affect the resources of other tenants. We could effectively
have two levels of authentication, and add a command line option to
varnishadm that would translate to a varnish-cli auth option.

Here is a straw man:

sub vcl_recv {
ban("obj.status != 0");
}

In a multi-tenant system, we should be confident that it will not
affect the cache of other tenants, even if they share storage.

> Random notes:
>
> * Ideally the help command output is also filtered through the rules.

The output is already filtered based on prior auth success, we can
make that work with auth levels. If a VCL is bound to a tenant, and
its VMOD wants to add commands to the CLI, they should also be visible
only to that tenant.

> * Varnishd should identify itself (-i/-n) in the 107 message so that the
>   client can pick which secret file to use if it has access to multiple.

If each "account" (admin or tenant) has one dedicated secret,
this is probably not needed.

> * Varnishadm could look for secret files in ~/.varnish/${-i/-n arg}

I'm not against the idea, but I would prefer to leave this as an
exercise to the user. Chances are that if they expose CLI directly to
their tenants, they have other problems to solve like not doing CLI in
clear text. Please note also that the multi-tenant scenario described
above can also be used entirely by the admin persona, similarly to how
MGT performs privilege (des)escalation on demand to ensure that
operations made on behalf of a tenant have a blast radius limited to
that tenant (modulus interesting details like child CLI timeout).

> Comments ?

All of the above ;-)

Best,
Dridi

[1] https://varnish-cache.org/docs/trunk/phk/barriers.html
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

VDD23Q1 Oslo

[Dridi Boukelmoune]

Greetings,

We are hosting our next Varnish Developer Days (VDD) in Varnish
Software's office in Oslo on February 6 and 7.

During these two days, Varnish core developers and VMOD authors can
meet in person to discuss current challenges and future changes in the
Varnish design. We have a limited number of seats, so please reach out
to me if you would like to join. Attendees must have some degree of
familiarity with Varnish internals, and some HTTP knowledge.

dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Bugwash today at 15:00 EU time...

[Dridi Boukelmoune]

On Mon, May 23, 2022 at 9:16 AM Poul-Henning Kamp  wrote:
>
> Can we please try to get the bugwash back on track ?
>
> If the monday 15:00 has become inconvenient for people we should try
> to find another and better timeslot.

Not sure about everyone else, but Monday is still the best option for
me and the current time slot is firmly locked. My main bugwash
impediment these days is a lack of availability to follow up on
tickets updates.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: bo access from VFP

[Dridi Boukelmoune]

On Fri, Jan 7, 2022 at 2:40 AM Guillaume Quintard
 wrote:
>
> As usual, thanks a bunch for all your help!
>
> Using the THR_* functions work: 
> https://github.com/gquintard/vmod_rers/blob/270a2f4272be49f8c1c904b899ec91dafcf2f873/src/lib.rs#L227
>  and they allow me to merge most of the vfp/vdp init code together (helped by 
> the fact the same rust structure is both a VDP and a VFP)
>
> It would be even more legible if we had access to the vrt_ctx directly (for 
> both vfp AND vdp) so I wouldn't have to build my own, OR if we hade 
> VRT_priv_task_raw_parts(const req *, const busyobj *, const void *) so I 
> didn't have to build my own context, but that is already pretty sweet

https://github.com/varnishcache/varnish-cache/pull/3772
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: bo access from VFP

[Dridi Boukelmoune]

On Fri, Dec 31, 2021 at 11:48 PM Guillaume Quintard
 wrote:
>
> Small follow-up on that one: it would be nice for VFP to be able to disable 
> streaming (bo->do_stream), so that's an extra argument to be able to access 
> the busyobj
>
> --
> Guillaume Quintard
>
>
> On Sun, Dec 26, 2021 at 6:11 PM Guillaume Quintard 
>  wrote:
>>
>> Happy holidays to everyone,

Happy new year !

>> Unless I'm mistaken, there's no way for a VFP to access the busyobj, and was 
>> wondering if that could change as I need it to retrieve a vmod_priv.

I think your statement is correct.

>> For some context, I stumbled on this while working on this vmod: 
>> https://github.com/gquintard/vmod_rers/tree/v0.0.2
>> (It's in rust, and I'm using it to explore what kind of bindings we need. I 
>> already have a couple of examples for VDP[1] and VFP[2] in the crate[3] 
>> repo, but of course they don't need as much as a real vmod.)
>>
>> With vmod_rers, the user is able to add regex/sub pairs on a per-request 
>> basis [4], and they will be acted upon by the VDP. To store and retrieve the 
>> pairs I use the regular "fake vrt_ctx trick"[4], since the VDP has access to 
>> the req struct, it's very easy to build.
>> However, I'm unable to do the same thing on the VFP side since I have 
>> neither a req (fair) nor a bo.

The big difference between both is that we only use VDPs to deliver
client responses, but we use VFPs to fetch both beresp and req bodies,
so the VFP operates at the objcore level.

>> Is there a big reason for not having access to it, or is it just that nobody 
>> asked for it until now?

Probably nobody never asked for it?

Passing a VRT_CTX to the VFP init function could help grab a req or bo
and stash it in vfp_entry::priv1, but I don't see a way to do that
today without cheating.

*cough* THR_GetBusyobj() *cough*

Cheers,
Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Making libunwind the default

[Dridi Boukelmoune]

On Mon, Sep 7, 2020 at 10:28 AM Poul-Henning Kamp  wrote:
>
> ----
> Dridi Boukelmoune writes:
>
> > After a brief discussion on IRC I agreed to provide comparative stack
> > traces, I'll find some time to make a couple up.
>
> Status in FreeBSD-land:
>
> The libunwind backtrace does not look obviously superior, or for
> that matter significantly different on FreeBSD.
>
> Libunwind does not seem to support 32-bit Arm well, at least on FreeBSD.
>
> Would it make sense to make this a packaging decision ?

Quick followup to close this thread for now.

It was already a packaging decision, but we changed the default from
"no" to "auto". Having libunwind present at build time is enough to
select it.

https://github.com/varnishcache/varnish-cache/pull/3739

Cheers,
Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Packaging: why does the RPM spec have both Provides & Obsoletes for the same packages?

[Dridi Boukelmoune]

Hey Geoff,

On Tue, Aug 24, 2021 at 2:40 PM Geoff Simmons  wrote:
>
> Hello,
>
> The spec file for RPM packages:
>
> https://raw.githubusercontent.com/varnishcache/pkg-varnish-cache/master/redhat/varnish.spec
>
> ... has this passage:
>
> Provides:  varnish-libs%{?_isa} = %{version}-%{release}
> Provides:  varnish-libs = %{version}-%{release}
> Obsoletes: varnish-libs
>
> Provides:  varnish-docs = %{version}-%{release}
> Obsoletes: varnish-docs
>
> Provides:  varnish-debuginfo%{?_isa} = %{version}-%{release}
> Provides:  varnish-debuginfo = %{version}-%{release}
> Obsoletes: varnish-debuginfo
>
> This may be an RPM technique that I'm not familiar with, but why does it
> have both Provides and Obsoletes for -libs, -docs and -debuginfo?
>
> Since the Obsoletes don't specify a version, my best guess is that it's
> something about updating to newer versions. But wouldn't newer versions
> (newer version and RPM release number) be enough for that?

They should ideally provide a version, but since we have packaging
that could compete with first party packages we can't really predict
what version we would go against.

Keeping Provides doesn't break an installation of -docs for example,
because ultimately the package containing the docs is going to be
installed thanks to the Provides.

> Asking because I'm building a custom RPM package for which I'd also like
> to have the debuginfo. Creating the debuginfo RPM is easy enough, by
> removing this line from the spec:
>
> %global debug_package %{nil}
>
> But then when you try to run debuginfo-install, it says that the
> debuginfo package is obsoleted by the Varnish package.
>
> I can get the debuginfo to install by removing the Obsoletes line for
> varnish-debuginfo, and rebuilding the RPMs. But I'm confused about why
> it was there in the first place, and concerned that I may have broken
> something else by doing so.

I think the reason why we didn't want the traditional separation
debuginfo package was the poor quality of panic backtraces. Once
split, installing the separate debuginfo wouldn't improve the
backtraces.

I think this specific point was a result of:

https://github.com/varnishcache/varnish-cache/commit/95437e6c882f2c2b332be94060a7ac96907db322

It's been a while, I don't remember the details.

Cheers,
Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Reintroduce a req.uncacheable flag

[Dridi Boukelmoune]

Hi Geoff,

It's been a while, I hope you are doing well :)

On Fri, May 28, 2021 at 5:22 PM Geoff Simmons  wrote:
>
> On 5/28/21 17:50, Dridi Boukelmoune wrote:
> >
> > First, we already have a relationship between bereq.uncacheable and
> > bereq.uncacheable: the former implies the latter.
>
> Presumably you meant to write that bereq.uncacheable implies
> beresp.uncacheable.

Correct, a tragic copy-pasta accident and yet I managed to make a
silly but valid statement anyway \o/

> My quick and superficial first reaction to the proposal -- I don't quite
> have my head around what you would write in VCL if you do in fact want
> to return early out of vcl_recv, and you want to say that the response
> will be uncacheable.
>
> You could set req.uncacheable to true, and also say return(pass), but it
> feels like saying the same thing twice.
>
> VCL could also have one but not the other: req.uncacheable=false and
> return(pass), or req.uncacheable=true and return(lookup). Which one
> determines whether a cache lookup is attempted or bypassed?

I didn't want to jump too soon into implementation details but I have
to dip a toe now :)

Let's ignore bereq.uncacheable since it's read-only and determined
strictly by what happened on the client side. You can decide to make a
beresp uncacheable but you can't undo it. My assumption is that we
would want the same for req.uncacheable where assigning true takes
effect and false does nothing.

> The combination req.uncacheable=true and return(lookup) in particular
> feels like a contradiction. But if the point-oh release eliminates
> return(pass) as you considered, isn't return(lookup) the only way to
> return early out of vcl_recv, unless you're going to something like pipe
> or synth or fail? If so, then is req.uncacheable=true, suggesting no
> lookup, and then return(lookup) the only way to accomplish what
> return(pass) does now?

Consider the backend transitions today:

- return(deliver)
- return(pass[...])

You have an explicit transition and a general one that will be
influenced by beresp.uncacheable.

On the client side that would be the same:

- return(hash) => influenced by req.uncacheable
- return(pass)

> I guess this is starting to sound like I'm critical of the idea, which I
> didn't intend, because there may be good answers to all of these
> questions. Just trying to get it sorted in my head.

Well, critical doesn't imply against, and your reply shows that my
first message was lacking a bit of context.

I'm not suggesting to remove the pass transition on the client side,
but to add a flag to signal that a lookup attempt should be neutered.
I am however suggesting that the built-in VCL replaces return(pass)
transitions with raising the req.uncacheable flag. This would for
example allow you to keep your cookie handling in vcl_req_cookie even
if your POST request was made uncacheable earlier.

Next implementation details, req.uncacheable would be read-write in
vcl_recv and vcl_hash, and read-only everywhere else.

If you return(pass) from vcl_recv, you can expect req.uncaheable to be
true in vcl_pass, vcl_deliver and vcl_synth.

The req.uncacheable flag would be reset to false after a return(restart).

I hope this clarifies things a bit.

Cheers,
Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Reintroduce a req.uncacheable flag

[Dridi Boukelmoune]

Greetings,

I would like to reopen a discussion started by Guillaume via a pull
request:

https://github.com/varnishcache/varnish-cache/pull/3457

For reasons other than the ones he exposed I have reached the
conclusion that such a flag would be both relevant and useful. Not
req.hash_always_pass as originally suggested but req.uncacheable
(and to Guillaume's credit he did make the suggestion first in the pull
request description).

I didn't take part in the pull request discussion and don't remember
if I took a position but chances are I would have opposed a flag
called hash_always_pass. On the other hand, here are the reasons why I
think req.uncacheable would make sense.

First, we already have a relationship between bereq.uncacheable and
bereq.uncacheable: the former implies the latter. I believe the same
relationship could exist between req and bereq.

Second, since the VCL built-in split it makes even more sense to get
the ability to mark a request as uncacheable. If you wish to keep your
code organized as per the split, but don't want an early return to
prevent a later step to be taken, then a flag instead of a
return(pass) lets the flow of execution continue and guarantees that a
lookup attempt would result in a pass transition.

As we are heading towards a dot-zero release, we could even break the
built-in VCL to replace pass transitions with req flagging. That would
be a breaking change in the sense that we would no longer break the
flow of execution, but in practice that does not change the outcome of
existing VCL not taking advantage of the split (for example upgrading
from 6.0 LTS) and for VCL authors who already adopted the split, I
assume they'd be savvy enough to check their code.

Third, one more reason to bring it back as req.uncacheable instead of
hash_always_pass is consistency across the board.

I'm not diving into implementation details on purpose, my goal with
this thread is to discuss the possibility to reintroduce a new
iteration on #3457 and shine a new light on it from the perspective
of the recent split and the additional opportunity it grants us.

Cheers,
Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Making libunwind the default

[Dridi Boukelmoune]

> Adding a new dependency by default might be worth it all things considered.

After a brief discussion on IRC I agreed to provide comparative stack
traces, I'll find some time to make a couple up.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Decide Name/number of 2020-09-15 release

[Dridi Boukelmoune]

On Mon, Aug 31, 2020 at 8:00 AM Poul-Henning Kamp  wrote:
>
> We're going to nail the name/number of the 2020-09-15 release at the
> bugwash today.  Please come prepared.

The consensus is 6.5 unless we made a change that warrants a 7.0 bump,
Nils will try to determine that tomorrow.

Coincidentally, tomorrow we start the freeze period and stop pushing
anything that is not documentation or related to release notes. The
freeze period lasts for the whole month of September.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Support for AARCH64

[Dridi Boukelmoune]

> No, I wasn't aware of this discussion.
> The weekly package installed successfully now!
> Thank you!

And I lost track of this thread, but all's well that ends well ;-)

We are looking forward to feedback regarding the weeklies, please make
sure to upgrade frequently and let us know as soon as something goes
wrong.

Cheers,
Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Making libunwind the default

[Dridi Boukelmoune]

On Mon, Jul 6, 2020 at 10:36 PM Guillaume Quintard
 wrote:
>
> Hi all,
>
> https://github.com/varnishcache/varnish-cache/pull/3052 was merged in 
> September and I was wondering if we had enough feedback (or lack thereof) to 
> make a decision and on making the libunwind dependency official.
>
> All the weeklies are being built with libunwind since the end of October, and 
> the 6.4 packages on packagecloud. And the official alpine packages use it 
> too, which isn't surprising as the change was made to accommodate the lack of 
> proper libexecinfo for that platform.
>
> Notably, if Nils has some feedback about the more "exotic" platforms he uses, 
> I'm interested!

Also some backtrace implementations like libexecinfo use GCC builtins [1,2]
that are documented [3] as unsafe:

> Calling this function with a nonzero argument can have unpredictable effects, 
> including crashing the calling program.

Adding a new dependency by default might be worth it all things considered.

Dridi

[1] __builtin_frame_address
[2] __builtin_return_address
[3] https://gcc.gnu.org/onlinedocs/gcc/Return-Address.html
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Support for AARCH64

[Dridi Boukelmoune]

On Wed, Jun 17, 2020 at 3:05 PM Geoff Simmons  wrote:
>
> On 6/17/20 16:56, Nils Goroll wrote:
> > On 17/06/2020 10:00, Emilio Fernandes wrote:
> >> 1.1) curl -s
> >> https://packagecloud.io/install/repositories/varnishcache/varnish-weekly/script.deb.sh
> >> | sudo bash
> >
> > The fact that, with my listmaster head on, I have not censored this posting,
> > does not, *by any stretch*, imply any form of endorsement of this practice.
> >
> > My personal 2 cents: DO NOT DO THIS. EVER. AND DO NOT POST THIS AS ADVISE 
> > TO OTHERS.
> >
> > Thank you
>
> +1
> To point fingers at the right people, this is what the packagecloud docs
> tell you do.
>
> But ... the *packagecloud docs* tell you to do that!
>
> If I could have them arrested for it, I'd think about it.
>
> Piping the response from a web site into a root shell is stark, raving
> madness.

Dudes, chill out and live with your time.

It's not like attackers taking control of packagecloud could send a
different payload depending on whether you curl to disk to audit the
script or yolo curl to pipe.

https://www.idontplaydarts.com/2016/04/detecting-curl-pipe-bash-server-side/

We've known for years that it isn't possible.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VSB_quote has gotten messy

[Dridi Boukelmoune]

On Mon, Apr 20, 2020 at 9:45 AM Poul-Henning Kamp  wrote:
>
> I finally got around to look at VSB_QUOTE_GLOB feature Guillaume committed
> by accident some time ago, and it doesn't work correctly as far as I
> can tell, for instance, the difference between inputs:
> []
> and
> ["]
> makes no sense to me.
>
> However, I can hardly blame Guillaume, because it is not very
> consistent or clear how VSB_QUOTE is supposed to work in the first
> place, I just spent 4 hours trying to find out, because we sort of
> made it up as we went.

As discussed previously, it was also on my review queue, and still is.

> I propose that b0d1a40f326f... gets backed out before it has any
> use in the tree, and put an errata on the 6.4 release page to the
> effect of "do not use VSB_QUOTE_GLOB".

Agreed.

> I also propose that we should deprecate VSB_quote*() in its current
> form, ie: leave around for the benefit of VMODers for 7.x, remove
> in 8.x.

No opinion, but if we replace the old API with a new one, maybe we can
manage to translate old API calls to new ones?

> Finally, I propose a new and more well thought, and better documented
> replacement, VSB_encode(), to be added shortly.
>
> Comments ?

One annoying limitation with the current quote logic is that calls
must be self-contained. If I want to quote a string that will come in
multiple parts, I have to assemble it (eg. with a VSB!) prior to
feeding it to VSB_quote().

If we land a new API, it would be nice to be able to delimit begin and
end steps, either with flags or more functions:

AZ(VSB_encode_begin(vsb, VSB_ENCODE_JSON)); /* adds the leading quote */
/* encode a VCL_STRANDS in a loop */
AZ(VSB_encode_end(vsb); /* adds the trailing quote */

If we move struct strands to libvarnish, we can also have
VSB_encode_strands(), but being able to encode multiple string
components independently of struct strands would be appreciated.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Support for AARCH64

[Dridi Boukelmoune]

> We do, in the sense that distributions do the work, but I believe the 
> question was about the packagecloud repos.
>
> Fedora is possibly a good student he, providing timely packages (Dridi 
> appears in 3..2..1..) but we definitely cannot expect the same thing from the 
> debians.

*appears*

Fedora builds packages for a bunch of architectures, and builds
packages for Red Hat Enterprise Linux and derivatives via its EPEL
project. So Fedora has "official" Varnish packages outside the x86_64
realm (power pc, system/390 mainframes, arm boards) but we don't.

*disappears*
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Travis macos job

[Dridi Boukelmoune]

On Sat, Mar 7, 2020 at 8:02 PM Federico Schwindt  wrote:
>
> Should be fixed now.

Thanks!
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Travis macos job

[Dridi Boukelmoune]

Hi,

Can someone with knowledge of both travis and macos have a look?

Since this build it fails to find rst2man:

https://travis-ci.org/varnishcache/varnish-cache/builds/653055383

Thanks,
Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Time for release notes

[Dridi Boukelmoune]

On Wed, Feb 19, 2020 at 11:12 AM Dridi Boukelmoune  wrote:
>
> Dear Varnish developers,
>
> It's that time of the semester already, the time where we all enjoy
> revisiting what happened in the last 6 months to produce release and
> upgrade notes.
>
> As such I'll spend the rest of today and tomorrow trying to make
> progress on 6.3 documentation.

I did, and finished a sweep of all the commits between 6.2.0 and
6.3.0, after what Nils kindly wrote upgrade notes for the VCL
temperature change.

The upgrade notes are mostly C developer-centric. It is either my
biais, or a reflection of how little 6.3 breaks anything compared to
6.2 besides the usual VRT suspects.

This is the last call for double checking, I will otherwise time this
operation out at some undefined time later this week, remove the
"incomplete" release notes markers and back-port everything in the
6.3 branch.


Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Time for release notes

[Dridi Boukelmoune]

Dear Varnish developers,

It's that time of the semester already, the time where we all enjoy
revisiting what happened in the last 6 months to produce release and
upgrade notes.

As such I'll spend the rest of today and tomorrow trying to make
progress on 6.3 documentation.

At this point you might think "wait a minute the next release is 6.4"
and you would be right but it turns out we didn't wrap up that
documentation before last September release.

As I'm going to revisit the 6.3 release, motivated contributors may
start filling up the skeleton for the upcoming 6.4 release so that I
don't run into conflicts and can be done faster and eventually
back-port the missing bits to the 6.3 branch.

Thanks,
Dridi

PS. I'm not pointing fingers, I also let it slip
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: changelog automation?

[Dridi Boukelmoune]

On Thu, Jan 16, 2020 at 4:06 PM Nils Goroll  wrote:
>
> does anyone have experience with something like
>
> https://github.com/hawkowl/towncrier
>
> at first sight, would this help with our changelog maintenance?

It came to my attention recently because someone suggested that for
Fedora's RPM changelogs, I gave up after the "philosophy" paragraph in
the readme.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: centos_8 branch

[Dridi Boukelmoune]

On Mon, Dec 23, 2019 at 8:29 AM Guillaume Quintard
 wrote:
>
> Hi,
>
> I open that one just to work on it with Dridi being able to look on it as I 
> progress. but I can move it back to my repo before opening the PR if you 
> prefer.

I thought you opened a branch to do CircleCI trial commits
without polluting the master branch's history.

If that doesn't work in your private fork, I don't mind a temporary
branch in the canonical repository.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

2019Q4 packaging update

[Dridi Boukelmoune]

Greetings,

Today I back-ported patches that have been living happily in the
weekly branch and now we only rely on Python3 dependencies also
for 6.x releases.

In the process I tried Debian 10 and RHEL 8 and we can now build
Varnish 6.3 on those systems without any obvious problems. The
devel might be in the details, like for example difference in memory
footprint with an updated jemalloc, or for that matter any other
dependency update on those platforms.

Currently we can't build 6.0 LTS releases on RHEL 8 because
/usr/bin/python is no longer provided by the distribution, on purpose,
so any script using plain unversioned python in its shebang will fail
to run at either build or run time.

This work was done for the 6.3 release so it's only a matter of
back-porting existing patches to 6.0 and while we could get away with
appending a '3' to our shebangs we would miss some of the Python2
cleanup that was done in the process.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Not attending the 2019-11-25 bugwash

[Dridi Boukelmoune]

Greetings,

I will probably be in transit at the time of the bugwash so I will
work with Nils to get the requested draft for #3134.

If we manage to finish the proposal in time, he will speak on our
behalf during the bugwash.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: [VDD] VSL query extensions

[Dridi Boukelmoune]

Greetings,

Just a quick word to say that with the release of 6.3 the following
extensions are now part of the VSL query language:

On Mon, May 7, 2018 at 8:08 AM Dridi Boukelmoune  wrote:
>

> 1) Multi-line queries
>
> When someone wishes to capture logs for multiple reasons, they need
> to OR them, possibly with brackets. Instead the proposal is to support
> one query per line, ignore empty lines, and treat tokens starting with
> '#' as comments.
>
> Example:
>
> varnishlog -q '(my-first-query) or (my-second-query)'
> varnishlog -q '
> my-first-query
> my-second-query
> ...
> '
>
> 2) Read queries from a file
>
> Similar to reading varnishncsa's format from a file:
>
> cat >bad-things.txt < # internal errors
> RespStatus >= 500
>
> # backend problems
> ...
>
> # internal incident 4253
> ...
>
> # varnish issue 1799
> ...
> EOF
> varnishlog -Q bad-things.txt


Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: TLS sandboxing

[Dridi Boukelmoune]

On Wed, Sep 4, 2019 at 8:02 AM Poul-Henning Kamp  wrote:
>
> 
> In message , Nils Goroll 
> writ
> es:
>
> >Yet with the H3/QUIC madness on the horizon,
>
> No, they actually dealt with this in the design, so that "keyless"
> operation is more or less the natural architecture for QUIC.
>
> If we want to do TCP/TLS, we should also aim firmly for the "keyless" model.
>
> I'm hoping we can to raid the hitch source code to build the "keymaster" 
> process.

I have given more thought to this and I'm torn between "this is 2019
and we still don't support HTTPS" and the very good reasons why we
don't. However I still think we should support it and I gave a tiny
bit more thoughts to this.

First, in-kernel TLS is coming, and while it will likely take forever
to become ubiquitous *cough* enterprise linux *cough* in the long run
it will probably be our best option: we keep our usual read and
write[v] calls and nothing changes for us.

Until then though, we might want to add support for "session
operations" to replace the read/write[v] calls with
SSL_read/SSL_write[v] for example. Ideally that would be with a
vmod-openssl that later could compete with a vmod-ktls or
vmod-other_tls_library, possibly written by third parties *cough*
uplex? *cough* since we can't reasonably support them all ;-)

With customizable session operations, we can now also replace other
calls like accept or connect. That might be our cue to add session
subroutines in VCL.

But once we have that we circle back to handshakes. And I very much
agree that a keyless approach would be best. However this is still
2019 so I think we should own it, and not rely on a third party. We
should provide a comprehensive solution, for either HTTPS or http/3,
and therefore we should provide a minimalist keyserver implementation.
Whether it is a new varnishkeyserver program, or a new varnishd -k
flag with a 3rd long-lived varnishd process, possibly working with a
socketpair like my MXC thought experiment, I think we should provide
that.

Have a nice VDD!

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: TLS sandboxing

[Dridi Boukelmoune]

Hi Nils,

On Wed, Sep 4, 2019 at 7:57 AM Nils Goroll  wrote:
>
> That is an interesting exercise, thank you, Dridi.

Thanks for reading.

> For TLS on TCP, I would hope that passing the session key and file descriptor
> would work. Have you checked already to which extend this is supported by
> existing library code?

I haven't, I really only had a short window to think about this and
give it a try, and I burned a large amount of that spare time to play
with asciinema...

> Yet with the H3/QUIC madness on the horizon, I am not sure if connect()ing the
> SOCK_DGRAM and passing the fd would work. The way I read the QUIC draft,
> connections are primarily identified by their ID and migrations need to be
> supported. I have made no coding attempt on my own, but my impression was that
> the natural implementation the authors had in mind was a recvfrom(2) loop
> matching packets based on their connection ID with spoof detection.
>
> So, Dridi, have you had a closer look yet if/how your idea could work with 
> QUIC?

Not at all. The proposed model falls short as soon as you have any
kind of key renegotiation, except that being a UDS you could in theory
pass the fd back and forth whenever you need private key privileges. I
really really don't like the idea of a two-way channel though.

> Somehow related: How about having the process owning the private keys also
> handle all receives into multiple ringbuffers, somehow similar to vsm, but 
> with
> overrun protection?

No opinion, I haven't thought about this. I'm literally back today and
will be away again for the rest of the week.

I will reply in greater details to Poul-Henning's response.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

TLS sandboxing

[Dridi Boukelmoune]

Greetings,

I have been kept forcefully awake for a few nights in a row and I let
my brain fart a bunch, ran a quick experiment, and shared my thoughts
on TLS support directly in varnishd.

TL;DR: I think we realisticly could, if so we should.

https://dridi.fedorapeople.org/post/tls-sandboxing/

I will be back for fakes on Sep 23th (because conference) and for
realsies on Sep 30th, possibly with travel privileges.

Cheers,
Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: STRING_LIST deorbit burn completed

[Dridi Boukelmoune]

> Hmm, I guess I should actually grep for STRING_LIST rather than
> the functions operating on it :-)

I found an old ASAT in my garage and STRING_LIST usage is now
gone from trunk.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: STRING_LIST deorbit burn completed

[Dridi Boukelmoune]

> Hmm, I guess I should actually grep for STRING_LIST rather than
> the functions operating on it :-)

I'm spoiled by git grep, it has so many killer features that I don't
see myself using anything other than git for source code :)

You can also grep in other branches without checking them out locally \o/

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: STRING_LIST deorbit burn completed

[Dridi Boukelmoune]

On Fri, Jun 21, 2019 at 1:01 PM Poul-Henning Kamp  wrote:
>
> 
> In message 
> 
> , Dridi Boukelmoune writes:
> >On Thu, Jun 20, 2019 at 9:29 AM Poul-Henning Kamp  
> >wrote:
> >>
> >> I have removed the remaining uses of STRING_LIST in the tree, and
> >> made vmodtool.py be slightly annoying if it is used.
> >
> >Not quite:
>
> Right now I'm trying to get the message out, the annoyance will be
> adjusted continuously.

My point was rather that "all uses of STRING_LIST in the tree" aren't
removed yet:

$ git grep STRING_LIST -- lib/libvmod_*
lib/libvmod_directors/vmod.vcc:$Method BACKEND .backend(STRING_LIST)
lib/libvmod_vtc/vmod.vcc:$Function VOID panic(STRING_LIST)

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: STRING_LIST deorbit burn completed

[Dridi Boukelmoune]

On Thu, Jun 20, 2019 at 9:29 AM Poul-Henning Kamp  wrote:
>
> I have removed the remaining uses of STRING_LIST in the tree, and
> made vmodtool.py be slightly annoying if it is used.

Not quite:

Making all in libvmod_directors


STRING_LIST will be discontinued before the 2019-09-15 release

Please switch to STRANDS


Same for vmod-vtc.

Dridi

> vmodtool.py will get more and more annoying about this over summer
> and around august 1st, it will refuse to compile it.
>
> The replacement is of course STRANDS.
>
> --
> Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
> p...@freebsd.org | TCP/IP since RFC 956
> FreeBSD committer   | BSD since 4.3-tahoe
> Never attribute to malice what can adequately be explained by incompetence.
> ___
> varnish-dev mailing list
> varnish-dev@varnish-cache.org
> https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

[bugwash] Retire hash_data() from VCL 4.2

[Dridi Boukelmoune]

Hello,

I would like to add the following item to the next bugwash agenda:

> 11:34 < dridi> phk: around?
> 11:35 < dridi> on the topic of vcl 4.2, and considering your recent commits
> 11:36 < dridi> would it make sense to retire the hash_data function in vcl 
> 4.2?
> 11:36 < dridi> replacement would be hash_string, hash_lowercase (host is 
> case-insensitive) and hash_blob (mainly req body)
> 11:37 < dridi> of course hash_data would have to remain for vcl < 4.2
> 11:38 < dridi> but the new functions could be made available in vcl 4.[01] too
> 11:52 < phk> dridi, for a few minutes
> 11:52 < phk> dridi, I think that is bugwash material ?

I would also encourage changing the built-in VCL to use the new
functions in vcl_hash.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VIP23 (VSL refactoring) design sketch

[Dridi Boukelmoune]

On Fri, May 3, 2019 at 3:07 PM Dridi Boukelmoune  wrote:
>
> On Fri, May 3, 2019 at 12:07 PM Geoff Simmons  wrote:
> >
> > On 5/3/19 10:51, Dridi Boukelmoune wrote:
> > >
> > > First we lose the ability to treat the whole record as a single sting
> > > out of the box. The client will need to reconstruct the string from
> > > the various field types that may constitute a record. Think string and
> > > regex operators.
> >
> > Yes, Martin and I recently had a conversation about this on #hacking.
> > For the string equality comparisons (eq and ne), we could conceivably
> > look into the comparison string and see if it has blanks that cross
> > field boundaries. But as you pointed out, some VSL payloads have blanks
> > that are not field separators (VCL_Log, FetchError, etc).
> >
> > For regex comparisons, where the pattern could include things like "\s",
> > ".*", "\W" and so forth, detecting whether the pattern might match a
> > field-separating blank has the look of a sophisticated computer
> > science-y problem.
>
> Does it have to though? We only use those as raw strings, and never as
> regular expression patterns.

By the way, yes I'm aware that quoting fields containing blanks would
introduce the need of escaping at the very least double quotes and the
escape character.

> > Martin and I agreed that, at least in the first iteration, for VSLQ
> > queries with a string operator (equality or regex) we should render the
> > formatted payload into a temporary buffer and run the comparison against
> > that. Once we have it all working, maybe we could consider more clever
> > solutions, but I'd be surprised if we'd gain much from getting clever.
>
> Ack, that's the logical thing to start with.

One request I forgot to make in this area, when you rebuild the
complete record, it would be nice to enclose fields that contain
blanks with double quotes, so that we can have the same syntax on 6.0
since we're going to maintain it for a while and I would like to solve
the querying problem there too.


> > I think that an important part of this will be to emphasize in the docs
> > that authors of VSL queries should use field[n] indices and numeric
> > comparisons whenever it's appropriate. My (admittedly evidence-free)
> > impression is that not everyone uses them when they could.
>
> Well, today the fields need to be dup'ed anyway when we need a null
> terminated string so that will indeed need to be documented that field
> operations become much cheaper than record operations.
>
> > Say you're scanning backend logs for fetches to backend foo, like this:
> >
> > -q 'BackendOpen ~ "foo"'
> >
> > Since the backend name is the second field, it would be better as:
> >
> > -q 'BackendOpen[2] eq "foo"'
> >
> > Or to look for backend fetches that take longer than 1 second, it
> > wouldn't surprise me if some people are doing something like:
> >
> > -q 'Timestamp:Beresp ~ "[1-9]\.\d*$"'
> >
> > When it should be:
> >
> > -q 'Timestamp:Beresp[3] > 1.0'
>
> Agreed, I sometimes realize after running a long query on a VSL of the
> GB persuasion that I could have done better.
>
> > We'll do much better if we can narrow the query to a field, and if we
> > don't have to convert numbers at all. And we should make sure that users
> > know about it.
>
> Make the field index mandatory with [0] meaning the whole record and
> people will learn :p
>
> > > Finally, regarding the VSL file header that we'd hypothetically dump
> > > with varnishlog -w in the future (which I really hope we do), we could
> > > consider making this a VSL record itself that is not subject to
> > > vsl_reclen, querying or filtering, for the needs of dynamic VSLs, so
> > > that whenever something changes VSL clients see an update. It would
> > > then be varnishlog -w's job to build such a synthetic record in the
> > > VSL file before dumping actual logs.
> >
> > I agree, and I'd say that when we do have meta-data entries in the log,
> > it should be possible for them to appear at any time, in both log files
> > and in-memory logs. That would put us on the path to dynamic SLT tags.
> >
> > But we're not planning for that in the first iteration. phk has said
> > that he doesn't foresee dynamic VSL by the September release. And I
> > agree that we should get the rest working first.
>
> I disagree. If we draft a plan that 1) changes the in-memory AND
> on-disk format and 2) has later a possibility for dy

Re: VIP23 (VSL refactoring) design sketch

[Dridi Boukelmoune]

On Fri, May 3, 2019 at 12:07 PM Geoff Simmons  wrote:
>
> On 5/3/19 10:51, Dridi Boukelmoune wrote:
> >
> > First we lose the ability to treat the whole record as a single sting
> > out of the box. The client will need to reconstruct the string from
> > the various field types that may constitute a record. Think string and
> > regex operators.
>
> Yes, Martin and I recently had a conversation about this on #hacking.
> For the string equality comparisons (eq and ne), we could conceivably
> look into the comparison string and see if it has blanks that cross
> field boundaries. But as you pointed out, some VSL payloads have blanks
> that are not field separators (VCL_Log, FetchError, etc).
>
> For regex comparisons, where the pattern could include things like "\s",
> ".*", "\W" and so forth, detecting whether the pattern might match a
> field-separating blank has the look of a sophisticated computer
> science-y problem.

Does it have to though? We only use those as raw strings, and never as
regular expression patterns.

> Martin and I agreed that, at least in the first iteration, for VSLQ
> queries with a string operator (equality or regex) we should render the
> formatted payload into a temporary buffer and run the comparison against
> that. Once we have it all working, maybe we could consider more clever
> solutions, but I'd be surprised if we'd gain much from getting clever.

Ack, that's the logical thing to start with.

> I think that an important part of this will be to emphasize in the docs
> that authors of VSL queries should use field[n] indices and numeric
> comparisons whenever it's appropriate. My (admittedly evidence-free)
> impression is that not everyone uses them when they could.

Well, today the fields need to be dup'ed anyway when we need a null
terminated string so that will indeed need to be documented that field
operations become much cheaper than record operations.

> Say you're scanning backend logs for fetches to backend foo, like this:
>
> -q 'BackendOpen ~ "foo"'
>
> Since the backend name is the second field, it would be better as:
>
> -q 'BackendOpen[2] eq "foo"'
>
> Or to look for backend fetches that take longer than 1 second, it
> wouldn't surprise me if some people are doing something like:
>
> -q 'Timestamp:Beresp ~ "[1-9]\.\d*$"'
>
> When it should be:
>
> -q 'Timestamp:Beresp[3] > 1.0'

Agreed, I sometimes realize after running a long query on a VSL of the
GB persuasion that I could have done better.

> We'll do much better if we can narrow the query to a field, and if we
> don't have to convert numbers at all. And we should make sure that users
> know about it.

Make the field index mandatory with [0] meaning the whole record and
people will learn :p

> > Finally, regarding the VSL file header that we'd hypothetically dump
> > with varnishlog -w in the future (which I really hope we do), we could
> > consider making this a VSL record itself that is not subject to
> > vsl_reclen, querying or filtering, for the needs of dynamic VSLs, so
> > that whenever something changes VSL clients see an update. It would
> > then be varnishlog -w's job to build such a synthetic record in the
> > VSL file before dumping actual logs.
>
> I agree, and I'd say that when we do have meta-data entries in the log,
> it should be possible for them to appear at any time, in both log files
> and in-memory logs. That would put us on the path to dynamic SLT tags.
>
> But we're not planning for that in the first iteration. phk has said
> that he doesn't foresee dynamic VSL by the September release. And I
> agree that we should get the rest working first.

I disagree. If we draft a plan that 1) changes the in-memory AND
on-disk format and 2) has later a possibility for dynamic VSLs we
should make this part of the first iteration. We shouldn't change the
format of such a critical piece of the infrastructure every other
release.

Also by making this both a synthetic record for live log consumer by
libvarnishapi and in the future something that varnishd may produce
then it becomes a no brainer for varnishlog. It just writes it on disk
like any other log record. It may need some intelligence though when
logs are rotated, like varnishlog asking for a synthetic record again.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VIP23 (VSL refactoring) design sketch

[Dridi Boukelmoune]

On Fri, Apr 12, 2019 at 9:24 PM Poul-Henning Kamp  wrote:
>
> 
> In message 
> , Dridi 
> Boukelmoune writes:
>
> >Interesting, for textual logs the effect of vsl_reclen is
> >straightforward but how should we deal with binary records truncation?
>
> Maybe redefine the limit to apply to individual strings (ie: header
> values) rather than the full record ?
>
> >> Some other lessons learned:
> >>
> >> The binary records are not necessarily shorter than the ASCII-formatted
> >> records.
>
> It is incredible how often people overlook this.  I've been ranting
> about it for almost two decades now, in particular in context of the
> userland/kernel API.
>
> Imagine if in addition to errno there also were a const char *errstr
> which the kernel could fill out, that would do *wonders*, in particular
> for EINVAL.
>
> My favourite example was a Nx64 card where the driver spent more
> than 35kloc of source code on defining a ton of structs for ioctl(2)
> usage but no matter what the kernel found deficient, the only thing
> it told you was return(EINVAL).
>
> I threw out the entire thing and added a single ioctl which passed a
> struct with an input string, containing a command like
> "set chan 4 ts 4-12"
> and another string out with an error message like:
> "channel 11 already allocated to chan 5"
> didn't even spend 1kloc on it...

I have given a bit more thought to this and going with structured
binary VSL records will have more consequences than a blank-separated
list of fields.

First we lose the ability to treat the whole record as a single sting
out of the box. The client will need to reconstruct the string from
the various field types that may constitute a record. Think string and
regex operators.

Then we further the disconnect between today's VSL parsing and VSL
query parsing. Query parsing of strings does already support quoted
strings, and they are not mandatory. We can even query a prefix
containing blanks by quoting the prefix:

-q 'VCL_Log:"just because" eq "we can"'

My attempt at solving this (#2872) was bringing back consistency by
allowing fields to be quoted. I'm glad with the turbo-encabulator-friendly
outcome for Backend_health, but we need to consider the std.log()
escape hatch over which we have no input control, and for which we
can't currently provide a VCC interface to describe the fields, pass
them as a vararg and still ensure at "compile time" that arguments
match the spec. Sure, VCC could learn to do that but it sounds a tiny
bit complicated.

Finally, regarding the VSL file header that we'd hypothetically dump
with varnishlog -w in the future (which I really hope we do), we could
consider making this a VSL record itself that is not subject to
vsl_reclen, querying or filtering, for the needs of dynamic VSLs, so
that whenever something changes VSL clients see an update. It would
then be varnishlog -w's job to build such a synthetic record in the
VSL file before dumping actual logs.


Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: on the vsl format - VIP23 (VSL refactoring) design sketch

[Dridi Boukelmoune]

On Fri, Apr 12, 2019 at 6:27 PM Nils Goroll  wrote:
>
> On 12/04/2019 17:57, Dridi Boukelmoune wrote:
> > The prefix, which may be optional, and by nature is supposed to be the
> > first field, and coincidentally is of variable length, is a problem.
>
> I do not understand why. The VSL client code which matches the prefix can do 
> so
> against the string field as before, it is just not the start of the record 
> payload.

The prefix is too important as an optional field to move to the last
position and jeopardize its presence because of vsl_reclen.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: on the vsl format - VIP23 (VSL refactoring) design sketch

[Dridi Boukelmoune]

On Fri, Apr 12, 2019 at 5:08 PM Nils Goroll  wrote:
>
> Hi,
>
> I spend some time pondering the VSL format, here's what I got at the moment.
>
> First of all, a recap of the existing format. In short:
>
> * a record can be an end- or wrapmarker. These are not relevant in the current
>   context
>
> * batch records are containers for individual records as produced by the VSLb*
>   functions on a vsl buffer
>
> * records are tag, length, vxid and payload
>
> endmarker:
>
> | 31 ... 24 | 23 ... 0 |
> | 0xfe  | 0x454545 |
>
> wrapmarker:
>
> | 31 ... 24 | 23 ... 0 |
> | 0xfe  | 0x575757 |
>
> batch:
>
> | 31 ... 24 | 23 ... 0 |
> | 0xff  | zero |
>
> | 31 ... 0 |
> | len  |
>
> ...
>
> record:
>
> | 31 ... 24 | 23 ... 2 | 1 .. 0 |
> | tag   | len >> 2 | zero   |
>
> | 31 | 30 | 29... 0 |
> |  B |  C | vxid|
>
> | (len bytes) |
> | log |
>
> B = BACKENDMARKER
> C = CLIENTMARKER
> actually those are contained in the vxid passed to VSL
>
>
> There is a number of VSL Tags for which this format can remain unchanged,
> because they either contain exactly one string or are relevant only for
> debugging/tracing purposes.
>
> PROPOSAL:
>
> For the binary VSL records we already agreed that fixed length fields always
> come first. Because we have the length and know the size of the fixed part, we
> can could put in another STRING or header as follows:

The prefix, which may be optional, and by nature is supposed to be the
first field, and coincidentally is of variable length, is a problem.

Although if we reserve one more bit for a prefix flag we can make it
the last field. Upon truncation though, not having the prefix could be
a big deal.

> fixed:
>
> | 31 ... 24 | 23 ... 2 | 1 .. 0 |
> | tag   | len >> 2 | 10 |
>
> | 31 | 30 | 29... 0 |
> |  B |  C | vxid|
>
> ...[]
>
> HDR: 
>
>
> This would accommodate exactly one additional STRING or, with one extra byte 
> to
> indicate the header name length, one additional header.
>
> I would also propose to indicate this format by setting one of the lower bits 
> of
> the length.
>
> This format would accommodate more VSL Tags where exactly one string/header is
> logged.
>
> The question then is if, for additional variable data, we should actually 
> embed
> it in one VSL record. We could also use additional VSL records, under the
> condition that such "continuation records" are written within the same Batch:
>
> variable:
>
> like record, but
> - field instead of tag
> - p[0] & 3 == 3
> - no xvid, data starts at p[1]
>
> | 31 ... 24 | 23 ... 2 | 1 .. 0 |
> | field | len >> 2 | 11 |
>
> | (len bytes) |
> | log |
>
> The advantage would be that we get the full 24 bits length per string.
>
> But my main motivation was actually pondering about a different function
> interface, for which this format would be advantageous because the caller 
> would
> write directly to VSL. Here's a taste of this suggestion, but this is not 
> cooked
> yet. Feedback welcome anyway:
>
> struct SLTS_XXX t[1];
>
> VSLS_Open(t, vsl, SLT_XXX, vxid);
> t->fixed->dbl = 0.5;
> t->fixed->int = 42;
> VSLS_printf(t, t->var->brz, "%s_%s", bar, baz);
> VSLS_printf(t, t->var->foo, "%s/%s", bar, baz);
> VSLS_Close(t);
>
> Nils
>
>
> --
>
> ** * * UPLEX - Nils Goroll Systemoptimierung
>
> Scheffelstraße 32
> 22301 Hamburg
>
> tel +49 40 28805731
> mob +49 170 2723133
> fax +49 40 42949753
>
> xmpp://sl...@jabber.int.uplex.de/
>
> http://uplex.de/
>
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VIP23 (VSL refactoring) design sketch

[Dridi Boukelmoune]

On Fri, Apr 12, 2019 at 4:00 PM Geoff Simmons  wrote:
>
> So I said we shouldn't get to optimizing too soon, and here I am doing
> something like that. Because the discussion made me curious.
>
> In the attachment you'll find:
>
> - A first implementation of a function VSLwv() to write binary fields
> into a log record buffer, using a printf-like format string and a va_list.
>
> - Test calls for Timestamp, ReqAcct, ReqHeader and RespStatus (these are
> #if-0'd out here).
>
> - printf() calls to output the data from the binary log record (also
> #if-0'd out). This confirms that we can get the data back out as it went
> in, and experiments at how we could go about that.
>
> - A benchmark with ca. one gazillion runs of VSLwv() and the
> corresponding sprintf() calls, to compare the binary function with
> library printf code.
>
> - Results of running that through callgrind and perf record. To view the
> results, you'll need a tool like kcachegrind, and perf report.
>
> VSLwv() doesn't have bounds checking, to make sure that we don't write
> past vsl_reclen, which of course a real version must have. That will
> mean more conditional branching, but the branches will be predicted
> almost always.

Interesting, for textual logs the effect of vsl_reclen is
straightforward but how should we deal with binary records truncation?

> I grepped through current VSL*() calls, and these are more or less all
> of the format characters we currently use: %s, %.*s, %.ns (for a length
> n, as in %.20s), %d, %u, %x, %f, %j* and %z* (for (u)intmax_t and size_t
> types). There may be one or two more or less, but that's close to all of
> it. I added %hu for a response status as unsigned short (we currently
> have %u for that). (There are some %p's for tags like ExpKill, mostly
> only interesting for devs.)
>
> Note that for the binary writes, we don't have to care if the data type
> is unsigned, just copy the right number of bytes. We'll want the %u
> formats for type checking, but VSLwv() can do the same thing for %d and
> %u, and %zd and %zu, and so on. It's the output code that needs to get
> signedness right.

Should we though? If someone runs Varnish in a virtual machine on a
mainframe server and runs varnishlog -w don't we want to be able to
varnishlog -r that file on a "regular" workstation?

> This version always writes a length field as an unsigned short before a
> string (maybe it should be uint16_t), and string fields are not
> NUL-terminated.

Until the vsl_reclen question is sorted out, you can always copy the
string and grab its length while doing so, and once it's written and
you have the length you can write it at the field length's address.

I agree we should settle for uin16_t for that.

Having null-terminated strings open a zero-copy opportunity on the VSL
consumer side.

> So a final version would need the bounds checking, may handle one or two
> more or fewer format chars, and may handle strings differently. Other
> than that, I think this is about everything we'll need it to do.
>
> What did the performance tools say?
>
> Overall, VSLwv() did noticeably better than the library printf calls on
> the various measures: fewer CPU cycles, better memory cache hit rates
> (both instruction and data), and, generally, fewer branch mispredicts.
> Not surprising, because library printf has quite a bit more to do.
>
> As @Nils suspected, the weak spot is branch prediction. My gcc compiled
> the switch statement as a jump table (jmpq *rax), and kcachegrind
> reports 86% of indirect branch mispredicts there, far more than anything
> else -- glibc vfprintf comes in next with 13%.
>
> On the other hand, the library printf code has about twice as many
> mispredicts for conditional branches, resulting in more mispredicts for
> all types of branches.
>
> So: this first shot at coding something up has a sore spot, but overall
> appears to be measurably better than the printf stuff.
>
> Some other lessons learned:
>
> The binary records are not necessarily shorter than the ASCII-formatted
> records. Particularly ReqAcct, which we have as 6 uintmax_t fields. That
> comes to 48 bytes on my machine, about twice as long as a string like
> "110 0 110 210 224 434". I don't think we can do much about that.
> Request/responses can transfer up to gigabytes, which is why we need
> uintmax_t. Unless we want to try something like variable-length integer
> fields (which I doubt).

We could, but then that would force us to pre-compute variable-length
field sizes. (are there others than strings?)

> HTTP header records, with two variable-length strings, also won't be any
> shorter than the ASCII format. If they have two 2-byte length fields,
> they may come out as a couple of bytes longer (especially if they're
> also NUL-terminated).

I think we never discussed the prefix filtering. With binary logs it
should become explicit and a different kind of field that is
essentially a string for a specific purpose.

Since the presence of a prefix may be 

Re: VIP23 (VSL refactoring) design sketch

[Dridi Boukelmoune]

On Wed, Apr 10, 2019 at 8:08 AM Nils Goroll  wrote:
>
> Hi,
>
> I am +1 (or more, if I got more votes) overall, in particular, the incremental
> approach seems to make a lot of sense to me. Depending on the state of the 
> rest
> of varnish-cache core, I would try get this into production ASAP as I have 
> been
> for the last couple of years.
>
> Some additional points:
>
> - It would make a lot of sense to include the field definitions for VSL 
> clients
> in a VSM segment such that log clients do not need to be recompiled when we
> change fields.

I think the definitions should be the first step. The second one
should be changing the VSL file format magic number to include a
version to accommodate future changes, so that varnishlog -w can also
write the field definitions for vut -r to pick it up offline too.


> - This would also pave the way for possible future dynamic VSL records (added
> while varnishd runs, referenced as "VMOD log meta-data" in VIP23). At some
> point, phk mentioned he would see them as a requirement for H3, but last I 
> know
> this is not the case, so we can put this to the end of the list. Yet we should
> head into the right direction to eventually get there.
>
> - We should make it clear that the VSL format does not constitute an API, so 
> log
> access is only supported by using the vsl client in libvarnishapi
>
> - We might want to consider making the "file" format written by varnishlog -w 
> an
> API, but I guess the advantage over libvarnishapi as the API is pretty minor.
>
> - Geoff, You mentioned once over a burger that the actual binary 
> representation
> of doubles is not standardized. By the same argument you give for the byte
> order, I do not think that this bears any practical relevance for now, but we
> might want to at least mention this and, for future cross-platform support,
> might have some kind of "this log uses that representation" info similar to 
> the BOM.
>
> Nils
>
> --
>
> ** * * UPLEX - Nils Goroll Systemoptimierung
>
> Scheffelstraße 32
> 22301 Hamburg
>
> tel +49 40 28805731
> mob +49 170 2723133
> fax +49 40 42949753
>
> xmpp://sl...@jabber.int.uplex.de/
>
> http://uplex.de/
>
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: GH006: Protected branch update failed for refs/heads/master

[Dridi Boukelmoune]

On Wed, Apr 3, 2019 at 2:47 PM Dridi Boukelmoune  wrote:
>
> On Wed, Apr 3, 2019 at 2:11 PM Dridi Boukelmoune  wrote:
> >
> > It looks like it's complaining that Travis CI needs to validate the
> > attached patch first, I'm assuming via a pull request.
> >
> > Can someone else give it a try?
>
> More trivial patches attached.

Fed solved this somehow, I pushed the patches to master.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: GH006: Protected branch update failed for refs/heads/master

[Dridi Boukelmoune]

On Wed, Apr 3, 2019 at 2:11 PM Dridi Boukelmoune  wrote:
>
> It looks like it's complaining that Travis CI needs to validate the
> attached patch first, I'm assuming via a pull request.
>
> Can someone else give it a try?

More trivial patches attached.
From 52132483928766aa0dfc485182be9b552392bbc5 Mon Sep 17 00:00:00 2001
From: Dridi Boukelmoune 
Date: Wed, 3 Apr 2019 14:46:22 +0200
Subject: [PATCH 3/3] Assert

---
 bin/varnishd/cache/cache_obj.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/bin/varnishd/cache/cache_obj.c b/bin/varnishd/cache/cache_obj.c
index 4c0ec76bb..5a807e869 100644
--- a/bin/varnishd/cache/cache_obj.c
+++ b/bin/varnishd/cache/cache_obj.c
@@ -367,6 +367,7 @@ ObjBocDone(struct worker *wrk, struct objcore *oc, struct boc **boc)
 	const struct obj_methods *m;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
+	CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
 	AN(boc);
 	CHECK_OBJ_NOTNULL(*boc, BOC_MAGIC);
 	if (oc->stobj->stevedore != NULL) {
-- 
2.20.1

From 7ee86fbbb9edd5a6fbe4b989488e1ce3d0c16108 Mon Sep 17 00:00:00 2001
From: Dridi Boukelmoune 
Date: Wed, 3 Apr 2019 14:44:09 +0200
Subject: [PATCH 2/3] Polish

---
 bin/varnishd/storage/storage_simple.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/bin/varnishd/storage/storage_simple.c b/bin/varnishd/storage/storage_simple.c
index 0606f997e..83e8d713c 100644
--- a/bin/varnishd/storage/storage_simple.c
+++ b/bin/varnishd/storage/storage_simple.c
@@ -497,8 +497,7 @@ sml_bocdone(struct worker *wrk, struct objcore *oc, struct boc *boc)
 	if (boc->stevedore_priv != NULL) {
 		/* Free any leftovers from Trim */
 		CAST_OBJ_NOTNULL(st, boc->stevedore_priv, STORAGE_MAGIC);
-		boc->stevedore_priv = 0;
-		CHECK_OBJ_NOTNULL(st, STORAGE_MAGIC);
+		boc->stevedore_priv = NULL;
 		sml_stv_free(stv, st);
 	}
 
-- 
2.20.1

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

GH006: Protected branch update failed for refs/heads/master

[Dridi Boukelmoune]

It looks like it's complaining that Travis CI needs to validate the
attached patch first, I'm assuming via a pull request.

Can someone else give it a try?

According to GitHub I'm still a member of the varnishcache
organization, so I'm not sure what went wrong:

> Enumerating objects: 11, done.
> Counting objects: 100% (11/11), done.
> Delta compression using up to 8 threads
> Compressing objects: 100% (6/6), done.
> Writing objects: 100% (6/6), 525 bytes | 262.00 KiB/s, done.
> Total 6 (delta 5), reused 0 (delta 0)
> remote: Resolving deltas: 100% (5/5), completed with 5 local objects.
> remote: error: GH006: Protected branch update failed for refs/heads/master.
> remote: error: Required status check "continuous-integration/travis-ci" is 
> expected.
> To github.com:varnishcache/varnish-cache
>  ! [remote rejected] master -> master (protected branch hook declined)
> error: failed to push some refs to 
> 'g...@github.com:varnishcache/varnish-cache'

Thanks,
Dridi
From f87c8e33ceeb35819e89e18dc21520f74db6e6d0 Mon Sep 17 00:00:00 2001
From: Dridi Boukelmoune 
Date: Wed, 3 Apr 2019 13:53:28 +0200
Subject: [PATCH] Polish

---
 bin/varnishd/cache/cache_obj.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/bin/varnishd/cache/cache_obj.c b/bin/varnishd/cache/cache_obj.c
index d46f4e4da..4c0ec76bb 100644
--- a/bin/varnishd/cache/cache_obj.c
+++ b/bin/varnishd/cache/cache_obj.c
@@ -116,13 +116,10 @@ obj_deleteboc(struct boc **p)
 {
 	struct boc *boc;
 
-	AN(p);
-	boc = *p;
-	*p = NULL;
+	TAKE_OBJ_NOTNULL(boc, p, BOC_MAGIC);
 	Lck_Delete(>mtx);
 	AZ(pthread_cond_destroy(>cond));
-	if (boc->vary != NULL)
-		free(boc->vary);
+	free(boc->vary);
 	FREE_OBJ(boc);
 }
 
-- 
2.20.1

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Coccinelle

[Dridi Boukelmoune]

On Wed, Mar 27, 2019 at 8:38 AM Nils Goroll  wrote:
>
> Re Dridi in 4edaa2836c6aba837899b78866a2567c704e336b:
>
> > Asking again, could we consider keeping Coccinelle patches around?
>
> I think it would make a lot of sense to automate coding conventions. Ideally,
> make check would fail if any of them are not met.

Since we have no Coccinelle expert in the team to my knowledge, I'd
rather start with having a collection of semantic patches available to
people (be it for in-tree or out-of-tree use). I know of other
projects failing their build on Coccinelle checks but I think it's a
premature discussion at this point.

> As I got zero experience with coccinelle my only concern would be how to mark
> exceptions. Is there anything similar to flexelint hints?

I don't know enough about that. I know you can define isomorphic
expressions so that you don't have to deal with differences like

if (ptr)
if (ptr != NULL)
if (!ptr)
if (ptr = NULL)

but regarding exceptions, I don't know.

> At any rate, +1 for adding the patches to the tree.

At the very least the semantic patches I wrote for Varnish are all
available on purpose in the commit log (when I considered them
relevant and was the one checking changes in).

git log --grep Coccinelle

3 patches are available as of today in the commit log so that shouldn't
be too intrusive.


Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Doc updates on v-c.o

[Dridi Boukelmoune]

On Thu, Mar 14, 2019 at 9:23 PM Geoffrey Simmons  wrote:
>
> @Dridi, the sentence about Python under “for developers” in “Changes” implies 
> that python 2 is still an option. I assume that’s no longer correct? That 
> went in fairly early, before the decision about python 3 was made.

Right, I did not catch that one and added the note about dropping
python 2 in the Upgrading document.

I'm fixing it now.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: 2019-03-15 release

[Dridi Boukelmoune]

On Thu, Mar 14, 2019 at 8:43 PM Poul-Henning Kamp  wrote:
>
> Do not cut the release until I give the OK.
>
> I/We have a couple of i's to dots and t's to dash first.

And coverity reported a bunch of things too!

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Doc updates on v-c.o

[Dridi Boukelmoune]

On Thu, Mar 14, 2019 at 8:31 PM Poul-Henning Kamp  wrote:
>
> 
> In message , Geoffrey Simmons 
> wr
> ites:
>
> Seems like python3  fallout. should be fixed now

Also https://varnish-cache.org/docs/ says:

   Documentation for the latest release 6.0

6.1 never made it in the docs :-/

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: tabular CLI output proof of concept hack

[Dridi Boukelmoune]

On Tue, Feb 12, 2019 at 12:22 PM Poul-Henning Kamp  wrote:
>
> 
> In message 
> , Dridi 
> Boukelmoune writes:
> >> But I am also seriously wondering if we should go even further and
> >> have varnishd *only* produce JSON output, and leave the render-for-humans
> >> aspect to varnishapi or even varnishadm.
> >
> >Ideally implement it in libvarnish.a and expose it in libvarnishapi,
> >with a way for varnishadm to specify a window size in interactive
> >mode. This way varnishd -d will be able to produce the plain text
> >output.
>
> I think you need to unpack that a bit before I follow ?

My bad!

When you run varnishd -d from a terminal you may send CLI commands
from stdin and read responses from stdout. So whether it's in a VTC
log or run interactively, I think it would be harder to read the JSON
output (parsing with eyeballs=).

So my thought was that varnishd -d should stay human-friendly and
stick to plain text.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: tabular CLI output proof of concept hack

[Dridi Boukelmoune]

> But I am also seriously wondering if we should go even further and
> have varnishd *only* produce JSON output, and leave the render-for-humans
> aspect to varnishapi or even varnishadm.

Ideally implement it in libvarnish.a and expose it in libvarnishapi,
with a way for varnishadm to specify a window size in interactive
mode. This way varnishd -d will be able to produce the plain text
output.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VMOD havoc generating patch

[Dridi Boukelmoune]

> >No, I mean when I look at the generated vcc_$VMOD_if.h, I see many
> >occurrences of the $Prefix that are not guarded by VPFX() macros.
>
> The VPFX/VARGS/VENUM macros are mostly intended as convenience for the
> files the vmod-author writes.

Ack.

> I didn't VPFX the prototypes to not _force_ people to use the VPFX
> in their code if they dont want to, but if the consensus is that
> we should be pushing VPFX() usage, changing it is just stylistic.

I will leave it to $Prefix users that need to support both 6.0 and the
new 6.2 vmodtool to leave an opinion here. I don't use the $Prefix
stanza.

Thanks,
Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VMOD havoc generating patch

[Dridi Boukelmoune]

On Mon, Feb 4, 2019 at 8:30 PM Poul-Henning Kamp  wrote:
>
> 
> In message 
> , Dridi 
> Boukelmoune writes:
> >On Mon, Jan 28, 2019 at 1:55 PM Poul-Henning Kamp  
> >wrote:
>
> >I waited until my weekly CI against master would choke on this change,
> >and it's very easy to support both before and after if your baseline
> >is 6.0, which is more than fine since 4.1 only has a couple months to
> >live:
> >
> >#include "vcc_$VMOD_if.h"
> >
> >/* Varnish < 6.2 compat */
> >#ifndef VPFX
> > #define VPFX(a) vmod_ ## a
> > #define VARGS(a) vmod_ ## a ## _arg
> > #define VENUM(a) vmod_enum_ ## a
> >#endif
> >
> >I didn't try on a vmod with a $Prefix stanza, but I'm sure it can be
> >helped if that snippet is not enough.
>
> Yes, that would go a long way towards compatibility.

It seems incomplete though, currently the $Prefix is applied all over
the place and the VPFX macro isn't used for function names. Is that
still work in progress?

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VMOD havoc generating patch

[Dridi Boukelmoune]

On Mon, Jan 28, 2019 at 1:55 PM Poul-Henning Kamp  wrote:
>
> Ticket 2810 is about the names generated by vmodtool and vcc, and
> while there is a good intellectual argument for getting it right,
> I am a little bit worried about how much havoc that causes.
>
> This is a WIP patch headed in that direction, and I would like to
> hear input from VMOD writers.
>
> Ideally with this stuff finished, VMOD writers can version their
> vmods using $Prefix and you will then be able to import multiple
> different versions of the same VMOD in the same VCL.  Not sure that
> is a good thing to do, but it proves that the name-space issue is
> solved.
>
> See the changes to the in-tree vmods for how this will look for you.

I waited until my weekly CI against master would choke on this change,
and it's very easy to support both before and after if your baseline
is 6.0, which is more than fine since 4.1 only has a couple months to
live:

#include "vcc_$VMOD_if.h"

/* Varnish < 6.2 compat */
#ifndef VPFX
 #define VPFX(a) vmod_ ## a
 #define VARGS(a) vmod_ ## a ## _arg
 #define VENUM(a) vmod_enum_ ## a
#endif

I didn't try on a vmod with a $Prefix stanza, but I'm sure it can be
helped if that snippet is not enough.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: [6.1] 2f2387038 Fix gensequences for BusyBox awk

[Dridi Boukelmoune]

On Wed, Nov 7, 2018 at 12:01 PM Poul-Henning Kamp  wrote:
>
> 
>
> Why is this fix not in -trunk ?

It is:

$ git branch --contains 9f50c4bd12303674890ca742ce6b9592a29e3ba9
  6.1
* master

But it was lost during backports. It was accidentally lost in both my
6.0 and Pål's 6.1 backports.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: mini benchmark vtim formatting

[Dridi Boukelmoune]

> So now the question is how this looks for other platforms. @All, Please send
> your results!

$ ./double
real: 0.004972s / 10 = 49.715042ns - tst val 153899435413393.562500
mono: 0.003554s / 10 = 35.541058ns - tst val 1075376231.264834
printf %.6f: 0.059387s / 10 = 593.865160ns - tst val
510.00 10753.764099
printf %ju.%06ju: 0.010162s / 10 = 101.621330ns - tst val
510.00 10753.823494

$ ./uint_nsec
1 vt is 1 nsec
real: 5054881ns / 10 = 50ns - tst val 16651824334676002549
mono: 2728750ns / 10 = 27ns - tst val 1030850150182733345
%lu.%06lu: 11545090ns / 10 = 115ns - tst val 56010308.502862
%lu.%06lu diff: 10600594ns / 10 = 106ns - tst val 560   10308.514411
%06lu: 7568966ns / 10 = 75ns - tst val 560  10308525016
2printf: 15149373ns / 10 = 151ns - tst val 560  10308.532590

$ ./uint_usec8
1 vt is 125 nsec
real: 5313625ns / 10 = 53ns - tst val 13710030185021002820
mono: 3063625ns / 10 = 30ns - tst val 8258753524632712
%lu.%06lu: 12164125ns / 10 = 121ns - tst val 51010323.443427
%lu.%06lu diff: 10665000ns / 10 = 106ns - tst val 510   10323.455602
%06lu: 7729250ns / 10 = 77ns - tst val 510  10323466276
2printf: 15788875ns / 10 = 157ns - tst val 510  10323.474012

$ uname -a
Linux dridi 4.18.11-200.fc28.x86_64 #1 SMP Sun Sep 30 15:31:40 UTC
2018 x86_64 x86_64 x86_64 GNU/Linux

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: RFC: unify directors and backends vcl interface

[Dridi Boukelmoune]

On Wed, Sep 19, 2018 at 12:19 PM Nils Goroll  wrote:
>
> my example was missing the point
>
> On 19/09/2018 12:14, Nils Goroll wrote:
> > backend "b" which you now want to layer under director "d": all instances 
> > of "b"
> > in the vcl now need to be replaced with something like "d.backend()".
>
> ... while, if we had a common accessor, b could be renamed to bb and the
> director be called b.

That won't work when .backend() takes arguments *cough* hash *cough*.

I think the confusion is mostly that we make an amalgam of the VMOD
object and the underlying director. I'm not strictly against adding some
magic to VCL, in this case saying that having a no-arg method returning
a VCL_BACKEND could be used automagically when casting an
"expression" to a backend and even enforce that the name of the no-arg
method is .backend().

I suggested in the past that "vmod-object ~ arg" syntax could automatically
be turned into "vmod-object.match(arg)" when we have a matching (pun
intended) "BOOL .match(SINGLE-ARG)" method.

In case it's not clear, I think it's a good idea, and I'm merely discussing
caveats to consider. Mainly, disambiguation.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: CLI command vmod.tell

[Dridi Boukelmoune]

On Fri, May 18, 2018 at 4:07 PM, Poul-Henning Kamp <p...@phk.freebsd.dk> wrote:
> 
> In message 
> <CABoVN9CmtyVAo_CS9QK+m1Z7FY295=wanluvca5kdjqsfpu...@mail.gmail.com>
> , Dridi Boukelmoune writes:
>
>>> vmod.tell [VCL_PATTERN:]VMODNAME arguments [...]
>
>>Can we also pass a nonce to the VMOD? So that "global" actions can be
>>skipped once done when a VMOD like xkey is targeted with a VCL pattern
>>matching more than once.
>
> You are entirely in control of what "arguments" are and how
> they are interpreted.

I mean an argument generated by varnish, not something I would entrust
to the CLI user.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: CLI command vmod.tell

[Dridi Boukelmoune]

On Fri, May 18, 2018 at 11:30 AM, Poul-Henning Kamp  wrote:
> 
> In message , Geoff Simmons 
> write
> s:
>
>>>  vmod.tell  somevcl::somevmod "bla bla bla"
>
> I looked a bit at this.
>
> There is a hurt of locking issues for VMOD writers trying to do
> anything smart, but trivial stuff like setting global options for
> the VMOD etc, shouldn't cause trouble.
>
> VCC wise I think this will be a "$CLI" stanza, and the function
> will get called with a VRT_CTX and a char **, and return an enum
> VCLI_status_e.

Can we also pass a nonce to the VMOD? So that "global" actions can be
skipped once done when a VMOD like xkey is targeted with a VCL pattern
matching more than once.

> I think I prefer this naming:
>
> vmod.tell [VCL_PATTERN:]VMODNAME arguments [...]
>
> The default VCL_PATTERN will be the active VCL.
>
> In other words:
>
> vmod.tell std bugger off
>
> only hits the std vmod in the active vcl, whereas
>
> vmod.tell *:std bugger off
>
> will hit the std vmod in all vcls which has imported it.
>
> When used without a pattern:
> 
>
> If the active VCL has not imported a vmod of that name
> status is CLIS_CANT.
>
> If that VMOD does not implement $CLI it will be CLIS_UNIMPL.
>
> Otherwise the VMODs $CLI determines the status.
>
> When used with a pattern:
> -
>
> If no VCLs are matched by pattern:  CLIS_CANT
>
> If the VMOD is not loaded in any of the matched VCLs:  CLIS_CANT
>
> If no hit VMOD implements $CLI: CLIS_UNIMPL.
>
> If any hit VMOD's $CLI returns non-200:  new status CLIS_KABOOM (=202)
>
> Otherwise CLIS_OK
>
> Still waffling on:
> --
>
> Should names of VCLs be put into cli output as subheadings when
> pattern is used so VMODs won't have to do that, or should they
> only be added in front of VMODs which produce cli output.
>
> --
> Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
> p...@freebsd.org | TCP/IP since RFC 956
> FreeBSD committer   | BSD since 4.3-tahoe
> Never attribute to malice what can adequately be explained by incompetence.
> ___
> varnish-dev mailing list
> varnish-dev@varnish-cache.org
> https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VDD summary

[Dridi Boukelmoune]

On Wed, May 16, 2018 at 9:20 AM, Poul-Henning Kamp  wrote:
> 
> In message 
> 

Re: VDD summary

[Dridi Boukelmoune]

> => new CLI command: backend.tell  args
> => ... if glob matches multiple backends, error-returns are ignored
> => ... Should "?" args be magic ?

It occurred to me that directors listening to the CLI via a backend.tell
command is a very special case that we could otherwise generalize
to a vmod.cmd CLI command. This would allow modules like xkey
to purge either from VCL or the CLI, just like ban:

vmod.cmd  command [args...]

Example:

vmod.cmd xkey help
help []
purge [-s]  [...]

vmod.cmd xkey help purge
purge [-s]  [...]
Perform a purge of all objects matching at least one id,
or a soft purge when the -s flag is present.

vmod.cmd xkey purge 123
42 objects purged.

This way it's up to directors to have their respective VMODs expose
various ways to change their state.

Thanks for the VDD and thanks to Nils for his patience and explanations
of their backend/director advanced needs and how we can keep the
user interface manageable in VCL.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

[VDD] Client-side latency

[Dridi Boukelmoune]

Hello everyone,

I'd like to not put this item on the agenda for next Monday so I'll
briefly present it here since nobody at Varnish Software thinks it
will be controversial. Attendees outside of Varnish Software, please
ack that this should have a de-facto "send patches" green light or we
can discuss this further next Monday.

I have recently run into simple latency problems that were tricky to
observe in Varnish.

First, I ran into a case where we didn't have enough workers and
sessions were queued. The client would report response times that
wouldn't match what we observed in varnishlog because Timestamp
record don't account for queuing (and it becomes tricky with h2).

Second, we then realized that the client was also too eager on
pipelining and many requests would pile up a "long" time before
Varnish would process them. Once again everything looks good in
varnishlog.

Proposed feature:

A new %{Varnish:date}x variable in varnishncsa that converts
req.http.date or beresp.http.date into a Timestamp-like format. This
should help spot "significant" differences between what the peer
claims and what the Timestamp:Start or Timestamp:Beresp records
say.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

[VDD] VSL query extensions

[Dridi Boukelmoune]

Hello everyone,

I'd like to not put this item on the agenda for next Monday so I'll
briefly present it here since nobody at Varnish Software thinks it
will be controversial. Attendees outside of Varnish Software, please
ack that this should have a de-facto "send patches" green light or we
can discuss this further next Monday.

It comes in 3 simple steps.

1) Multi-line queries

When someone wishes to capture logs for multiple reasons, they need
to OR them, possibly with brackets. Instead the proposal is to support
one query per line, ignore empty lines, and treat tokens starting with
'#' as comments.

Example:

varnishlog -q '(my-first-query) or (my-second-query)'
varnishlog -q '
my-first-query
my-second-query
...
'

2) Read queries from a file

Similar to reading varnishncsa's format from a file:

cat >bad-things.txt <= 500

# backend problems
...

# internal incident 4253
...

# varnish issue 1799
...
EOF
varnishlog -Q bad-things.txt

3) Query scoping

The current scope is always the transaction group, but sometimes we
may want conditions to apply to a subset of the `-g` option (for
example two fields of the same VSL record).

Proposed solution: inside brackets have the possibility to reduce the
scope with the very first token. The scope cannot outlive the parent
scope, and the query keeps its default @group transaction.

Example:

# beresp cached more than 2m before v_b_r{}
@tag TTL[1] eq RFC and TTL[2] >= 120

The groups would be:

- @group (default when omitted, matches -g option)
- @request (same as -g request)
- @vxid (same as -g vxid)
- @tag (single record)

Things that shouldn't work:

varnishlog -q '@request ...'
varnishlog -q '@tag ... and (@group ...)'
varnishlog -g vxid -q '@request ...'
varnishlog -q '@tag SomeRecord and DifferentRecord'

My opinion was that the current VSL query architecture should
accommodate such changes nicely, and Martin is also not too worried
about it.

4) Bonus topic

I also wanted to put a small performance topic on the agenda: mmap'ing
regular files when reading offline VSL. The current code treats files
(-r option) the same way live SHMLOG VSL is and allocates copies.

The idea is to mmap regular files and avoid needless copies, and keep
the current `-r file` handling for the stdin case. I hope to speed up
processing when it comes to dumps in the GBs.

Someone asked about binary VSL future plans. All of this should be
orthogonal to changes in the underlying VSL format. Nothing should
require an soname major bump in libvarnishapi (minor bump for the -Q
option).

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: backend/director admin states

[Dridi Boukelmoune]

On Fri, May 4, 2018 at 9:42 AM, Poul-Henning Kamp  wrote:
> 

> The reason we have include/tbl/cli_cmds.h in the first place was
> that I wanted some kind of "schema-light" for UX use, but that
> didn't happen.

Yes, that experiment failed, but I somehow succeeded in doing
something similar for varnishtest: a schema for commands and
a generic parser dispatches TheRightThing(tm) to callbacks. It
worked with nested commands too although I only implemented
a dozen in total. The generic parser squared some of the
concurrency issues too by keeping track of all parties.

I don't have time these days for such experiments :(


> I don't see a need for varnishd to ship code to varnishadm though,
> so I'd be happy with a design where varnishd CLI always spits out
> JSON, and varnishadm is responsible for formatting it, if required.

The reason I wanted to centralize the json2txt code is to also ensure
that varnishd -d and varnishtest logs stay readable. If we want
varnishadm to take advantage of its interactive mode to align columns
and whatnots, we can always have a json2matrix step and then render
it.

> As long as somebody who is not me writes the code for it...
>
> PS: We have a solid JSON parser in the tree already.

All we need is TheNewGuy(tm) to dump this on their desk :p

Cheers
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: backend/director admin states

[Dridi Boukelmoune]

>>Maybe we should do that the other way around? Have all CLI commands
>>reply with a JSON data structure, and CLI consumers like varnishadm,
>>varnishtest or varnishd -d be able to output it in plain text (with
>>this code centralized in libvarnish and exposed in libvarnishapi).
>
> I thought about that, but didn't move on it, as I didn't want to
> write all the code myself ;-)

I experimented with this a while back and tried this:

- have a CLI commands reply with a JSON payload
  - same as today: fixed-length size, newline, payload
  - includes metadata such as the response status
- JSON payload includes the "plain text" renderer's name
  - example: "backend.list", "backend.list -p" etc
  - one per command plus variants that change the output format
  - didn't go as far as plain text rendering
- replace the response JSON payload with custom byte code
  - the render chooses between JSON or text rendering
  - byte code is JSON-like, but easier to parse
  - one generic JSON renderer for all commands
  - started experimenting with plain text conversion
- code generation from a schema for each output type
  - example: "backend.list", "backend.list -p" etc (same as before)
  - defensive assertions that let us know when we break the output
  - the idea was to start actually versioning the CLI from 2.0 on
  - and at least bump y in 2.y when we add stuff to the CLI

I never completed the last experiment because the time I allocated
eventually ran out and I lost motivation to nail down the schema part.
This was in the varnishreload [1] time frame, when I realized that
changes to the CLI broke the reload scripts we ship in the [4.1,5.x]
series and since we were maintaining them out of tree we had no easy
way to keep track of that. Which is why I also wanted to import
varnishreload upstream and have proper coverage.

I have thrown the code away, but the idea stuck. And it was a fun
experiment :)

Dridi

[1] https://github.com/varnishcache/pkg-varnish-cache/pull/70
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: backend/director admin states

[Dridi Boukelmoune]

On Thu, May 3, 2018 at 1:04 PM, Poul-Henning Kamp  wrote:
> 
> In message 
> 
> , Guillaume Quintard writes:
>
>>All commands should have a JSON option, I reckon.
>
> Somebody should write more code for it then, I reckon :-)

Maybe we should do that the other way around? Have all CLI commands
reply with a JSON data structure, and CLI consumers like varnishadm,
varnishtest or varnishd -d be able to output it in plain text (with
this code centralized in libvarnish and exposed in libvarnishapi).

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: backend/director admin states

[Dridi Boukelmoune]

> Best proposal for "???" seems to be "auto", even though it is not
> entirely on point.
>
> The implementation can control its view of the health two ways
>
>   A) Call VRT_SetHealth() and set it on the director.
>  (useful for probes as we know them)
>
>   B) Provide a ->healthy() method which will be asked whenever
>  anybody wants to know.
>  (useful for selecting directors like rr/fb/rnd/hash)

"auto" is what we have today in the CLI, and I don't remember ever
being confused by that when I was only a user. And it seems to
still match your A) and B) cases.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Springtime for VDD ?

[Dridi Boukelmoune]

On Fri, Apr 13, 2018 at 9:00 AM, Poul-Henning Kamp  wrote:
> A bugwash or two ago we talked about having a VDD "soonish",
> did anybody do more about that ?
>
> Does anybody want to do more about that ?

As I'm going to be away while the VDD is being organized, I'm dumping
here the topics I'd like to cover for the September release (this is
also a reminder for my future self).

First, actionable items I can/wish to deliver. I have discussed each
with at least one developer from VS and a VDD would be a good time to
quickly present them (as they are quite small in scope) to the rest:

- selinux
- extensions to the VSL query syntax
- (some) performance improvements for VSL consumers
- client side latency

That's in no particular order. If I can't make it to the VDD I'll
either start one thread for each on this list or bring them up during
bugwashes. As those have already been discussed, they shouldn't take
much time to cover. This is more about getting an OK before working on
them now that they have been fleshed out.

There's another topic I'd like to present around connection pools,
only to figure whether I'm not alone thinking this is a problem, and
also figure out new problems it could create. Once again if it doesn't
land on the VDD's agenda I'll either start a thread, open an issue or
bring it up during a bugwash.

Cheers,
Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: UDS decisions

[Dridi Boukelmoune]

On Tue, Feb 13, 2018 at 3:47 PM, Nils Goroll  wrote:
> WFM, but one thing:
>
>> 1. We will use bogo-IP numbers for client UDS connections
>
> As long as we get VCL access to the accept socket name, we should not need the
> uds socket path. But we should have a way to differentiate between
> /untrusted/external.socket and /highly/secure/internal.socket

That would be "named listen addresses" described in the same VIP as
UDS, and currently half-implemented in trunk (names exist, but aren't
usable in VCL).

While at it, WFM too.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: calling for a new workspace sizing concept - Re: auto-tune ws / headroom poc

[Dridi Boukelmoune]

On Tue, Jan 30, 2018 at 2:11 PM, Nils Goroll <sl...@schokola.de> wrote:
> On 30/01/18 14:08, Dridi Boukelmoune wrote:
>> Instead to new xxx_headroom knobs, why not recycle the existing workspace_xxx
>> parameters to take their values _in addition to_ related parameters and maybe
>> document it as such?
>
> I wouldn't oppose to this option, but this would lead to a significant 
> increase
> of actual workspace sizes for those users who are unaware of the change.

Agreed, but in both cases we'd need to change our defaults to match
the change and users should read release notes before upgrading :)

> Using new parameter names and making the old ones read-only would raise
> awareness for the change.
>
> My preference would be the latter, but I'm ok with the former if we reach 
> consensus.
>
> Nils
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: calling for a new workspace sizing concept - Re: auto-tune ws / headroom poc

[Dridi Boukelmoune]

On Tue, Jan 30, 2018 at 12:17 PM, Nils Goroll  wrote:

> * forbid any nested sizing parameters as in "if you tune knob a, you also need
>   to tune knob b" -> knob b should be replaced by a knob b' which is 
> independent
>   of knob a and any derivation should be calculated automatically.
>
> * make workspace sizes read only parameters which users can query to estimate
>   their memory consumption
>
> * replace the workspace tunables with xxx_headroom tuning just the space
>   to remain available on the workspace after deduction of all known 
> allocations

Hi,

Instead to new xxx_headroom knobs, why not recycle the existing workspace_xxx
parameters to take their values _in addition to_ related parameters and maybe
document it as such?

This way the description could tell that workspace_client is the space allocated
to VCL processing on the client side, and possibly (would we need to?) mention
that the total workspace size of a client transaction is "this much".

Knowing the formula would help capacity planning, so documenting it somewhere
sounds sensible all things considered.

I overall agree that we should prevent users from getting a configuration that
guarantees transactions failures.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VFP and VDP configurations

[Dridi Boukelmoune]

>> I think the position should be mapped closer to HTTP semantics
>
> I think this makes too many assumptions? For example, where would security
> processors go? Knowing what I know about whats possible with these things, I
> think the processor universe might be bigger than the 4 categories you
> listed out.

I'm a bit perplex regarding theoretical security processors...

> I think this brings up an important point, which is that for us to be
> successful here, we really need to bring forward some new processors to be
> our seeds for building this new framework. This will drive the requirements
> that we need. I think there will be a lot of uncertainty if we build this
> based on theoretical processors.

...since you explicitly advise against designing for theory.

With the 4 categories I listed I can fit real-life processors in all of them:

- assembly: esi, edgestash, probably other kinds of include-able templates
- content: minification, obfuscation, regsub, exif cleanup, resizing,
watermarking
- encoding: gzip, br
- transfer: identity, chunked, trailers

My examples were VDP-oriented (from storage to proto) but would work
the other way around too (except assembly that I can't picture in a
VFP). You can map encoding and transfer processors to headers:
imagining that both gzip and brotli processors are registered, core
code could pick one or none based on good old content negotiation.

Now where would I put security processors? The only places where it
would make sense to me is content. But then again, please define
security (I see two cases off the top of my head, both would run on
content).

> I think its alright if these new processors
> are simple and our new framework starts off simple as well. This can then
> evolve as we learn more. For me, I have written a handful of processors
> already, so a lot of what I am proposing here comes from past experience.

Sure, with the ongoing work to clarify vmod ABIs this one should
definitely start as "strict" until we get to something stable. However
on the VCL side it is not that simple, because we don't want to break
"vcl x.y" if we can avoid it.

We could mimic the feature/debug parameters:

set beresp.deliver = "[+-]value(,...)*";

A + would append a processor to the right step (depending on where it
was registered), a - would remove it from the pipeline, and a lack of
prefix would replace the list altogether. That would create an
equivalent for the `do_*` properties, or even better the `do_*`
properties could be syntactic sugar:

set beresp.do_esi = true;
set beresp.do_br = true;
# same as
set beresp.deliver = "+esi,br";

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VFP and VDP configurations

[Dridi Boukelmoune]

> - format specifiers:
>
> - have:
>
>   (gzip), (plain) *1), (esi)
>
> - ideas:
>
>   (br), (buffertext) *2)
>
> esi being a format which can contain gzip segments, but that's would
> be opaque to other vfps
[...]
> *1) "(text)" in reza's concept

Or "identity" to match HTTP vocabulary.

> *2) not sure if this is a good idea, maybe multi segment regexen are the 
> better
> idea

For a lack of better place to comment, in my previous message I put
`content` before `assembly`. On second thought it should be the other
way around, otherwise  tags break the content type.

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VFP and VDP configurations

[Dridi Boukelmoune]

> So from VCL, here is how we add VFPs:
>
> VOID add_vfp(VFP init, ENUM position = DEFAULT);
>
> VFP is "struct vfp" and any VMOD can return that, thus registering itself as
> a VFP. This contains all the callback and its input and output requirements.
>
> position is: DEFAULT, FRONT, MIDDLE, LAST, FETCH, STEVEDORE
>
> DEFAULT lets the VMOD recommend a position, otherwise it falls back to LAST.
> FETCH and STEVEDORE are special positions which tells Varnish to put the VFP
> in front or last, regardless of actual FRONT and LAST.

I think the position should be mapped closer to HTTP semantics:

$Enum {
content,
assembly,
encoding,
transfer,
};

The `content` value would map to Accept/Content-Type headers, working
on the original body. The order shouldn't matter (otherwise you are
changing the content type) and you could for example chain operations:

- js-minification
- js-obfuscation

You should expect the same results regardless of the order, of course
the simplest would be to keep the order set in VCL. The `content` step
would feed from storage where the body is buffered.

The `assembly` value would map to ESI-like features, and would feed
from the content, with built-in support for Varnish's subset of ESI.

The `encoding` value would map to Accept-Encoding/Content-Encoding
headers. With built-in support for gzip and opening support for other
encodings. It would feed from the contents after an optional assembly.

The `transfer` value would map to Transfer-Encoding headers, with
built-in support for chunked encoding. ZeGermans could implement
trailers this way.

Would this step make sense in h2? If not, should Varnish just ignore them?

Now problems arise if you have an `encoding` step in a VFP (eg. gzip'd
in storage) and use `content` or `assembly` steps in a VDP for that
same object, or a different encoding altogether. But in your proposal
you don't seem bothered by this prospect. Neither am I, because that's
only a classic memory vs cpu trade off. But it might be hard to implement
the current ESI+gzip optimization if we go this route (or a good reason to
go back to upstream zlib).

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

VIP20: Varnish ABI and packaging

[Dridi Boukelmoune]

Hello everyone,

As requested last week, I started a VIP regarding VMODs, Varnish ABI,
and packaging.

While this is still incomplete, please let me know if anything needs a
clarification so far. The research took me a while so I'll continue
later, probably next week.

https://github.com/varnishcache/varnish-cache/wiki/VIP20%3A-Varnish-ABI-and-packaging

Cheers,
Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: RFC for VIP17: unix domain sockets for listen and backend addresses

[Dridi Boukelmoune]

> OK, that should be documented though (there is other software that
> works with UDSen that does in fact unlink before bind).

There should be a limit to hand-holding. I have bitter memories of
tracking down processes holding unlinked files around and leaving a
partition with no space. The upside was that it introduced me to
lsof(1) at the time.

I would invoke POLA, at least it would feel surprising to me, not
being sure what service I'm connecting to when I pick a path.

>> The question here is more whether we need something like
>> beresp.backend.path in addition to the ip field. Same question for
>> peer credentials, they probably don't make sense for backends (and
>> that would keep the new std functions limited to the listen
>> addresses type).
>
> For the long-winded reasons stated above, I disagree that it doesn't
> make sense. %^) Especially since connect(2) doesn't create the path as
> bind(2) does, getting peer credentials on a backend address is about
> the only thing that could be expressed in VCL about who the peer is
> intended to be, that goes beyond assuming that the admin got it right.

Well, I was cautious enough to say probably. I'm basing my reasoning
on the fact that Varnish tends to trust the backend. It was recently-ish
reconfirmed [1] by phk.

>> Is the question of naming from the original draft still relevant?
>
> That was about VTCP_name, _hisname and _myname, so one way or another
> we'll have to decide what happens with those. We could drop them and
> have them do what they do some other way, and introduce something else
> again where they're currently called, when it's a UDS. git grep on
> those function names currently gets over 30 hits, so that would be a
> bit of a chore. The suggestion in the original draft was to avoid the
> pain -- leave them as they are, and have them create names for UDSen
> in way that doesn't change their interfaces.

I will confess I haven't researched this part thoroughly, that's why I
left it as an open question and linked directly to your original draft.

Dridi

[1] 
https://github.com/varnishcache/varnish-cache/issues/2197#issuecomment-275055034

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: RFC for VIP17: unix domain sockets for listen and backend addresses

[Dridi Boukelmoune]

> +1 I just had a use case for this yesterday, which might also be a general use
> case: cross-container communication (in docker). Sharing a file system with a
> UDS (read only) between container is safe and easy, while configuring a shared
> network between containers is not.

The VIP now covers both -a -T and -M.

> I must say though that this use case calls for more finely grained access
> control for cli connections. Sounds like we could want a cli vcl?

The varnish-cli is already its own language so I don't see how a "cli
vcl" would fit in the picture.

I think that loopback+secret or uds[+secret] is already quite fine.
You need a local access somehow and local credentials with enough
privileges to even use the CLI.

Dridi

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: VSC self-documentation

[Dridi Boukelmoune]

> I have a small, 500 LOC, json parser in C we can use, so that is
> not going to drag in a dependency.
>
> If we're going to bite the JSON bullet, the other places it makes
> sense to use it, is for the vcc_if.c::Vmod_Spec data structure
> which explains the contents of a VMOD to VCC.
>
> Given that we are going to expand what VMODs can contain, that
> "minilanguage" needs expansion too, so all in all, I think we
> are over the threshold were JSON makes sense.
>
> ... but I still feel dirty using JSON to pass data structures
> from one C-program to another, or as it may be, from a python
> program to another.

There are other solutions for this kind of descriptors, and you could
probably get a msgpack [1] subset implementation with the same-ish
amount of code and get the benefits of actual typing (int, string,
etc) and easier parsing.

Dridi

[1] http://msgpack.org/

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: RFC for VIP17: unix domain sockets for listen and backend addresses

[Dridi Boukelmoune]

On Mon, May 15, 2017 at 11:41 AM, Dridi Boukelmoune <dr...@varni.sh> wrote:


Hello,

Making slow progress on the draft, but getting there. While gathering
details from the first v6wash and the original draft, I found that
named listen addresses could once again solve one of the issues.

In this case, that would be ACLs containing paths. With named listen
address you could already implement name-based access control (with
strong typing) and I figured we could do path matching too (via string
conversion).

https://github.com/varnishcache/varnish-cache/wiki/VIP-17:-Enable-Unix-domain-sockets-for-listen-and-backend-addresses/_compare/5b5ca04148c0420789af4006f0f1b2bf1815f53c...caa8cf86af2ab1e8c37f2a80bee29f4a16333898

Thoughts?

Thanks,
Dridi

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: RFC for VIP17: unix domain sockets for listen and backend addresses

[Dridi Boukelmoune]

> First of all (a lesser matter): OK, we can go with commas as a
> separator, with the consequence that UDS paths may not have a comma.
> There is a precedent for that with -sfile.

Yes, we can probably get away with no commas in file names.

Also see this comment from phk:

https://github.com/varnishcache/varnish-cache/issues/2325#issuecomment-301413244

> The more important matter is the mode. *If* we want to do this -- set
> access restrictions on a listen UDS for UID and/or GID --, then if I'm
> understanding the various platform quirks correctly, we can't have a
> mode option. Because if you want to restrict access to a user and/or
> group, then the management process will have to set permissions a
> certain way, and the user cannot have any options.
>
> For connect(2), BSD-derived systems don't care about permissions on
> the UDS path at all. That is, the permissions can look as if you
> couldn't access it, if it were a regular file, but nevertheless
> connect will succeed. However, you can restrict access by restricting
> the directory to which the UDS belongs. (This is what I read in the
> docs, haven't tried it myself.)
>
> With Linux, however, the process that calls connect(2) must have both
> read and write permissions on the path.
>
> I believe that covers all the possibilities for the platforms that
> Varnish supports.

Well, we can always set a mode and let users refer to the UDS docs of
their platforms.

> So it seems to me that this would have to happen (remember that bind
> creates the path):
>
> - - If uid and/or gid are specified in -a
>   - bind and set read access on the containing directory for uid
> and/or gid, no access for other

I'm not fond of fiddling with directories we don't own, for example,
what if the user picks /tmp/varnish.sock?

>   - set read/write access on the path for uid and/or gid, no access
> for other
> - - otherwise (the UDS should be accessible to everyone)
>   - bind and set read/write access for everyone on the path
>
> That would mean that users don't have any options for setting the
> mode, the management process has to control that. Setting mode in -a,
> in the best case, would get it just right, but would most likely screw
> it up.
>
> If you don't do anything, then yes, the permissions on the path
> created by bind depend on umask. But if we want to implement access
> restrictions, then I believe we'd have to do something like this.
>
> Another possibility would leave the "otherwise" case to whatever
> results from umask. Or that we don't implement this feature at all, at
> least not in the next release. But in either case, I'm not sure if
> "leave it to the umask" is a good idea, it might result in other
> processes being unable to connect, forcing users to set a umask before
> starting Varnish. They would have to know to do that, for one thing,
> and they might have to set a wide-open umask that we wouldn't want
> otherwise.

My take on it is do nothing unless a mode was specified when the
socket is created. Simple for us, leaving the users in control.

> (Incidentally, we'll have to decide what Varnish does if the UDS path
> already exists before the bind, which causes bind to throw EADDRINUSE.
> unlink and then bind, or throw the error, requiring users to make sure
> it doesn't exist before starting Varnish?)

If we can't bind, we should just fail the startup. Varnish shouldn't
do the sysadmin's job.

Cheers,
Dridi

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: RFC for VIP17: unix domain sockets for listen and backend addresses

[Dridi Boukelmoune]

>> So I say do it right from the beginning this time, and make it
>> possible to use relative paths and just change the uds_path parameter
>> when you have to.
>
> Define "when you have to", as I said earlier listen addresses are
> immutable once the manager is started. That's a condition that
> we must hold since we are to expose them in VCL.

Just when I decide to move to something else for the rest of the day I
get hit hard with this thought:

A uds_path would actually make sense for backend definitions, so
actually I'm not against the idea. For the reasons exposed previously
I still think we should stick to absolute paths on the client/listen
side.

Cheers,
Dridi

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: [master] 3d60db6 Sort out some signed/unsigned mismatches

[Dridi Boukelmoune]

> We'll talk about that as we approach the next release.  There will
> be a lot more API work before then.

I already mentioned it in the #2318 ticket, I figured it belonged to
the 6.0 planning ;)

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

6.0 planning planning

[Dridi Boukelmoune]

Hello,

Next Monday we're supposed to find out what we want to plan for
September:

https://github.com/varnishcache/varnish-cache/issues/2318

Next Monday is also May 1st, a holiday. Does Tuesday 2nd at the
usual bugwash time work for everyone?

Cheers,
Dridi

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: RFC for VIP17: unix domain sockets for listen and backend addresses

[Dridi Boukelmoune]

> # Synopsis
> Allow unix domain sockets as a listen address for Varnish (``-a``
> option) and as addresses for backends, and for use in ACLs. Obtain
> credentials of the peer process connected on a UDS, such as uid and
> gid, for use in VCL.

Except for ACLs, I find the idea compelling so far. I would even like
to see UDS support for admin sockets (-T option).

> # Why?
> * Eliminate the overhead of TCP/loopback for connections with peers
> that are colocated on a host with Varnish.

Yes.

> * Restrict who can send requests to Varnish by setting permissions on
> the UDS path of the listen address.

The whole point of UDSs IMO.

>   * (But see the discussion below about getting this right portably.)
> * Make it possible for a backend peer to require restricted
> credentials for the Varnish process by setting permissions on the UDS
> path on which it listens.

It is technically possible to implement a UDS backend if one is brave
enough to re-implement all the VBE logic. So I'm strongly in favor of
having this capability in varnishd.

> * Peer credentials make it possible to:
>   * Make information about the peer available in VCL and the log.

Why not, no opinion.

>   * Extend ACLs to make it possible to place further restrictions on
> peers connecting to the listen address.

I would use a regex instead of messing with ACLs.

<...snip...>

> I would like to make this contribution for the September 2017 release.
> With the VIP I'd like to clarify:
>
> * Are there any changes planned for VTCP and VSA in the September
> release that would make adding UDS to those interfaces less trivial
> than it is now?

I wouldn't mix UDS with VSA, there's probably room for a different solution.

> * Every platform has a way to get peer credentials from a UDS, but
> there's no standard and it's highly platform-dependent. So how do we
> want to handle that?

Maybe we could start by not having them, being able to use UDSs is
already a huge win IMO.

> * Additions/changes to VCL and other changes in naming, such as the
> ``-a`` option and backend definitions.

I don't think we need to change -a or -T, as long as we force absolute
names we should be able to get away with the current syntax. An address
starting with a slash (/) would denote a UDS. [1]

> * If someone knows a reason why we shouldn't do this at all, this is
> the place to say so.
>
> # How?
> ## Address notation
> I suggest that we require a prefix such as ``unix:`` to identify UDS
> addresses (nginx uses ``unix:``, haproxy uses ``unix@``):
> ```
> varnishd -a unix:/path/to/uds

This should be enough:

varnishd -a /path/to/uds

> backend uds { .host = "unix:/path/to/uds"; }

I would instead go for a .path field mutually exclusive with .host and
.port, removing ambiguity at vcl.load-time (error messages etc).

> ```
> That makes the interpretation unambiguous. We could simply interpret
> paths as UDS addresses when they appear in those places, but then we
> would need logic like: if the argument cannot be resolved as a host or
> parsed as an IP address, then assume it's a path for UDS, but if the
> path does not exist or cannot be accessed, then fail. So better to
> just make it unambiguous.

As I said earlier, I think a slash [1] is enough to remove ambiguity.

> Parsing UDS addresses would be an extension of ``VSS_Resolver``.

Not if we don't mix paths with IPs/domains

> The name ``.host`` in a backend definition becomes a bit peculiar if
> its value can also be a UDS (we will see a number or examples like
> this). We could:
>
> * stay with the name ``.host``, and document the fact that it might
> not identify a host in some cases
> * replace ``.host`` with a name like ``.peer``, sacrificing backward
> compatibility
> * introduce ``.peer``, retain ``.host`` as a deprecated alias, and
> remove ``.host`` in a future release
>
> I suggest the last option, comments welcome.

Once again, I suggest we don't mix them up so that we don't need to
break anything. I also find .peer ambiguous.

> ``.port`` in a backend definition is already optional, and is
> unnecessary for a UDS. Should it be an error to specify a port when a
> UDS is specified, or should it be ignored? Comments welcome.

As stated above, mutually exclusive with the .path field.

> ## Access permissions on the listen address
> For the ``-a`` address, I suggest an optional means of specifying who
> can access the UDS:
> ```
> varnishd -a unix:/path/to/uds:uid=foo,gid=bar
> ```
> There's an issue here in that the separator (``:`` in the example)
> could not appear in any UDS path. We might just have to forbid a
> certain character in UDS paths. Fortunately we don't have a such a
> problem with backend addresses (which are generated by another server,
> so we have less freedom to impose restrictions on the path names).

I would use the comma separator like -j and -s options for jails and
storage backends. Possibly named parameters like in -j so that order
doesn't matter. But that means breaking the 

Re: sub probe_resp - VIP RFC

[Dridi Boukelmoune]

> # Why?

No opinion on the why, I get the point and I see the benefits but no
opinion. It also leaves out the non-VBE backends, although as of today
I m only aware of Martin's fsbackend implementation of an out-of-tree
backend.

> * Add a default vcl_probe_resp sub to the builtin.vcl which implements the
>   current behavior. Probes without an explicit response sub definition will
>   use vcl_probe_resp

I'd rather have vcl_probe_response, akin to existing v_b_r.

> * in probe_resp context, make the following objects available
>
>   - analogous to beresp
>
> - prresp.backend
> - prresp.http.*
> - prresp.proto
> - prresp.status
> - prresp.reason

Why not beresp? It's a response from the backend, I don't think
reusing the name would be confusing.

> * a probe_resp sub may return with the following
>
> healthy
> sick

Or we could reuse "ok" and the universal "fail" like in vcl_init.

> * The default vcl_probe_resp:
>
> sub vcl_probe_resp {
> if (prresp.proto ~ "^HTTP/\d+\.\d+$" &&

^HTTP/1\.[01]$

Dridi

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

SVG change for hitpass

[Dridi Boukelmoune]

Hi,

Here is a partial diff of the change I'm ready to push if the result
is OK for everyone:

-label="[...]{deliver|{304?|other?"
+label="[...]{deliver or pass|{304?|other?"

Please find the resulting SVG attached. It doesn't follow the usual
pattern but I don't think having two boxes with "304?/other?" sub
boxes (or whatever it is called) would bring value.

And there's still return(fail) to sneak in the diagrams.

Thoughts?

Dridi
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: Commits digest

[Dridi Boukelmoune]

> Do I need to join the list? I'm just lazy and read the git logs, would
> rather not get the spam (too much of that as it is).

Dunno about your email setup but with my VS address I have no
problem setting up filters to deal with lists (in my case I tag them
as commits). Once you're sure it won't get in the way, please join.
It's very convenient to follow what's happening.

Cheers

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Commits digest

[Dridi Boukelmoune]

Hello,

Do the keepers of the blessed script that sends git commit emails
know why sometimes commit don't land in varnish-commit?

The only pattern I have identified so far is that we don't get Geoff's
commits, see the archive for this month so far:

https://www.varnish-cache.org/lists/pipermail/varnish-commit/2017-March/date.html

Dridi

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: [master] cbe8047 Reintroduce hit-for-pass with new and better syntax:

[Dridi Boukelmoune]

On Wed, Feb 22, 2017 at 2:04 PM, Poul-Henning Kamp  wrote:
>
> commit cbe804760d78b917fbbb8a1d69b2236a2d98a028
> Author: Poul-Henning Kamp 
> Date:   Wed Feb 22 10:40:58 2017 +
>
> Reintroduce hit-for-pass with new and better syntax:
>
> sub vcl_backend_response {
> return (pass(2s));
> }

We need to clear the confusion of turning hitpass into a hitmiss in
5.0, while keeping the MAIN.cache_hitpass counter as is. Especially
now that hitpass lives alongside hitmiss.

Also, should the built-in VCL make a distinction between hitmiss and
hitpass? For example if the response contains a Set-Cookie header it
might be a one-time thing and hitmiss would be preferred, otherwise it
looks like something reliably not cacheable: so hitpass.

Thoughts?

Cheers

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: [master] a4e07fc Correct this test for good

[Dridi Boukelmoune]

> As for the solution to this particular problem, I find it quite
> elegant naming aside.

Except that it fails on FreeBSD and SunOS, I'm pushing a revert :(

Dridi

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: [master] a4e07fc Correct this test for good

[Dridi Boukelmoune]

On Sat, Jan 28, 2017 at 7:03 PM, Federico G. Schwindt  wrote:
>
> commit a4e07fc5a611062afddd2fb114d43d793054c15f
> Author: Federico G. Schwindt 
> Date:   Sat Jan 28 14:38:50 2017 +
>
> Correct this test for good
>
> (Ab)use the bound address used for the bad_backend macro to expose a
> random port and avoid the dreaded collisions.

This is a topic I actually wanted to bring up to the next bugwash (I
couldn't attend last Monday) and while ${bad_backend} is convenient
for testing, it relies on brittle behavior from the way varnish does
host/port resolution. It doesn't question tests relying on this macro
and the new ${random_port}. I found both names misleading too.

As for the solution to this particular problem, I find it quite
elegant naming aside.

Dridi

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: [master] b6d0ff8 Merge tests and reshuffle things

[Dridi Boukelmoune]

On Fri, Jan 27, 2017 at 9:21 PM, Federico Schwindt  wrote:
> Not happy about hiding the error but it will do it for now.
>
> The tests are not using a hardcoded port anymore and should fail with the
> expected error now.

Your test case is misleading because it pretends to be safe, but either
${v1_port} is bound when you're using it the test can't [1] possibly pass,
or it's available for anyone to bind and you run into the same
concurrency problem again.

> Why this is a problem in sunos is beyond to me at the moment.

SunOS, FreeBSD, and my Linux box. I tried to get rid of the hard-coded
port too but didn't manage, so I kept it as close as the original test.

This is very simple: varnishtest doesn't offer enough isolation to safely
run this test, especially with concurrent jobs.

Dridi

[1] Except maybe with SO_REUSEPORT, but... can of worms?

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: [master] b6d0ff8 Merge tests and reshuffle things

[Dridi Boukelmoune]

On Fri, Jan 27, 2017 at 9:11 PM, Dridi Boukelmoune <dr...@varni.sh> wrote:
> On Fri, Jan 27, 2017 at 7:37 PM, Federico G. Schwindt <fg...@lodoss.net> 
> wrote:
>>
>> commit b6d0ff84fabfce8bffad426c02421eb891ef6a7b
>> Author: Federico G. Schwindt <fg...@lodoss.net>
>> Date:   Fri Jan 27 18:35:22 2017 +
>>
>> Merge tests and reshuffle things
>
> Instead of shuffling things *right after* I stabilized them we could
> have discussed this. It now fails even more frequently. And yes I
> know my fix would hide errors (not actual test failures) but at least
> it was better than noise considering the hard-coded ports.
>
> I suggest a revert of this commit and then a reshuffle without the
> merge of r1813. I'm aware it is covering varnishd usage but it's also
> a regression test so I can live with "have same address" being its own
> test. For some systems vtest is currently reporting 100% of failures
> caused by c3.vtc for 9a652c7.

I see you've fixed it again while I was writing this message.

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Re: autoconf madness

[Dridi Boukelmoune]

On Thu, Jan 19, 2017 at 6:00 PM, Poul-Henning Kamp <p...@phk.freebsd.dk> wrote:
> 
> In message 
> <CABoVN9DKswgGkEVQ+Jb-HUvx1PNyyPc0o5+nrzZ=r732q9g...@mail.gmail.com>
> , Dridi Boukelmoune writes:
>
> Yes, we are probably poor users of autocrap.
>
> But I don't want to make excuses for that.
>
> We're trying to build 8 small binaries, three static and four dynamic
> libraries.
>
> We should not have to even think about build infrastructure *at all*.
>
> *It* *should* *just* *work*.

I'm afraid compilers don't make it easy when it comes to just working
and portability. At least on the dependency management side pkg-config
does a great job (if *.pc files are properly written).

> And that is why I want to get rid of autocrap:  It doesn't just work,
> not even close.

There are things from autotools themselves and from the
autoconf-archive that certainly don't just work, or even work.

>>Also please note that Guillaume once submitted a patch for a
>>non-recursive automake build, which I think would also help contain
>>the complexity^Wmadness.
>
> Absolutely, and I'd love to have that going in.

So what's the goal? Are we still getting rid of autocrap?

> The only change I ask is that each of the lib/bin subdirectories
> have a trivial makefile along the lines of:
>
> *:
> cd ../.. && make $(*)
>
> (no, make(1) doesn't grok that, but you get the idea...

Yes, although make's design was very spot on IMO it also lacks many
useful things today. As for myself, I haven't felt the need to cd into
a sub-directory for a long time, so I would no longer hurt from having
a single Makefile.

Dridi

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev