Felix Huettner <[email protected]> writes:
> This will remove the inefficiencies we would have when just iterating
> using a position.
>
> Signed-off-by: Felix Huettner <[email protected]>
> ---
> lib/conntrack.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
[....]
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index eb8248ed9..12109f03b 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -23,6 +23,7 @@
> #include <string.h>
>
> #include "conntrack.h"
> +#include "cmap.h"
> #include "conntrack-private.h"
> #include "conntrack-tp.h"
> #include "coverage.h"
> @@ -1505,7 +1506,6 @@ ct_sweep_zone(struct conntrack *ct, uint16_t zone, long
> long now,
> struct conntrack_zone *cz;
> unsigned int conn_count = 0;
> struct conn *conn;
> - struct cmap_node *node;
> long long expiration;
>
> cz = zone_lookup(ct, zone);
> @@ -1517,9 +1517,12 @@ ct_sweep_zone(struct conntrack *ct, uint16_t zone,
> long long now,
> *current_position = xzalloc(sizeof(**current_position));
> }
>
> - while ((node = cmap_next_position(&cz->conns, *current_position))) {
> - keyn = OBJECT_CONTAINING(node, keyn, cm_node);
> + struct cmap_cursor cursor = cmap_position_to_cursor(&cz->conns,
> + *current_position);
> +
> + CMAP_CURSOR_FOR_EACH_CONTINUE (keyn, cm_node, &cursor) {
> if (conn_count > limit) {
> + cmap_cursor_to_position(&cursor, *current_position);
CMAP_CURSOR_FOR_EACH_CONTINUE will advance as the first thing it does,
and cmap_cursor_to_position will also advance the element per the
documentation. So will we accidentally skip two nodes here? I'm
worried about a weird case where there are two nodes and we just keep
advancing past them. It's a weird case, agreed.
But I think it's less theoretical given the conn_count > limit has a
case to be hit since (clean_end - count) is the argument that is passed
for the limit (so earlier zones exhaust the budet, and we just cycle
past the nodes).
Apart from that one thing though, I guess it is more a weirdness that we
may want to document in the code with a comment because the
double-advance isn't obvious on first glance. The practical side effect
is that we're already over the limit anyway and we will just do a double
advance here but (hopefully) the next time through we will start from
the beginning and clean the nodes anyway.
> return conn_count;
> }
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
