Skip to content

Commit

Permalink
tls: set flag whenever app session is freed
Browse files Browse the repository at this point in the history 
Type: fix

Signed-off-by: Florin Coras <[email protected]>
Change-Id: I3d44ff851da00573343e15712284af3b9c3912e3
  • Loading branch information
@florincoras
florincoras authored and Dave Barach committed Jan 23, 2024
1 parent 77680ae commit fad689e
Showing 1 changed file with 9 additions and 5 deletions.
14 changes: 9 additions & 5 deletions src/vnet/tls/tls.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ tls_notify_app_accept (tls_ctx_t * ctx)
{
TLS_DBG (1, "failed to allocate fifos");
session_free (app_session);
ctx->no_app_session = 1;
return rv;
}
ctx->app_session_handle = session_handle (app_session);
Expand Down Expand Up @@ -455,12 +456,15 @@ tls_session_reset_callback (session_t * s)
tls_disconnect_transport (ctx);
}
else
if ((app_session =
session_get_if_valid (ctx->c_s_index, ctx->c_thread_index)))
{
session_free (app_session);
ctx->c_s_index = SESSION_INVALID_INDEX;
tls_disconnect_transport (ctx);
app_session = session_get_if_valid (ctx->c_s_index, ctx->c_thread_index);
if (app_session)
{
session_free (app_session);
ctx->c_s_index = SESSION_INVALID_INDEX;
ctx->no_app_session = 1;
tls_disconnect_transport (ctx);
}
}
}

Expand Down

6 comments on commit fad689e

@fjccc
Copy link

@fjccc fjccc commented on fad689e Feb 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in tls_session_disconnect_callback function, session_free(app_session) after this, fifo may not dealloc

@florincoras
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, let’s move the conversation to gerrit.fd.io.

Regarding your comment, this only marks the ctx as not having an app session after it was freed. So functionally this should not be affecting fifo cleanup. Could you provide more details about the issue you’re hitting?

@fjccc
Copy link

@fjccc fjccc commented on fad689e Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have met core,bt stack is as follow
received signal SIGSEGV, PC 0x7f8677282c79, faulting address 0x7f6f0000008f
Jan 31 16:12:37 localhost vnet[1539562]: #0 0x00007f8677321be4 0x7f8677321be4
Jan 31 16:12:37 localhost vnet[1539562]: #1 0x00007f8675ba2e00 0x7f8675ba2e00
Jan 31 16:12:37 localhost vnet[1539562]: #2 0x00007f8677282c79 svm_fifo_enqueue + 0x19
Jan 31 16:12:37 localhost vnet[1539562]: #3 0x00007f8574ff1ca9 0x7f8574ff1ca9
Jan 31 16:12:37 localhost vnet[1539562]: #4 0x00007f8574ff1ea2 0x7f8574ff1ea2
Jan 31 16:12:37 localhost vnet[1539562]: #5 0x00007f8574ff6a44 0x7f8574ff6a44
Jan 31 16:12:37 localhost vnet[1539562]: #6 0x00007f8574ff6ece 0x7f8574ff6ece
Jan 31 16:12:37 localhost vnet[1539562]: #7 0x00007f8574fbcb3d 0x7f8574fbcb3d
Jan 31 16:12:37 localhost vnet[1539562]: #8 0x00007f86772c9437 0x7f86772c9437
Jan 31 16:12:37 localhost vnet[1539562]: #9 0x00007f8675b7de7c 0x7f8675b7de7c
Jan 31 16:12:38 localhost systemd[1]: vpp.service: Main process exited, code=dumped, status=6/ABRT

the address of session is all ok, but the fifo address is wrong. Now suspecting that the core dump is related to issues with fifo allocation and release。Test Scenarios Include Frequent Creation and Teardown of TLS Links”

@florincoras
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately that stack does not tell us much beyond the fact that svm_fifo_enqueue is trying to add data to a fifo. Any chance of getting the rest of the symbols? Also, is this master latest code?

As luck would have it, I'm working these days on some similar tests but have not hit any issues with the fifo enqueues. Two possibilities come to mind 1) session is actually freed and this is a spurious rx event, e.g., app session was freed and tls should not try to enqueue to it 2) fifo allocation actually failed because it ran out of memory and the event is not properly handled somewhere.

@fjccc
Copy link

@fjccc fjccc commented on fad689e Feb 21, 2024 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@florincoras
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can get symbols, try to print the session that own the fifo and check what state it is in.

Please sign in to comment.