search

Edgio / hurl

http(s)+h2 server load tester
153 stars 34 forks source link

Add KTLS support to hurl #13

Closed Griffin86 closed 5 years ago

Griffin86 commented 5 years ago

The proposed changes (to add KTLS support) are still in a draft stage.

Initial points for review/discussion:

  1. In its current state, the proposed patch will ALWAYS enable KTLS if available. It may be worthwhile to make this configurable (possibly adding a command line argument?).
  2. The BIO changes made do not appear to really be necessary for KTLS support and could possibly be omitted. This may be influenced by whether or not the "nc_set_accepting" code is ever hit (after an initial SSL conneciton, a subsequent call to SSL_set_fd will destroy the current BIOs attached to a TCP socket and create new ones - could influence KTLS behaviour).
  3. The edit to the CMakeLists.txt. file is a bit hacky (was done for a custom OpenSSL build done in a non-conventional way for a specific use case). It may be a good idea to amend or even omit this change.
tinselcity commented 5 years ago

Let me know if you're ready to PR -the changes all look fine to me now.

Griffin86 commented 5 years ago

Thanks for the feedback. Appreciate it. I don't have any more changes from my side for the PR. Moved PR from draft mode to 'ready for review'. You should be able to do the merge now.