[systemd-devel] [PATCH] Add sabridge for socket activation of traditional daemons
Zbigniew Jędrzejewski-Szmek
zbyszek at in.waw.pl
Mon Oct 14 22:10:22 PDT 2013
On Mon, Oct 14, 2013 at 04:44:23PM -0700, david at davidstrauss.net wrote:
> From: David Strauss <david at davidstrauss.net>
>
> ---
> .gitignore | 1 +
> Makefile-man.am | 1 +
> Makefile.am | 20 +-
> man/systemd-sabridge.xml | 264 ++++++++++++++++++++++++
> src/sabridge/Makefile | 28 +++
> src/sabridge/sabridge.c | 519 +++++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 832 insertions(+), 1 deletion(-)
> create mode 100644 man/systemd-sabridge.xml
> create mode 100644 src/sabridge/Makefile
> create mode 100644 src/sabridge/sabridge.c
>
> diff --git a/.gitignore b/.gitignore
> index 5e63b2a..d2d5da5 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -71,6 +71,7 @@
> /systemd-reply-password
> /systemd-rfkill
> /systemd-run
> +/systemd-sabridge
> /systemd-shutdown
> /systemd-shutdownd
> /systemd-sleep
> diff --git a/Makefile-man.am b/Makefile-man.am
> index 6c9b790..e78a8a2 100644
> --- a/Makefile-man.am
> +++ b/Makefile-man.am
> @@ -66,6 +66,7 @@ MANPAGES += \
> man/systemd-nspawn.1 \
> man/systemd-remount-fs.service.8 \
> man/systemd-run.1 \
> + man/systemd-sabridge.1 \
> man/systemd-shutdownd.service.8 \
> man/systemd-sleep.conf.5 \
> man/systemd-suspend.service.8 \
> diff --git a/Makefile.am b/Makefile.am
> index 6601244..ce740ef 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -5,6 +5,7 @@
> # Copyright 2010-2012 Lennart Poettering
> # Copyright 2010-2012 Kay Sievers
> # Copyright 2013 Zbigniew Jędrzejewski-Szmek
> +# Copyright 2013 David Strauss
> #
> # systemd is free software; you can redistribute it and/or modify it
> # under the terms of the GNU Lesser General Public License as published by
> @@ -298,7 +299,8 @@ bin_PROGRAMS = \
> systemd-detect-virt \
> systemd-delta \
> systemd-analyze \
> - systemd-run
> + systemd-run \
> + systemd-sabridge
>
> dist_bin_SCRIPTS = \
> src/kernel-install/kernel-install
> @@ -3213,6 +3215,22 @@ EXTRA_DIST += \
> units/systemd-journal-gatewayd.service.in
>
> # ------------------------------------------------------------------------------
> +
> +systemd_sabridge_SOURCES = \
> + src/sabridge/sabridge.c
> +
> +systemd_sabridge_LDADD = \
> + libsystemd-shared.la \
> + libsystemd-logs.la \
> + libsystemd-journal-internal.la \
> + libsystemd-id128-internal.la \
> + libsystemd-daemon.la \
> + libsystemd-bus.la
> +
> +systemd_sabridge_CFLAGS = \
> + $(AM_CFLAGS)
> +
> +# ------------------------------------------------------------------------------
> if ENABLE_COREDUMP
> systemd_coredump_SOURCES = \
> src/journal/coredump.c
> diff --git a/man/systemd-sabridge.xml b/man/systemd-sabridge.xml
> new file mode 100644
> index 0000000..d7afb0e
> --- /dev/null
> +++ b/man/systemd-sabridge.xml
> @@ -0,0 +1,264 @@
> +<?xml version="1.0"?>
> +<!--*-nxml-*-->
> +<!DOCTYPE refentry PUBLIC "-//OASIS//DTD DocBook XML V4.2//EN"
> + "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd">
> +<!--
> + This file is part of systemd.
> +
> + Copyright 2013 David Strauss
> +
> + systemd is free software; you can redistribute it and/or modify it
> + under the terms of the GNU Lesser General Public License as published by
> + the Free Software Foundation; either version 2.1 of the License, or
> + (at your option) any later version.
> +
> + systemd is distributed in the hope that it will be useful, but
> + WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public License
> + along with systemd; If not, see <http://www.gnu.org/licenses/>.
> +-->
> +<refentry id="systemd-sabridge">
> + <refentryinfo>
> + <title>systemd-sabridge</title>
> + <productname>systemd</productname>
> + <authorgroup>
> + <author>
> + <contrib>Developer</contrib>
> + <firstname>David</firstname>
> + <surname>Strauss</surname>
> + <email>david at davidstrauss.net</email>
> + </author>
> + </authorgroup>
> + </refentryinfo>
> + <refmeta>
> + <refentrytitle>systemd-sabridge</refentrytitle>
> + <manvolnum>1</manvolnum>
> + </refmeta>
> + <refnamediv>
> + <refname>systemd-sabridge</refname>
> + <refpurpose>Inherit a socket. Bidirectionally
> + proxy.</refpurpose>
> + </refnamediv>
> + <refsynopsisdiv>
> + <cmdsynopsis>
> + <command>systemd-sabridge</command>
> + <arg choice="opt" rep="repeat">OPTIONS</arg>
> + <arg choice="plain"><replaceable>HOSTNAME-OR-IP</replaceable></arg>
> + <arg choice="plain"><replaceable>PORT-OR-SERVICE</replaceable></arg>
> + </cmdsynopsis>
> + <cmdsynopsis>
> + <command>systemd-sabridge</command>
> + <arg choice="opt" rep="repeat">OPTIONS</arg>
> + <arg choice="plain"><replaceable>UNIX-DOMAIN-SOCKET-PATH</replaceable>
> + </arg>
> + </cmdsynopsis>
> + </refsynopsisdiv>
> + <refsect1>
> + <title>Description</title>
> + <para>
> + <command>systemd-sabridge</command>provides a proxy
> + to socket-activate services that do not yet support
> + native socket activation. On behalf of the daemon,
> + the proxy inherits the socket from systemd, accept
accepts
> + each client connection, opens a server connection
opens a connection to the server
> + for each client, and then bidirectionally forwards
> + data between the two.</para>
> + <para>This utility's behavior is similar to
> + <citerefentry><refentrytitle>socat</refentrytitle>
> + <manvolnum>1</manvolnum> </citerefentry>.
We tend to keep <citerefentry>s as one line...
> The main
> + differences for <command>systemd-sabridge</command> are
> + support for socket activation with
> + <literal>Accept=false</literal> and an event-driven
> + design that scales better with the number of
> + connections.</para>
> + </refsect1>
> + <refsect1>
> + <title>Options</title>
> + <para>The following options are understood:</para>
> + <variablelist>
> + <varlistentry>
> + <term>
> + <option>-h</option>
> + </term>
Please make those <term>s one line for consistency with the rest of pages.
> + <term>
> + <option>--help</option>
> + </term>
> + <listitem>
> + <para>Prints a short help
> + text and exits.</para>
> + </listitem>
> + </varlistentry>
> + <varlistentry>
> + <term>
> + <option>--version</option>
> + </term>
> + <listitem>
> + <para>Prints a version
> + string and exits.</para>
> + </listitem>
> + </varlistentry>
> + <varlistentry>
> + <term>
> + <option>
> + --ignore-env</option>
> + </term>
> + <listitem>
> + <para>Skips verification of
> + the expected PID and file
> + descriptor numbers. Use if
> + invoked indirectly, for
> + example with a shell script
> + rather than with
> + <literal>
> + ExecStart=/usr/bin/systemd-sabridge</literal></para>
<literal> is correct, strictly speaking, but has quotes and looks
bad, <option> is better.
> + </listitem>
> + </varlistentry>
> + </variablelist>
> + </refsect1>
> + <refsect1>
> + <title>Exit status</title>
> + <para>On success 0 is returned, a non-zero failure
> + code otherwise.</para>
Is the return value from the daemon carried over?
> + </refsect1>
> + <refsect1>
> + <title>Examples</title>
> + <refsect2>
> + <title>Direct-Use Example</title>
> + <para>Use two services with a dependency
> + and no namespace isolation.</para>
> + <example label="bridge socket unit">
> + <title>/etc/systemd/system/bridge-to-nginx.socket</title>
> + <programlisting>
> +<![CDATA[[Socket]
> +ListenStream=80
> +
> +[Install]
> +WantedBy=socket.target]]>
There are no xml special characters here, so CDATA isn't needed.
> +</programlisting>
> + </example>
> + <example label="bridge service unit">
> + <title>/etc/systemd/system/bridge-to-nginx.service</title>
> + <programlisting>
> +<![CDATA[[Unit]
> +After=nginx.service
> +Requires=nginx.service
> +
> +[Service]
> +ExecStart=/usr/bin/systemd-sabridge /tmp/nginx.sock
> +PrivateTmp=true
> +PrivateNetwork=true]]>
> +</programlisting>
> + </example>
> + <example label="nginx configuration">
> + <title>/etc/nginx/nginx.conf</title>
> + <programlisting>
> +<![CDATA[[...]
> +server {
> + listen unix:/tmp/nginx.sock;
> + [...]]]>
> +</programlisting>
> + </example>
> + <example label="commands">
> + <programlisting>
> +<![CDATA[$ sudo systemctl --system daemon-reload
> +$ sudo systemctl start bridge-to-nginx.socket
> +$ sudo systemctl enable bridge-to-nginx.socket
> +$ curl http://localhost:80/]]>
> +</programlisting>
> + </example>
> + </refsect2>
> + <refsect2>
> + <title>Indirect-Use Example</title>
> + <para>Use a shell script to isolate the
> + service and bridge into the same namespace.
> + This is particularly useful for running
> + TCP-only daemons without the daemon
> + affecting ports on regular
> + interfaces.</para>
> + <example label="combined bridge and nginx socket unit">
> +
> + <title>
> + /etc/systemd/system/bridge-with-nginx.socket</title>
> + <programlisting>
> +<![CDATA[[Socket]
> +ListenStream=80
> +
> +[Install]
> +WantedBy=socket.target]]>
> +</programlisting>
> + </example>
> + <example label="combined bridge and nginx service unit">
> +
> + <title>
> + /etc/systemd/system/bridge-with-nginx.service</title>
> + <programlisting>
> +<![CDATA[[Unit]
> +After=syslog.target remote-fs.target nss-lookup.target
> +
> +[Service]
> +ExecStartPre=/usr/sbin/nginx -t
> +ExecStart=/usr/bin/sabridge-nginx.sh
> +PrivateTmp=true
> +PrivateNetwork=true]]>
> +</programlisting>
> + </example>
> + <example label="shell script">
> + <title>
> + /usr/bin/sabridge-nginx.sh</title>
> + <programlisting>
> +<![CDATA[#!/bin/sh
> +/usr/sbin/nginx
> +while [ ! -f /tmp/nginx.pid ]
> + do
> + /usr/bin/inotifywait /tmp/nginx.pid
Isn't this a race?
> + done
> +/usr/bin/systemd-sabridge --ignore-env localhost 8080]]>
Using the script is really ugly. Why can't sabridge launch the daemon
itself?
> +</programlisting>
> + </example>
> + <example label="nginx configuration">
> + <title>
> + /etc/nginx/nginx.conf</title>
> + <programlisting>
> +<![CDATA[[...]
> +server {
> + listen 8080;
> + listen unix:/tmp/nginx.sock;
> + [...]]]>
> +</programlisting>
> + </example>
> + <example label="commands">
> + <programlisting>
> +<![CDATA[$ sudo systemctl --system daemon-reload
> +$ sudo systemctl start bridge-with-nginx.socket
> +$ sudo systemctl enable bridge-with-nginx.socket
> +$ curl http://localhost:80/]]>
> +</programlisting>
> + </example>
> + </refsect2>
> + </refsect1>
> + <refsect1>
> + <title>See Also</title>
> + <para>
> + <citerefentry>
> + <refentrytitle>
> + systemd.service</refentrytitle>
> + <manvolnum>5</manvolnum>
> + </citerefentry>,
Those are usally one line, which make it easier to copy them over.
> + <citerefentry>
> + <refentrytitle>
> + systemd.socket</refentrytitle>
> + <manvolnum>5</manvolnum>
> + </citerefentry>,
> + <citerefentry>
> + <refentrytitle>systemctl</refentrytitle>
> + <manvolnum>1</manvolnum>
> + </citerefentry>,
> + <citerefentry>
> + <refentrytitle>socat</refentrytitle>
> + <manvolnum>1</manvolnum>
> + </citerefentry></para>
> + </refsect1>
> +</refentry>
> diff --git a/src/sabridge/Makefile b/src/sabridge/Makefile
> new file mode 100644
> index 0000000..9d07505
> --- /dev/null
> +++ b/src/sabridge/Makefile
This file should be a link.
> @@ -0,0 +1,28 @@
> +# This file is part of systemd.
> +#
> +# Copyright 2010 Lennart Poettering
> +#
> +# systemd is free software; you can redistribute it and/or modify it
> +# under the terms of the GNU Lesser General Public License as published by
> +# the Free Software Foundation; either version 2.1 of the License, or
> +# (at your option) any later version.
> +#
> +# systemd is distributed in the hope that it will be useful, but
> +# WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public License
> +# along with systemd; If not, see <http://www.gnu.org/licenses/>.
> +
> +# This file is a dirty trick to simplify compilation from within
> +# emacs. This file is not intended to be distributed. So, don't touch
> +# it, even better ignore it!
> +
> +all:
> + $(MAKE) -C ..
> +
> +clean:
> + $(MAKE) -C .. clean
> +
> +.PHONY: all clean
> diff --git a/src/sabridge/sabridge.c b/src/sabridge/sabridge.c
> new file mode 100644
> index 0000000..1f1b2cc
> --- /dev/null
> +++ b/src/sabridge/sabridge.c
> @@ -0,0 +1,519 @@
> +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
> +
> +/***
> + This file is part of systemd.
> +
> + Copyright 2013 David Strauss
> +
> + systemd is free software; you can redistribute it and/or modify it
> + under the terms of the GNU Lesser General Public License as published by
> + the Free Software Foundation; either version 2.1 of the License, or
> + (at your option) any later version.
> +
> + systemd is distributed in the hope that it will be useful, but
> + WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public License
> + along with systemd; If not, see <http://www.gnu.org/licenses/>.
> + ***/
> +
> +#include <arpa/inet.h>
> +#include <errno.h>
> +#include <getopt.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <netdb.h>
> +#include <sys/fcntl.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <unistd.h>
> +
> +#include "log.h"
> +#include "sd-daemon.h"
> +#include "sd-event.h"
> +#include "socket-util.h"
> +#include "util.h"
> +
> +#define BUFFER_SIZE 1024
This seems really small... Standard MTU is 1500, so after
removing the heading the data from one packet will still be
larget than that.
> +#define _cleanup_freeaddrinfo_ _cleanup_(freeaddrinfop)
> +
> +unsigned int total_clients = 0;
> +
> +struct proxy {
> + int listen_fd;
> + bool ignore_env;
> + bool remote_is_inet;
> + const char *remote_host;
> + const char *remote_service;
> +};
> +
> +struct connection {
> + int fd;
> + char buffer[BUFFER_SIZE];
> + ssize_t buffer_len;
The name is misleading, becuase it's not buffer lenght,
but filled bytes or somethign like that.
> + sd_event_source *w_recv;
> + sd_event_source *w_send;
> + struct connection *c_destination;
> +};
The buffer should be at the end, so that the rest fits in one cacheline.
> +
> +static inline void freeaddrinfop(struct addrinfo **ai) {
> + if (*ai)
> + freeaddrinfo(*ai);
> +}
DEFINE_TRIVIAL_CLENANUP_FUNC(struct addrinfo, freeaddrinfo);
> +
> +static void free_connection(struct connection *c) {
> + sd_event_source_unref(c->w_recv);
> + sd_event_source_unref(c->w_send);
> + close(c->fd);
> + free(c);
> +}
> +
> +static int transfer_data_cb(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
> + struct connection *c = (struct connection *) userdata;
> + int r = 0;
> +
> + assert(revents & (EPOLLIN | EPOLLOUT));
> +
> + if (revents & EPOLLIN) {
> + log_debug("About to recv up to %u bytes from fd=%d.", BUFFER_SIZE, fd);
> +
> + assert(s == c->w_recv);
> + c->buffer_len = recv(fd, &c->buffer, BUFFER_SIZE, 0);
Here you're assuming that the buffer is only read when it is completely empty.
It should instead be filled whenever there's free space in it and data is
available. Then buffer_len should not be updated directly, and can be size_t,
not ssize_t.
> + if (c->buffer_len == 0) {
> + log_debug("Clean disconnection.");
> + total_clients--;
> + free_connection(c->c_destination);
> + free_connection(c);
> + goto finish;
This can be just return 0, shorter and simpler. In all other places too, of course.
> + }
> + else if (c->buffer_len == -1) {
Brace on the same line.
> + log_error("Error %d in recv from fd=%d: %s", errno, fd, strerror(errno));
strerror(errno) → %m.
> + r = -1;
> + goto finish;
> + }
> +
> + /* Try sending the data immediately. */
> + r = send(c->c_destination->fd, &c->buffer, c->buffer_len, 0);
> + if (r < 0 && errno != EWOULDBLOCK && errno != EAGAIN) {
> + log_error("Error %d in send to fd=%d: %s", errno, fd, strerror(errno));
Maybe log_error("Error in send to fd=%d: %m", fd); ?
> + goto finish;
> + }
> + else if (errno == EWOULDBLOCK || errno == EAGAIN) {
> + /* Switch to listening for a sendable state in destination. */
> + r = sd_event_source_set_enabled(c->w_recv, SD_EVENT_OFF);
Muting the source should only be done when the buffer is actually full.
Also, partial send()s are possible, you don't seem to take that into account.
> + if (r < 0) {
> + log_error("Error %d muting recv for fd=%d: %s", r, fd, strerror(r));
> + goto finish;
> + }
> +
> + r = sd_event_source_set_enabled(c->c_destination->w_send, SD_EVENT_ON);
> + if (r < 0) {
> + log_error("Error %d unmuting send for fd=%d: %s", r, c->c_destination->fd, strerror(-r));
> + goto finish;
> + }
> + log_debug("Done with recv for fd=%d. Waiting on send for fd=%d.", fd, c->c_destination->fd);
> + goto finish;
> + }
> + log_debug("Done with recv for fd=%d and send for fd=%d.", fd, c->c_destination->fd);
> + }
> + else {
> + log_debug("About to send up to %u bytes to fd=%d.", BUFFER_SIZE, fd);
Should be c->buffer_len, not BUFFER_SIZE.
> +
> + assert(s == c->w_send);
> + assert(c->buffer != NULL);
> +
> + r = send(fd, &c->buffer, c->buffer_len, 0);
> + if (r < 0) {
> + log_error("Error %d in send to fd=%d: %s", errno, fd, strerror(errno));
> + goto finish;
> + }
Again, partial sends.
> + /* Switch to listening for a receivable state in destination. */
> + r = sd_event_source_set_enabled(c->w_send, SD_EVENT_OFF);
> + if (r < 0) {
> + log_error("Error %d muting send for fd=%d: %s", r, fd, strerror(-r));
> + goto finish;
> + }
> +
> + r = sd_event_source_set_enabled(c->c_destination->w_recv, SD_EVENT_ON);
> + if (r < 0) {
> + log_error("Error %d unmuting recv for fd=%d: %s", r, c->c_destination->fd, strerror(-r));
> + goto finish;
> + }
> +
> + log_debug("Done with send for fd=%d. Waiting on recv for fd=%d.", fd, c->c_destination->fd);
> + }
> +
> +finish:
> + return r;
> +}
> +
> +/* Once sending to the server is unblocked, set up the real watchers. */
> +static int connected_to_server_cb(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
> + struct sd_event *e = NULL;
> + struct connection *c_server_to_client = (struct connection *) userdata;
> + struct connection *c_client_to_server = c_server_to_client->c_destination;
> + int r;
> +
> + assert(revents & EPOLLOUT);
> +
> + e = sd_event_get(s);
> +
> + /* Cancel the initial write watcher for the server. */
> + sd_event_source_unref(s);
> +
> + log_debug("Connected to server. Initializing watchers for receiving data.");
> +
> + /* A disabled send watcher for the server. */
> + r = sd_event_add_io(e, c_server_to_client->fd, EPOLLOUT, transfer_data_cb, c_server_to_client, &c_server_to_client->w_send);
> + if (r < 0) {
> + log_error("Error %d creating send watcher for fd=%d: %s", r, c_server_to_client->fd, strerror(-r));
> + goto fail;
> + }
> + r = sd_event_source_set_enabled(c_server_to_client->w_send, SD_EVENT_OFF);
> + if (r < 0) {
> + log_error("Error %d muting send for fd=%d: %s", r, c_server_to_client->fd, strerror(-r));
> + goto finish;
> + }
> +
> + /* A recv watcher for the server. */
> + r = sd_event_add_io(e, c_server_to_client->fd, EPOLLIN, transfer_data_cb, c_server_to_client, &c_server_to_client->w_recv);
> + if (r < 0) {
> + log_error("Error %d creating recv watcher for fd=%d: %s", r, c_server_to_client->fd, strerror(-r));
> + goto fail;
> + }
> +
> + /* A disabled send watcher for the client. */
> + r = sd_event_add_io(e, c_client_to_server->fd, EPOLLOUT, transfer_data_cb, c_client_to_server, &c_client_to_server->w_send);
> + if (r < 0) {
> + log_error("Error %d creating send watcher for fd=%d: %s", r, c_client_to_server->fd, strerror(-r));
> + goto fail;
> + }
> + r = sd_event_source_set_enabled(c_client_to_server->w_send, SD_EVENT_OFF);
> + if (r < 0) {
> + log_error("Error %d muting send for fd=%d: %s", r, c_client_to_server->fd, strerror(-r));
> + goto finish;
> + }
> +
> + /* A recv watcher for the client. */
> + r = sd_event_add_io(e, c_client_to_server->fd, EPOLLIN, transfer_data_cb, c_client_to_server, &c_client_to_server->w_recv);
> + if (r < 0) {
> + log_error("Error %d creating recv watcher for fd=%d: %s", r, c_client_to_server->fd, strerror(-r));
> + goto fail;
> + }
> +
> +goto finish;
return r;
> +
> +fail:
> + free_connection(c_client_to_server);
> + free_connection(c_server_to_client);
> +
> +finish:
> + return r;
> +}
> +
> +static int get_server_connection_fd(const struct proxy *proxy) {
> + int server_fd;
> + int r = -EBADF;
> + int len;
> +
> + if (proxy->remote_is_inet) {
> + struct addrinfo hints = {};
> + _cleanup_freeaddrinfo_ struct addrinfo *result = NULL;
> + int s;
> +
> + hints.ai_family = AF_UNSPEC; /* IPv4 or IPv6 */
> + hints.ai_socktype = SOCK_STREAM; /* TCP */
> + hints.ai_flags = AI_PASSIVE; /* Any IP address */
Just say
struct addrinfo hints = {
.ai_family = AF_UNSPEC,
...
}
> +
> + //log_error("Looking up address info for %s:%s", proxy->remote_host, proxy->remote_service);
log_debug?
> + s = getaddrinfo(proxy->remote_host, proxy->remote_service, &hints, &result);
> + if (s != 0) {
> + log_error("getaddrinfo error (%d): %s", s, gai_strerror(s));
> + goto finish;
> + }
> +
> + if (result == NULL) {
> + log_error("getaddrinfo: no result");
> + goto finish;
> + }
> +
> + /* @TODO: Try connecting to all results instead of just the first. */
> + server_fd = socket(result->ai_family, result->ai_socktype | SOCK_NONBLOCK, result->ai_protocol);
> + if (server_fd < 0) {
> + log_error("Error %d creating socket: %s", errno, strerror(errno));
> + goto finish;
> + }
> +
> + r = connect(server_fd, result->ai_addr, result->ai_addrlen);
> + /* Ignore EINPROGRESS errors because they're expected for a non-blocking socket. */
> + if (r < 0 && errno != EINPROGRESS) {
> + log_error("Error %d while connecting to socket %s:%s: %s", errno, proxy->remote_host, proxy->remote_service, strerror(errno));
> + goto finish;
> + }
> + }
> + else {
> + struct sockaddr_un remote;
> +
> + server_fd = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0);
> + if (server_fd < 0) {
> + log_error("Error %d creating socket: %s", errno, strerror(errno));
> + r = -EBADFD;
> + goto finish;
> + }
> +
> + remote.sun_family = AF_UNIX;
> + strncpy(remote.sun_path, proxy->remote_host, sizeof(remote.sun_path));
> + len = strlen(remote.sun_path) + sizeof(remote.sun_family);
> + r = connect(server_fd, (struct sockaddr *) &remote, len);
> + if (r < 0 && errno != EINPROGRESS) {
> + log_error("Error %d while connecting to Unix domain socket %s: %s", errno, proxy->remote_host, strerror(errno));
> + r = -EBADFD;
> + goto finish;
> + }
> + }
> +
> + log_debug("Server connection is fd=%d", server_fd);
> + r = server_fd;
> +
> +finish:
> + return r;
> +}
> +
> +static int accept_cb(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
> + struct proxy *proxy = (struct proxy *) userdata;
> + struct connection *c_server_to_client = malloc(sizeof(struct connection));
> + struct connection *c_client_to_server = malloc(sizeof(struct connection));
oom checking!
> + int r = 0;
> + union sockaddr_union sa;
> + socklen_t salen = sizeof(sa);
> +
> + assert(revents & EPOLLIN);
> +
> + c_server_to_client->fd = get_server_connection_fd(proxy);
> + if (c_server_to_client->fd < 0) {
> + log_error("Error initiating server connection.");
> + goto fail;
> + }
> +
> + /* Sockets inheriting from the non-blocking listen socket will
> + * also be non-blocking. */
> + c_client_to_server->fd = accept(fd, (struct sockaddr *) &sa, &salen);
> + if (c_client_to_server->fd < 0) {
> + log_error("Error accepting client connection.");
> + goto fail;
> + }
> +
> + if (sa.sa.sa_family & (AF_INET | AF_INET6)) {
I don't think that those are bit masks, just numbers.
> + char sa_str[INET6_ADDRSTRLEN];
> + const char *success;
> +
> + success = inet_ntop(sa.sa.sa_family, &sa.in6.sin6_addr, sa_str, INET6_ADDRSTRLEN);
> + if (success == NULL)
> + log_warning("Error %d calling inet_ntop: %s", errno, strerror(errno));
> + else
> + log_debug("Accepted client connection from %s as fd=%d", sa_str, c_client_to_server->fd);
> + }
> + else {
> + log_debug("Accepted client connection (non-IP) as fd=%d", c_client_to_server->fd);
> + }
> +
> + total_clients++;
> + log_debug("Client successfully connected. Total clients: %u", total_clients);
> +
> + /* Initialize watcher for send to server; this shows connectivity. */
> + r = sd_event_add_io(sd_event_get(s), c_server_to_client->fd, EPOLLOUT, connected_to_server_cb, c_server_to_client, &c_server_to_client->w_send);
> + if (r < 0) {
> + log_error("Error %d creating connectivity watcher for fd=%d: %s", r, c_server_to_client->fd, strerror(r));
> + goto fail;
> + }
> +
> + /* Allow lookups of the opposite connection. */
> + c_server_to_client->c_destination = c_client_to_server;
> + c_client_to_server->c_destination = c_server_to_client;
> +
> + goto finish;
> +
> +fail:
> + log_warning("Accepting a client connection or connecting to the server failed.");
> + free_connection(c_client_to_server);
> + free_connection(c_server_to_client);
> +
> +finish:
> + /* Preserve the main loop even if a single proxy setup fails. */
> + return 0;
> +}
> +
> +static int run_main_loop(struct proxy *proxy) {
> + int r = EXIT_SUCCESS;
> + struct sd_event *e = NULL;
> + sd_event_source *w_accept = NULL;
> +
> + r = sd_event_new(&e);
> + if (r < 0)
> + goto finish;
> +
> + r = fd_nonblock(proxy->listen_fd, true);
> + if (r < 0)
> + goto finish;
> +
> + log_debug("Initializing main listener fd=%d", proxy->listen_fd);
> +
> + sd_event_add_io(e, proxy->listen_fd, EPOLLIN, accept_cb, proxy, &w_accept);
> +
> + log_debug("Initialized main listener. Entering loop.");
> +
> + sd_event_loop(e);
> +
> +finish:
> + sd_event_source_unref(w_accept);
> + sd_event_unref(e);
> +
> + return r;
> +}
> +
> +static int help(void) {
> +
> + printf("%s hostname-or-ip port-or-service\n"
> + "%s unix-domain-socket-path\n\n"
> + "Inherit a socket. Bidirectionally proxy.\n\n"
> + " -h --help Show this help\n"
> + " --version Print version and exit\n"
> + " --ignore-env Ignore expected systemd environment\n",
> + program_invocation_short_name,
> + program_invocation_short_name);
> +
> + return 0;
> +}
> +
> +static void version(void) {
> + puts(PACKAGE_STRING " sabridge");
> +}
> +
> +static int parse_argv(int argc, char *argv[], struct proxy *p) {
> +
> + enum {
> + ARG_VERSION = 0x100,
> + ARG_IGNORE_ENV
> + };
> +
> + static const struct option options[] = {
> + { "help", no_argument, NULL, 'h' },
> + { "version", no_argument, NULL, ARG_VERSION },
> + { "ignore-env", no_argument, NULL, ARG_IGNORE_ENV},
> + { NULL, 0, NULL, 0 }
> + };
> +
> + int c;
> +
> + assert(argc >= 0);
> + assert(argv);
> +
> + while ((c = getopt_long(argc, argv, "h", options, NULL)) >= 0) {
> +
> + switch (c) {
> +
> + case 'h':
> + help();
> + return 0;
> +
> + case '?':
> + return -EINVAL;
> +
> + case ARG_VERSION:
> + version();
> + return 0;
> +
> + case ARG_IGNORE_ENV:
> + p->ignore_env = true;
> + continue;
> +
> + default:
> + log_error("Unknown option code %c", c);
> + return -EINVAL;
> + }
> + }
> +
> + if (optind + 1 != argc && optind + 2 != argc) {
> + help();
It's just better to print an error message here, instead of forcing the user
to read a long usage description. Also, the output should not be to stdout,
but stderr.
> + return -EINVAL;
> + }
> +
> + p->remote_host = argv[optind];
> + assert(p->remote_host);
> +
> + p->remote_is_inet = (p->remote_host[0] != '/');
No ().
> +
> + if (optind == argc - 2) {
> + if (!p->remote_is_inet) {
> + /* A port or service is not allowed for Unix socket destinations. */
> + help();
Yes, just make the comment a log_error().
> + return -EINVAL;
> + }
> + p->remote_service = argv[optind + 1];
> + assert(p->remote_service);
> + }
> + else if (p->remote_is_inet) {
> + /* A port or service is required for IP destinations. */
> + help();
> + return -EINVAL;
> + }
> +
> + return 1;
> +}
> +
> +int main(int argc, char *argv[]) {
> + struct proxy p = {};
> + int r;
> +
> + log_parse_environment();
> + log_open();
> +
> + r = parse_argv(argc, argv, &p);
> + if (r <= 0)
> + goto finish;
> +
> + p.listen_fd = SD_LISTEN_FDS_START;
> +
> + if (!p.ignore_env) {
> + int n;
> + n = sd_listen_fds(1);
> + if (n == 0) {
> + log_error("Found zero inheritable sockets. Are you sure this is running as a socket-activated service?");
> + r = EXIT_FAILURE;
> + goto finish;
> + }
> + else if (n < 0) {
> + log_error("Error %d while finding inheritable sockets: %s", n, strerror(-n));
> + r = EXIT_FAILURE;
> + goto finish;
> + }
> + else if (n > 1) {
> + log_error("Can't listen on more than one socket.");
> + r = EXIT_FAILURE;
> + goto finish;
> + }
> + }
> +
> + /* @TODO: Check if this proxy can work with datagram sockets. */
> + r = sd_is_socket(p.listen_fd, 0, SOCK_STREAM, 1);
> + if (r < 0) {
> + log_error("Error %d while checking inherited socket: %s", r, strerror(-r));
> + goto finish;
> + }
> +
> + log_info("Starting the socket activation bridge with listener fd=%d.", p.listen_fd);
> +
> + r = run_main_loop(&p);
> + if (r < 0) {
> + log_error("Error %d from main loop.", r);
> + goto finish;
> + }
> +
> +finish:
> + log_close();
> + return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
> +}
Zbyszek
PS. I think the name *is* OK.
More information about the systemd-devel
mailing list