ligang/vpp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

nat: fix per-vrf session bookkeeping

Browse Source 
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
This commit is contained in:
Jing Peng
2022-07-15 15:12:29 -04:00
committed by Ole Tr�an
parent 9ff833d8f4
commit 61fdfd51d1
3 changed files with 26 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
src/plugins/nat/nat44-ed/nat44_ed.c
View File

@ -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

2
src/plugins/nat/nat44-ed/nat44_ed.h
View File

@ -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;

30
src/plugins/nat/nat44-ed/nat44_ed_inlines.h
View File

@ -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;
}