| Message ID | 20240701122721.622994-5-pvalerio@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | aaron conole |
| Headers | show |
| Series | Fix default zone limit | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
Paolo Valerio <pvalerio@redhat.com> writes: > while at it, changes struct zone_limit initialization in > zone_limit_create() in order to use atomic init operations instead of > relying on memset() which, although correctly initializes the struct, > is semantially not aware of atomics. semantically > Signed-off-by: Paolo Valerio <pvalerio@redhat.com> > --- > lib/conntrack-private.h | 2 +- > lib/conntrack.c | 19 ++++++++++++------- > 2 files changed, 13 insertions(+), 8 deletions(-) > > diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h > index 2c625d710..2770470d1 100644 > --- a/lib/conntrack-private.h > +++ b/lib/conntrack-private.h > @@ -201,7 +201,7 @@ enum ct_ephemeral_range { > > struct conntrack_zone_limit { > int32_t zone; > - uint32_t limit; > + atomic_uint32_t limit; > atomic_count count; > uint32_t zone_limit_seq; /* Used to disambiguate zone limit counts. */ > }; > diff --git a/lib/conntrack.c b/lib/conntrack.c > index 0481a8c8a..ac0790e11 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -341,7 +341,7 @@ zone_limit_get(struct conntrack *ct, int32_t zone) > struct zone_limit *zl = zone_limit_lookup_or_default(ct, zone); > if (zl) { > czl.zone = zl->czl.zone; > - czl.limit = zl->czl.limit; > + atomic_read_relaxed(&zl->czl.limit, &czl.limit); > czl.count = atomic_count_get(&zl->czl.count); > } > return czl; > @@ -358,8 +358,9 @@ zone_limit_create(struct conntrack *ct, int32_t zone, uint32_t limit) > } > > if (zone >= DEFAULT_ZONE && zone <= MAX_ZONE) { > - zl = xzalloc(sizeof *zl); > - zl->czl.limit = limit; > + 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); > @@ -376,7 +377,7 @@ zone_limit_update(struct conntrack *ct, int32_t zone, uint32_t limit) > int err = 0; > struct zone_limit *zl = zone_limit_lookup(ct, zone); > if (zl) { > - zl->czl.limit = limit; > + atomic_store_relaxed(&zl->czl.limit, limit); > VLOG_INFO("Changed zone limit of %u for zone %d", limit, zone); > } else { > ovs_mutex_lock(&ct->ct_lock); > @@ -916,12 +917,16 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt, > } > > if (commit) { > + uint32_t czl_limit; > struct conn_key_node *fwd_key_node, *rev_key_node; > struct zone_limit *zl = zone_limit_lookup_or_default(ct, > ctx->key.zone); > - if (zl && atomic_count_get(&zl->czl.count) >= zl->czl.limit) { > - COVERAGE_INC(conntrack_zone_full); > - return nc; > + if (zl) { > + atomic_read_relaxed(&zl->czl.limit, &czl_limit); > + if (atomic_count_get(&zl->czl.count) >= czl_limit) { > + COVERAGE_INC(conntrack_zone_full); > + return nc; > + } > } > > unsigned int n_conn_limit;
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h index 2c625d710..2770470d1 100644 --- a/lib/conntrack-private.h +++ b/lib/conntrack-private.h @@ -201,7 +201,7 @@ enum ct_ephemeral_range { struct conntrack_zone_limit { int32_t zone; - uint32_t limit; + atomic_uint32_t limit; atomic_count count; uint32_t zone_limit_seq; /* Used to disambiguate zone limit counts. */ }; diff --git a/lib/conntrack.c b/lib/conntrack.c index 0481a8c8a..ac0790e11 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -341,7 +341,7 @@ zone_limit_get(struct conntrack *ct, int32_t zone) struct zone_limit *zl = zone_limit_lookup_or_default(ct, zone); if (zl) { czl.zone = zl->czl.zone; - czl.limit = zl->czl.limit; + atomic_read_relaxed(&zl->czl.limit, &czl.limit); czl.count = atomic_count_get(&zl->czl.count); } return czl; @@ -358,8 +358,9 @@ zone_limit_create(struct conntrack *ct, int32_t zone, uint32_t limit) } if (zone >= DEFAULT_ZONE && zone <= MAX_ZONE) { - zl = xzalloc(sizeof *zl); - zl->czl.limit = limit; + 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); @@ -376,7 +377,7 @@ zone_limit_update(struct conntrack *ct, int32_t zone, uint32_t limit) int err = 0; struct zone_limit *zl = zone_limit_lookup(ct, zone); if (zl) { - zl->czl.limit = limit; + atomic_store_relaxed(&zl->czl.limit, limit); VLOG_INFO("Changed zone limit of %u for zone %d", limit, zone); } else { ovs_mutex_lock(&ct->ct_lock); @@ -916,12 +917,16 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt, } if (commit) { + uint32_t czl_limit; struct conn_key_node *fwd_key_node, *rev_key_node; struct zone_limit *zl = zone_limit_lookup_or_default(ct, ctx->key.zone); - if (zl && atomic_count_get(&zl->czl.count) >= zl->czl.limit) { - COVERAGE_INC(conntrack_zone_full); - return nc; + if (zl) { + atomic_read_relaxed(&zl->czl.limit, &czl_limit); + if (atomic_count_get(&zl->czl.count) >= czl_limit) { + COVERAGE_INC(conntrack_zone_full); + return nc; + } } unsigned int n_conn_limit;
while at it, changes struct zone_limit initialization in zone_limit_create() in order to use atomic init operations instead of relying on memset() which, although correctly initializes the struct, is semantially not aware of atomics. Signed-off-by: Paolo Valerio <pvalerio@redhat.com> --- lib/conntrack-private.h | 2 +- lib/conntrack.c | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-)