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] -=-=-=-=-=-=-=-=-=-=-=-