<p></p>
<p><b>@benpicco</b> commented on this pull request.</p>

<p>Sorry for not taking a look at this earlier.<br>
There are two conceptual things that irk me, but that might also be due to me not being familiar with the CoAP/DTLS/async socket implementation.</p>
<ul>
<li>Why is DTLS sock not just an extension to UDP sock? The DTLS implementation would then be specific to the network stack (e.g. GNRC), but that would still be preferable to pushing it to the application layer IMHO</li>
<li>Why is there a need for <code>dtls/dsm</code> - can't we attach the session directly to the socket?</li>
</ul><hr>

<p>In <a href="https://github.com/RIOT-OS/RIOT/pull/15549#discussion_r629710844">sys/net/dsm/dsm.c</a>:</p>
<pre style='color:#555'>> +    for (uint8_t i=0; i < DTLS_PEER_MAX; i++) {
+        _sessions[i].state = SESSION_STATE_NONE;
+        memset(&_sessions[i], 0, sizeof(dsm_session_t));
+    }
</pre>
<p>That's redundant as <code>_sessions</code> is already <code>static</code> and thus initialized with 0.</p>

<hr>

<p>In <a href="https://github.com/RIOT-OS/RIOT/pull/15549#discussion_r631899609">sys/include/net/dsm.h</a>:</p>
<pre style='color:#555'>> +dsm_state_t dsm_store(sock_dtls_t *sock, sock_dtls_session_t *session,
+                      dsm_state_t new_state, bool restore);
</pre>
<p>Why not extend <code>sock_dtls</code> instead? Is this so there can be multiple sessions per socket?</p>
<div class="highlight highlight-source-diff"><pre><span class="pl-md">--- a/pkg/tinydtls/include/sock_dtls_types.h</span>
<span class="pl-mi1">+++ b/pkg/tinydtls/include/sock_dtls_types.h</span>
<span class="pl-mdr">@@ -81,6 +81,10 @@</span> struct sock_dtls {
     dtls_peer_type role;                    /**< DTLS role of the socket */
     sock_dtls_client_psk_cb_t client_psk_cb;/**< Callback to determine PSK credential for session */
     sock_dtls_rpk_cb_t rpk_cb;              /**< Callback to determine RPK credential for session */
<span class="pl-mi1"><span class="pl-mi1">+</span></span>
<span class="pl-mi1"><span class="pl-mi1">+</span>    sock_dtls_session_t session;</span>
<span class="pl-mi1"><span class="pl-mi1">+</span>    dsm_state_t state;</span>
<span class="pl-mi1"><span class="pl-mi1">+</span>    uint64_t last_used;</span>
 };
 
 /**</pre></div>

<hr>

<p>In <a href="https://github.com/RIOT-OS/RIOT/pull/15549#discussion_r631928584">sys/net/application_layer/gcoap/gcoap.c</a>:</p>
<pre style='color:#555'>> @@ -110,6 +126,17 @@ static event_queue_t _queue;
 static uint8_t _listen_buf[CONFIG_GCOAP_PDU_BUF_SIZE];
 static sock_udp_t _sock_udp;
 
+#if IS_ACTIVE(CONFIG_GCOAP_ENABLE_DTLS)
+/* DTLS variables and definitions */
+#define SOCK_DTLS_CLIENT_TAG (2)
+
+static sock_dtls_t _sock_dtls;
</pre>
<p>What if instead we did</p>
<div class="highlight highlight-source-diff"><pre><span class="pl-md">--- a/pkg/tinydtls/include/sock_dtls_types.h</span>
<span class="pl-mi1">+++ b/pkg/tinydtls/include/sock_dtls_types.h</span>
<span class="pl-mdr">@@ -39,8 +39,8 @@</span> extern "C" {
  * @brief Information about DTLS sock
  */
 struct sock_dtls {
<span class="pl-mi1"><span class="pl-mi1">+</span>    sock_udp_t udp_sock;                    /**< Underlying UDP sock to use */</span>
     dtls_context_t *dtls_ctx;               /**< TinyDTLS context for sock */
<span class="pl-md"><span class="pl-md">-</span>    sock_udp_t *udp_sock;                   /**< Underlying UDP sock to use */</span>
 #if defined(SOCK_HAS_ASYNC) || defined(DOXYGEN)
     /**
      * @brief   Network stack internal buffer context.</pre></div>
<p>And use the <code>.flags</code> field to indicate there is a DTLS context attached to the socket?</p>
<p>Could that save the application from dealing with DTLS entirely (save for calling a DTLS setup function on the DTLS socket) and leave the rest to GNRC?</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/RIOT-OS/RIOT/pull/15549#pullrequestreview-656119150">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABE7WYE3NOVFUIIYWKYP2KDTNP4M3ANCNFSM4UL2PR7Q">unsubscribe</a>.<img src="https://github.com/notifications/beacon/ABE7WYC6KX3RRGGXZQKAJMTTNP4M3A5CNFSM4UL2PR72YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOE4NZK3Q.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/RIOT-OS/RIOT/pull/15549#pullrequestreview-656119150",
"url": "https://github.com/RIOT-OS/RIOT/pull/15549#pullrequestreview-656119150",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>