Dear Andrew This is a good explanation of how session add and delete works, I think this not a benign operation, I could produce the rare scenario you explained. I will send backtrace and other details tomorrow.
On Tue, Dec 12, 2017 at 2:46 PM, Andrew 👽 Yourtchenko <[email protected]> wrote: > Dear Khers, > > I think you are right. Normally the entry in the session hash table is > deleted before any operations with the per-worker pool, so we should > not end up on that line. Also, the deletion itself usually happens as > a result of the idle timeout - meaning, no packets hit the session for > a comparatively very long time. Also, the deletion happens in the > interrupt handler on the worker, so that worker will not be able to > use that session anyway. The only theoretically possible scenario I > can see this happening is if the interrupt handler on the worker > owning the session starts the process of its deletion, and then the > worker handling the other leg of the connection receives the packet > just in time to do the lookup of the session in the global table just > before it gets deleted *AND* then the per-worker session deletion > happens earlier than we get the pointer for the session. Initially I > did not have the check when getting the pointer to the session, so in > this rare case we would reset the timeout or update the flags on the > free session. So this would be a benign operation. But as some of the > places used the ~0 as index, that was a bit problematic so i have > added that check. Coincidentally this is also the exact place which > appears to have triggered the other issue that you saw. > > So I think I might just simplify the check to ensure the index is > within the bounds of the allowed indices for the pool. > > Hope this clarifies the logic... > > --a > > On 12/11/17, khers <[email protected]> wrote: > > Dear Andrew > > > > I'm working on d594711a5d79859a7d0bde83a516f7ab52051d9b commit on > > stable/1710 branch. sorry for less info. > > I can't reproduce last issue I have reported, forgot the commit I were > > working on. > > > > Regards, > > Khers > > > > On Mon, Dec 11, 2017 at 12:24 PM, Andrew Yourtchenko <[email protected] > > > > wrote: > > > >> Dear Khers, > >> > >> At least the exact commit# you are working with to get more context > would > >> be useful - line 1029 on master points to a call acl_fill_5tuple to > me... > >> > >> Also, I have not heard - were you able to reproduce the issue you > >> contacted about a while ago ? > >> > >> --a > >> > >> > On 11 Dec 2017, at 08:46, khers <[email protected]> wrote: > >> > > >> > Dear VPP folks, > >> > > >> > The get_session_ptr function may return null pointer, while we do not > >> check this situation in code, for example fa_node.c line 1029, if the > >> sess > >> equals null, we get segmentation fault in next usage of sess. > >> > Please share your thought about this. > >> > > >> > Regards, > >> > Khers > >> > _______________________________________________ > >> > vpp-dev mailing list > >> > [email protected] > >> > https://lists.fd.io/mailman/listinfo/vpp-dev > >> > > >
_______________________________________________ vpp-dev mailing list [email protected] https://lists.fd.io/mailman/listinfo/vpp-dev
