Hi Paolo,
On 4/19/23 2:40 PM, Paolo Valerio wrote:
During the creation of a new connection, there's a chance both key and
rev_key end up having the same hash. This is more common in the case
of all-zero snat with no collisions. In that case, once the
connection is expired, but not cleaned up, if a new packet with the
same 5-tuple is received, an assertion failure gets triggered in
conn_update_state() because of a previous failure of retrieving a
CT_CONN_TYPE_DEFAULT connection.
Fix it by releasing the nat_conn during the connection creation in the
case of same hash for both key and rev_key.
Sorry for reviving a two month-old thread, but we recently started
seeing this issue which seemed to also be related to [0], but I can't
find it in patchworks or the tree. Was there a plan to update it?
Thanks,
-Brian
[0] https://www.mail-archive.com/[email protected]/msg08945.html
Reported-by: Michael Plato <[email protected]>
Fixes: 61e48c2d1db2 ("conntrack: Handle SNAT with all-zero IP address.")
Signed-off-by: Paolo Valerio <[email protected]>
---
In this thread [0] there are some more details. A similar
approach here could be to avoid to add the nat_conn to the cmap and
letting the sweeper release the memory for nat_conn once the whole
connection gets freed.
That approach could still be ok, but the drawback is that it could
require a different patch for older branches that don't include
3d9c1b855a5f ("conntrack: Replace timeout based expiration lists with
rculists."). It still worth to be considered.
[0] https://mail.openvswitch.org/pipermail/ovs-discuss/2023-April/052339.html
---
lib/conntrack.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 7e1fc4b1f..d2ee127d9 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1007,14 +1007,19 @@ conn_not_found(struct conntrack *ct, struct dp_packet
*pkt,
}
nat_packet(pkt, nc, false, ctx->icmp_related);
- memcpy(&nat_conn->key, &nc->rev_key, sizeof nat_conn->key);
- memcpy(&nat_conn->rev_key, &nc->key, sizeof nat_conn->rev_key);
- nat_conn->conn_type = CT_CONN_TYPE_UN_NAT;
- nat_conn->nat_action = 0;
- nat_conn->alg = NULL;
- nat_conn->nat_conn = NULL;
- uint32_t nat_hash = conn_key_hash(&nat_conn->key, ct->hash_basis);
- cmap_insert(&ct->conns, &nat_conn->cm_node, nat_hash);
+ uint32_t nat_hash = conn_key_hash(&nc->rev_key, ct->hash_basis);
+ if (nat_hash != ctx->hash) {
+ memcpy(&nat_conn->key, &nc->rev_key, sizeof nat_conn->key);
+ memcpy(&nat_conn->rev_key, &nc->key, sizeof nat_conn->rev_key);
+ nat_conn->conn_type = CT_CONN_TYPE_UN_NAT;
+ nat_conn->nat_action = 0;
+ nat_conn->alg = NULL;
+ nat_conn->nat_conn = NULL;
+ cmap_insert(&ct->conns, &nat_conn->cm_node, nat_hash);
+ } else {
+ free(nat_conn);
+ nat_conn = NULL;
+ }
}
nc->nat_conn = nat_conn;
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
