Hi, 

Apologies, I missed your original point and only though about the large bitmap 
we create at startup. So yes, go for s->session_index + 1.

Regards,
Florin

> On Oct 24, 2022, at 9:11 PM, Zhang Dongya <fortitude.zh...@gmail.com> wrote:
> 
> Hi,
> 
> Can you elaborate a bit on that, If session index is 64, if we do not 
> increase by 1, it will only make one 64B vec for the bitmap, which may not 
> hold the session index.
> 
> Florin Coras <fcoras.li...@gmail.com <mailto:fcoras.li...@gmail.com>> 
> 于2022年10月25日周二 01:14写道:
> Hi, 
> 
> Could you replace s->session_index by s->session_index ? : 1 in the patch? 
> 
> Regards, 
> Florin
> 
>> On Oct 24, 2022, at 12:23 AM, Zhang Dongya <fortitude.zh...@gmail.com 
>> <mailto:fortitude.zh...@gmail.com>> wrote:
>> 
>> Hi list,
>> 
>> Recently I am testing my TCP application in a plugin, what I did is to 
>> initiate a TCP client in my plugin, however, when I build the debug image 
>> and test, the vpp
>> will crash and complaint about out of memory.
>> 
>> After doing some research, it seems the following code may cause the crash:
>> 
>> always_inline session_t *
>> ho_session_alloc (void)
>> {
>>   session_t *s;
>>   ASSERT (vlib_get_thread_index () == 0);
>>   s = session_alloc (0);
>>   s->session_state = SESSION_STATE_CONNECTING;
>>   s->flags |= SESSION_F_HALF_OPEN;
>>   /* Not ideal. Half-opens are only allocated from main with worker barrier
>>    * but can be cleaned up, i.e., session_half_open_free, from main without
>>    * a barrier. In debug images, the free_bitmap can grow while workers peek
>>    * the sessions pool, e.g., session_half_open_migrate_notify, and as a
>>    * result crash while validating the session. To avoid this, grow the 
>> bitmap
>>    * now. */
>>   if (CLIB_DEBUG)
>>     {
>>       session_t *sp = session_main.wrk[0].sessions;
>>       clib_bitmap_validate (pool_header (sp)->free_bitmap, s->session_index);
>>     }
>>   return s;
>> }
>> 
>> since the clib_bitmap_validate is defined as:
>> 
>> /* Make sure that a bitmap is at least n_bits in size */
>> #define clib_bitmap_validate(v,n_bits) \
>>   clib_bitmap_vec_validate ((v), ((n_bits) - 1) / BITS (uword))
>> 
>> 
>> The first half open session have a session_index with zero, so 0-1 will make 
>> a overflow which cause it try to allocate (UINT64_MAX-1)/ 64 memory which 
>> make
>> the vppinfra abort.
>> 
>> I think we should modify the code above with s->session_index + 1, if that's 
>> correct, I will submit a patch later.
>> 
>> 
>> 
> 
> 
> 
> 
> 
> 

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#22071): https://lists.fd.io/g/vpp-dev/message/22071
Mute This Topic: https://lists.fd.io/mt/94529335/21656
Group Owner: vpp-dev+ow...@lists.fd.io
Unsubscribe: https://lists.fd.io/g/vpp-dev/leave/1480452/21656/631435203/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to