For performance reasons, make 'conn' lock protocol specific. Signed-off-by: Darrell Ball <dlu...@gmail.com> --- lib/conntrack-private.h | 8 +++---- lib/conntrack-tcp.c | 43 +++++++++++++++++++++++++++++++++---- lib/conntrack.c | 56 ++++++++++++++++++++++++++++++++++++------------- 3 files changed, 83 insertions(+), 24 deletions(-)
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h index 3d838e4..ac891cc 100644 --- a/lib/conntrack-private.h +++ b/lib/conntrack-private.h @@ -81,16 +81,11 @@ struct alg_exp_node { bool nat_rpl_dst; }; -struct OVS_LOCKABLE ct_ce_lock { - struct ovs_mutex lock; -}; - struct conn { struct conn_key key; struct conn_key rev_key; /* Only used for orig_tuple support. */ struct conn_key master_key; - struct ct_ce_lock lock; long long expiration; struct ovs_list exp_node; struct cmap_node cm_node; @@ -142,6 +137,9 @@ struct ct_l4_proto { long long now); void (*conn_get_protoinfo)(const struct conn *, struct ct_dpif_protoinfo *); + void (*conn_lock)(struct conn *); + void (*conn_unlock)(struct conn *); + void (*conn_destroy)(struct conn *); }; /* Timeouts: all the possible timeout states passed to update_expiration() diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c index 19fdf1d..9805332 100644 --- a/lib/conntrack-tcp.c +++ b/lib/conntrack-tcp.c @@ -54,6 +54,7 @@ struct tcp_peer { struct conn_tcp { struct conn up; struct tcp_peer peer[2]; + struct ovs_mutex lock; }; enum { @@ -144,10 +145,34 @@ tcp_get_wscale(const struct tcp_header *tcp) return wscale; } +static void +tcp_conn_lock(struct conn *conn_) + OVS_NO_THREAD_SAFETY_ANALYSIS +{ + struct conn_tcp *conn = conn_tcp_cast(conn_); + ovs_mutex_lock(&conn->lock); +} + +static void +tcp_conn_unlock(struct conn *conn_) + OVS_NO_THREAD_SAFETY_ANALYSIS +{ + struct conn_tcp *conn = conn_tcp_cast(conn_); + ovs_mutex_unlock(&conn->lock); +} + +static void +tcp_conn_destroy(struct conn *conn_) +{ + struct conn_tcp *conn = conn_tcp_cast(conn_); + ovs_mutex_destroy(&conn->lock); +} + static enum ct_update_res tcp_conn_update(struct conn *conn_, struct dp_packet *pkt, bool reply, long long now) { + tcp_conn_lock(conn_); struct conn_tcp *conn = conn_tcp_cast(conn_); struct tcp_header *tcp = dp_packet_l4(pkt); /* The peer that sent 'pkt' */ @@ -156,20 +181,23 @@ tcp_conn_update(struct conn *conn_, struct dp_packet *pkt, bool reply, struct tcp_peer *dst = &conn->peer[reply ? 0 : 1]; uint8_t sws = 0, dws = 0; uint16_t tcp_flags = TCP_FLAGS(tcp->tcp_ctl); + enum ct_update_res rc = CT_UPDATE_VALID; uint16_t win = ntohs(tcp->tcp_winsz); uint32_t ack, end, seq, orig_seq; uint32_t p_len = tcp_payload_length(pkt); if (tcp_invalid_flags(tcp_flags)) { - return CT_UPDATE_INVALID; + rc = CT_UPDATE_INVALID; + goto out; } if (((tcp_flags & (TCP_SYN | TCP_ACK)) == TCP_SYN) && dst->state >= CT_DPIF_TCPS_FIN_WAIT_2 && src->state >= CT_DPIF_TCPS_FIN_WAIT_2) { src->state = dst->state = CT_DPIF_TCPS_CLOSED; - return CT_UPDATE_NEW; + rc = CT_UPDATE_NEW; + goto out; } if (src->wscale & CT_WSCALE_FLAG @@ -385,10 +413,13 @@ tcp_conn_update(struct conn *conn_, struct dp_packet *pkt, bool reply, src->state = dst->state = CT_DPIF_TCPS_TIME_WAIT; } } else { - return CT_UPDATE_INVALID; + rc = CT_UPDATE_INVALID; + goto out; } - return CT_UPDATE_VALID; +out: + tcp_conn_unlock(conn_); + return rc; } static bool @@ -448,6 +479,7 @@ tcp_new_conn(struct dp_packet *pkt, long long now) src->state = CT_DPIF_TCPS_SYN_SENT; dst->state = CT_DPIF_TCPS_CLOSED; conn_init_expiration(&newconn->up, CT_TM_TCP_FIRST_PACKET, now); + ovs_mutex_init_adaptive(&newconn->lock); return &newconn->up; } @@ -490,4 +522,7 @@ struct ct_l4_proto ct_proto_tcp = { .valid_new = tcp_valid_new, .conn_update = tcp_conn_update, .conn_get_protoinfo = tcp_conn_get_protoinfo, + .conn_lock = tcp_conn_lock, + .conn_unlock = tcp_conn_unlock, + .conn_destroy = tcp_conn_destroy, }; diff --git a/lib/conntrack.c b/lib/conntrack.c index 182a651..d88732d 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -583,16 +583,43 @@ alg_src_ip_wc(enum ct_alg_ctl_type alg_ctl_type) } static void +conn_entry_lock(struct conn *conn) +{ + struct ct_l4_proto *class = l4_protos[conn->key.nw_proto]; + if (class->conn_lock) { + class->conn_lock(conn); + } +} + +static void +conn_entry_unlock(struct conn *conn) +{ + struct ct_l4_proto *class = l4_protos[conn->key.nw_proto]; + if (class->conn_unlock) { + class->conn_unlock(conn); + } +} + +static void +conn_entry_destroy(struct conn *conn) +{ + struct ct_l4_proto *class = l4_protos[conn->key.nw_proto]; + if (class->conn_destroy) { + class->conn_destroy(conn); + } +} + +static void handle_alg_ctl(const struct conn_lookup_ctx *ctx, struct dp_packet *pkt, - enum ct_alg_ctl_type ct_alg_ctl, const struct conn *conn, + enum ct_alg_ctl_type ct_alg_ctl, struct conn *conn, long long now, bool nat) { /* ALG control packet handling with expectation creation. */ if (OVS_UNLIKELY(alg_helpers[ct_alg_ctl] && conn && conn->alg)) { - ovs_mutex_lock(&conn->lock.lock); + conn_entry_lock(conn); alg_helpers[ct_alg_ctl](ctx, pkt, conn, now, CT_FTP_CTL_INTEREST, nat); - ovs_mutex_unlock(&conn->lock.lock); + conn_entry_unlock(conn); } } @@ -929,7 +956,6 @@ conn_not_found(struct dp_packet *pkt, struct conn_lookup_ctx *ctx, } nc->nat_conn = nat_conn; - ovs_mutex_init_adaptive(&nc->lock.lock); nc->conn_type = CT_CONN_TYPE_DEFAULT; cmap_insert(&cm_conns, &nc->cm_node, ctx->hash); nc->inserted = true; @@ -1085,23 +1111,23 @@ conn_update_state_alg(struct dp_packet *pkt, struct conn_lookup_ctx *ctx, if (is_ftp_ctl(ct_alg_ctl)) { /* Keep sequence tracking in sync with the source of the * sequence skew. */ - ovs_mutex_lock(&conn->lock.lock); + conn_entry_lock(conn); if (ctx->reply != conn->seq_skew_dir) { handle_ftp_ctl(ctx, pkt, conn, now, CT_FTP_CTL_OTHER, !!nat_action_info); /* conn_update_state locks for unrelated fields, so unlock. */ - ovs_mutex_unlock(&conn->lock.lock); + conn_entry_unlock(conn); *create_new_conn = conn_update_state(pkt, ctx, conn, now); } else { /* conn_update_state locks for unrelated fields, so unlock. */ - ovs_mutex_unlock(&conn->lock.lock); + conn_entry_unlock(conn); *create_new_conn = conn_update_state(pkt, ctx, conn, now); - ovs_mutex_lock(&conn->lock.lock); + conn_entry_lock(conn); if (*create_new_conn == false) { handle_ftp_ctl(ctx, pkt, conn, now, CT_FTP_CTL_OTHER, !!nat_action_info); } - ovs_mutex_unlock(&conn->lock.lock); + conn_entry_unlock(conn); } return true; } @@ -1296,16 +1322,16 @@ ct_sweep(long long now, size_t limit) for (unsigned i = 0; i < N_CT_TM; i++) { LIST_FOR_EACH_SAFE (conn, next, exp_node, &cm_exp_lists[i]) { if (conn->conn_type == CT_CONN_TYPE_DEFAULT) { - ovs_mutex_lock(&conn->lock.lock); + conn_entry_lock(conn); if (conn->exp_list_id != NO_UPD_EXP_LIST) { ovs_list_remove(&conn->exp_node); ovs_list_push_back(&cm_exp_lists[conn->exp_list_id], &conn->exp_node); conn->exp_list_id = NO_UPD_EXP_LIST; - ovs_mutex_unlock(&conn->lock.lock); + conn_entry_unlock(conn); } else if (!conn_expired(conn, now) || count >= limit) { /* Not looking at conn changable fields. */ - ovs_mutex_unlock(&conn->lock.lock); + conn_entry_unlock(conn); min_expiration = MIN(min_expiration, conn->expiration); if (count >= limit) { /* Do not check other lists. */ @@ -1315,7 +1341,7 @@ ct_sweep(long long now, size_t limit) break; } else { /* Not looking at conn changable fields. */ - ovs_mutex_unlock(&conn->lock.lock); + conn_entry_unlock(conn); if (conn->inserted) { conn_clean(conn); } else { @@ -2173,7 +2199,7 @@ delete_conn(struct conn *conn) if (conn->conn_type == CT_CONN_TYPE_DEFAULT) { free(conn->nat_info); free(conn->alg); - ovs_mutex_destroy(&conn->lock.lock); + conn_entry_destroy(conn); if (conn->nat_conn) { free(conn->nat_conn); } @@ -2187,7 +2213,7 @@ delete_conn_one(struct conn *conn) if (conn->conn_type == CT_CONN_TYPE_DEFAULT) { free(conn->nat_info); free(conn->alg); - ovs_mutex_destroy(&conn->lock.lock); + conn_entry_destroy(conn); } free(conn); } -- 1.9.1 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev