Hi,

I have made a patch and submit it to gerrit for review.

https://gerrit.fd.io/r/c/vpp/+/37567

I have run against vpp unit test for session feature and no regression
found yet.

Florin Coras <fcoras.li...@gmail.com> 于2022年11月1日周二 23:42写道:

> Hi,
>
> Will you be pushing the fix or should I do it?
>
> Regards,
> Florin
>
> On Oct 25, 2022, at 9:26 AM, Florin Coras via lists.fd.io <
> fcoras.lists=gmail....@lists.fd.io> wrote:
>
> 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> 于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>
>> 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 (#22114): https://lists.fd.io/g/vpp-dev/message/22114
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