Skip to content

tinydtls: add sock_async support for sock_dtls #12907

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.

Sign up for GitHub

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

Merged
miri64 merged 4 commits into from
Jun 22, 2020
Merged

tinydtls: add sock_async support for sock_dtls #12907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1b23037
sock_async: change flags for new sock_dtls behavior
miri64 May 22, 2020
4013c61
tinydtls: add `sock_async` support for `sock_dtls`
miri64 Dec 9, 2019
7dc4f78
sock_async: add missing license header to `net/sock/async/types.h`
miri64 Jun 16, 2020
155953f
tests: provide test for tinydtls with sock_async
miri64 May 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
224 changes: 198 additions & 26 deletions pkg/tinydtls/contrib/sock_dtls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,15 @@
*/

#include "dtls.h"
#include "log.h"
#include "net/sock/dtls.h"
#include "net/credman.h"

#if SOCK_HAS_ASYNC
#include "net/sock/async.h"
#include "net/sock/async/event.h"
#endif

#define ENABLE_DEBUG (0)
#include "debug.h"
#include "dtls_debug.h"
Expand Down Expand Up @@ -77,6 +83,11 @@ static int _read(struct dtls_context_t *ctx, session_t *session, uint8_t *buf,
sock->buffer.data = buf;
sock->buffer.datalen = len;
sock->buffer.session = session;
#ifdef SOCK_HAS_ASYNC
if (sock->async_cb != NULL) {
sock->async_cb(sock, SOCK_ASYNC_MSG_RECV, sock->async_cb_arg);
}
#endif
return len;
}

