summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorliuyacan <liuyacan@corp.netease.com>2021-04-25 20:11:30 +0800
committerFlorin Coras <florin.coras@gmail.com>2021-04-26 04:25:40 +0000
commit9f299030fd1214eb1fc076cf0c7f44559a7c8f6b (patch)
tree160e1c524b4e14cfd0c63ae86e9b80a336fc7c50
parentb14c49d2275f6348640572b7b481dad43f3a00d0 (diff)
session: fix race condition in fifo allocation
Under some timing conditions,VCL may receive CONNECTED/ACCEPTED event before ADD_SEGMENT event. Timing example: 2 threads call segment_manager_alloc_session_fifos() parallelly Thread 1 Thread 2 sm read lock | | | try to alloc fifo =>failed | | | sm read unlock | | | sm write lock | | | add segment | | | sm write unlock | | sm read lock | | | try to alloc fifo=>successful sm read lock | | sm read unlock | | | emit CONNECTED/ACCEPTED emit ADD_SEGMENT event | sm read unlock This commit move ADD_SEGMENT notification under the protection of the write lock in some scenarios. Type: fix Signed-off-by: liuyacan <liuyacan@corp.netease.com> Change-Id: I25d5475c5e6d37cfccefa9506f6030c26ce8ee9b
-rw-r--r--src/plugins/unittest/segment_manager_test.c2
-rw-r--r--src/vnet/session/application_local.c2
-rw-r--r--src/vnet/session/segment_manager.c77
-rw-r--r--src/vnet/session/segment_manager.h3
4 files changed, 40 insertions, 44 deletions
diff --git a/src/plugins/unittest/segment_manager_test.c b/src/plugins/unittest/segment_manager_test.c
index c6f52972e51..31b417aef24 100644
--- a/src/plugins/unittest/segment_manager_test.c
+++ b/src/plugins/unittest/segment_manager_test.c
@@ -448,7 +448,7 @@ segment_manager_test_fifo_balanced_alloc (vlib_main_t * vm,
}
/* add another 2MB segment */
- fs_index = segment_manager_add_segment (sm, size_2MB);
+ fs_index = segment_manager_add_segment (sm, size_2MB, 0);
SEG_MGR_TEST ((fs_index == 1), "fs_index %d", fs_index);
/* allocate fifos : 4KB x2
diff --git a/src/vnet/session/application_local.c b/src/vnet/session/application_local.c
index 478fc7a933b..12209527cb9 100644
--- a/src/vnet/session/application_local.c
+++ b/src/vnet/session/application_local.c
@@ -188,7 +188,7 @@ ct_init_accepted_session (app_worker_t * server_wrk,
seg_size = 4 * (round_rx_fifo_sz + round_tx_fifo_sz + margin);
sm = app_worker_get_listen_segment_manager (server_wrk, ll);
- seg_index = segment_manager_add_segment (sm, seg_size);
+ seg_index = segment_manager_add_segment (sm, seg_size, 0);
if (seg_index < 0)
{
clib_warning ("failed to add new cut-through segment");
diff --git a/src/vnet/session/segment_manager.c b/src/vnet/session/segment_manager.c
index 560560c8d64..73b2f7177bd 100644
--- a/src/vnet/session/segment_manager.c
+++ b/src/vnet/session/segment_manager.c
@@ -88,7 +88,8 @@ segment_manager_segment_index (segment_manager_t * sm, fifo_segment_t * seg)
* to avoid affecting any of the segments pool readers.
*/
int
-segment_manager_add_segment (segment_manager_t * sm, uword segment_size)
+segment_manager_add_segment (segment_manager_t *sm, uword segment_size,
+ u8 notify_app)
{
segment_manager_main_t *smm = &sm_main;
segment_manager_props_t *props;
@@ -162,6 +163,16 @@ segment_manager_add_segment (segment_manager_t * sm, uword segment_size)
fs->h->pct_first_alloc = props->pct_first_alloc;
fs->h->flags &= ~FIFO_SEGMENT_F_MEM_LIMIT;
+ if (notify_app)
+ {
+ app_worker_t *app_wrk;
+ u64 fs_handle;
+ fs_handle = segment_manager_segment_handle (sm, fs);
+ app_wrk = app_worker_get (sm->app_wrk_index);
+ rv = app_worker_add_segment_notify (app_wrk, fs_handle);
+ if (rv)
+ return rv;
+ }
done:
if (vlib_num_workers ())
@@ -376,7 +387,7 @@ segment_manager_init_first (segment_manager_t * sm)
/* Allocate the segments */
for (i = 0; i < approx_segment_count + 1; i++)
{
- fs_index = segment_manager_add_segment (sm, max_seg_size);
+ fs_index = segment_manager_add_segment (sm, max_seg_size, 0);
if (fs_index < 0)
{
clib_warning ("Failed to preallocate segment %d", i);
@@ -398,7 +409,7 @@ segment_manager_init_first (segment_manager_t * sm)
return 0;
}
- fs_index = segment_manager_add_segment (sm, first_seg_size);
+ fs_index = segment_manager_add_segment (sm, first_seg_size, 0);
if (fs_index < 0)
{
clib_warning ("Failed to allocate segment");
@@ -665,8 +676,6 @@ segment_manager_alloc_session_fifos (segment_manager_t * sm,
segment_manager_props_t *props;
fifo_segment_t *fs = 0, *cur;
u32 sm_index, fs_index;
- u8 added_a_segment = 0;
- u64 fs_handle;
props = segment_manager_properties_get (sm);
@@ -700,45 +709,12 @@ segment_manager_alloc_session_fifos (segment_manager_t * sm,
segment_manager_segment_reader_unlock (sm);
-alloc_check:
-
- if (!alloc_fail)
- {
-
- alloc_success:
-
- ASSERT (rx_fifo && tx_fifo);
- sm_index = segment_manager_index (sm);
- fs_index = segment_manager_segment_index (sm, fs);
- (*tx_fifo)->segment_manager = sm_index;
- (*rx_fifo)->segment_manager = sm_index;
- (*tx_fifo)->segment_index = fs_index;
- (*rx_fifo)->segment_index = fs_index;
-
- if (added_a_segment)
- {
- app_worker_t *app_wrk;
- fs_handle = segment_manager_segment_handle (sm, fs);
- app_wrk = app_worker_get (sm->app_wrk_index);
- rv = app_worker_add_segment_notify (app_wrk, fs_handle);
- }
- /* Drop the lock after app is notified */
- segment_manager_segment_reader_unlock (sm);
- return rv;
- }
-
/*
* Allocation failed, see if we can add a new segment
*/
if (props->add_segment)
{
- if (added_a_segment)
- {
- clib_warning ("Added a segment, still can't allocate a fifo");
- segment_manager_segment_reader_unlock (sm);
- return SESSION_E_SEG_NO_SPACE2;
- }
- if ((new_fs_index = segment_manager_add_segment (sm, 0)) < 0)
+ if ((new_fs_index = segment_manager_add_segment (sm, 0, 1)) < 0)
{
clib_warning ("Failed to add new segment");
return SESSION_E_SEG_CREATE;
@@ -748,14 +724,33 @@ alloc_check:
props->rx_fifo_size,
props->tx_fifo_size,
rx_fifo, tx_fifo);
- added_a_segment = 1;
- goto alloc_check;
+ if (alloc_fail)
+ {
+ clib_warning ("Added a segment, still can't allocate a fifo");
+ segment_manager_segment_reader_unlock (sm);
+ return SESSION_E_SEG_NO_SPACE2;
+ }
}
else
{
SESSION_DBG ("Can't add new seg and no space to allocate fifos!");
return SESSION_E_SEG_NO_SPACE;
}
+
+alloc_success:
+ ASSERT (rx_fifo && tx_fifo);
+
+ sm_index = segment_manager_index (sm);
+ fs_index = segment_manager_segment_index (sm, fs);
+ (*tx_fifo)->segment_manager = sm_index;
+ (*rx_fifo)->segment_manager = sm_index;
+ (*tx_fifo)->segment_index = fs_index;
+ (*rx_fifo)->segment_index = fs_index;
+
+ /* Drop the lock after app is notified */
+ segment_manager_segment_reader_unlock (sm);
+
+ return rv;
}
void
diff --git a/src/vnet/session/segment_manager.h b/src/vnet/session/segment_manager.h
index 7c3434b70d3..e73a70fda16 100644
--- a/src/vnet/session/segment_manager.h
+++ b/src/vnet/session/segment_manager.h
@@ -101,7 +101,8 @@ segment_manager_t *segment_manager_get (u32 index);
segment_manager_t *segment_manager_get_if_valid (u32 index);
u32 segment_manager_index (segment_manager_t * sm);
-int segment_manager_add_segment (segment_manager_t * sm, uword segment_size);
+int segment_manager_add_segment (segment_manager_t *sm, uword segment_size,
+ u8 notify_app);
void segment_manager_del_segment (segment_manager_t * sm,
fifo_segment_t * fs);
void segment_manager_lock_and_del_segment (segment_manager_t * sm,