Implement DTLSSocket and fix non-blocking connections on TLSSocket #8659

SeppoTakalo · 2018-11-06T18:47:12Z

Description

Implement DTLSSocketWrapper and fix non-blocking connections on TLSSocket

DTLSSocketWrapper is equivalent of TLSSocketWrapper but uses datagram mode
and timers for handling Mbed TLS timeouts.

Non-blocking connections were not working earlier, now fixed for both
secure socket modes.

This requires changes once #8651 goes in, so pending for that one, or if we merge this before, I need to rework the #8651

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[X] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

SeppoTakalo · 2018-11-06T18:47:24Z

@kjbracey-arm Please review

cmonr · 2018-11-13T22:58:37Z

@kjbracey-arm When you get the chance, this needs a review.
@SeppoTakalo When you get a chance, this'll need a rebase.

cmonr · 2018-11-13T23:01:35Z

@SeppoTakalo We'll be aiming to take in #8651 first since its last test failures were not due to the PR.

kjbracey

Seems fine, just formatting nits and note for the future.

kjbracey · 2018-11-14T08:06:21Z

features/netsocket/DTLSSocket.cpp

+ close();
+}
+
+#endif // MBEDTLS_SSL_CLI_C


Missing newline. Same in lots of other files

kjbracey · 2018-11-14T08:16:21Z

features/netsocket/DTLSSocketWrapper.cpp

