[WIP/RFC v4 0/5] Support abstract Unix sockets

[Maxim Dounin]

Hello!

On Wed, Aug 24, 2022 at 08:15:40PM +0200, Alejandro Colomar wrote:

> Hello Maxim!
> 
> On 8/24/22 18:00, Maxim Dounin wrote:
> [...]
> >> For having nginx as a reverse proxy in front of a server, such as unit,
> >> it's faster to communicate through Unix sockets (UDS), rather than TCP
> >> (localhost).
> > 
> > I would rather say "it's a common misconception that it's faster
> > to communicate through Unix sockets".  While it Unix sockets can
> > be beneficial for some microbenchmarks,
> 
> Yeah, I don't know how much of an improvement it will really bring.  I'm 
> going to develop some benchmark soon, as a side effect of another task, 
> but it will still be a bit artificial, and not a real load.  But it will 
> help see how much of an improvement UDS have in the context of Unit.
> 
> > in most production setups
> > it makes no difference, yet used to introduce various issues.
> 
> The only one I can think of is security, since you don't have file 
> permissions, but one could also argue that the security can't be lower 
> than that of TCP sockets.

Apart from being non-scalable to multiple hosts and various issues 
with socket files, Unix sockets used to trigger various kernel 
bugs in different OSes.  In particular, there were multiple 
sendfile() issues, at least on FreeBSD and IIRC on Linux.

> >> Nginx already supports UDS.  But UDS has a problem: the
> >> kernel creates a file in the fs, and it's not always trivial to clean up
> >> those files.  Then, if the application is restarted, there's no
> >> SO_REUSEADDR to allow reusing the socket file, so the application will
> >> just fail.
> >>
> >> This happens in nginx Unit, which creates listener sockets from a
> >> privileged thread, and then uses them from unprivileged threads.  When
> >> the unprivileged thread stops using the socket, it can't remove the
> >> file, and doing so would require huge complexity to implement.  It's
> >> easier to just tell the kernel we want an abstract UDS (AUDS), so that
> >> there's no file at all.  Then if the user restarts Unit, it'll be able
> >> to recreate the AUDS.
> >>
> >> A user reported this problem with normal UDS and we concluded that the
> >> easiest solution would be to add support for AUDS.  His set-up is a
> >> kubernetes pod, where a container uses nginx and another container uses
> >> Unit.  Communicating through an AUDS would be trivial and fast.
> > 
> > So, you are trying to implement abstract namespace sockets as a
> > bandaid for Unit bug, which is not able to properly remove Unix
> > sockets in some cases and fails to restart, correct? 
> 
> Yeah, it helps avoid fixing that bug.  But that's why the Linux kernel 
> implemented that feature in the first place: so that programs don't need 
> to cleanup the filesystem in these cases, recognizing that it may not be 
> always trivial.
> 
> > This does
> > not look like a valid reason to me, especially given that a)
> > abstract namespace sockets are Linux-only, and
> 
> Since other systems can use localhost, I don't think it's so important 
> to support abstract sockets in them (it's not losing a big feature, but 
> rather a minor improvement).  What Unit (and some other web servers do) 
> is just report an error in the configuration.

The point is that you'll have to fix the bug anyway, since the bug 
is there and, even assuming the workaround is good enough, it's 
Linux-specific.

If you are looking for a workaround, a readily available one is 
using TCP sockets.

> > b) there are
> > multiple approaches to filing the abstract socket address.
> 
> I don't understand this last point.  Do you mean the NUL and @ 
> alternatives?  NUL is the one supported by the kernel, and so the most 
> obvious one.  Since NUL is not usable by most tools, most programs use 
> the alternative syntax @.  Some programs only allow @; but since it 
> doesn't hurt to allow NUL, and it makes for a very easy implementation, 
> I'd do it that way.

Since null bytes in the socket address are not special, there is 
more than one way to fill the sockaddr structure.  The patch I've 
referenced fills the struct sockaddr_un's sun_path with null 
bytes, and uses socklen set to sizeof(struct sockaddr_un): this 
approach is compatible with HAProxy's abns@ and socat with 
unix-tightsocklen=0.  In contrast, curl uses different approach 
and instead uses socklen set to exact length of the name provided 
with leading null bytes and no trailing null bytes (and this is 
what you've tried in your patches).

-- 
Maxim Dounin
http://mdounin.ru/