-
Notifications
You must be signed in to change notification settings - Fork 374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Unix domain sockets as -a listen addresses and backend addresses #2371
Conversation
sockaddr_un storage management, VSAs and session workspaceA VSA now looks like this:
So the UDS member is a pointer to
When Varnish starts, When a session is created for a connection accepted via UDS, its VSA is duplicated as the So we actually use less session workspace for UDSen -- one VSA for |
Backends and TCP pools
The second pointer points to
VMOD debug has been extended to support dynamic creation of backends that listen at UDSen via Edit: |
VTCP_*nameThese are called about 30 times in the code, and are conceived for IPs/ports, so it's a chore to get them right for UDS. The solution here is:
But that by itself doesn't solve all of the problems, because So the code has to decide, on a case-by-case basis, whether we're dealing with a UDS, and whether it's bound or connected, and do the right thing. That in turn requires knowing that it's a UDS, which isn't always possible (the |
VSL outputsThe output formats are consistent with formats used for IP addresses -- there is always the same number of non-whitespace positions in the output. But some of those positions may be
|
-a path creation and deletion
Precedents:
I'm a bit of wary of deleting an existing path at startup -- someone might think they're using it for something, and Varnish comes along with root permissions and deletes it. But if we were to do it the nginx way, then:
That last is the reason why nginx fails to restart when you use |
X-Forwarded-For and backend Host headersThis implementation leaves X-F-F unchanged if the client connection came over a UDS. Maybe we should add something like Precedents: with haproxy and nginx, you get whatever you get when you have a configuration like In backend definitions thus far, if Maybe we should require Because of the fact that |
vtcTo test Varnish with a UDS listen address, call it with The The easy way to do this is to use
If a server is listening at UDS, then both If a client or server is using a UDS address, then the vtc variables However, I noticed while doing that that |
Socket optionsThe table of socket options in Socket options not set for UDS are currently exactly the same as those with level is |
PROXY protocolThe PR has code to send UDS addresses to a backend for PROXYv2, but there are no tests for that yet. At this point I don't think we should support receiving UDS addresses via PROXY:
We could theoretically support receiving UDS addresses via PROXYv2, but I can't think of any place they could go besides session workspace, with all of the IMO this should be something for a future iteration. |
HTTP/2There is one test for HTTP/2 over UDS addresses -- It seems that the socket address shouldn't matter to the transport protocol. But I'm afraid that I don't really grok HTTP/2 or the vtc commands for it, so I am unaware of any surprises that may turn up after all. Someone who knows HTTP/2 better than I do should put some thought to that. |
Backend_healthMay now emit a |
*.ip, std.ip() and std.port()All of the I didn't see the point in Because of the fact that the I updated
The potential NULL valiue for the
is always false for every x. That is, all of the |
ACLsUDS addresses don't work in any way with ACLs. They can't be listed in an ACL, and matching a |
getaddrinfoOn Linux, AF_UNIX addresses are not supported for getaddrinfo, but they are on FreeBSD. That is, if the string provided happens to match a path that is a socket, then FreeBSD getaddrinfo will return a sockaddr-as-sockaddr_un for it. In a few places in the code, getaddrinfo is called with the intent of converting a string to an IP address, for example in the IP resolver called by VCC or in |
Weirdnesses
|
I tried this on FreeBSD and sent geoff email with the results. |
8adecfe
to
334055e
Compare
Consensus from bugwash: merge after next release, re-think the VCL interface |
Just to detail the discussion yesterday: The main worries is if the VCL side of this is where we want to go, and the plan is to start working on this right after 5.2 is out. |
… path. On FreeBSD, getaddrinfo(3) may convert a path to an AF_UNIX sockaddr.
Note that as a consequence of the *.ip variables returning NULL for UDS, the fallback argument to std.ip() may now be NULL.
Otherwise, since client.ip is NULL for a UDS, unset client.identity would always return NULL, and hence would be the same for every client. That would undermine the purposes it's meant for, such as the hash director.
These are NULL (match <undef>) for UDSen.
NULL for an IP address (matches <undef>), the path for a UDS.
There's currently only one left, e826 in cache_acceptor.c, due to assigning void * from VSA_Get_Sockaddr to struct sockaddr_un *.
This affects the log output for BackendStart.
Shouldn't we close this one? |
I was keeping it open for reference, since it's not entirely OBE yet (the backend part is still pending), but yes it was going to be closed eventually. We can of course look back at a closed PR, same difference. |
As I see it, the backend side is happening in #2579. I just went through some triage frenzy so if anything should be kept open, don't mind me and reopen it. |
This is a partial implementation of the ideas proposed in VIP17.
In VIP17 but not implemented yet:
-T
and-M
args).uds_path
as a search path for relative paths in backend definitions./
.Update: At the end of VIP17 (section "Implementation"), I've written down some proposals for how these features could be implemented --
-T
and-M
,uds_path
, and additions to VMOD std.The big picture:
-a
addresses can be specified as:If you specify any of
user
,group
and/ormode
(mode in octal), then the path will be created with those permissions. (The sub-args can appear in any order.)A backend definition may have:
.path
XOR.host
must be specified for a backend.You can also use
-b /path/to/uds
to define a backend on the command line.This code has been tested on:
There are quite a few details and issues worth considering for a review. So as to break it up into small pieces, I'll add more comments about those issues (and point out some interesting quirks about UDSen that I discovered along the way).
Update: Branches with the fork rebased to Varnish 5.2.0 and 5.2.1 live here:
RPMs for el7 for the two forks live at: https://packagecloud.io/uplex/varnish-uds