NDT-DEV email list created

[Peter Boothe <>]

(this time I managed to hit reply-all and push my changes before sending the email)

Hi Aaron,

I'm glad you like it!  I have updated the pull request, and my responses are inline:

On Thu, Nov 12, 2015 at 9:00 PM, Aaron Brown <> wrote:

Hey Peter,

This looks great. I'm curious what's the performance difference between NDT
over WS vs. WSS?

The performance differences between WS vs WSS are minimal. SSL is designed to be fast, and modern processors are much faster than they need to be to create SSL streams at line rate (even in phones!). I saw no speed differences between WS and WSS.

 

A few comments (none modulo the snapshot semantics give me pause from this
being merged):

shutdown_connection does SSL_shutdown/SSL_free, but close_connection just does
the SSL_free. Should close_connection be doing SSL_shutdown too? It'd be nice
if those could somehow be merged, but given there's only one user of shutdown,
and it's unlikely to need changed much, that's probably not a big deal.

My understanding is that SSL_free() behaves like close(), but SSL_shutdown() behaves like shutdown(). So I used one where we had the one and the other where we had the other.

Looking at this set of changes, I think it changes the snapshot semantics:

-      lastThroughputSnapshot->time = secs() - tmptime;
+      lastThroughputSnapshot->time = secs() - start_time;

I think tmptime got updated with the end of the previous snapshot, so the
'time' field will be the duration of that snapshot. With the above change, the
time will be the duration from test start till end of the snapshot.

Looking at the old code, tmptime was set once before the loop at line 387 and then is never set again until after the loop on line 448 where it is changed to be the elapsed time.  During the loop, I think tmptime remains unchanged? If I am reading things wrong, please let me know and I will update my code.

 

The test_s2c_srv has a raw_write function for maximum speed. The test_c2s_srv
has a similar thing, but it isn't split out into a function. A raw_read would
probably make sense there.

I have made a raw_read() function.

 

There are a few instances of "Connection conn = {0, NULL};". Given that 0 is
stdin, it may make sense to make that {-1, NULL}. I doubt it affects anything,
though.

Nice idea! I have updated the default initialization.

 

I'm guessing the SFW test isn't used in TLS mode so TLS isn't used when

setting up the server to client connection?

TEST_SFW involves making a connection from the server back to the host. I decided it was unlikely that the average host can act as an SSL server so I didn't add SSL support for that test.

 

There's a line "struct testoptions;  // declare it, but don't define it". Is there a
recursive header definition issue?

There was such an issue at one point in this large refactoring. That line turns out to be vestigial - other refactorings fixed the problem - and I have now deleted it.

  -Peter

 

On Wed, Nov 11, 2015 at 1:03 PM, Peter Boothe <> wrote:

SSL finally works in NDT. Please take a look. It's already been raked over the coals by my co-worker, so I think the average code quality level is pretty high.

  https://github.com/ndt-project/ndt/pull/198

Once someone else who has the power to commit to NDT gives me the go-ahead, I will merge this into master and NDT will support SSL.

  -Peter

--

ᴹ̶LAB | Measure the Internet, save the data, and make it universally accessible and useful.

--

ᴹ̶LAB | Measure the Internet, save the data, and make it universally accessible and useful.