+
+ if (context->_timer_event_id == 0) {
+ return -1;
+ } else if (context->_timer_expired) {


Seems current implementation of mbed TLS doesn't rely on the intermediate delay, but nothing in the documentation says you can skip it. I'd suggest putting it in for completeness in a later patch.

SeppoTakalo · 2018-11-14T14:52:37Z

Unittests added and fixed.
whitespace/line endings fixed
rebased on top of master
Added comparison for intermediate timeouts
Manually tested against DTLS server from Mbed TLS
unittests passing.
Refactored the DTLSSocket to use UDPSocket::getpeername() instead of having copy of the endpoint address.

+---------------+---------+-------------+-------------+-----------+-----------+
| Testcase      | Verdict | Fail Reason | Skip Reason | platforms |  duration |
+---------------+---------+-------------+-------------+-----------+-----------+
| DTLS          |   pass  |             |             |    K64F   | 11.730098 |
| DTLS_NONBLOCK |   pass  |             |             |    K64F   | 12.998959 |
+---------------+---------+-------------+-------------+-----------+-----------+

cmonr · 2018-11-14T14:58:41Z

/morph build

mbed-ci · 2018-11-14T20:00:38Z

Build : SUCCESS

Build number : 3629
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8659/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-11-15T01:32:01Z

Exporter Build : FAILURE

Build number : 3236
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/8659/

cmonr · 2018-11-15T01:54:45Z

Export falures do not appear to be due to PR.
(Jenkins Nodes dropped)

Will restart when able.

mbed-ci · 2018-11-15T12:43:57Z

Test : FAILURE

Build number : 3412
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8659/3412

SeppoTakalo · 2018-11-15T16:33:53Z

Unrelated test failure:
tests-mbed_hal-rtc_time_conv

cmonr · 2018-11-15T17:19:24Z

@SeppoTakalo Yup. Will restart when able.

0xc0170 · 2018-11-15T17:21:28Z

This failure is tracked in the issues and one potentional fix was created as I understood. We will restart CI as soon as possible (one rollup now in there). We might create another rollup of few 5.11 unrelated PR (this would be a good candidate to be included).

cmonr · 2018-11-15T22:42:03Z

@SeppoTakalo Fyi, in trying to create #8760 (Rollup PR) with this PR (to expidite retesting while keeping CI load low), I found that a merge conflict is created when this PR is merged with #8647.

I've put 8647 in instead of this PR because 1) the other PR is a day older, and 2) I suspect that rebase resolution will be more straightforward with this PR than for 8647.

TL;DR: Once #8760 passes and is merged, this PR will need a rebase because of a merge conflict with #8647.

SeppoTakalo · 2018-11-19T15:58:29Z

will add.. just a sec

SeppoTakalo · 2018-11-19T16:00:22Z

@0xc0170 fixed!

mbed-ci · 2018-11-19T16:48:09Z

Build : FAILURE

Build number : 3676
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8659/

mbed-ci · 2018-11-19T17:40:41Z

Exporter Build : FAILURE

Build number : 3277
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/8659/

0xc0170

Build failures look related, there are unrecognized tokens reported in 2 header files

0xc0170 · 2018-11-19T17:48:24Z

features/netsocket/DTLSSocketWrapper.h

@@ -0,0 +1,22 @@
+#ifndef DTLSSOCKETWRAPPER_H


this file misses the license header

SeppoTakalo · 2018-11-20T09:50:00Z

Oh nice... UTF-8.
This means is not the same as first one is space, second one is non-breaking space. You can easily enter non-breaking space when you hold the same keys as when you produce | (alt-space on Mac)

Both issues fixed now.

100% tests passed, 0 tests failed out of 43



Total Test time (real) =   0.57 sec

0xc0170 · 2018-11-20T10:52:33Z

/morph build

mbed-ci · 2018-11-20T12:11:40Z

Build : SUCCESS

Build number : 3683
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8659/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-11-20T14:21:07Z

Exporter Build : SUCCESS

Build number : 3286
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/8659/

mbed-ci · 2018-11-20T14:53:21Z

Test : FAILURE

Build number : 3459
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8659/3459

0xc0170 · 2018-11-20T15:01:35Z

^^ known failures , still investigating

0xc0170 · 2018-11-22T12:58:08Z

Info: This PR has been re-bundled into a new rollup PR (#8838 ).

No further work is needed here, as once that PR is merged, this PR will also be closed and marked as merged.
If any more commits are made in this PR, this PR will remain open and have to go through CI on its own.

cmonr added needs: preceding PR release-version: 5.11.0-rc1 labels Nov 7, 2018

cmonr requested a review from kjbracey November 7, 2018 03:12

SeppoTakalo force-pushed the dtls branch from b71bad5 to fe1f908 Compare November 7, 2018 14:36

michalpasztamobica mentioned this pull request Nov 9, 2018

Add unit tests for TLSSocket and TLSSocketWrapper SeppoTakalo/mbed-os#3

Merged

cmonr added the needs: work label Nov 13, 2018

kjbracey approved these changes Nov 14, 2018

View reviewed changes

SeppoTakalo force-pushed the dtls branch from d899723 to 339abcc Compare November 14, 2018 14:14

kjbracey approved these changes Nov 14, 2018

View reviewed changes

cmonr added needs: CI and removed needs: preceding PR needs: work labels Nov 14, 2018

SeppoTakalo mentioned this pull request Nov 14, 2018

DTLS using TLS Sockets #8610

Closed

cmonr added rollup PR: B and removed rollup PR: B labels Nov 15, 2018

cmonr added needs: work needs: preceding PR labels Nov 15, 2018

0xc0170 reopened this Nov 19, 2018

0xc0170 added the needs: CI label Nov 19, 2018

SeppoTakalo mentioned this pull request Nov 19, 2018

Add Secure socket documentations ARMmbed/mbed-os-5-docs#759

Merged

0xc0170 approved these changes Nov 19, 2018

View reviewed changes

0xc0170 added needs: work and removed needs: CI labels Nov 19, 2018

0xc0170 requested changes Nov 19, 2018

View reviewed changes

features/netsocket/DTLSSocketWrapper.h

@@ -0,0 +1,22 @@

#ifndef DTLSSOCKETWRAPPER_H

Copy link

Contributor

0xc0170 Nov 19, 2018

Choose a reason for hiding this comment

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

this file misses the license header

Seppo Takalo added 3 commits November 20, 2018 11:47

Include TLSSocket and DTLSSocket in Doxygen

0f5cec3

Add license headers into TLSSocket unittests

8e41cac

Add license headers into TLSSocket files

5459a7b

SeppoTakalo force-pushed the dtls branch from f309faa to 5459a7b Compare November 20, 2018 09:49

0xc0170 approved these changes Nov 20, 2018

View reviewed changes

0xc0170 added needs: CI and removed needs: work labels Nov 20, 2018

0xc0170 added the rollup PR label Nov 22, 2018

0xc0170 mentioned this pull request Nov 22, 2018

Rollup v2 for 5.11rc1 #8838

Merged

0xc0170 merged commit 5459a7b into ARMmbed:master Nov 24, 2018

0xc0170 removed the needs: CI label Nov 24, 2018

cmonr removed the rollup PR label Nov 26, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement DTLSSocket and fix non-blocking connections on TLSSocket #8659

Implement DTLSSocket and fix non-blocking connections on TLSSocket #8659

SeppoTakalo commented Nov 6, 2018

SeppoTakalo commented Nov 6, 2018

cmonr commented Nov 13, 2018

cmonr commented Nov 13, 2018

kjbracey left a comment

kjbracey Nov 14, 2018

kjbracey Nov 14, 2018

SeppoTakalo commented Nov 14, 2018 •

edited

Loading

cmonr commented Nov 14, 2018

mbed-ci commented Nov 14, 2018

mbed-ci commented Nov 15, 2018

cmonr commented Nov 15, 2018

mbed-ci commented Nov 15, 2018

SeppoTakalo commented Nov 15, 2018

cmonr commented Nov 15, 2018

0xc0170 commented Nov 15, 2018

cmonr commented Nov 15, 2018 •

edited

Loading

SeppoTakalo commented Nov 19, 2018

SeppoTakalo commented Nov 19, 2018

mbed-ci commented Nov 19, 2018

mbed-ci commented Nov 19, 2018

0xc0170 left a comment

0xc0170 Nov 19, 2018

SeppoTakalo commented Nov 20, 2018

0xc0170 commented Nov 20, 2018

mbed-ci commented Nov 20, 2018

mbed-ci commented Nov 20, 2018

mbed-ci commented Nov 20, 2018

0xc0170 commented Nov 20, 2018

0xc0170 commented Nov 22, 2018

Implement DTLSSocket and fix non-blocking connections on TLSSocket #8659

Implement DTLSSocket and fix non-blocking connections on TLSSocket #8659

Conversation

SeppoTakalo commented Nov 6, 2018

Description

Pull request type

SeppoTakalo commented Nov 6, 2018

cmonr commented Nov 13, 2018

cmonr commented Nov 13, 2018

kjbracey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SeppoTakalo commented Nov 14, 2018 • edited Loading

cmonr commented Nov 14, 2018

mbed-ci commented Nov 14, 2018

Triggering tests

mbed-ci commented Nov 15, 2018

cmonr commented Nov 15, 2018

mbed-ci commented Nov 15, 2018

SeppoTakalo commented Nov 15, 2018

cmonr commented Nov 15, 2018

0xc0170 commented Nov 15, 2018

cmonr commented Nov 15, 2018 • edited Loading

SeppoTakalo commented Nov 19, 2018

SeppoTakalo commented Nov 19, 2018

mbed-ci commented Nov 19, 2018

mbed-ci commented Nov 19, 2018

0xc0170 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SeppoTakalo commented Nov 20, 2018

0xc0170 commented Nov 20, 2018

mbed-ci commented Nov 20, 2018

Triggering tests

mbed-ci commented Nov 20, 2018

mbed-ci commented Nov 20, 2018

0xc0170 commented Nov 20, 2018

0xc0170 commented Nov 22, 2018

SeppoTakalo commented Nov 14, 2018 •

edited

Loading

cmonr commented Nov 15, 2018 •

edited

Loading