Skip to content

Commit

Permalink
conntrack: Remove nat_conn introducing key directionality.
Browse files Browse the repository at this point in the history
The patch avoids the extra allocation for nat_conn.
Currently, when doing NAT, the userspace conntrack will use an extra
conn for the two directions in a flow. However, each conn has actually
the two keys for both orig and rev directions. This patch introduces a
key_node[CT_DIRS] member as per Aaron's suggestion in the conn which
consists of a key, direction, and a cmap_node for hash lookup so
addressing the feedback received by the original patch [0].

With this adjustment, we also remove the assertion that connections in
the table are DEFAULT while updating connection state and/or removing
connections.

[0] https://patchwork.ozlabs.org/project/openvswitch/patch/20201129033255.64647-2-hepeng.0320@bytedance.com/

Reported-by: Michael Plato <michael.plato@tu-berlin.de>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2022-September/052065.html
Signed-off-by: Peng He <hepeng.0320@bytedance.com>
Co-authored-by: Paolo Valerio <pvalerio@redhat.com>
Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
Tested-by: Frode Nordahl <frode.nordahl@canonical.com>
Acked-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Simon Horman <horms@ovn.org>
  • Loading branch information
2 people authored and Simon Horman committed Mar 6, 2024
1 parent b5f2ed2 commit ff0e477
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 227 deletions.
19 changes: 11 additions & 8 deletions lib/conntrack-private.h
Expand Up @@ -49,6 +49,12 @@ struct ct_endpoint {
* hashing in ct_endpoint_hash_add(). */
BUILD_ASSERT_DECL(sizeof(struct ct_endpoint) == sizeof(union ct_addr) + 4);

enum key_dir {
CT_DIR_FWD = 0,
CT_DIR_REV,
CT_DIRS,
};

/* Changes to this structure need to be reflected in conn_key_hash()
* and conn_key_cmp(). */
struct conn_key {
Expand Down Expand Up @@ -112,20 +118,18 @@ enum ct_timeout {

#define N_EXP_LISTS 100

enum OVS_PACKED_ENUM ct_conn_type {
CT_CONN_TYPE_DEFAULT,
CT_CONN_TYPE_UN_NAT,
struct conn_key_node {
enum key_dir dir;
struct conn_key key;
struct cmap_node cm_node;
};

struct conn {
/* Immutable data. */
struct conn_key key;
struct conn_key rev_key;
struct conn_key_node key_node[CT_DIRS];
struct conn_key parent_key; /* Only used for orig_tuple support. */
struct cmap_node cm_node;
uint16_t nat_action;
char *alg;
struct conn *nat_conn; /* The NAT 'conn' context, if there is one. */
atomic_flag reclaimed; /* False during the lifetime of the connection,
* True as soon as a thread has started freeing
* its memory. */
Expand All @@ -150,7 +154,6 @@ struct conn {

/* Immutable data. */
bool alg_related; /* True if alg data connection. */
enum ct_conn_type conn_type;

uint32_t tp_id; /* Timeout policy ID. */
};
Expand Down
6 changes: 4 additions & 2 deletions lib/conntrack-tp.c
Expand Up @@ -253,7 +253,8 @@ conn_update_expiration(struct conntrack *ct, struct conn *conn,
}
VLOG_DBG_RL(&rl, "Update timeout %s zone=%u with policy id=%d "
"val=%u sec.",
ct_timeout_str[tm], conn->key.zone, conn->tp_id, val);
ct_timeout_str[tm], conn->key_node[CT_DIR_FWD].key.zone,
conn->tp_id, val);

atomic_store_relaxed(&conn->expiration, now + val * 1000);
}
Expand All @@ -273,7 +274,8 @@ conn_init_expiration(struct conntrack *ct, struct conn *conn,
}

VLOG_DBG_RL(&rl, "Init timeout %s zone=%u with policy id=%d val=%u sec.",
ct_timeout_str[tm], conn->key.zone, conn->tp_id, val);
ct_timeout_str[tm], conn->key_node[CT_DIR_FWD].key.zone,
conn->tp_id, val);

conn->expiration = now + val * 1000;
}

0 comments on commit ff0e477

Please sign in to comment.