Am Mon, Aug 24, 2026 at 12:42:31PM +0200 schrieb Paolo Valerio:
> On 21 Jul 2026 at 12:56:37 PM, Felix Huettner via dev
> <[email protected]> wrote:
>
> > This lock will in the future be the only thing that is needed to insert
> > or remove connections. For now it is there additionally to ct_lock which
> > is still needed for the rculists.
> >
> > Signed-off-by: Felix Huettner <[email protected]>
> > ---
> >
> > Notes:
> > v3->v4: split to 4 patches, this is 2/4 of the previous patch 5
> >
> > lib/conntrack-private.h | 1 +
> > lib/conntrack.c | 19 ++++++++++++++++---
> > 2 files changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
> > index a0aaf5bc3..3409b91e2 100644
> > --- a/lib/conntrack-private.h
> > +++ b/lib/conntrack-private.h
> > @@ -208,6 +208,7 @@ struct conntrack_zone_limit {
> > };
> >
> > struct conntrack_zone {
> > + struct ovs_mutex zone_lock; /* Protects the following fields. */
>
Hi Paolo,
thanks for the feedback
>
> This introduces roughly 3 MB of memory overhead that adds up to the cmaps.
> This footprint increase is worth mentioning in the commit message maybe
> explaining why such increase is a needed trade-off.
I'll update the commit message
>
> While at it I have a general question, what's the average zone usage in
> your use-cases?
On our OVN gateway chassis we have around 10k-30k zones allocated by
ovn-controller.
Out of these around 1k zones have some amount of connections (Average
around 1500k connections). Some go sometimes as high as 1 million conns.
>
> > struct cmap conns;
> > };
> >
> > diff --git a/lib/conntrack.c b/lib/conntrack.c
> > index f313eaa15..f82258775 100644
> > --- a/lib/conntrack.c
> > +++ b/lib/conntrack.c
> > @@ -261,6 +261,7 @@ struct conntrack *
> > conntrack_init(void)
> > {
> > static struct ovsthread_once setup_l4_once =
> > OVSTHREAD_ONCE_INITIALIZER;
> > + struct conntrack_zone *cz;
> > struct conntrack *ct = xzalloc(sizeof *ct);
> >
> > /* This value can be used during init (e.g. timeout_policy_init()),
> > @@ -277,7 +278,9 @@ conntrack_init(void)
> > ovs_mutex_init_adaptive(&ct->ct_lock);
> > ovs_mutex_lock(&ct->ct_lock);
> > for (unsigned i = 0; i < ARRAY_SIZE(ct->zones); i++) {
> > - cmap_init(&ct->zones[i].conns);
> > + cz = zone_lookup(ct, i);
> > + ovs_mutex_init_adaptive(&cz->zone_lock);
> > + cmap_init(&cz->conns);
> > }
> > for (unsigned i = 0; i < ARRAY_SIZE(ct->exp_lists); i++) {
> > rculist_init(&ct->exp_lists[i]);
> > @@ -584,6 +587,7 @@ conn_clean__(struct conntrack *ct, struct conn *conn)
> > cz = zone_lookup(ct, fwd_zone);
> >
> > hash = conn_key_hash(&conn->key_node[CT_DIR_FWD].key, ct->hash_basis);
> > + ovs_mutex_lock(&cz->zone_lock);
> > cmap_remove(&cz->conns,
> > &conn->key_node[CT_DIR_FWD].cm_node, hash);
> >
> > @@ -596,6 +600,7 @@ conn_clean__(struct conntrack *ct, struct conn *conn)
> > }
> >
> > rculist_remove(&conn->node);
> > + ovs_mutex_unlock(&cz->zone_lock);
> > }
> >
> > /* Also removes the associated nat 'conn' from the lookup
> > @@ -634,6 +639,7 @@ conn_force_expire(struct conn *conn)
> > void
> > conntrack_destroy(struct conntrack *ct)
> > {
> > + struct conntrack_zone *cz;
> > struct conn *conn;
> >
> > latch_set(&ct->clean_thread_exit);
> > @@ -664,7 +670,11 @@ conntrack_destroy(struct conntrack *ct)
> >
> > ovs_mutex_lock(&ct->ct_lock);
> > for (unsigned i = 0; i < ARRAY_SIZE(ct->zones); i++) {
> > - cmap_destroy(&ct->zones[i].conns);
> > + cz = zone_lookup(ct, i);
> > + ovs_mutex_lock(&cz->zone_lock);
> > + cmap_destroy(&cz->conns);
> > + ovs_mutex_unlock(&cz->zone_lock);
> > + ovs_mutex_destroy(&cz->zone_lock);
> > }
> > cmap_destroy(&ct->zone_limits);
> > cmap_destroy(&ct->timeout_policies);
> > @@ -1054,7 +1064,7 @@ conn_insert(struct conntrack *ct, struct
> > conntrack_zone *cz,
> > const struct nat_action_info_t *nat_action_info,
> > const char *helper, const struct alg_exp_node *alg_exp,
> > enum ct_alg_ctl_type ct_alg_ctl, uint32_t tp_id)
> > - OVS_REQUIRES(ct->ct_lock)
> > + OVS_REQUIRES(ct->ct_lock, cz->zone_lock)
> > {
> > struct conn_key_node *fwd_key_node, *rev_key_node;
> > struct conn *nc = NULL;
> > @@ -1203,15 +1213,18 @@ conn_maybe_not_found(struct conntrack *ct, struct
> > dp_packet *pkt,
> > * analysis. */
> > if (commit) {
> > ovs_mutex_lock(&ct->ct_lock);
> > + ovs_mutex_lock(&cz->zone_lock);
> > bool found = conn_lookup_zone(ct, cz, &ctx->key, now, NULL, NULL);
> > if (!found) {
> > if (!pkt_validate_and_set_new_ct_state(pkt, ctx, alg_exp)) {
> > + ovs_mutex_unlock(&cz->zone_lock);
> > ovs_mutex_unlock(&ct->ct_lock);
> > return nc;
> > }
> > nc = conn_insert(ct, cz, pkt, ctx, now, nat_action_info,
> > helper, alg_exp, ct_alg_ctl, tp_id);
> > }
> > + ovs_mutex_unlock(&cz->zone_lock);
> > ovs_mutex_unlock(&ct->ct_lock);
> > } else {
> > bool found = conn_lookup_zone(ct, cz, &ctx->key, now, NULL, NULL);
> > --
> > 2.43.0
> >
> >
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
