A previous conversation [1] about optionally enabling socket options for
Listen seemed to gain consensus around adding an optional argument,
rather than multiple alternative Listen-like directives.
I've attached a patch based on work by both Jan Kaluza and Lubos
Uhliarik which implements this, updated for trunk. Syntax used is:
Listen [IP-address:]portnumber [protocol] [options=keyword,keyword,...]
where options is a comma-separated list of keywords. In this patch the
"reuseport" and "freebind" keywords are supported for APR_SO_REUSEPORT
and APR_SO_FREEBIND respectively. Key/value keywords could be used to
allow per-listener backlog, TCP buffer sizes etc, though non-boolean
options will require extending ap_listen_rec so that needs care.
Opinions? Is there still consensus that extending Listen like this is a
good idea?
Regards, Joe
[1]
http://mail-archives.apache.org/mod_mbox/httpd-dev/201603.mbox/%[email protected]%3e
diff --git a/include/ap_listen.h b/include/ap_listen.h
index 9cbaaa4910..2329cae70c 100644
--- a/include/ap_listen.h
+++ b/include/ap_listen.h
@@ -38,6 +38,11 @@ typedef struct ap_slave_t ap_slave_t;
typedef struct ap_listen_rec ap_listen_rec;
typedef apr_status_t (*accept_function)(void **csd, ap_listen_rec *lr,
apr_pool_t *ptrans);
+/* Flags for ap_listen_rec.flags */
+#define AP_LISTEN_SPECIFIC_ERRORS (0x0001)
+#define AP_LISTEN_FREEBIND (0x0002)
+#define AP_LISTEN_REUSEPORT (0x0004)
+
/**
* @brief Apache's listeners record.
*
@@ -73,10 +78,9 @@ struct ap_listen_rec {
ap_slave_t *slave;
/**
- * Allow the accept_func to return a wider set of return codes
+ * Various AP_LISTEN_* flags.
*/
- int use_specific_errors;
-
+ apr_uint32_t flags;
};
/**
diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 24ac648ac9..1271ce18ed 100644
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -628,14 +628,15 @@
* 20200331.2 (2.5.1-dev) Add ap_proxy_should_override to mod_proxy.h
* 20200331.3 (2.5.1-dev) Add ap_parse_request_line() and
* ap_check_request_header()
+ * 20200420.0 (2.5.1-dev) Add flags to listen_rec in place of
use_specific_errors
*/
#define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
#ifndef MODULE_MAGIC_NUMBER_MAJOR
-#define MODULE_MAGIC_NUMBER_MAJOR 20200331
+#define MODULE_MAGIC_NUMBER_MAJOR 20200420
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 3 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 0 /* 0...n */
/**
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a
diff --git a/os/unix/unixd.c b/os/unix/unixd.c
index bde859022b..3b0e695727 100644
--- a/os/unix/unixd.c
+++ b/os/unix/unixd.c
@@ -323,7 +323,7 @@ AP_DECLARE(apr_status_t) ap_unixd_accept(void **accepted,
ap_listen_rec *lr,
}
/* Let the caller handle slightly more varied return values */
- if (lr->use_specific_errors && ap_accept_error_is_nonfatal(status)) {
+ if ((lr->flags & AP_LISTEN_SPECIFIC_ERRORS) &&
ap_accept_error_is_nonfatal(status)) {
return status;
}
diff --git a/server/listen.c b/server/listen.c
index 87a4bafe80..3657ba65c7 100644
--- a/server/listen.c
+++ b/server/listen.c
@@ -150,7 +150,8 @@ static apr_status_t make_sock(apr_pool_t *p, ap_listen_rec
*server, int do_bind_
#endif
#if defined(SO_REUSEPORT)
- if (ap_have_so_reuseport && ap_listencbratio > 0) {
+ if (server->flags & AP_LISTEN_REUSEPORT
+ || (ap_have_so_reuseport && ap_listencbratio > 0)) {
int thesock;
apr_os_sock_get(&thesock, s);
if (setsockopt(thesock, SOL_SOCKET, SO_REUSEPORT,
@@ -166,6 +167,21 @@ static apr_status_t make_sock(apr_pool_t *p, ap_listen_rec
*server, int do_bind_
}
#endif
+
+#if defined(APR_SO_FREEBIND)
+ if (server->flags & AP_LISTEN_FREEBIND) {
+ if (apr_socket_opt_set(s, APR_SO_FREEBIND, one) < 0) {
+ stat = apr_get_netos_error();
+ ap_log_perror(APLOG_MARK, APLOG_CRIT, stat, p, APLOGNO()
+ "make_sock: apr_socket_opt_set: "
+ "error setting APR_SO_FREEBIND");
+ apr_socket_close(s);
+ return stat;
+ }
+ }
+#endif
+
+
if (do_bind_listen) {
#if APR_HAVE_IPV6
if (server->bind_addr->family == APR_INET6) {
@@ -467,7 +483,7 @@ static int find_listeners(ap_listen_rec **from,
ap_listen_rec **to,
static const char *alloc_listener(process_rec *process, const char *addr,
apr_port_t port, const char* proto,
const char *scope_id, void *slave,
- apr_pool_t *temp_pool)
+ apr_pool_t *temp_pool, apr_uint32_t flags)
{
ap_listen_rec *last;
apr_status_t status;
@@ -511,6 +527,7 @@ static const char *alloc_listener(process_rec *process,
const char *addr,
new->next = 0;
new->bind_addr = sa;
new->protocol = apr_pstrdup(process->pool, proto);
+ new->flags = flags;
/* Go to the next sockaddr. */
sa = sa->next;
@@ -795,7 +812,7 @@ AP_DECLARE(int) ap_setup_listeners(server_rec *s)
}
for (lr = ap_listeners; lr; lr = lr->next) {
- lr->use_specific_errors = ap_accept_errors_nonfatal;
+ if (ap_accept_errors_nonfatal) lr->flags |= AP_LISTEN_SPECIFIC_ERRORS;
num_listeners++;
found = 0;
for (ls = s; ls && !found; ls = ls->next) {
@@ -885,6 +902,7 @@ AP_DECLARE(apr_status_t) ap_duplicate_listeners(apr_pool_t
*p, server_rec *s,
apr_sockaddr_info_get(&sa, hostname, APR_UNSPEC, port, 0, p);
duplr->bind_addr = sa;
duplr->next = NULL;
+ duplr->flags = lr->flags;
stat = apr_socket_create(&duplr->sd, duplr->bind_addr->family,
SOCK_STREAM, 0, p);
if (stat != APR_SUCCESS) {
@@ -1015,20 +1033,48 @@ AP_DECLARE(int)
ap_accept_error_is_nonfatal(apr_status_t status)
|| APR_STATUS_IS_ECONNRESET(status);
}
+/* Parse optional flags argument for Listen. Currently just boolean
+ * flags handled; would need to be extended to incorporate
+ * ListenBacklog */
+static const char *parse_listen_flags(apr_pool_t *temp_pool, const char *arg,
+ apr_uint32_t *flags_out)
+{
+ apr_uint32_t flags = 0;
+ char *str = apr_pstrdup(temp_pool, arg), *token, *state = NULL;
+
+ token = apr_strtok(str, ",", &state);
+ while (token) {
+ if (ap_cstr_casecmp(token, "freebind") == 0)
+ flags |= AP_LISTEN_FREEBIND;
+ else if (ap_cstr_casecmp(token, "reuseport") == 0)
+ flags |= AP_LISTEN_REUSEPORT;
+ else
+ return apr_psprintf(temp_pool, "Unknown Listen option '%s' in
'%s'",
+ token, arg);
+
+ token = apr_strtok(NULL, ",", &state);
+ }
+
+ *flags_out = flags;
+
+ return NULL;
+}
+
AP_DECLARE_NONSTD(const char *) ap_set_listener(cmd_parms *cmd, void *dummy,
int argc, char *const argv[])
{
- char *host, *scope_id, *proto;
+ char *host, *scope_id, *proto = NULL;
apr_port_t port;
apr_status_t rv;
const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+ apr_uint32_t flags = 0;
if (err != NULL) {
return err;
}
- if (argc < 1 || argc > 2) {
- return "Listen requires 1 or 2 arguments.";
+ if (argc < 1 || argc > 3) {
+ return "Listen requires 1-3 arguments.";
}
#ifdef HAVE_SYSTEMD
if (use_systemd == -1) {
@@ -1058,7 +1104,24 @@ AP_DECLARE_NONSTD(const char *)
ap_set_listener(cmd_parms *cmd, void *dummy,
return "Port must be specified";
}
- if (argc != 2) {
+ if (argc == 3) {
+ if (strncasecmp(argv[2], "options=", 8)) {
+ return "Third argument to Listen must be options=...";
+ }
+
+ err = parse_listen_flags(cmd->temp_pool, argv[2] + 8, &flags);
+ if (err) {
+ return err;
+ }
+ }
+
+ if (argc == 2 && strncasecmp(argv[1], "options=", 8) == 0) {
+ err = parse_listen_flags(cmd->temp_pool, argv[1] + 8, &flags);
+ if (err) {
+ return err;
+ }
+ }
+ else if (argc < 3) {
if (port == 443) {
proto = "https";
} else {
@@ -1077,7 +1140,7 @@ AP_DECLARE_NONSTD(const char *) ap_set_listener(cmd_parms
*cmd, void *dummy,
#endif
return alloc_listener(cmd->server->process, host, port, proto,
- scope_id, NULL, cmd->temp_pool);
+ scope_id, NULL, cmd->temp_pool, flags);
}
AP_DECLARE_NONSTD(const char *) ap_set_listenbacklog(cmd_parms *cmd,
