Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue: 3759513 Fix blocking SSL_sendfile() #141

Open
wants to merge 1 commit into
base: vNext
Choose a base branch
from

Conversation

pasis
Copy link
Member

@pasis pasis commented Apr 23, 2024

Description

SSL_sendfile() creates 3 iov elements, however, for a blocking socket only a signle element is expected in partial write handling.

What

Fix partial write handling in blocking TLS offload path.

Why ?

Blocking SSL_sendfile() fix.

Change type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Tests
  • Other

Check list

  • Code follows the style de facto guidelines of this project
  • Comments have been inserted in hard to understand places
  • Documentation has been updated (if necessary)
  • Test has been added (if possible)

SSL_sendfile() creates 3 iov elements, however, for a blocking socket
only a signle element is expected in partial write handling.

Signed-off-by: Dmytro Podgornyi <dmytrop@nvidia.com>
@pasis pasis added the bug Something isn't working label Apr 26, 2024
if (m_p_zc_owner) {
/*
* For zerocopy case we create 3 scatter-gather elements in this order:
* [0] TLS header which includes IV for TLS 1.2
* [1] Payload
* [2] Trailer which contains TAG and TLS 1.3 type if applicable
*/
assert(iov_max >= 3);
(void)iov_max;
tx_attr.attr.sz_iov = 3;
Copy link
Collaborator

@AlexanderGrissik AlexanderGrissik May 12, 2024

Choose a reason for hiding this comment

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

Should we use TLS_IOV_SIZE here?
and in the static_assert above

@@ -186,6 +186,8 @@ enum : size_t {
TLS_RECORD_MAX = 16384U,
/* Block size big enough to hold TLS header/trailer for zerocopy records. */
TLS_ZC_BLOCK = 32U,
/* Maximum number of scatter-gather elements for TCP send operation. */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add to the comment why 3 ...

@AlexanderGrissik AlexanderGrissik added the question Further information is requested label Jun 2, 2024
@AlexanderGrissik
Copy link
Collaborator

Last check introduces some degradation that should be investigated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants