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/%3c56dd68e5.2040...@redhat.com%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,