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;
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).
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
