Move response timeout logic into comm layer

Author: AlexLanzano

docs/draft/timeout.md

@@ -4,7 +4,7 @@
 
 The timeout feature uses a callback-based abstraction (similar to the lock feature) that allows platform-specific timer implementations without introducing OS dependencies in core wolfHSM code. A platform port provides a callback table implementing the timer operations, and the core timeout module delegates to these callbacks.
 
-When creating a client, you provide a `whTimeoutConfig` specifying the platform callbacks, platform context, and an optional application-level expired callback:
+The timeout lives in the comm layer. When creating a client, you provide a `whTimeoutConfig` in the `whCommClientConfig`:
 ```c
 /* Platform-specific setup (e.g. POSIX) */
 posixTimeoutContext posixCtx = {0};
@@ -18,62 +18,36 @@ whTimeoutConfig timeoutCfg = {
     .expiredCb  = myTimeoutHandler,    /* optional app callback on expiry */
     .expiredCtx = myAppContext,        /* context passed to app callback */
 };
-whClientConfig clientCfg = {
-    .comm              = &commConfig,
+whCommClientConfig commConfig = {
+    .transport_cb      = &transportCb,
+    .transport_context = &transportCtx,
+    .transport_config  = &transportCfg,
+    .client_id         = 1,
     .respTimeoutConfig = &timeoutCfg,  /* attach timeout config */
 };
+whClientConfig clientCfg = {
+    .comm = &commConfig,
+};
 wh_Client_Init(&clientCtx, &clientCfg);
 ```
 
-During `wh_Client_Init`, the config is used to initialize an embedded `whTimeout respTimeout` inside the client context via `wh_Timeout_Init()`. This calls the platform `init` callback to set up timer resources but doesn't start any timer yet.
-If `respTimeoutConfig` is NULL (or `cb` is NULL), the timeout is disabled and all operations become no-ops (timeout never expires).
+During `wh_CommClient_Init`, the timeout is initialized via `wh_Timeout_Init()`. This calls the platform `init` callback to set up timer resources but doesn't start any timer yet.
+If `respTimeoutConfig` is NULL (or `cb` is NULL), the timeout enters no-op mode and never expires.
 
-## 2. What Happens During a Crypto Call
+## 2. How the Timeout Works
 
-Before the timeout feature, every crypto function in `wh_client_crypto.c` had this pattern after sending a request:
-```c
-/* Old pattern -- infinite busy-wait */
-do {
-    ret = wh_Client_RecvResponse(ctx, &group, &action, &res_len, dataPtr);
-} while (ret == WH_ERROR_NOTREADY);
-```
+The timeout is handled transparently in the comm layer:
 
-If the server never responded, the client would spin forever.
-This is replaced with a single helper `_recvCryptoResponse()` (`src/wh_client_crypto.c`):
-```c
-static int _recvCryptoResponse(whClientContext* ctx,
-                               uint16_t* group, uint16_t* action,
-                               uint16_t* size, void *data)
-{
-    int ret;
-#ifdef WOLFHSM_CFG_ENABLE_TIMEOUT
-    ret = wh_Client_RecvResponseBlockingWithTimeout(ctx, group, action,
-                                                     size, data);
-#else
-    do {
-        ret = wh_Client_RecvResponse(ctx, group, action, size, data);
-    } while (ret == WH_ERROR_NOTREADY);
-#endif
-    return ret;
-}
-```
+1. **`wh_CommClient_SendRequest`**: After a successful send, starts the response timer via `wh_Timeout_Start()`.
+2. **`wh_CommClient_RecvResponse`**: When the transport returns `WH_ERROR_NOTREADY`, checks `wh_Timeout_Expired()`. If expired, returns `WH_ERROR_TIMEOUT`. On successful receive, stops the timer via `wh_Timeout_Stop()`.
+
+This means every `do { ... } while (ret == WH_ERROR_NOTREADY)` loop in the codebase automatically gets timeout support -- crypto, NVM, keystore, cert, SHE, keywrap, and all other client operations.
 
-When timeout is enabled, it delegates to `wh_Client_RecvResponseBlockingWithTimeout`. When disabled, the old infinite-loop behavior is preserved.
-
-## 3. The Timeout Receive Loop
-`wh_Client_RecvResponseBlockingWithTimeout` (`src/wh_client.c`) does this:
-1. **Starts the timer** -- calls `wh_Timeout_Start()` which delegates to the platform `start` callback (e.g. captures the current time).
-2. **Polls for a response** -- calls `wh_Client_RecvResponse()` in a loop.
-3. **On each `WH_ERROR_NOTREADY`**, checks `wh_Timeout_Expired()`:
-   - Delegates to the platform `expired` callback to check elapsed time
-   - If expired: invokes the application `expiredCb` (if set), then returns `WH_ERROR_TIMEOUT`
-   - If not expired: loops again
-4. **On any other return value** (success or error), returns immediately.
 ```
-Client App                    _recvCryptoResponse                 whTimeout
+Client App                      CommClient                       whTimeout
     |                                |                                |
-    |-- wh_Client_AesCbc() --------> |                                |
-    |                                |-- wh_Timeout_Start --------> cb->start()
+    |-- wh_Client_AesCbc() -------->|                                |
+    |                                |-- SendRequest ------> cb->start()
     |                                |                                |
     |                                |-- RecvResponse (NOTREADY)      |
     |                                |-- Expired? -------> cb->expired() -> no
@@ -86,11 +60,12 @@ Client App                    _recvCryptoResponse                 whTimeout
     |<-- WH_ERROR_TIMEOUT -----------|                                |
 ```
 
