Previously zone limits where stored in a separate cmap and needed to be looked up on each created or removed connection. As we now have a struct conntrack_zone we add them directly in there. This makes the insertion and deletion path significantly easier, since do not need a separate lookup to find correct conntrack_zone_limit entry. In addition it removes another instance which needed to be guarded via ct_lock which was the insertion of new conntrack_zone_limit entries when inserting a new connection.
Signed-off-by: Felix Huettner <[email protected]> --- Notes: v3->v4: split to 4 patches, this is 3/4 of the previous patch 5 lib/conntrack-private.h | 26 ++-- lib/conntrack.c | 293 +++++++++------------------------------- 2 files changed, 77 insertions(+), 242 deletions(-) diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h index 3409b91e2..c0818a2fe 100644 --- a/lib/conntrack-private.h +++ b/lib/conntrack-private.h @@ -144,10 +144,6 @@ struct conn { uint32_t mark; int seq_skew; - /* Immutable data. */ - int32_t admit_zone; /* The zone for managing zone limit counts. */ - uint32_t zone_limit_seq; /* Used to disambiguate zone limit counts. */ - /* Mutable data. */ bool seq_skew_dir; /* TCP sequence skew direction due to NATTing of FTP * control messages; true if reply direction. */ @@ -198,29 +194,27 @@ enum ct_ephemeral_range { #define FOR_EACH_PORT_IN_RANGE(curr, min, max) \ FOR_EACH_PORT_IN_RANGE__(curr, min, max, OVS_JOIN(idx, __COUNTER__)) -#define ZONE_LIMIT_CONN_DEFAULT -1 - -struct conntrack_zone_limit { - int32_t zone; - atomic_int64_t limit; - atomic_count count; - uint32_t zone_limit_seq; /* Used to disambiguate zone limit counts. */ -}; +#define CONN_LIMIT_NONE -1 +#define CONN_LIMIT_USE_DEFAULT -2 struct conntrack_zone { struct ovs_mutex zone_lock; /* Protects the following fields. */ struct cmap conns; + + /* Limits */ + atomic_int64_t limit; /* Currently active limit. */ + atomic_int64_t requested_limit; /* User requested limit. May be + * ZONE_LIMIT_CONN_DEFAULT if it should use + * the default limit. */ + atomic_count count; /* Number of connections currently tracked. */ }; struct conntrack { struct ovs_mutex ct_lock; /* Protects the following fields. */ struct conntrack_zone zones[UINT16_MAX + 1]; struct rculist exp_lists[N_EXP_LISTS]; - struct cmap zone_limits; struct cmap timeout_policies; - uint32_t zone_limit_seq OVS_GUARDED; /* Used to disambiguate zone limit - * counts. */ - atomic_uint32_t default_zone_limit; + atomic_int64_t default_zone_limit; uint32_t hash_basis; /* Salt for hashing a connection key. */ pthread_t clean_thread; /* Periodically cleans up connection tracker. */ diff --git a/lib/conntrack.c b/lib/conntrack.c index f82258775..1c47bea84 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -89,11 +89,6 @@ enum ct_alg_ctl_type { CT_ALG_CTL_SIP, }; -struct zone_limit { - struct cmap_node node; - struct conntrack_zone_limit czl; -}; - static bool conn_key_extract(struct conntrack *, struct dp_packet *, ovs_be16 dl_type, struct conn_lookup_ctx *, uint16_t zone); @@ -281,12 +276,12 @@ conntrack_init(void) cz = zone_lookup(ct, i); ovs_mutex_init_adaptive(&cz->zone_lock); cmap_init(&cz->conns); + cz->limit = CONN_LIMIT_NONE; + cz->requested_limit = CONN_LIMIT_USE_DEFAULT; } for (unsigned i = 0; i < ARRAY_SIZE(ct->exp_lists); i++) { rculist_init(&ct->exp_lists[i]); } - cmap_init(&ct->zone_limits); - ct->zone_limit_seq = 0; timeout_policy_init(ct); ovs_mutex_unlock(&ct->ct_lock); @@ -294,7 +289,7 @@ conntrack_init(void) atomic_init(&ct->n_conn_limit, DEFAULT_N_CONN_LIMIT); atomic_init(&ct->tcp_seq_chk, true); atomic_init(&ct->sweep_ms, 20000); - atomic_init(&ct->default_zone_limit, 0); + atomic_init(&ct->default_zone_limit, CONN_LIMIT_NONE); latch_init(&ct->clean_thread_exit); ct->clean_thread = ovs_thread_create("ct_clean", clean_thread_main, ct); ct->ipf = ipf_init(); @@ -314,115 +309,19 @@ conntrack_init(void) return ct; } -static uint32_t -zone_key_hash(int32_t zone, uint32_t basis) -{ - size_t hash = hash_int((OVS_FORCE uint32_t) zone, basis); - return hash; -} - static int64_t -zone_limit_get_limit__(struct conntrack_zone_limit *czl) +zone_read_limit(struct conntrack_zone *cz) { int64_t limit; - atomic_read_relaxed(&czl->limit, &limit); + atomic_read_relaxed(&cz->limit, &limit); return limit; } -static int64_t -zone_limit_get_limit(struct conntrack *ct, struct conntrack_zone_limit *czl) -{ - int64_t limit = zone_limit_get_limit__(czl); - - if (limit == ZONE_LIMIT_CONN_DEFAULT) { - atomic_read_relaxed(&ct->default_zone_limit, &limit); - limit = limit ? limit : -1; - } - - return limit; -} - -static struct zone_limit * -zone_limit_lookup_protected(struct conntrack *ct, int32_t zone) - OVS_REQUIRES(ct->ct_lock) -{ - uint32_t hash = zone_key_hash(zone, ct->hash_basis); - struct zone_limit *zl; - CMAP_FOR_EACH_WITH_HASH_PROTECTED (zl, node, hash, &ct->zone_limits) { - if (zl->czl.zone == zone) { - return zl; - } - } - return NULL; -} - -static struct zone_limit * -zone_limit_lookup(struct conntrack *ct, int32_t zone) -{ - uint32_t hash = zone_key_hash(zone, ct->hash_basis); - struct zone_limit *zl; - CMAP_FOR_EACH_WITH_HASH (zl, node, hash, &ct->zone_limits) { - if (zl->czl.zone == zone) { - return zl; - } - } - return NULL; -} - -static struct zone_limit * -zone_limit_create__(struct conntrack *ct, int32_t zone, int64_t limit) - OVS_REQUIRES(ct->ct_lock) -{ - struct zone_limit *zl = NULL; - - if (zone > DEFAULT_ZONE && zone <= MAX_ZONE) { - zl = xmalloc(sizeof *zl); - atomic_init(&zl->czl.limit, limit); - atomic_count_init(&zl->czl.count, 0); - zl->czl.zone = zone; - zl->czl.zone_limit_seq = ct->zone_limit_seq++; - uint32_t hash = zone_key_hash(zone, ct->hash_basis); - cmap_insert(&ct->zone_limits, &zl->node, hash); - } - - return zl; -} - -static struct zone_limit * -zone_limit_create(struct conntrack *ct, int32_t zone, int64_t limit) - OVS_REQUIRES(ct->ct_lock) -{ - struct zone_limit *zl = zone_limit_lookup_protected(ct, zone); - - if (zl) { - return zl; - } - - return zone_limit_create__(ct, zone, limit); -} - -/* Lazily creates a new entry in the zone_limits cmap if default limit - * is set and there's no entry for the zone. */ -static struct zone_limit * -zone_limit_lookup_or_default(struct conntrack *ct, int32_t zone) - OVS_REQUIRES(ct->ct_lock) -{ - struct zone_limit *zl = zone_limit_lookup_protected(ct, zone); - - if (!zl) { - uint32_t limit; - atomic_read_relaxed(&ct->default_zone_limit, &limit); - - if (limit) { - zl = zone_limit_create__(ct, zone, ZONE_LIMIT_CONN_DEFAULT); - } - } - - return zl; -} - -struct conntrack_zone_info +/* Returns the conntrack_zone_info for the requested zone. + * Note: to be compatible with the kernel implementation we return empty + * entries if the default limit is to be used. + * This can be improved upon later. */struct conntrack_zone_info zone_limit_get(struct conntrack *ct, int32_t zone) { struct conntrack_zone_info czl = { @@ -430,82 +329,66 @@ zone_limit_get(struct conntrack *ct, int32_t zone) .limit = 0, .count = 0, }; - struct zone_limit *zl = zone_limit_lookup(ct, zone); - if (zl) { - int64_t czl_limit = zone_limit_get_limit__(&zl->czl); - if (czl_limit > ZONE_LIMIT_CONN_DEFAULT) { - czl.zone = zl->czl.zone; - czl.limit = czl_limit; - } else { - atomic_read_relaxed(&ct->default_zone_limit, &czl.limit); - } - czl.count = atomic_count_get(&zl->czl.count); + if (zone == DEFAULT_ZONE) { + atomic_read_relaxed(&ct->default_zone_limit, &czl.limit); } else { - atomic_read_relaxed(&ct->default_zone_limit, &czl.limit); + struct conntrack_zone *cz = zone_lookup(ct, zone); + uint64_t req_limit; + atomic_read_relaxed(&cz->requested_limit, &req_limit); + if (req_limit != CONN_LIMIT_USE_DEFAULT) { + czl.zone = zone; + czl.limit = zone_read_limit(cz); + czl.count = atomic_count_get(&cz->count); + } + } + + if (czl.limit == CONN_LIMIT_NONE) { + czl.limit = 0; } return czl; } -static void -zone_limit_clean__(struct conntrack *ct, struct zone_limit *zl) - OVS_REQUIRES(ct->ct_lock) +static bool +zone_limit_reset(struct conntrack *ct, struct conntrack_zone *cz) { - uint32_t hash = zone_key_hash(zl->czl.zone, ct->hash_basis); - cmap_remove(&ct->zone_limits, &zl->node, hash); - ovsrcu_postpone(free, zl); -} - -static void -zone_limit_clean(struct conntrack *ct, struct zone_limit *zl) - OVS_REQUIRES(ct->ct_lock) -{ - uint32_t limit; + int64_t limit; + atomic_read_relaxed(&cz->requested_limit, &limit); + if (limit == CONN_LIMIT_USE_DEFAULT) { + return false; + } atomic_read_relaxed(&ct->default_zone_limit, &limit); - /* Do not remove the entry if the default limit is enabled, but - * simply move the limit to default. */ - if (limit) { - atomic_store_relaxed(&zl->czl.limit, ZONE_LIMIT_CONN_DEFAULT); - } else { - zone_limit_clean__(ct, zl); - } + atomic_store_relaxed(&cz->limit, limit); + atomic_store_relaxed(&cz->requested_limit, CONN_LIMIT_USE_DEFAULT); + return true; } static void -zone_limit_clean_default(struct conntrack *ct) - OVS_REQUIRES(ct->ct_lock) +zone_limit_update_default(struct conntrack *ct, uint32_t limit) { - struct zone_limit *zl; - int64_t czl_limit; + int64_t cz_req_limit; - atomic_store_relaxed(&ct->default_zone_limit, 0); + atomic_store_relaxed(&ct->default_zone_limit, limit); - CMAP_FOR_EACH (zl, node, &ct->zone_limits) { - atomic_read_relaxed(&zl->czl.limit, &czl_limit); - if (zone_limit_get_limit__(&zl->czl) == ZONE_LIMIT_CONN_DEFAULT) { - zone_limit_clean__(ct, zl); + for (unsigned i = 0; i < ARRAY_SIZE(ct->zones); i++) { + atomic_read_relaxed(&ct->zones[i].requested_limit, &cz_req_limit); + if (cz_req_limit == CONN_LIMIT_USE_DEFAULT) { + atomic_store_relaxed(&ct->zones[i].limit, limit); } } } static bool zone_limit_delete__(struct conntrack *ct, int32_t zone) - OVS_REQUIRES(ct->ct_lock) { - struct zone_limit *zl = NULL; - if (zone == DEFAULT_ZONE) { - zone_limit_clean_default(ct); + zone_limit_update_default(ct, CONN_LIMIT_NONE); + return false; } else { - zl = zone_limit_lookup_protected(ct, zone); - if (zl) { - zone_limit_clean(ct, zl); - } + return zone_limit_reset(ct, zone_lookup(ct, zone)); } - - return zl != NULL; } int @@ -513,9 +396,7 @@ zone_limit_delete(struct conntrack *ct, int32_t zone) { bool deleted; - ovs_mutex_lock(&ct->ct_lock); deleted = zone_limit_delete__(ct, zone); - ovs_mutex_unlock(&ct->ct_lock); if (zone != DEFAULT_ZONE) { VLOG_INFO(deleted @@ -527,46 +408,33 @@ zone_limit_delete(struct conntrack *ct, int32_t zone) return 0; } -static void -zone_limit_update_default(struct conntrack *ct, int32_t zone, uint32_t limit) -{ - /* limit zero means delete default. */ - if (limit == 0) { - ovs_mutex_lock(&ct->ct_lock); - zone_limit_delete__(ct, zone); - ovs_mutex_unlock(&ct->ct_lock); - } else { - atomic_store_relaxed(&ct->default_zone_limit, limit); - } -} - int zone_limit_update(struct conntrack *ct, int32_t zone, uint32_t limit) { - struct zone_limit *zl; + struct conntrack_zone *cz; int err = 0; + int64_t conn_limit = limit; + + if (conn_limit == 0) { + conn_limit = CONN_LIMIT_NONE; + } if (zone == DEFAULT_ZONE) { - zone_limit_update_default(ct, zone, limit); - VLOG_INFO("Set default zone limit to %u", limit); + zone_limit_update_default(ct, conn_limit); + VLOG_INFO("Set default zone limit to %"PRId64, conn_limit); return err; } - zl = zone_limit_lookup(ct, zone); - if (zl) { - atomic_store_relaxed(&zl->czl.limit, limit); - VLOG_INFO("Changed zone limit of %u for zone %d", limit, zone); + cz = zone_lookup(ct, zone); + if (cz) { + atomic_store_relaxed(&cz->limit, conn_limit); + atomic_store_relaxed(&cz->requested_limit, conn_limit); + VLOG_INFO("Set zone limit of %"PRId64" for zone %d", conn_limit, zone); } else { - ovs_mutex_lock(&ct->ct_lock); - err = zone_limit_create(ct, zone, limit) == NULL; - ovs_mutex_unlock(&ct->ct_lock); - if (!err) { - VLOG_INFO("Created zone limit of %u for zone %d", limit, zone); - } else { - VLOG_WARN("Request to create zone limit for invalid zone %d", - zone); - } - } + VLOG_WARN("Request to create zone limit for invalid zone %d", + zone); + err = 1; + } return err; } @@ -590,6 +458,7 @@ conn_clean__(struct conntrack *ct, struct conn *conn) ovs_mutex_lock(&cz->zone_lock); cmap_remove(&cz->conns, &conn->key_node[CT_DIR_FWD].cm_node, hash); + atomic_count_dec(&cz->count); if (conn->nat_action) { ovs_assert(fwd_zone == conn->key_node[CT_DIR_REV].key.zone); @@ -618,11 +487,6 @@ conn_clean(struct conntrack *ct, struct conn *conn) conn_clean__(ct, conn); ovs_mutex_unlock(&ct->ct_lock); - struct zone_limit *zl = zone_limit_lookup(ct, conn->admit_zone); - if (zl && zl->czl.zone_limit_seq == conn->zone_limit_seq) { - atomic_count_dec(&zl->czl.count); - } - ovsrcu_postpone(delete_conn, conn); atomic_count_dec(&ct->n_conn); } @@ -652,14 +516,6 @@ conntrack_destroy(struct conntrack *ct) } } - struct zone_limit *zl; - CMAP_FOR_EACH (zl, node, &ct->zone_limits) { - uint32_t hash = zone_key_hash(zl->czl.zone, ct->hash_basis); - - cmap_remove(&ct->zone_limits, &zl->node, hash); - ovsrcu_postpone(free, zl); - } - struct timeout_policy *tp; CMAP_FOR_EACH (tp, node, &ct->timeout_policies) { uint32_t hash = hash_int(tp->policy.id, ct->hash_basis); @@ -676,7 +532,6 @@ conntrack_destroy(struct conntrack *ct) ovs_mutex_unlock(&cz->zone_lock); ovs_mutex_destroy(&cz->zone_lock); } - cmap_destroy(&ct->zone_limits); cmap_destroy(&ct->timeout_policies); ovs_mutex_unlock(&ct->ct_lock); @@ -1072,15 +927,11 @@ conn_insert(struct conntrack *ct, struct conntrack_zone *cz, COVERAGE_INC(conntrack_insert); - struct zone_limit *zl = zone_limit_lookup_or_default(ct, - ctx->key.zone); - if (zl) { - czl_limit = zone_limit_get_limit(ct, &zl->czl); - if (czl_limit >= 0 && - atomic_count_get(&zl->czl.count) >= czl_limit) { - COVERAGE_INC(conntrack_zone_full); - return nc; - } + czl_limit = zone_read_limit(cz); + if (czl_limit != CONN_LIMIT_NONE && + atomic_count_get(&cz->count) >= czl_limit) { + COVERAGE_INC(conntrack_zone_full); + return nc; } unsigned int n_conn_limit; @@ -1114,13 +965,6 @@ conn_insert(struct conntrack *ct, struct conntrack_zone *cz, fwd_key_node->dir = CT_DIR_FWD; rev_key_node->dir = CT_DIR_REV; - if (zl) { - nc->admit_zone = zl->czl.zone; - nc->zone_limit_seq = zl->czl.zone_limit_seq; - } else { - nc->admit_zone = INVALID_ZONE; - } - if (nat_action_info) { nc->nat_action = nat_action_info->nat_action; @@ -1150,10 +994,7 @@ conn_insert(struct conntrack *ct, struct conntrack_zone *cz, &fwd_key_node->cm_node, ctx->hash); conn_expire_push_front(ct, nc); atomic_count_inc(&ct->n_conn); - - if (zl) { - atomic_count_inc(&zl->czl.count); - } + atomic_count_inc(&cz->count); ctx->conn = nc; /* For completeness. */ -- 2.43.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
