Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proxy address reuse and expiry #367

Closed
wants to merge 19 commits into from
Closed

Proxy address reuse and expiry #367

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
5b4effb
mod_proxy: Add/use ap_proxy_determine_address()
ylavic Jun 29, 2023
2fe3248
mod_proxy: Make ap_proxy_worker_name() pool-less and thread-safe.
ylavic Jul 5, 2023
24faad1
mod_proxy: Factorize code in ap_proxy_get_worker_ex(), no functional …
ylavic Jul 5, 2023
f8d273c
mod_proxy: Single "proxy:forward" worker and use of conf->pool to def…
ylavic Jul 5, 2023
0d3792b
mod_proxy: Update some comments, axe redundant socket_cleanup().
ylavic Jul 5, 2023
97ce1e9
mod_proxy: Consistently close the socket on failure to reuse the conn…
ylavic Jul 21, 2023
8ad4da6
Address Ruediger's comments (to squash in "Add/use ap_proxy_determine…
ylavic Jul 10, 2023
deb8ab5
Address Ruediger's comment (to squash in "Update some comments, ...")
ylavic Jul 10, 2023
52126bf
Address Ruediger's comment (to squash in "Add/use ap_proxy_determine_…
ylavic Jul 21, 2023
975bb2c
Merge branch 'trunk' of https://github.com/apache/httpd into proxy_ad…
ylavic Jul 21, 2023
e47e574
Merge branch 'trunk' of https://github.com/apache/httpd into proxy_ad…
ylavic Jul 21, 2023
7ed69ec
mod_proxy: Set new APLOGNOs (to squash in "Add/use ap_proxy_determine…
ylavic Jul 21, 2023
7026745
address_ttl in seconds.
ylavic Sep 20, 2023
f1e6d9e
Fix some atomic/lifetime issues.
ylavic Sep 20, 2023
6d11e75
Add flags to ap_proxy_determine_address() for better extendability.
ylavic Sep 20, 2023
b975293
Minor bump for ap_proxy_determine_address()
ylavic Sep 20, 2023
aa79132
Add ap_proxy_worker_get_name() deprecating ap_proxy_worker_name()
ylavic Sep 20, 2023
176de10
Add PROXY_DETERMINE_ADDRESS_CHECK flag for ap_proxy_determine_address()
ylavic Sep 21, 2023
08a1940
Fix proxy_addrs_equal().
ylavic Sep 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 52 additions & 30 deletions modules/proxy/mod_proxy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,24 @@ static const char *set_worker_param(apr_pool_t *p,
return "EnableReuse must be On|Off";
worker->s->disablereuse_set = 1;
}
else if (!strcasecmp(key, "addressttl")) {
/* Address TTL in seconds
*/
apr_interval_time_t ttl;
if (strcmp(val, "-1") == 0) {
worker->s->address_ttl = -1;
}
else if (ap_timeout_parameter_parse(val, &ttl, "s") == APR_SUCCESS
&& (ttl <= apr_time_from_sec(APR_INT32_MAX))
&& (ttl % apr_time_from_sec(1)) == 0) {
worker->s->address_ttl = ttl;
}
else {
return "AddressTTL must be -1 or a number of seconds not "
"exceeding " APR_STRINGIFY(APR_INT32_MAX);
}
worker->s->address_ttl_set = 1;
}
ylavic marked this conversation as resolved.
Show resolved Hide resolved
else if (!strcasecmp(key, "route")) {
/* Worker route.
*/
Expand Down Expand Up @@ -2223,14 +2241,14 @@ static const char *
reuse = 1;
ap_log_error(APLOG_MARK, APLOG_INFO, 0, cmd->server, APLOGNO(01145)
"Sharing worker '%s' instead of creating new worker '%s'",
ap_proxy_worker_name(cmd->pool, worker), new->real);
ap_proxy_worker_name(NULL, worker), new->real);
}

