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

Fix #918: release connection if its' reference count is zero. #983

Merged
merged 4 commits into from
Mar 30, 2018
Merged

Fix #918: release connection if its' reference count is zero. #983

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bba7ccf
Fix #918: release connection if its' reference count is zero.
aleksostapenko Mar 27, 2018
22e4263
Add TFW_CONN_B_ACTIVE flag and fixes for appending connections case (…
aleksostapenko Mar 28, 2018
566affb
Corrections according review comments (PR#983).
aleksostapenko Mar 30, 2018
6d53b12
Increment version (PR#983).
aleksostapenko Mar 30, 2018
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
16 changes: 11 additions & 5 deletions tempesta_fw/connection.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,17 @@ typedef struct {

/* Connection flags are defined by the bit number. */
enum {
TFW_CONN_B_RESEND = 0, /* Need to re-send requests. */
TFW_CONN_B_QFORWD, /* Need to forward requests in the queue. */
TFW_CONN_B_HASNIP, /* Has non-idempotent requests. */

TFW_CONN_B_DEL /* Remove connection */
/* Need to re-send requests. */
TFW_CONN_B_RESEND = 0,
/* Need to forward requests in the queue. */
TFW_CONN_B_QFORWD,
/* Has non-idempotent requests. */
TFW_CONN_B_HASNIP,

/* Remove connection */
TFW_CONN_B_DEL,
/* Connection is in use or at least scheduled to be established. */
TFW_CONN_B_ACTIVE
};

#define TFW_CONN_F_RESEND (1 << TFW_CONN_B_RESEND)
Expand Down
38 changes: 34 additions & 4 deletions tempesta_fw/sock_srv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,10 +304,17 @@ tfw_srv_conn_release(TfwSrvConn *srv_conn)
* in deferred context after a short pause (in a timer
* callback). The only reason not to start new reconnect
* attempt is removing server from the current configuration.
*
* TFW_CONN_B_ACTIVE bit is checked here to see if the connection has
* already got a server's reference; if so, then server's reference must
* be put. If bit is not set, this means that connection didn't have
* time or was never scheduled to reach server (due to some error in
* sock_srv start procedure) and consequently, it needn't to put
* server here (during stop procedure).
*/
if (likely(!test_bit(TFW_CONN_B_DEL, &srv_conn->flags)))
tfw_sock_srv_connect_try_later(srv_conn);
else
else if (test_bit(TFW_CONN_B_ACTIVE, &srv_conn->flags))
tfw_server_put((TfwServer *)srv_conn->peer);
}

Expand Down Expand Up @@ -469,6 +476,18 @@ tfw_sock_srv_disconnect(TfwConn *conn)
* for not having a global list of all TfwSrvConn{} objects, and for storing
* not-yet-established connections in the TfwServer->conn_list.
*/

/*
* Get reference to server and mark the connection as active, which means
* that server must be put during connection release procedure.
*/
static inline void
tfw_sock_srv_conn_activate(TfwServer *srv, TfwSrvConn *srv_conn)
{
tfw_server_get(srv);
set_bit(TFW_CONN_B_ACTIVE, &srv_conn->flags);
}

static void
tfw_sock_srv_connect_srv(TfwServer *srv)
{
Expand All @@ -483,7 +502,7 @@ tfw_sock_srv_connect_srv(TfwServer *srv)
* that parallel execution can't happen with the same socket.
*/
list_for_each_entry(srv_conn, &srv->conn_list, list) {
tfw_server_get(srv);
tfw_sock_srv_conn_activate(srv, srv_conn);
tfw_sock_srv_connect_try_later(srv_conn);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like tfw_sock_srv_connect_try_later() is always called after tfw_sock_srv_conn_activate(). Let's move it there.

Copy link
Contributor Author

@aleksostapenko aleksostapenko Mar 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also tfw_sock_srv_connect_try_later() call here

tempesta/tempesta_fw/sock_srv.c

Line 309 in f4ef380

tfw_sock_srv_connect_try_later(srv_conn);
and it seems that we needn't increase server reference count in this path.

}
}
Expand Down Expand Up @@ -540,6 +559,12 @@ tfw_srv_conn_alloc(void)
INIT_LIST_HEAD(&srv_conn->nip_queue);
spin_lock_init(&srv_conn->fwd_qlock);

/*
* Initialization into special value for force releasing
* of taken server's reference counter on connection removing.
*/
atomic_set(&srv_conn->refcnt, TFW_CONN_DEATHCNT);

__setup_retry_timer(srv_conn);
ss_proto_init(&srv_conn->proto, &tfw_sock_srv_ss_hooks, Conn_HttpSrv);

Expand Down Expand Up @@ -580,6 +605,7 @@ tfw_sock_srv_append_conns_n(TfwServer *srv, size_t conn_n)
for (i = 0; i < conn_n; ++i) {
if (!(srv_conn = tfw_sock_srv_new_conn(srv)))
return -ENOMEM;
tfw_sock_srv_conn_activate(srv, srv_conn);
tfw_sock_srv_connect_try_later(srv_conn);
tfw_srv_loop_sched_rcu();
}
Expand Down Expand Up @@ -1981,7 +2007,7 @@ tfw_cfgop_update_srv(TfwServer *orig_srv, TfwCfgSrvGroup *sg_cfg)
orig_srv->weight = srv->weight;

if (orig_srv->conn_n < srv->conn_n) {
r = tfw_sock_srv_append_conns_n(srv,
r = tfw_sock_srv_append_conns_n(orig_srv,
srv->conn_n - orig_srv->conn_n);
if (r)
return r;
Expand Down Expand Up @@ -2035,7 +2061,11 @@ tfw_cfgop_update_sg_srv_list(TfwCfgSrvGroup *sg_cfg)

TFW_DBG2("Update server list for group '%s'\n", sg_cfg->orig_sg->name);

__tfw_sg_for_each_srv(sg_cfg->orig_sg, __tfw_cfgop_update_sg_srv_list, sg_cfg);
r = __tfw_sg_for_each_srv(sg_cfg->orig_sg,
__tfw_cfgop_update_sg_srv_list,
sg_cfg);
if (r)
return r;

/* Add new servers. */
list_for_each_entry_safe(srv, tmp, &sg_cfg->parsed_sg->srv_list, list) {
Expand Down
2 changes: 1 addition & 1 deletion tempesta_fw/tempesta_fw.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

#define TFW_AUTHOR "Tempesta Technologies, Inc"
#define TFW_NAME "Tempesta FW"
#define TFW_VERSION "0.5.0"
#define TFW_VERSION "0.6.0"

#define DEF_MAX_PORTS 8

Expand Down