First implementation of backends@UDS - slimhazard/varnish-cache GitHub Wiki

This commit is a first attempt at implementing Unix domain sockets as backend addresses. As with the code for listen addresses, this does not cover everything that needs to be done, and not all of the ideas proposed in VIP17. It's meant to be a basis for discussion, to decide if this is conceptually the way we want to go.

As with the listen addresses, this implementation does not require a VSA destructor, but rather defines an "owner" of the storage for sockaddr_un, who is responsible for freeing it. In this case, the owner is struct backend, and the sockaddr is freed in VBE_Delete.

The code successfully connects to and fetches from a backend listening at a UDS, and passes all of make check. v00038.vtc is updated to check the modified syntax for backend definitions, but otherwise the tests are not updated for UDS. That will have to be done, for the listen addresses as well, but varnishtest must be updated to support UDSen.

VCL

A .path field has been added, and a backend def must have exactly one of .path or .host. Since there are no issues of resolving IP addresses, VCC does not go through all of the stress of resolving and creating a binary suckaddr; the creation of the VSA for UDS is deferred to VRT_new_backend. VCC does check if the path is too long for a legal UDS.

The backend is created so that either the path is NULL or both of the IP suckaddrs are NULL.

struct backend

struct backend has two additional fields uds_suckaddr and uds_addr. Both are NULL if the backend uses IP addresses, otherwise uds_addr points to sockaddr_un, and VBE_* is responsible for allocating and freeing the sockaddr_un.

In VRT_new_backend, if the path is not NULL, the UDS VSA and sockaddr_un are allocated and assigned to uds_suckaddr and uds_addr. The path is also checked for legal length for a UDS, since VRT_new_backend may be called to dynamically create a backend. When a TCP pool is created for the UDS VSA, in which the VSA is duplicated, they point back to the sockaddr_un in uds_addr.

VBE_Delete frees both the UDS VSA and the sockaddr_un.

TCP pools

TCP pools are now identified by either a pair of IP addresses (as before) or a UDS address. VBT_Ref now has two additional arguments for the UDS VSA and the pointer to sockaddr_un from struct backend. If it's UDS, then the duplicated VSA in the TCP pool points back to the sockaddr_un owned by struct backend. When a TCP pool is deleted, it frees the duplicated VSA, but does not touch the sockaddr_un.

Other issues, weirdnesses and TBDs

  • Previously, if .hosthdr is not set in a backend defintion, we set it to the value of .host. But how do we set the Host header if there is no .host, but rather a .path? In this implementation, .hosthdr gets set to localhost in this case.

  • This implementation currently does not check if the path begins with /. I believe there would be an assertion failure otherwise (haven't tested it). Some of the options are:

    • Always require / at the start of the path, hence always require an absolute path.

    • Allow paths not starting with /, and the path will located in wherever the CWD is for the child process.

    • Add the param uds_path discussed in VIP17, and search for the socket in that path.

  • VCC could do some stricter checking: Ensure that the path exists, stat it, and ensure that it's a socket. A UDS MUST exist prior to attempting connect(2), otherwise the connect fails. So VCC could do more to ensure that there is indeed something to connect to (a bit like resolving host names and failing the compile if the resolve fails). Currently, if the UDS does not exist or is not a socket, the VCL loads, but every fetch results in no backend connection.

  • I haven't done anything yet about the contents of BackendOpen and BackendStart in the log when the backend is UDS. Both of these get their contents from VTCP_*name, which are not conceived with UDSen in mind (they expect appropriately sized buffers for an IP address and port), and the result isn't right:

-   BackendOpen    25 boot.u localhost /var/run/nginx. localhost 
-   BackendStart   localhost /var/run/nginx.

The UDS path was actually /var/run/nginx.sock, but it got truncated to the length of a port number buffer.

The omnipresence of VTCP_*name makes for a messy problem in the code. git grep -e 'VTCP[a-zA-Z_]*name' tells me that there are 34 matches in the repo. Mostly they're needed for some kind of output: SHM log, CLI, varnishtest log and macros and VRT_IP_string. Also for the PROXY protocol line. Doing something like if (it's UDS) do_this; else do_that; in all of these places would be a mess, we'll need a newly conceived solution.

Update: It gets better. If a UDS is bound (for listening), then getsockname (called by VTCP_myname) returns the path, but the address returned from getpeername (called by VTCP_hisname) is unnamed, i.e. the sun_path field in sockaddr_un contains garbage. If the UDS is connected (as in to a backend), then getpeername returns the path, but the address returned from getsockname is unnamed.

This means that for BackendOpen for example, for which both VTCP_myname and VTCP_hisname are currently called to emit both the local and peer addresses, only VTCP_hisname can be used when connected to a backend via UDS.

That in turn means that VSL output formats will have to change incompatibly. Since the September release is meant to be a major release, we can do that, but we'll be having different output formats for some tags depending on whether a UDS or IP socket is in use. Which may complicate some things for varnishncsa.

At this point, I don't see any alternative to grimly going through all of the 30-odd uses of VTCP_*name and deciding on a case-by-case what to do when the socket is UDS.