for (i = 0; i < arr->nelts; i++) {
if (reuse) {
ap_log_error(APLOG_MARK, APLOG_WARNING, 0, cmd->server, APLOGNO(01146)
"Ignoring parameter '%s=%s' for worker '%s' because of worker sharing",
elts[i].key, elts[i].val, ap_proxy_worker_name(cmd->pool, worker));
elts[i].key, elts[i].val, ap_proxy_worker_name(NULL, worker));
} else {
const char *err = set_worker_param(cmd->pool, s, worker, elts[i].key,
elts[i].val);
Expand Down Expand Up @@ -2775,13 +2793,13 @@ static const char *add_member(cmd_parms *cmd, void *dummy, const char *arg)
return apr_pstrcat(cmd->temp_pool, "BalancerMember ", err, NULL);
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, cmd->server, APLOGNO(01148)
"Defined worker '%s' for balancer '%s'",
ap_proxy_worker_name(cmd->pool, worker), balancer->s->name);
ap_proxy_worker_name(NULL, worker), balancer->s->name);
PROXY_COPY_CONF_PARAMS(worker, conf);
} else {
reuse = 1;
ap_log_error(APLOG_MARK, APLOG_INFO, 0, cmd->server, APLOGNO(01149)
"Sharing worker '%s' instead of creating new worker '%s'",
ap_proxy_worker_name(cmd->pool, worker), name);
ap_proxy_worker_name(NULL, worker), name);
}
if (!worker->section_config) {
worker->section_config = balancer->section_config;
Expand All @@ -2793,7 +2811,7 @@ static const char *add_member(cmd_parms *cmd, void *dummy, const char *arg)
if (reuse) {
ap_log_error(APLOG_MARK, APLOG_WARNING, 0, cmd->server, APLOGNO(01150)
"Ignoring parameter '%s=%s' for worker '%s' because of worker sharing",
elts[i].key, elts[i].val, ap_proxy_worker_name(cmd->pool, worker));
elts[i].key, elts[i].val, ap_proxy_worker_name(NULL, worker));
} else {
err = set_worker_param(cmd->pool, cmd->server, worker, elts[i].key,
elts[i].val);
Expand Down Expand Up @@ -3350,6 +3368,8 @@ static int proxy_status_hook(request_rec *r, int flags)

static void child_init(apr_pool_t *p, server_rec *s)
{
proxy_server_conf *main_conf;
proxy_worker *forward = NULL;
proxy_worker *reverse = NULL;

apr_status_t rv = apr_global_mutex_child_init(&proxy_mutex,
Expand All @@ -3361,8 +3381,8 @@ static void child_init(apr_pool_t *p, server_rec *s)
exit(1); /* Ugly, but what else? */
}

/* TODO */
while (s) {
main_conf = ap_get_module_config(s->module_config, &proxy_module);
for (; s; s = s->next) {
void *sconf = s->module_config;
proxy_server_conf *conf;
proxy_worker *worker;
Expand All @@ -3375,32 +3395,36 @@ static void child_init(apr_pool_t *p, server_rec *s)
*/
worker = (proxy_worker *)conf->workers->elts;
for (i = 0; i < conf->workers->nelts; i++, worker++) {
ap_proxy_initialize_worker(worker, s, p);
ap_proxy_initialize_worker(worker, s, conf->pool);
}

/* Create and initialize forward worker if defined */
if (conf->req_set && conf->req) {
proxy_worker *forward;
ap_proxy_define_worker(conf->pool, &forward, NULL, NULL,
if (conf->req_set && conf->req && !forward) {
ap_proxy_define_worker(main_conf->pool, &forward, NULL, NULL,
"http://www.apache.org", 0);
conf->forward = forward;
PROXY_STRNCPY(conf->forward->s->name, "proxy:forward");
PROXY_STRNCPY(conf->forward->s->hostname, "*"); /* for compatibility */
PROXY_STRNCPY(conf->forward->s->hostname_ex, "*");
PROXY_STRNCPY(conf->forward->s->scheme, "*");
conf->forward->hash.def = conf->forward->s->hash.def =
ap_proxy_hashfunc(conf->forward->s->name, PROXY_HASHFUNC_DEFAULT);
conf->forward->hash.fnv = conf->forward->s->hash.fnv =
ap_proxy_hashfunc(conf->forward->s->name, PROXY_HASHFUNC_FNV);
PROXY_STRNCPY(forward->s->name, "proxy:forward");
PROXY_STRNCPY(forward->s->hostname, "*"); /* for compatibility */
PROXY_STRNCPY(forward->s->hostname_ex, "*");
PROXY_STRNCPY(forward->s->scheme, "*");
forward->hash.def = forward->s->hash.def =
ap_proxy_hashfunc(forward->s->name, PROXY_HASHFUNC_DEFAULT);
forward->hash.fnv = forward->s->hash.fnv =
ap_proxy_hashfunc(forward->s->name, PROXY_HASHFUNC_FNV);
/* Do not disable worker in case of errors */
conf->forward->s->status |= PROXY_WORKER_IGNORE_ERRORS;
forward->s->status |= PROXY_WORKER_IGNORE_ERRORS;
/* Mark as the "generic" worker */
conf->forward->s->status |= PROXY_WORKER_GENERIC;
ap_proxy_initialize_worker(conf->forward, s, p);
/* Disable address cache for generic forward worker */
conf->forward->s->is_address_reusable = 0;
forward->s->status |= PROXY_WORKER_GENERIC;
/* Disable connection and address reuse for generic workers */
forward->s->is_address_reusable = 0;
ap_proxy_initialize_worker(forward, s, main_conf->pool);
}
if (conf->req_set && conf->req) {
conf->forward = forward;
}

/* Create and initialize the generic reserse worker once only */
if (!reverse) {
ap_proxy_define_worker(conf->pool, &reverse, NULL, NULL,
ap_proxy_define_worker(main_conf->pool, &reverse, NULL, NULL,
"http://www.apache.org", 0);
PROXY_STRNCPY(reverse->s->name, "proxy:reverse");
PROXY_STRNCPY(reverse->s->hostname, "*"); /* for compatibility */
Expand All @@ -3414,13 +3438,11 @@ static void child_init(apr_pool_t *p, server_rec *s)
reverse->s->status |= PROXY_WORKER_IGNORE_ERRORS;
/* Mark as the "generic" worker */
reverse->s->status |= PROXY_WORKER_GENERIC;
conf->reverse = reverse;
ap_proxy_initialize_worker(conf->reverse, s, p);
/* Disable address cache for generic reverse worker */
/* Disable connection and address reuse for generic workers */
reverse->s->is_address_reusable = 0;
ap_proxy_initialize_worker(reverse, s, main_conf->pool);
}
conf->reverse = reverse;
s = s->next;
}
}

Expand Down
35 changes: 32 additions & 3 deletions modules/proxy/mod_proxy.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,8 @@ typedef struct {
apr_array_header_t* cookie_domains;
} proxy_req_conf;

struct proxy_address; /* opaque TTL'ed and refcount'ed address */

typedef struct {
conn_rec *connection;
request_rec *r; /* Request record of the backend request
Expand All @@ -294,6 +296,9 @@ typedef struct {
* and its scpool/bucket_alloc (NULL before),
* must be left cleaned when used (locally).
*/
apr_pool_t *uds_pool; /* Subpool for reusing UDS paths */
apr_pool_t *fwd_pool; /* Subpool for reusing ProxyRemote infos */
struct proxy_address *address; /* Current remote address */
} proxy_conn_rec;

typedef struct {
Expand Down Expand Up @@ -488,6 +493,9 @@ typedef struct {
unsigned int was_malloced:1;
unsigned int is_name_matchable:1;
unsigned int response_field_size_set:1;
unsigned int address_ttl_set:1;
apr_time_t address_ttl; /* backend address' TTL */
apr_uint32_t address_expiry; /* backend address' next expiry time */
} proxy_worker_shared;

#define ALIGNED_PROXY_WORKER_SHARED_SIZE (APR_ALIGN_DEFAULT(sizeof(proxy_worker_shared)))
Expand All @@ -504,6 +512,8 @@ struct proxy_worker {
#endif
void *context; /* general purpose storage */
ap_conf_vector_t *section_config; /* <Proxy>-section wherein defined */
struct proxy_address *address; /* current worker address (if reusable) */
const char *uds_name; /* "unix:/uds/path|worker-URL" */
};

/* default to health check every 30 seconds */
Expand Down Expand Up @@ -755,12 +765,13 @@ typedef __declspec(dllimport) const char *
/* Connection pool API */
/**
* Return the user-land, UDS aware worker name
* @param p memory pool used for displaying worker name
* @param unused memory pool unused
* @param worker the worker
* @return name
* @note Even though the returned name is non constant char*, the string
* it points to is shared and should *not* be modified by the caller!
*/
ylavic marked this conversation as resolved.
Show resolved Hide resolved

PROXY_DECLARE(char *) ap_proxy_worker_name(apr_pool_t *p,
PROXY_DECLARE(char *) ap_proxy_worker_name(apr_pool_t *unused,
proxy_worker *worker);

/**
Expand Down Expand Up @@ -1041,6 +1052,24 @@ PROXY_DECLARE(int) ap_proxy_post_request(proxy_worker *worker,
request_rec *r,
proxy_server_conf *conf);

/**
* Resolve an address, reusing the one of the worker if any.
* @param proxy_function calling proxy scheme (http, ajp, ...)
* @param conn proxy connection the address is used for
* @param hostname host to resolve (should be the worker's if reusable)
* @param hostport port to resolve (should be the worker's if reusable)
* @param r current request (if any)
* @param s current server (or NULL if r != NULL and ap_proxyerror()
* should be called on error)
* @return APR_SUCCESS or an error
*/
PROXY_DECLARE(apr_status_t) ap_proxy_determine_address(const char *proxy_function,
proxy_conn_rec *conn,
const char *hostname,
apr_port_t hostport,
request_rec *r,
server_rec *s);

/**
* Determine backend hostname and port
* @param p memory pool used for processing
Expand Down
35 changes: 12 additions & 23 deletions modules/proxy/mod_proxy_ajp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,8 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
if (status != APR_SUCCESS) {
conn->close = 1;
ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00868)
"request failed to %pI (%s:%d)",
conn->worker->cp->addr,
conn->worker->s->hostname_ex,
(int)conn->worker->s->port);
"request failed to %pI (%s:%hu)",
conn->addr, conn->hostname, conn->port);
if (status == AJP_EOVERFLOW)
return HTTP_BAD_REQUEST;
else {
Expand Down Expand Up @@ -334,10 +332,8 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
conn->close = 1;
apr_brigade_destroy(input_brigade);
ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00876)
"send failed to %pI (%s:%d)",
conn->worker->cp->addr,
conn->worker->s->hostname_ex,
(int)conn->worker->s->port);
"send failed to %pI (%s:%hu)",
conn->addr, conn->hostname, conn->port);
/*
* It is fatal when we failed to send a (part) of the request
* body.
Expand Down Expand Up @@ -376,10 +372,8 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
conn->close = 1;
apr_brigade_destroy(input_brigade);
ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00878)
"read response failed from %pI (%s:%d)",
conn->worker->cp->addr,
conn->worker->s->hostname_ex,
(int)conn->worker->s->port);
"read response failed from %pI (%s:%hu)",
conn->addr, conn->hostname, conn->port);

/* If we had a successful cping/cpong and then a timeout
* we assume it is a request that cause a back-end timeout,
Expand Down Expand Up @@ -676,10 +670,8 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,
}
else {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00892)
"got response from %pI (%s:%d)",
conn->worker->cp->addr,
conn->worker->s->hostname_ex,
(int)conn->worker->s->port);
"got response from %pI (%s:%hu)",
conn->addr, conn->hostname, conn->port);

if (ap_proxy_should_override(conf, r->status)) {
/* clear r->status for override error, otherwise ErrorDocument
Expand All @@ -701,10 +693,8 @@ static int ap_proxy_ajp_request(apr_pool_t *p, request_rec *r,

if (backend_failed) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00893)
"dialog to %pI (%s:%d) failed",
conn->worker->cp->addr,
conn->worker->s->hostname_ex,
(int)conn->worker->s->port);
"dialog to %pI (%s:%hu) failed",
conn->addr, conn->hostname, conn->port);
/*
* If we already send data, signal a broken backend connection
* upwards in the chain.
Expand Down Expand Up @@ -846,9 +836,8 @@ static int proxy_ajp_handler(request_rec *r, proxy_worker *worker,
if (status != APR_SUCCESS) {
backend->close = 1;
ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00897)
"cping/cpong failed to %pI (%s:%d)",
worker->cp->addr, worker->s->hostname_ex,
(int)worker->s->port);
"cping/cpong failed to %pI (%s:%hu)",
backend->addr, backend->hostname, backend->port);
status = HTTP_SERVICE_UNAVAILABLE;
retry++;
continue;
Expand Down
Loading