From: Aurelien DARRAGON Date: Fri, 9 Sep 2022 13:32:57 +0000 (+0200) Subject: MINOR: listener: small API change X-Git-Tag: v2.7-dev6~38 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0013288;p=thirdparty%2Fhaproxy.git MINOR: listener: small API change A minor API change was performed in listener(.c/.h) to restore consistency between stop_listener() and (resume/pause)_listener() functions. LISTENER_LOCK was never locked prior to calling stop_listener(): lli variable hint is thus not useful anymore. Added PROXY_LOCK locking in (resume/pause)_listener() functions with related lpx variable hint (prerequisite for #1626). It should be backported to 2.6, 2.5 and 2.4 --- diff --git a/include/haproxy/listener.h b/include/haproxy/listener.h index 1b918b19ac..e5f8236794 100644 --- a/include/haproxy/listener.h +++ b/include/haproxy/listener.h @@ -42,23 +42,29 @@ void listener_set_state(struct listener *l, enum li_state st); * closes upon SHUT_WR and refuses to rebind. So a common validation path * involves SHUT_WR && listen && SHUT_RD. In case of success, the FD's polling * is disabled. It normally returns non-zero, unless an error is reported. + * It will need to operate under the proxy's lock. The caller is + * responsible for indicating in lpx whether the proxy locks is + * already held (non-zero) or not (zero) so that the function picks it. */ -int pause_listener(struct listener *l); +int pause_listener(struct listener *l, int lpx); /* This function tries to resume a temporarily disabled listener. * The resulting state will either be LI_READY or LI_FULL. 0 is returned * in case of failure to resume (eg: dead socket). + * It will need to operate under the proxy's lock. The caller is + * responsible for indicating in lpx whether the proxy locks is + * already held (non-zero) or not (zero) so that the function picks it. */ -int resume_listener(struct listener *l); +int resume_listener(struct listener *l, int lpx); /* * This function completely stops a listener. It will need to operate under the - * proxy's lock, the protocol's lock, and the listener's lock. The caller is - * responsible for indicating in lpx, lpr, lli whether the respective locks are + * proxy's lock and the protocol's lock. The caller is + * responsible for indicating in lpx, lpr whether the respective locks are * already held (non-zero) or not (zero) so that the function picks the missing * ones, in this order. */ -void stop_listener(struct listener *l, int lpx, int lpr, int lli); +void stop_listener(struct listener *l, int lpx, int lpr); /* This function adds the specified listener's file descriptor to the polling * lists if it is in the LI_LISTEN state. The listener enters LI_READY or diff --git a/src/listener.c b/src/listener.c index 6f8d4ad620..98d384cda9 100644 --- a/src/listener.c +++ b/src/listener.c @@ -332,13 +332,14 @@ void enable_listener(struct listener *listener) /* * This function completely stops a listener. It will need to operate under the - * proxy's lock, the protocol's lock, and the listener's lock. The caller is - * responsible for indicating in lpx, lpr, lli whether the respective locks are - * already held (non-zero) or not (zero) so that the function picks the missing - * ones, in this order. The proxy's listeners count is updated and the proxy is + * It will need to operate under the proxy's lock and the protocol's lock. + * The caller is responsible for indicating in lpx, lpr whether the + * respective locks are already held (non-zero) or not (zero) so that the + * function picks the missing ones, in this order. + * The proxy's listeners count is updated and the proxy is * disabled and woken up after the last one is gone. */ -void stop_listener(struct listener *l, int lpx, int lpr, int lli) +void stop_listener(struct listener *l, int lpx, int lpr) { struct proxy *px = l->bind_conf->frontend; @@ -355,8 +356,7 @@ void stop_listener(struct listener *l, int lpx, int lpr, int lli) if (!lpr) HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); - if (!lli) - HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock); + HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock); if (l->state > LI_INIT) { do_unbind_listener(l); @@ -367,8 +367,7 @@ void stop_listener(struct listener *l, int lpx, int lpr, int lli) proxy_cond_disable(px); } - if (!lli) - HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock); + HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock); if (!lpr) HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); @@ -457,12 +456,18 @@ int default_resume_listener(struct listener *l) * closes upon SHUT_WR and refuses to rebind. So a common validation path * involves SHUT_WR && listen && SHUT_RD. In case of success, the FD's polling * is disabled. It normally returns non-zero, unless an error is reported. + * It will need to operate under the proxy's lock. The caller is + * responsible for indicating in lpx whether the proxy locks is + * already held (non-zero) or not (zero) so that the function picks it. */ -int pause_listener(struct listener *l) +int pause_listener(struct listener *l, int lpx) { struct proxy *px = l->bind_conf->frontend; int ret = 1; + if (!lpx) + HA_RWLOCK_WRLOCK(PROXY_LOCK, &px->lock); + HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock); if (l->state <= LI_PAUSED) @@ -481,6 +486,10 @@ int pause_listener(struct listener *l) } end: HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock); + + if (!lpx) + HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &px->lock); + return ret; } @@ -493,13 +502,19 @@ int pause_listener(struct listener *l) * state, it's totally rebound. This can happen if a pause() has completely * stopped it. If the resume fails, 0 is returned and an error might be * displayed. + * It will need to operate under the proxy's lock. The caller is + * responsible for indicating in lpx whether the proxy locks is + * already held (non-zero) or not (zero) so that the function picks it. */ -int resume_listener(struct listener *l) +int resume_listener(struct listener *l, int lpx) { struct proxy *px = l->bind_conf->frontend; int was_paused = px && px->li_paused; int ret = 1; + if (!lpx) + HA_RWLOCK_WRLOCK(PROXY_LOCK, &px->lock); + HA_RWLOCK_WRLOCK(LISTENER_LOCK, &l->lock); /* check that another thread didn't to the job in parallel (e.g. at the @@ -530,6 +545,10 @@ int resume_listener(struct listener *l) } end: HA_RWLOCK_WRUNLOCK(LISTENER_LOCK, &l->lock); + + if (!lpx) + HA_RWLOCK_WRUNLOCK(PROXY_LOCK, &px->lock); + return ret; } @@ -572,7 +591,7 @@ void dequeue_all_listeners() /* This cannot fail because the listeners are by definition in * the LI_LIMITED state. */ - resume_listener(listener); + resume_listener(listener, 0); } } @@ -585,7 +604,7 @@ void dequeue_proxy_listeners(struct proxy *px) /* This cannot fail because the listeners are by definition in * the LI_LIMITED state. */ - resume_listener(listener); + resume_listener(listener, 0); } } @@ -1155,7 +1174,7 @@ void listener_accept(struct listener *l) (!tick_isset(global_listener_queue_task->expire) || tick_is_expired(global_listener_queue_task->expire, now_ms))))) { /* at least one thread has to this when quitting */ - resume_listener(l); + resume_listener(l, 0); /* Dequeues all of the listeners waiting for a resource */ dequeue_all_listeners(); @@ -1174,7 +1193,7 @@ void listener_accept(struct listener *l) * Let's put it to pause in this case. */ if (l->rx.proto && l->rx.proto->rx_listening(&l->rx) == 0) { - pause_listener(l); + pause_listener(l, 0); goto end; } @@ -1212,7 +1231,7 @@ void listener_release(struct listener *l) _HA_ATOMIC_DEC(&l->thr_conn[tid]); if (l->state == LI_FULL || l->state == LI_LIMITED) - resume_listener(l); + resume_listener(l, 0); /* Dequeues all of the listeners waiting for a resource */ dequeue_all_listeners(); diff --git a/src/protocol.c b/src/protocol.c index 7da0727386..03f7085913 100644 --- a/src/protocol.c +++ b/src/protocol.c @@ -160,7 +160,7 @@ void protocol_stop_now(void) HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); list_for_each_entry(proto, &protocols, list) { list_for_each_entry_safe(listener, lback, &proto->receivers, rx.proto_list) - stop_listener(listener, 0, 1, 0); + stop_listener(listener, 0, 1); } HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); } @@ -180,7 +180,7 @@ int protocol_pause_all(void) HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); list_for_each_entry(proto, &protocols, list) { list_for_each_entry(listener, &proto->receivers, rx.proto_list) - if (!pause_listener(listener)) + if (!pause_listener(listener, 0)) err |= ERR_FATAL; } HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); @@ -202,7 +202,7 @@ int protocol_resume_all(void) HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); list_for_each_entry(proto, &protocols, list) { list_for_each_entry(listener, &proto->receivers, rx.proto_list) - if (!resume_listener(listener)) + if (!resume_listener(listener, 0)) err |= ERR_FATAL; } HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); diff --git a/src/proxy.c b/src/proxy.c index 7b1c96eb43..7d5989a6ca 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -2265,7 +2265,7 @@ int pause_proxy(struct proxy *p) goto end; list_for_each_entry(l, &p->conf.listeners, by_fe) - pause_listener(l); + pause_listener(l, 1); if (p->li_ready) { ha_warning("%s %s failed to enter pause mode.\n", proxy_cap_str(p->cap), p->id); @@ -2294,7 +2294,7 @@ void stop_proxy(struct proxy *p) HA_RWLOCK_WRLOCK(PROXY_LOCK, &p->lock); list_for_each_entry(l, &p->conf.listeners, by_fe) - stop_listener(l, 1, 0, 0); + stop_listener(l, 1, 0); if (!(p->flags & (PR_FL_DISABLED|PR_FL_STOPPED)) && !p->li_ready) { /* might be just a backend */ @@ -2323,7 +2323,7 @@ int resume_proxy(struct proxy *p) fail = 0; list_for_each_entry(l, &p->conf.listeners, by_fe) { - if (!resume_listener(l)) { + if (!resume_listener(l, 1)) { int port; port = get_host_port(&l->rx.addr); @@ -3012,7 +3012,7 @@ static int cli_parse_set_maxconn_frontend(char **args, char *payload, struct app px->maxconn = v; list_for_each_entry(l, &px->conf.listeners, by_fe) { if (l->state == LI_FULL) - resume_listener(l); + resume_listener(l, 1); } if (px->maxconn > px->feconn)