-## 4. What the Client Sees
-From the application's perspective, the crypto APIs (`wh_Client_AesCbc`, `wh_Client_RsaFunction`, `wh_Client_EccSign`, etc.) now return `WH_ERROR_TIMEOUT` (-2010) instead of hanging indefinitely. The application can then decide how to handle it -- retry, log, fail gracefully, etc.
+## 3. What the Client Sees
+
+From the application's perspective, any client API that waits for a server response can now return `WH_ERROR_TIMEOUT` (-2010) instead of hanging indefinitely. The application can then decide how to handle it -- retry, log, fail gracefully, etc.
 The `expiredCb` fires *before* the error is returned, so you can use it for logging or cleanup without needing to check the return code first.
 
-## 5. Overriding Expiration via the Callback
+## 4. Overriding Expiration via the Callback
 
 The application expired callback receives a pointer to the `isExpired` flag and can override it by setting `*isExpired = 0`. This suppresses the expiration for the current check, allowing the polling loop to continue. A common use case is to extend the timeout deadline: clear the flag, then call `wh_Timeout_Start()` to restart the timer.
 
@@ -129,7 +104,6 @@ whTimeoutConfig timeoutCfg = {
 };
 ```
 
-## 6. Scope Limitations
-A few things to note about the current design:
-- **Only crypto responses are covered.** Non-crypto client calls (key management, NVM operations, comm init) still use the old infinite-wait pattern. The timeout is specifically wired into `_recvCryptoResponse`.
-- **The timeout is per-client, not per-call.** All crypto operations for a given client share the same `respTimeout` context with the same duration. You can call `wh_Timeout_Set()` to change the duration between calls, but there's no per-operation override.
+## 5. Design Notes
+- **The timeout is per-comm-client, not per-call.** All operations for a given client share the same `respTimeout` context with the same duration. You can call `wh_Timeout_Set()` to change the duration between calls, but there's no per-operation override.
+- **Timer starts on send, checks on receive.** The timer window begins when a request is successfully sent, measuring the full round-trip wait.

src/wh_client.c

@@ -77,15 +77,6 @@ int wh_Client_Init(whClientContext* c, const whClientConfig* config)
 
     memset(c, 0, sizeof(*c));
 
-#ifdef WOLFHSM_CFG_ENABLE_TIMEOUT
-    if (config->respTimeoutConfig != NULL) {
-        rc = wh_Timeout_Init(&c->respTimeout, config->respTimeoutConfig);
-        if (rc != WH_ERROR_OK) {
-            return rc;
-        }
-    }
-#endif
-
     rc = wh_CommClient_Init(c->comm, config->comm);
 
 #ifndef WOLFHSM_CFG_NO_CRYPTO
@@ -138,10 +129,6 @@ int wh_Client_Cleanup(whClientContext* c)
     (void)wolfCrypt_Cleanup();
 #endif  /* !WOLFHSM_CFG_NO_CRYPTO */
 
-#ifdef WOLFHSM_CFG_ENABLE_TIMEOUT
-    (void)wh_Timeout_Cleanup(&c->respTimeout);
-#endif
-
     (void)wh_CommClient_Cleanup(c->comm);
 
     memset(c, 0, sizeof(*c));
@@ -208,46 +195,6 @@ int wh_Client_RecvResponse(whClientContext *c,
     return rc;
 }
 
-#ifdef WOLFHSM_CFG_ENABLE_TIMEOUT
-int wh_Client_RecvResponseBlockingWithTimeout(whClientContext* c,
-                                              uint16_t*        out_group,
-                                              uint16_t*        out_action,
-                                              uint16_t* out_size, void* data)
-{
-    int        ret;
-    whTimeout* timeout;
-
-    if (c == NULL) {
-        return WH_ERROR_BADARGS;
-    }
-
-    timeout = &c->respTimeout;
-
-    /* If no timeout configured, fall back to standard blocking loop */
-    if (timeout->cb == NULL) {
-        do {
-            ret = wh_Client_RecvResponse(c, out_group, out_action, out_size,
-                                         data);
-        } while (ret == WH_ERROR_NOTREADY);
-        return ret;
-    }
-
-    ret = wh_Timeout_Start(timeout);
-    if (ret != WH_ERROR_OK) {
-        return ret;
-    }
-
-    do {
-        ret = wh_Client_RecvResponse(c, out_group, out_action, out_size, data);
-        if ((ret == WH_ERROR_NOTREADY) && wh_Timeout_Expired(timeout)) {
-            return WH_ERROR_TIMEOUT;
-        }
-    } while (ret == WH_ERROR_NOTREADY);
-
-    return ret;
-}
-#endif /* WOLFHSM_CFG_ENABLE_TIMEOUT */
-
 int wh_Client_CommInitRequest(whClientContext* c)
 {
     whMessageCommInitRequest msg = {0};