Skip to content

DPI: Avoid concurrent access of ndpi_flow_struct by multiple lcores#9

Closed
shubham-cdot wants to merge 1 commit intodanos:masterfrom
shubham-cdot:DAN-316-fix
Closed

DPI: Avoid concurrent access of ndpi_flow_struct by multiple lcores#9
shubham-cdot wants to merge 1 commit intodanos:masterfrom
shubham-cdot:DAN-316-fix

Conversation

@shubham-cdot
Copy link
Copy Markdown
Contributor

Fixes DAN-316

@pjaitken
Copy link
Copy Markdown
Contributor

@shubham-cdot there are a couple of checkpatch issues. Please re-wrap the commit message so there are no lines > 75 chars, and change "Co-authored-by:" to "Co-developed-by:" so that checkpatch runs cleanly.

Thanks.

$ ./scripts/checkpatch_wrapper.sh origin/master origin/DAN-316
0c51dba7994921803c27945e33b96b4c4db86f82:11: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#11: 
session handled by it. The per packet data includes pointers to L3, L4 headers

0c51dba7994921803c27945e33b96b4c4db86f82:23: WARNING:BAD_SIGN_OFF: Non-standard signature: Co-authored-by:
#23: 
Co-authored-by: Anshul Thakur <athakur@cdot.in>

total: 0 errors, 2 warnings, 28 lines checked

For each flow, an instance of `ndpi_flow_struct` is maintained in the
respective row in the connection table. The `ndpi_flow_struct` structure
contains `ndpi_packet_struct` (`packet` in `struct ndpi_flow_struct`)
which holds per-packet data and is updated by nDPI library for each
packet of the session handled by it. The per packet data includes
pointers to L3, L4 headers and L4 payload which are contained inside the
`rte_mbuf` of the given packet.

Since the connection table is global and if there are not any explicit
locks, any of the lcores can access it at any time.

Under such circumstances, if the forward and backward packets from the
same flow are handled by different lcores, it is possible that the
`ndpi_flow_struct` of the session is concurrently accessed by the two
lcores.

Fixes DAN-316

Co-developed-by: Anshul Thakur <athakur@cdot.in>
Signed-off-by: Anshul Thakur <athakur@cdot.in>
Signed-off-by: Shubham Shrivastava <shubhams@cdot.in>
@shubham-cdot
Copy link
Copy Markdown
Contributor Author

@pjaitken Fixed checkpatch issues and added copyright notice to the modified file.

@nickbroon
Copy link
Copy Markdown
Contributor

This was included via 455e70b

@nickbroon nickbroon closed this Apr 21, 2021
nickbroon pushed a commit to nickbroon/vyatta-dataplane that referenced this pull request Apr 28, 2021
se_sen field in struct session may be NULL in two cases for newly
created sessions not yet added to sentry hash list, or after sesssion
removed from sentry hash list during reclaim in session gc.

This causes the following segmentaion fault infrequently.

[Current thread is 1 (Thread 0x7ff5c3fff700 (LWP 30235))]
 #0  0x000055ce31a8978d in csync_get_session_from_init_sentry (
    cse=<synthetic pointer>, cs=<synthetic pointer>, sp=0x7ff5f004ef36)
    at ../src/npf/csync/csync_session_unpack.c:38
 #1  csync_session_unpack_update (csu=0x7ff5f004ef2e)
    at ../src/npf/csync/csync_session_unpack.c:71
 #2  csync_unpack_session (size=<optimized out>, msg=0x7ff5f004ef26)
    at ../src/npf/csync/csync_session_unpack.c:421
 #3  csync_recv_session_update (frame=<optimized out>)
    at ../src/npf/csync/csync_session_unpack.c:501
 #4  0x000055ce31b2d57d in csync_restore_sessions (n=<optimized out>,
    flist=<optimized out>) at ../src/csync/csync_transfer.c:218
 #5  csync_pull_batch (info=0x7ff58400b880) at
../src/csync/csync_transfer.c:506
 #6  csync_xfer_backup (pipe=0x7ff58400b8e0, arg=0x7ff58400b880)
    at ../src/csync/csync_transfer.c:301
 #7  0x00007ff6629618d3 in ?? () from
/usr/lib/x86_64-linux-gnu/libczmq.so.4
 #8  0x00007ff6611ad4a4 in start_thread ()
   from /lib/x86_64-linux-gnu/libpthread.so.0
 danos#9  0x00007ff660eefd0f in clone () from /lib/x86_64-linux-gnu/libc.so.6

Fixed by doing a safe derefernce and checking for NULL.

VRVDR-54586
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants