aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJing Peng <jing@meter.com>2022-07-15 15:12:29 -0400
committerOle Tr�an <otroan@employees.org>2022-11-07 08:00:23 +0000
commit61fdfd51d1dd5bd4f3bc02b332b265c28e879221 (patch)
tree414b4893346d8dccddf44723fdccd802dee7db2b
parent9ff833d8f4ccccc475f9a302b8f70eed49963472 (diff)
nat: fix per-vrf session bookkeeping
Each NAT44 ED session has a per_vrf_sessions_index referencing an element in the thread-local vector per_vrf_sessions_vec. However this index can be possibly invalidated by vec_del1() in per_vrf_sessions_cleanup(), before a session is registered. Such a stale index can cause an assertion failure in function per_vrf_sessions_is_expired() when we use it to locate the per_vrf_sessions object. A possible sequence to reproduce is: 1. Create two NAT44 ED sessions s1, s2 so that two per_vrf_sessions are created: index 0: between VRF pair 10 and 11 (expired=0, ses_count=1) index 1: between VRF pair 20 and 21 (expired=0, ses_count=1) For the sessions we have: s1->per_vrf_sessions_index == 0 s2->per_vrf_sessions_index == 1 2. Delete the first session via CLI, now the two per_vrf_sessions become: index 0: between VRF pair 10 and 11 (expired=0, ses_count=0) index 1: between VRF pair 20 and 21 (expired=0, ses_count=1) For the sessions we have: s2->per_vrf_sessions_index == 1 3. Delete the VRF 11: index 0: between VRF pair 10 and 11 (expired=1, ses_count=0) index 1: between VRF pair 20 and 21 (expired=0, ses_count=1) For the sessions we have: s2->per_vrf_sessions_index == 1 4. Create a new session s3 between VRF pair 20 and 21 so that the first per_vrf_sessions will be deleted: index 0: between VRF pair 20 and 21 (expired=0, ses_count=2) For the sessions we have: s2->per_vrf_sessions_index == 1 s3->per_vrf_sessions_index == 0 Here, note that the actual index of per_vrf_session is changed due to vec_del1(). The new session is added after the cleanup so it gets the correct index. But the index held by the existing session is not updated. 5. Trigger the fast path of the session s2. To achieve this, session s2 could be created in step 1 by ping -i20 -Iiface_in_vrf_10 1.1.1.1 and steps 2-4 should then be performed within the 20-second interval. This patch fixes this by changing per_vrf_sessions_vec to a pool so that indicies are kept intact. Type: fix Signed-off-by: Jing Peng <jing@meter.com> Change-Id: I4c08f9bfd50134bcb5f08e50ad61af2bddbcb645
-rw-r--r--src/plugins/nat/nat44-ed/nat44_ed.c28
-rw-r--r--src/plugins/nat/nat44-ed/nat44_ed.h2
-rw-r--r--src/plugins/nat/nat44-ed/nat44_ed_inlines.h30
3 files changed, 26 insertions, 34 deletions
diff --git a/src/plugins/nat/nat44-ed/nat44_ed.c b/src/plugins/nat/nat44-ed/nat44_ed.c
index ed93f6cc55f..133c39e28c3 100644
--- a/src/plugins/nat/nat44-ed/nat44_ed.c
+++ b/src/plugins/nat/nat44-ed/nat44_ed.c
@@ -1633,19 +1633,19 @@ expire_per_vrf_sessions (u32 fib_index)
vec_foreach (tsm, sm->per_thread_data)
{
- vec_foreach (per_vrf_sessions, tsm->per_vrf_sessions_vec)
- {
- if ((per_vrf_sessions->rx_fib_index == fib_index) ||
- (per_vrf_sessions->tx_fib_index == fib_index))
- {
- per_vrf_sessions->expired = 1;
- }
- }
+ pool_foreach (per_vrf_sessions, tsm->per_vrf_sessions_pool)
+ {
+ if ((per_vrf_sessions->rx_fib_index == fib_index) ||
+ (per_vrf_sessions->tx_fib_index == fib_index))
+ {
+ per_vrf_sessions->expired = 1;
+ }
+ }
}
}
void
-update_per_vrf_sessions_vec (u32 fib_index, int is_del)
+update_per_vrf_sessions_pool (u32 fib_index, int is_del)
{
snat_main_t *sm = &snat_main;
nat_fib_t *fib;
@@ -1788,7 +1788,7 @@ nat44_ed_add_interface (u32 sw_if_index, u8 is_inside)
fib_index =
fib_table_get_index_for_sw_if_index (FIB_PROTOCOL_IP4, sw_if_index);
- update_per_vrf_sessions_vec (fib_index, 0 /*is_del*/);
+ update_per_vrf_sessions_pool (fib_index, 0 /*is_del*/);
if (!is_inside)
{
@@ -1903,7 +1903,7 @@ nat44_ed_del_interface (u32 sw_if_index, u8 is_inside)
fib_index =
fib_table_get_index_for_sw_if_index (FIB_PROTOCOL_IP4, sw_if_index);
- update_per_vrf_sessions_vec (fib_index, 1 /*is_del*/);
+ update_per_vrf_sessions_pool (fib_index, 1 /*is_del*/);
if (!is_inside)
{
@@ -2002,7 +2002,7 @@ nat44_ed_add_output_interface (u32 sw_if_index)
fib_index =
fib_table_get_index_for_sw_if_index (FIB_PROTOCOL_IP4, sw_if_index);
- update_per_vrf_sessions_vec (fib_index, 0 /*is_del*/);
+ update_per_vrf_sessions_pool (fib_index, 0 /*is_del*/);
outside_fib = nat44_ed_get_outside_fib (sm->outside_fibs, fib_index);
if (outside_fib)
@@ -2092,7 +2092,7 @@ nat44_ed_del_output_interface (u32 sw_if_index)
fib_index =
fib_table_get_index_for_sw_if_index (FIB_PROTOCOL_IP4, sw_if_index);
- update_per_vrf_sessions_vec (fib_index, 1 /*is_del*/);
+ update_per_vrf_sessions_pool (fib_index, 1 /*is_del*/);
outside_fib = nat44_ed_get_outside_fib (sm->outside_fibs, fib_index);
if (outside_fib)
@@ -3287,7 +3287,7 @@ nat44_ed_worker_db_free (snat_main_per_thread_data_t *tsm)
{
pool_free (tsm->lru_pool);
pool_free (tsm->sessions);
- vec_free (tsm->per_vrf_sessions_vec);
+ pool_free (tsm->per_vrf_sessions_pool);
}
void
diff --git a/src/plugins/nat/nat44-ed/nat44_ed.h b/src/plugins/nat/nat44-ed/nat44_ed.h
index bc69648dc3f..706511475cf 100644
--- a/src/plugins/nat/nat44-ed/nat44_ed.h
+++ b/src/plugins/nat/nat44-ed/nat44_ed.h
@@ -469,7 +469,7 @@ typedef struct
/* real thread index */
u32 thread_index;
- per_vrf_sessions_t *per_vrf_sessions_vec;
+ per_vrf_sessions_t *per_vrf_sessions_pool;
} snat_main_per_thread_data_t;
diff --git a/src/plugins/nat/nat44-ed/nat44_ed_inlines.h b/src/plugins/nat/nat44-ed/nat44_ed_inlines.h
index b99d152da26..04e5236b7f9 100644
--- a/src/plugins/nat/nat44-ed/nat44_ed_inlines.h
+++ b/src/plugins/nat/nat44-ed/nat44_ed_inlines.h
@@ -414,23 +414,16 @@ per_vrf_sessions_cleanup (u32 thread_index)
per_vrf_sessions_t *per_vrf_sessions;
u32 *to_free = 0, *i;
- vec_foreach (per_vrf_sessions, tsm->per_vrf_sessions_vec)
+ pool_foreach (per_vrf_sessions, tsm->per_vrf_sessions_pool)
{
- if (per_vrf_sessions->expired)
- {
- if (per_vrf_sessions->ses_count == 0)
- {
- vec_add1 (to_free, per_vrf_sessions - tsm->per_vrf_sessions_vec);
- }
- }
+ if (per_vrf_sessions->expired && per_vrf_sessions->ses_count == 0)
+ vec_add1 (to_free, per_vrf_sessions - tsm->per_vrf_sessions_pool);
}
- if (vec_len (to_free))
+ vec_foreach (i, to_free)
{
- vec_foreach (i, to_free)
- {
- vec_del1 (tsm->per_vrf_sessions_vec, *i);
- }
+ per_vrf_sessions = pool_elt_at_index (tsm->per_vrf_sessions_pool, *i);
+ pool_put (tsm->per_vrf_sessions_pool, per_vrf_sessions);
}
vec_free (to_free);
@@ -449,7 +442,7 @@ per_vrf_sessions_register_session (snat_session_t *s, u32 thread_index)
// s->per_vrf_sessions_index == ~0 ... reuse of old session
- vec_foreach (per_vrf_sessions, tsm->per_vrf_sessions_vec)
+ pool_foreach (per_vrf_sessions, tsm->per_vrf_sessions_pool)
{
// ignore already expired registrations
if (per_vrf_sessions->expired)
@@ -468,14 +461,13 @@ per_vrf_sessions_register_session (snat_session_t *s, u32 thread_index)
}
// create a new registration
- vec_add2 (tsm->per_vrf_sessions_vec, per_vrf_sessions, 1);
+ pool_get (tsm->per_vrf_sessions_pool, per_vrf_sessions);
clib_memset (per_vrf_sessions, 0, sizeof (*per_vrf_sessions));
-
per_vrf_sessions->rx_fib_index = s->in2out.fib_index;
per_vrf_sessions->tx_fib_index = s->out2in.fib_index;
done:
- s->per_vrf_sessions_index = per_vrf_sessions - tsm->per_vrf_sessions_vec;
+ s->per_vrf_sessions_index = per_vrf_sessions - tsm->per_vrf_sessions_pool;
per_vrf_sessions->ses_count++;
}
@@ -491,7 +483,7 @@ per_vrf_sessions_unregister_session (snat_session_t *s, u32 thread_index)
tsm = vec_elt_at_index (sm->per_thread_data, thread_index);
per_vrf_sessions =
- vec_elt_at_index (tsm->per_vrf_sessions_vec, s->per_vrf_sessions_index);
+ pool_elt_at_index (tsm->per_vrf_sessions_pool, s->per_vrf_sessions_index);
ASSERT (per_vrf_sessions->ses_count != 0);
@@ -511,7 +503,7 @@ per_vrf_sessions_is_expired (snat_session_t *s, u32 thread_index)
tsm = vec_elt_at_index (sm->per_thread_data, thread_index);
per_vrf_sessions =
- vec_elt_at_index (tsm->per_vrf_sessions_vec, s->per_vrf_sessions_index);
+ pool_elt_at_index (tsm->per_vrf_sessions_pool, s->per_vrf_sessions_index);
return per_vrf_sessions->expired;
}