Am Thu, Aug 06, 2026 at 10:58:02AM -0400 schrieb Aaron Conole: > Felix Huettner <[email protected]> writes: > > > 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(-) > > [...] > > > 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;
Hi Aaron, thanks for the review > > Here, we have a default_zone_limit that is signed, 64-bit, but... > > > +zone_limit_update_default(struct conntrack *ct, uint32_t limit) > > This function uses an unsigned 32-bit integer, and I'm worried on this > line: > > > + atomic_store_relaxed(&ct->default_zone_limit, limit); > > about whether there is really proper sign extension, etc. Actually, > these constants earlier: > > > +#define CONN_LIMIT_NONE -1 > > +#define CONN_LIMIT_USE_DEFAULT -2 > > may have some implicit conversion and mean that when switching between > int64_t and uint32_t types, they will have very different values. We > should declare them: > > +#define CONN_LIMIT_NONE -1LL > +#define CONN_LIMIT_USE_DEFAULT -2LL > > and let the compiler down convert them, and probably make sure that our > limit definitions are always signed values (or make sure we guarantee > proper sign-bit extension). Thats a good point. I'll update the defines and zone_limit_update_default in the next version. I will also updat zone_limit_get as it had similar issues. Thanks, Felix > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
