Re: [vpp-dev] weird crash when allocate new ho_session_alloc in debug image

2022-11-03 Thread Florin Coras
Thanks, merged!

Regards,
Florin


> On Nov 3, 2022, at 12:33 AM, 张东亚  wrote:
> 
> 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 mailto: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 
>>  > > 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 >> > 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 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 >>> > 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 (#22117): https://lists.fd.io/g/vpp-dev/message/22117
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]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [vpp-dev] weird crash when allocate new ho_session_alloc in debug image

2022-11-03 Thread Zhang Dongya
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  于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 
> 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  于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 
>> 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]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [vpp-dev] weird crash when allocate new ho_session_alloc in debug image

2022-10-25 Thread Florin Coras
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  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 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 > > 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]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [vpp-dev] weird crash when allocate new ho_session_alloc in debug image

2022-10-24 Thread Zhang Dongya
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  于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 
> 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 (#22069): https://lists.fd.io/g/vpp-dev/message/22069
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]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [vpp-dev] weird crash when allocate new ho_session_alloc in debug image

2022-10-24 Thread Florin Coras
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  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 (#22065): https://lists.fd.io/g/vpp-dev/message/22065
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]
-=-=-=-=-=-=-=-=-=-=-=-



[vpp-dev] weird crash when allocate new ho_session_alloc in debug image

2022-10-24 Thread Zhang Dongya
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 (#22064): https://lists.fd.io/g/vpp-dev/message/22064
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]
-=-=-=-=-=-=-=-=-=-=-=-