From 762cfd408b16b6ab43ade3ab491292b93bdeb9b3 Mon Sep 17 00:00:00 2001 From: Nathan Skrzypczak Date: Wed, 2 Feb 2022 19:31:43 +0100 Subject: [PATCH] cnat: Fix conflicting rsession When dNAT-ing to a VIP, it can happen that the return session conflicts with another forward session than the one we own. This patchs adds a rsession_flags CNAT_SESSION_RETRY_SNAT that makes cnat_session_create search for a free src port to use for the resulting return session. It also makes forward & return session share their fate in the session scanner. Type: fix Change-Id: Id0edf59abf8e5bc0c0d8941ba289c4563c77dee0 Signed-off-by: Nathan Skrzypczak --- src/plugins/cnat/cnat_node.h | 83 ++++++++++++++++++++---------------- src/plugins/cnat/cnat_node_feature.c | 3 +- src/plugins/cnat/cnat_session.c | 35 ++++++++++++++- src/plugins/cnat/cnat_session.h | 5 +++ 4 files changed, 87 insertions(+), 39 deletions(-) diff --git a/src/plugins/cnat/cnat_node.h b/src/plugins/cnat/cnat_node.h index 246fdb8ba57..80d803c4b61 100644 --- a/src/plugins/cnat/cnat_node.h +++ b/src/plugins/cnat/cnat_node.h @@ -846,11 +846,55 @@ cnat_session_create (cnat_session_t *session, cnat_node_ctx_t *ctx, cnat_bihash_kv_t rkey; cnat_session_t *rsession = (cnat_session_t *) & rkey; cnat_bihash_kv_t *bkey = (cnat_bihash_kv_t *) session; - cnat_bihash_kv_t rvalue; - int rv; + int rv, n_retries = 0; + static u32 sport_seed = 0; session->value.cs_ts_index = cnat_timestamp_new (ctx->now); - cnat_bihash_add_del (&cnat_session_db, bkey, 1); + + /* First create the return session */ + ip46_address_copy (&rsession->key.cs_ip[VLIB_RX], + &session->value.cs_ip[VLIB_TX]); + ip46_address_copy (&rsession->key.cs_ip[VLIB_TX], + &session->value.cs_ip[VLIB_RX]); + rsession->key.cs_proto = session->key.cs_proto; + rsession->key.cs_loc = rsession_location; + rsession->key.__cs_pad = 0; + rsession->key.cs_af = ctx->af; + rsession->key.cs_port[VLIB_RX] = session->value.cs_port[VLIB_TX]; + rsession->key.cs_port[VLIB_TX] = session->value.cs_port[VLIB_RX]; + + ip46_address_copy (&rsession->value.cs_ip[VLIB_RX], + &session->key.cs_ip[VLIB_TX]); + ip46_address_copy (&rsession->value.cs_ip[VLIB_TX], + &session->key.cs_ip[VLIB_RX]); + rsession->value.cs_ts_index = session->value.cs_ts_index; + rsession->value.cs_lbi = INDEX_INVALID; + rsession->value.flags = rsession_flags | CNAT_SESSION_IS_RETURN; + rsession->value.cs_port[VLIB_TX] = session->key.cs_port[VLIB_RX]; + rsession->value.cs_port[VLIB_RX] = session->key.cs_port[VLIB_TX]; + +retry_add_ression: + rv = cnat_bihash_add_del (&cnat_session_db, &rkey, + 2 /* add but don't overwrite */); + if (rv) + { + if (!(rsession_flags & CNAT_SESSION_RETRY_SNAT)) + return; + + /* return session add failed pick an new random src port */ + rsession->value.cs_port[VLIB_TX] = session->key.cs_port[VLIB_RX] = + random_u32 (&sport_seed); + if (n_retries++ < 100) + goto retry_add_ression; + else + { + clib_warning ("Could not find a free port after 100 tries"); + /* translate this packet, but don't create state */ + return; + } + } + + cnat_bihash_add_del (&cnat_session_db, bkey, 1 /* add */); if (!(rsession_flags & CNAT_SESSION_FLAG_NO_CLIENT)) { @@ -894,39 +938,6 @@ cnat_session_create (cnat_session_t *session, cnat_node_ctx_t *ctx, } } - /* create the reverse flow key */ - ip46_address_copy (&rsession->key.cs_ip[VLIB_RX], - &session->value.cs_ip[VLIB_TX]); - ip46_address_copy (&rsession->key.cs_ip[VLIB_TX], - &session->value.cs_ip[VLIB_RX]); - rsession->key.cs_proto = session->key.cs_proto; - rsession->key.cs_loc = rsession_location; - rsession->key.__cs_pad = 0; - rsession->key.cs_af = ctx->af; - rsession->key.cs_port[VLIB_RX] = session->value.cs_port[VLIB_TX]; - rsession->key.cs_port[VLIB_TX] = session->value.cs_port[VLIB_RX]; - - /* First search for existing reverse session */ - rv = cnat_bihash_search_i2 (&cnat_session_db, &rkey, &rvalue); - if (!rv) - { - /* Reverse session already exists - cleanup before creating for refcnts */ - cnat_session_t *found_rsession = (cnat_session_t *) & rvalue; - cnat_session_free (found_rsession); - } - /* add the reverse flow */ - ip46_address_copy (&rsession->value.cs_ip[VLIB_RX], - &session->key.cs_ip[VLIB_TX]); - ip46_address_copy (&rsession->value.cs_ip[VLIB_TX], - &session->key.cs_ip[VLIB_RX]); - rsession->value.cs_ts_index = session->value.cs_ts_index; - rsession->value.cs_lbi = INDEX_INVALID; - rsession->value.flags = rsession_flags | CNAT_SESSION_IS_RETURN; - rsession->value.cs_port[VLIB_TX] = session->key.cs_port[VLIB_RX]; - rsession->value.cs_port[VLIB_RX] = session->key.cs_port[VLIB_TX]; - - cnat_bihash_add_del (&cnat_session_db, &rkey, 1); } always_inline uword diff --git a/src/plugins/cnat/cnat_node_feature.c b/src/plugins/cnat/cnat_node_feature.c index aced4cd0a15..76aa893983d 100644 --- a/src/plugins/cnat/cnat_node_feature.c +++ b/src/plugins/cnat/cnat_node_feature.c @@ -321,7 +321,8 @@ cnat_output_feature_fn (vlib_main_t *vm, vlib_node_runtime_t *node, trace_flags |= CNAT_TRACE_SESSION_CREATED; cnat_session_create (session, ctx, CNAT_LOCATION_INPUT, - CNAT_SESSION_FLAG_NO_CLIENT); + CNAT_SESSION_FLAG_NO_CLIENT | + CNAT_SESSION_RETRY_SNAT); } if (AF_IP4 == ctx->af) diff --git a/src/plugins/cnat/cnat_session.c b/src/plugins/cnat/cnat_session.c index 216d2575c37..fa04de602b4 100644 --- a/src/plugins/cnat/cnat_session.c +++ b/src/plugins/cnat/cnat_session.c @@ -172,15 +172,43 @@ cnat_session_purge (void) return (0); } +void +cnat_reverse_session_free (cnat_session_t *session) +{ + cnat_bihash_kv_t bkey, bvalue; + cnat_session_t *rsession = (cnat_session_t *) &bkey; + int rv; + + ip46_address_copy (&rsession->key.cs_ip[VLIB_RX], + &session->value.cs_ip[VLIB_TX]); + ip46_address_copy (&rsession->key.cs_ip[VLIB_TX], + &session->value.cs_ip[VLIB_RX]); + rsession->key.cs_proto = session->key.cs_proto; + rsession->key.cs_loc = session->key.cs_loc == CNAT_LOCATION_OUTPUT ? + CNAT_LOCATION_INPUT : + CNAT_LOCATION_OUTPUT; + rsession->key.__cs_pad = 0; + rsession->key.cs_af = session->key.cs_af; + rsession->key.cs_port[VLIB_RX] = session->value.cs_port[VLIB_TX]; + rsession->key.cs_port[VLIB_TX] = session->value.cs_port[VLIB_RX]; + + rv = cnat_bihash_search_i2 (&cnat_session_db, &bkey, &bvalue); + if (!rv) + { + /* other session is in bihash */ + cnat_session_t *rsession = (cnat_session_t *) &bvalue; + cnat_session_free (rsession); + } +} + u64 cnat_session_scan (vlib_main_t * vm, f64 start_time, int i) { BVT (clib_bihash) * h = &cnat_session_db; int j, k; - /* Don't scan the l2 fib if it hasn't been instantiated yet */ if (alloc_arena (h) == 0) - return 0.0; + return 0; for ( /* caller saves starting point */ ; i < h->nbuckets; i++) { @@ -219,6 +247,9 @@ cnat_session_scan (vlib_main_t * vm, f64 start_time, int i) cnat_timestamp_exp (session->value.cs_ts_index)) { /* age it */ + cnat_reverse_session_free (session); + /* this should be last as deleting the session memset it to + * 0xff */ cnat_session_free (session); /* diff --git a/src/plugins/cnat/cnat_session.h b/src/plugins/cnat/cnat_session.h index 072bb10f96f..a0a28c9a818 100644 --- a/src/plugins/cnat/cnat_session.h +++ b/src/plugins/cnat/cnat_session.h @@ -129,6 +129,11 @@ typedef enum cnat_session_flag_t_ /* Debug flag marking return sessions */ CNAT_SESSION_IS_RETURN = (1 << 4), + + /** On conflicts when adding the return session, try to sNAT the + * forward session, and dNAT the return session with a random port */ + CNAT_SESSION_RETRY_SNAT = (1 << 5), + } cnat_session_flag_t; typedef enum cnat_session_location_t_ -- 2.16.6