Expand All @@ -103,7 +114,7 @@ static int _event(struct dtls_context_t *ctx, session_t *session,
(void)session;

sock_dtls_t *sock = dtls_get_app_data(ctx);
msg_t msg = { .type = code };
msg_t msg = { .type = code, .content.ptr = session };
#ifdef ENABLE_DEBUG
switch (code) {
case DTLS_EVENT_CONNECT:
Expand All @@ -120,6 +131,23 @@ static int _event(struct dtls_context_t *ctx, session_t *session,
if (!level && (code != DTLS_EVENT_CONNECT)) {
mbox_put(&sock->mbox, &msg);
}
#ifdef SOCK_HAS_ASYNC
if (sock->async_cb != NULL) {
switch (code) {
case DTLS_ALERT_CLOSE_NOTIFY:
Comment thread
miri64 marked this conversation as resolved.
/* peer closed their session */
sock->async_cb(sock, SOCK_ASYNC_CONN_FIN, sock->async_cb_arg);
break;
case DTLS_EVENT_CONNECTED:
/* we received a session handshake initialization */
sock->async_cb(sock, SOCK_ASYNC_CONN_RECV,
sock->async_cb_arg);
break;
default:
break;
}
}
#endif
return 0;
}

Expand Down Expand Up @@ -250,6 +278,10 @@ int sock_dtls_create(sock_dtls_t *sock, sock_udp_t *udp_sock,

sock->udp_sock = udp_sock;
sock->buffer.data = NULL;
#ifdef SOCK_HAS_ASYNC
sock->async_cb = NULL;
sock->buf_ctx = NULL;
#endif /* SOCK_HAS_ASYNC */
sock->role = role;
sock->tag = tag;
sock->dtls_ctx = dtls_new_context(sock);
Expand Down Expand Up @@ -320,14 +352,14 @@ void sock_dtls_session_destroy(sock_dtls_t *sock, sock_dtls_session_t *remote)
ssize_t sock_dtls_send(sock_dtls_t *sock, sock_dtls_session_t *remote,
const void *data, size_t len, uint32_t timeout)
{
int res;

assert(sock);
assert(remote);
assert(data);

/* check if session exists, if not create session first then send */
if (!dtls_get_peer(sock->dtls_ctx, &remote->dtls_session)) {
int res;

if (timeout == 0) {
return -ENOTCONN;
}
Expand Down Expand Up @@ -366,8 +398,47 @@ ssize_t sock_dtls_send(sock_dtls_t *sock, sock_dtls_session_t *remote,
}
}

return dtls_write(sock->dtls_ctx, &remote->dtls_session,
(uint8_t *)data, len);
res = dtls_write(sock->dtls_ctx, &remote->dtls_session,
(uint8_t *)data, len);
#ifdef SOCK_HAS_ASYNC
if ((res >= 0) && (sock->async_cb != NULL)) {
sock->async_cb(sock, SOCK_ASYNC_MSG_SENT, sock->async_cb_arg);
}
#endif /* SOCK_HAS_ASYNC */
return res;
}

#if SOCK_HAS_ASYNC
/**
* @brief Checks for and iterates for more data chunks within the network
* stacks anternal packet buffer
*
* When no more chunks exists, `data_ctx` assures cleaning up the internal
* buffer state and `sock_udp_recv_buf()` returns 0.
*
* @see @ref sock_udp_recv_buf().
*/
static void _check_more_chunks(sock_udp_t *udp_sock, void **data,
Comment thread
miri64 marked this conversation as resolved.
Comment thread
miri64 marked this conversation as resolved.
void **data_ctx, sock_udp_ep_t *remote)
{
ssize_t res;

while ((res = sock_udp_recv_buf(udp_sock, data, data_ctx, 0, remote)) > 0) {

@benpicco benpicco Aug 19, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm currently trying to implement sock_dtls_recv_buf_aux() so I got back to this function.
How is this supposed to work? The way this is implemented (the while loop) this will just flush (and discard) pending data on the socket.

On tangent: Is there even such a thing as 'chunked datagram payload'?
As far as I can see it is implemented in no network stack, it's always

  • first call returns pointer to the buffer
  • second call frees the buffer

The only way I could see we could get chunked datagram payload is if we didn't do IP fragment reassembly in the stack but pushed it to the user to handle.

That seems not like something anyone would want. And user would need to maintain a reassembly buffer, nullifying the zero-copy advantage of sock_udp_recv_buf().

Is there any real world use for this API that we can't do a single buffer lease -> return?

@miri64 miri64 Aug 29, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

On tangent: Is there even such a thing as 'chunked datagram payload'?

Yes, lwIP is returning ~500B---I don't remember the exact number and don't want to stall answering this question by looking it up---non-continuous chunks of data for larger UDP packets. Not sure if this is still the case, after the packet went to TinyDTLS or if other DTLS implementations might still opt to chunk the payload (e.g. by data record in the actual DTLS packet).

How is this supposed to work?

Due to the restrictions of TinyDTLS that you I believe fixed a proper implementation was not possible so far. The idea is to call this function repeatedly, until there are no chunks left (signified by return value 0). I think the lwip_sock_udp implementation provides a good example of how this is supposed to look like.

/* TODO: remove and adapt _copy_buffer() to add remaining data when
* tinydtls supports chunked datagram payload */
if (IS_ACTIVE(DEVELHELP)) {
LOG_ERROR("sock_dtls: Chunked datagram payload currently not "
"supported yet by tinydtls\n");
}
}
}
#endif

static inline void _copy_session(sock_dtls_t *sock, sock_dtls_session_t *remote)
{
memcpy(&remote->dtls_session, sock->buffer.session,
sizeof(remote->dtls_session));
_session_to_ep(&remote->dtls_session, &remote->ep);
}

static ssize_t _copy_buffer(sock_dtls_t *sock, sock_dtls_session_t *remote,
Expand All @@ -380,32 +451,83 @@ static ssize_t _copy_buffer(sock_dtls_t *sock, sock_dtls_session_t *remote,
if (buflen > max_len) {
return -ENOBUFS;
}
#if SOCK_HAS_ASYNC
if (sock->buf_ctx != NULL) {
memcpy(data, buf, sock->buffer.datalen);
_copy_session(sock, remote);
_check_more_chunks(sock->udp_sock, (void **)&buf, &sock->buf_ctx,
&remote->ep);
if (sock->async_cb &&
/* is there a message in the sock's mbox? */
cib_avail(&sock->mbox.cib)) {
if (sock->buffer.data) {
sock->async_cb(sock, SOCK_ASYNC_MSG_RECV,
sock->async_cb_arg);
}
else {
sock->async_cb(sock, SOCK_ASYNC_CONN_RECV,
sock->async_cb_arg);
}
}
return buflen;
}
#else
(void)remote;
#endif
/* use `memmove()` as tinydtls reuses `data` to store decrypted data with an
* offset in `buf`. This prevents problems with overlapping buffers. */
memmove(data, buf, buflen);
memcpy(&remote->dtls_session, sock->buffer.session,
sizeof(remote->dtls_session));
_session_to_ep(&remote->dtls_session, &remote->ep);
_copy_session(sock, remote);
return buflen;
}

static ssize_t _complete_handshake(sock_dtls_t *sock,
sock_dtls_session_t *remote,
const session_t *session)
{
memcpy(&remote->dtls_session, session, sizeof(remote->dtls_session));
#ifdef SOCK_HAS_ASYNC
if (sock->async_cb) {
sock_async_flags_t flags = SOCK_ASYNC_CONN_RDY;

if (cib_avail(&sock->mbox.cib)) {
if (sock->buffer.data) {
flags |= SOCK_ASYNC_MSG_RECV;
}
else {
flags |= SOCK_ASYNC_CONN_RECV;
Comment thread
miri64 marked this conversation as resolved.
}
}
sock->async_cb(sock, flags, sock->async_cb_arg);
}
#else
(void)sock;
#endif
return -SOCK_DTLS_HANDSHAKE;
}

ssize_t sock_dtls_recv(sock_dtls_t *sock, sock_dtls_session_t *remote,
void *data, size_t max_len, uint32_t timeout)
{
assert(sock);
assert(data);
assert(remote);

if (sock->buffer.data != NULL) {
/* there is already decrypted data available */
return _copy_buffer(sock, remote, data, max_len);
}

/* loop breaks when timeout or application data read */
while (1) {
ssize_t res;
uint32_t start_recv = xtimer_now_usec();
ssize_t res = sock_udp_recv(sock->udp_sock, data, max_len, timeout,
&remote->ep);
msg_t msg;

if (sock->buffer.data != NULL) {
return _copy_buffer(sock, remote, data, max_len);
}
else if (mbox_try_get(&sock->mbox, &msg) &&
msg.type == DTLS_EVENT_CONNECTED) {
return _complete_handshake(sock, remote, msg.content.ptr);
}
res = sock_udp_recv(sock->udp_sock, data, max_len, timeout,
&remote->ep);
if (res <= 0) {
DEBUG("sock_dtls: error receiving UDP packet: %d\n", (int)res);
return res;
Expand All @@ -418,21 +540,11 @@ ssize_t sock_dtls_recv(sock_dtls_t *sock, sock_dtls_session_t *remote,
if ((timeout != SOCK_NO_TIMEOUT) && (timeout != 0)) {
timeout = _update_timeout(start_recv, timeout);
}

msg_t msg;
if (sock->buffer.data != NULL) {
return _copy_buffer(sock, remote, data, max_len);
}
else if (mbox_try_get(&sock->mbox, &msg) &&
msg.type == DTLS_EVENT_CONNECTED) {
return -SOCK_DTLS_HANDSHAKE;
}
else if (timeout == 0) {
if (timeout == 0) {
DEBUG("sock_dtls: timed out while decrypting message\n");
return -ETIMEDOUT;
}
}

}

void sock_dtls_close(sock_dtls_t *sock)
Expand Down Expand Up @@ -468,4 +580,64 @@ static inline uint32_t _update_timeout(uint32_t start, uint32_t timeout)
return (diff > timeout) ? 0: timeout - diff;
}

#ifdef SOCK_HAS_ASYNC
void _udp_cb(sock_udp_t *udp_sock, sock_async_flags_t flags, void *ctx)
{
sock_dtls_t *sock = ctx;

if (flags & SOCK_ASYNC_MSG_RECV) {
sock_dtls_session_t remote;
void *data = NULL;
void *data_ctx = NULL;

ssize_t res = sock_udp_recv_buf(udp_sock, &data, &data_ctx, 0,
&remote.ep);
if (res <= 0) {
DEBUG("sock_dtls: error receiving UDP packet: %d\n", (int)res);
return;
}

/* prevent overriding already set `buf_ctx` */
if (sock->buf_ctx != NULL) {
Comment thread
miri64 marked this conversation as resolved.
DEBUG("sock_dtls: unable to store buffer asynchronously\n");
_check_more_chunks(udp_sock, &data, &data_ctx, &remote.ep);
return;
}
Comment on lines +601 to +605

@pokgak pokgak Apr 19, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I had to comment this out in my patch to get it working. From #12907 (comment) I understand that this check is for the chunked UDP payload support which is still not in tinydtls. Can you explain how checking for sock->buf_ctx can check for chunked UDP payload?

@pokgak pokgak Apr 19, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also why check for sock->buf_ctx when in line 532 and 534 you're using data_ctx?

_ep_to_session(&remote.ep, &remote.dtls_session);
sock->buf_ctx = data_ctx;
res = dtls_handle_message(sock->dtls_ctx, &remote.dtls_session,
data, res);
if (sock->buffer.data == NULL) {
_check_more_chunks(udp_sock, &data, &data_ctx, &remote.ep);
sock->buf_ctx = NULL;
}
}
if ((flags & SOCK_ASYNC_PATH_PROP) && sock->async_cb) {
/* just hand this event type up the stack */
sock->async_cb(sock, SOCK_ASYNC_PATH_PROP, sock->async_cb_arg);
}
}

void sock_dtls_set_cb(sock_dtls_t *sock, sock_dtls_cb_t cb, void *cb_arg)
{
sock->async_cb = cb;
sock->async_cb_arg = cb_arg;
if (IS_USED(MODULE_SOCK_ASYNC_EVENT)) {
sock_async_ctx_t *ctx = sock_dtls_get_async_ctx(sock);
if (ctx->queue) {
sock_udp_event_init(sock->udp_sock, ctx->queue, _udp_cb, sock);
return;
}
}
sock_udp_set_cb(sock->udp_sock, _udp_cb, sock);
}

#ifdef SOCK_HAS_ASYNC_CTX
sock_async_ctx_t *sock_dtls_get_async_ctx(sock_dtls_t *sock)
{
return &sock->async_ctx;
}
#endif /* SOCK_HAS_ASYNC_CTX */
#endif /* SOCK_HAS_ASYNC */

/** @} */
24 changes: 24 additions & 0 deletions pkg/tinydtls/include/sock_dtls_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
#include "dtls.h"
#include "net/sock/udp.h"
#include "net/credman.h"
#ifdef SOCK_HAS_ASYNC
#include "net/sock/async/types.h"
#endif

#ifdef __cplusplus
extern "C" {
Expand All @@ -37,6 +40,27 @@ extern "C" {
struct sock_dtls {
dtls_context_t *dtls_ctx; /**< TinyDTLS context for sock */
sock_udp_t *udp_sock; /**< Underlying UDP sock to use */
#if defined(SOCK_HAS_ASYNC) || defined(DOXYGEN)
/**
* @brief Network stack internal buffer context.
*/
void *buf_ctx;
/**
* @brief Asynchronous event callback
*
* @note Only available if @ref SOCK_HAS_ASYNC is defined
*/
sock_dtls_cb_t async_cb;
void *async_cb_arg; /**< asynchronous callback arg */
#if defined(SOCK_HAS_ASYNC_CTX) || defined(DOXYGEN)
/**
* @brief Asynchronous event context
*
* @note Only available if @ref SOCK_HAS_ASYNC_CTX is defined
*/
sock_async_ctx_t async_ctx;
#endif
#endif
mbox_t mbox; /**< Mailbox for internal event
handling */
msg_t mbox_queue[SOCK_DTLS_MBOX_SIZE]; /**< Queue for struct
Expand Down
12 changes: 10 additions & 2 deletions sys/include/net/sock/async/types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
/*
* Copyright (C) 2019-20 Freie Universität Berlin
*
* This file is subject to the terms and conditions of the GNU Lesser
* General Public License v2.1. See the file LICENSE in the top level
* directory for more details.
*/

/**
* @addtogroup net_sock_async Sock extension for asynchronous access
*
Expand Down Expand Up @@ -47,9 +55,9 @@ typedef struct sock_dtls sock_dtls_t; /**< forward declare for async */
*
* @param[in] sock The sock the event happened on
* @param[in] flags The event flags. Expected values are
* - @ref SOCK_ASYNC_CONN_RDY (if a session you created becomes ready),
* - @ref SOCK_ASYNC_CONN_RDY (if a session becomes ready),
* - @ref SOCK_ASYNC_CONN_FIN (if a created session was destroyed),
* - @ref SOCK_ASYNC_CONN_RECV (if a peer tries to create a session),
* - @ref SOCK_ASYNC_CONN_RECV (if a handshake message needs to be completed)
* - @ref SOCK_ASYNC_MSG_RECV,
* - @ref SOCK_ASYNC_MSG_SENT,
* - @ref SOCK_ASYNC_PATH_PROP, or
Expand Down
Loading