[ovs-dev] [PATCH v2 2/3] conntrack: Support conntrack flush by ct 5-tuple

Justin Pettit jpettit at gmail.com
Wed Dec 6 22:57:18 UTC 2017



> On Nov 21, 2017, at 5:00 PM, Yi-Hung Wei <yihung.wei at gmail.com> wrote:
> 
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index f5a3aa9fab34..c9077c07042c 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -2335,6 +2335,18 @@ ct_endpoint_to_ct_dpif_inet_addr(const struct ct_addr *a,
> }
> 
> static void
> +ct_dpif_inet_addr_to_ct_endpoint(const union ct_dpif_inet_addr *a,
> +                                 struct ct_addr *b,
> +                                 const uint16_t l3_type)
> +{
> +    if (l3_type == AF_INET) {
> +        b->ipv4_aligned = a->ip;
> +    } else if (l3_type == AF_INET6) {
> +        b->ipv6_aligned = a->in6;
> +    }
> +}

This is the reverse of the function ct_endpoint_to_ct_dpif_inet_addr(), and they both take a 16-bit argument describing the IP version, but their form is different.  I'd suggest adding comments to both to make it clearer, since it could lead to subtle bugs for people using them later:

-=-=-=-=-=-=-=-=-
/* Convert a conntrack address 'a' into an IP address 'b' of type 'dl_type'.
 *
 * Note that 'dl_type' should be either "ETH_TYPE_IP" or "ETH_TYPE_IPv6"
 * in network-byte order. */
static void
ct_endpoint_to_ct_dpif_inet_addr(const struct ct_addr *a,
                                 union ct_dpif_inet_addr *b,
                                 ovs_be16 dl_type)

....

/* Convert an IP address 'a' of type 'l3_type' into a conntrack address 'b'.
 *
 * Note that 'l3_type' should be either "AF_INET" or "AF_INET6". */
static void
ct_dpif_inet_addr_to_ct_endpoint(const union ct_dpif_inet_addr *a,
                                 struct ct_addr *b,
                                 const uint16_t l3_type)
-=-=-=-=-=-=-=-=-

> +int
> +conntrack_flush_tuple(struct conntrack *ct, const struct ct_dpif_tuple *tuple,
> +                      uint16_t zone)
> +{
> +    struct conn_lookup_ctx ctx;
> +    int error = 0;
> +
> +    memset(&ctx, 0, sizeof(ctx));
> +    tuple_to_conn_key(tuple, zone, &ctx.key);
> +    ctx.hash = conn_key_hash(&ctx.key, ct->hash_basis);
> +    unsigned bucket = hash_to_bucket(ctx.hash);
> +
> +    ct_lock_lock(&ct->buckets[bucket].lock);
> +    conn_key_lookup(&ct->buckets[bucket], &ctx, time_msec());
> +    if (ctx.conn) {
> +        conn_clean(ct, ctx.conn, &ct->buckets[bucket]);
> +    } else {
> +        error = ENOENT;
> +    }
> +    ct_lock_unlock(&ct->buckets[bucket].lock);
> +    return error;
> +}

I believe the kernel clears out expectations.  As we discussed off-line, I think it would make sense to follow similar behavior, since presumably if the control channel is being flushed, the related flows should be, too.

--Justin




More information about the dev mailing list