diff --git a/sdk/PATCHES.md b/sdk/PATCHES.md index c7b9c22..175a02b 100644 --- a/sdk/PATCHES.md +++ b/sdk/PATCHES.md @@ -33,15 +33,14 @@ Lib/GNU, Bin/secboot.img, Tools/GNU/link_w600_2m.ld (→ ld/) und wm_tool.c (→ `HTTP_CLIENT_ERROR_SOCKET_TIME_OUT` zurückkehren, statt endlos zu warten. Fix-Runde 2 (Re-Review von Fix-Runde 1) hat zwei Nachwirkungen der obigen Änderungen behoben: - - `Src/App/OTA/wm_http_fwup.c`, `HANDLE_BODY`: Lehnt `socket_fwup_recv()` einen Chunk ab (Checksummen- - /Trailer-Fehler oder ein anderer fataler fwup-Fehler), sprang der Code bisher nach `SHUTDOWN_LINK` - - da `recvLen` dabei nie den vollen `totalLen` erreicht, startete das eine komplette - Resume-Schleife (bis zu `NL_OTA_MAX_RETRY_ROUNDS` = 200 Range-Requests), deren Daten die fwup-Task - wegen des bereits gesetzten Fehler-Bits (siehe unten) jedes Mal sofort verwarf. `t_http_fwup()` kam - also erst nach Minuten mit dem richtigen Fehler zurück, `s_busy`/`ota_pending` standen so lange. - Springt jetzt direkt nach `QUIT_OTA` (mit `nRetCode = ret`, damit die `socket_fwup_err()`- - Bereinigung am Funktionsende trotzdem läuft) - der Abbruch ist damit sofort sichtbar, nicht erst - nach der vollen Resume-Schleife. + - `Src/App/OTA/wm_http_fwup.c`, `HANDLE_BODY`: Lehnt `socket_fwup_recv()` einen Chunk mit `ERR_ABRT` + ab, sprang der Code bisher nach `SHUTDOWN_LINK` - da `recvLen` dabei nie den vollen `totalLen` + erreicht, startete das eine komplette Resume-Schleife (bis zu `NL_OTA_MAX_RETRY_ROUNDS` = 200 + Range-Requests). Ursprünglich (Fix-Runde 2) mit der Begründung „die fwup-Task hat abgelehnt" + beschrieben - das war **zu ungenau**: `ERR_ABRT` bedeutet nicht zwingend eine echte fwup-Ablehnung, + siehe die Korrektur in Fix-Runde 3 unten. Springt seit Fix-Runde 3 nur noch bei einer **echten** + Ablehnung direkt nach `QUIT_OTA`; ein leerer Read (Verbindungsabriss/WLAN-Aussetzer) läuft weiterhin + in den Resume. - `Platform/Common/fwup/wm_fwup.c`, Trailer-Puffer-Allokation (`RSA_SHA256 = tls_mem_alloc(512)`): Schlug das fehl, sprang der Code nach `request_finish`, ohne ein Fehler-Bit zu setzen. Mit dem Fix-Runde-1-Reset-Guard (siehe oben, `!(fwup->current_state & TLS_FWUP_STATE_ERROR)`) wäre das der @@ -49,6 +48,27 @@ Lib/GNU, Bin/secboot.img, Tools/GNU/link_w600_2m.ld (→ ld/) und wm_tool.c (→ Trailer wäre dann nie geprüft worden. Setzt jetzt `TLS_FWUP_STATE_ERROR_MEM` / `TLS_FWUP_REQ_STATUS_FMEM` (wie die andere Allokationsprüfung in dieser Funktion) und `nl_ota_set_error("download")` (kein spezifischerer Wert aus dem Vokabular passt hier). + + Fix-Runde 3 (Re-Review von Fix-Runde 2) hat eine echte Regression aus Runde 2 korrigiert: + - `Src/App/OTA/wm_http_fwup.c`, `HANDLE_BODY`: `socket_fwup_recv()` liefert `ERR_ABRT` nicht nur bei + einer echten fwup-Ablehnung (`tls_fwup_request_sync()` meldet Checksummen-/Trailer-/Hash-Fehler, + oder `tls_fwup_enter()` lehnt eine noch belegte Session ab), sondern auch für ein leeres Paket: + `wm_socket_fwup.c` macht aus `current_pack.datalen == 0` per `goto err` denselben `ERR_ABRT`. Ein + leeres Paket entsteht regelmäßig **ohne** jeden fwup-Fehler, wenn `HTTPClient.c`s + `HTTPIntrnRecv()` (Server schließt mitten im Body, oder `recv()` liefert wegen eines + ECONNRESET/WLAN-Aussetzers 0 Bytes) oder der `nSocketEvents < 0`-Zweig `*nLength = 0` + zurückgeben, aber weiterhin `HTTP_CLIENT_SUCCESS`/`HTTP_CLIENT_EOS` melden. Der Fix-Runde-2-Fix + (QUIT_OTA sofort bei `ERR_ABRT`) hätte also genau den häufigsten Auslöser des Resume-Mechanismus + aus I1 (Runde 1) - einen kurzen WLAN-Aussetzer - fälschlich als endgültigen Abbruch behandelt, + obwohl kein Byte falsch geprüft oder geflasht wurde. Fix: Ein `nSize == 0`-Read wird jetzt schon + **vor** dem `pbuf_alloc()`/`socket_fwup_recv()`-Aufruf abgefangen und geht wie vor Runde 2 nach + `SHUTDOWN_LINK` (resumable); `socket_fwup_recv()` bekommt damit nie mehr ein leeres Paket, sein + `ERR_ABRT` ist ab dann eindeutig eine echte Ablehnung und geht weiterhin sofort nach `QUIT_OTA`. + Diese Unterscheidung über den SDK-eigenen Aufrufer zu treffen ist der direkteste verfügbare Weg - + `tls_fwup_current_state(session_id)` bräuchte die (in `wm_socket_fwup.c` statische, nicht + exportierte) `session_id`, wäre also ein größerer Eingriff für denselben Effekt. Nebenbei: + `nRetCode = ret` (ein lwIP-`err_t` in einer sonst `HTTP_CLIENT_*`-Codes tragenden Variable) durch + `HTTP_CLIENT_ERROR_BAD_STATE` ersetzt. 2. `Src/OS/RTOS/include/FreeRTOSConfig.h`: `configGENERATE_RUN_TIME_STATS 0`, QConnex-Debug-Hooks entfernt. 3. `undefined reference` beim Linken auf `EarlyInit` und `pre_gpio_config` (von `sdk/Platform/Sys/wm_main.c` als `extern`-Hooks erwartet, aber nur von der App bereitgestellt): diff --git a/sdk/Src/App/OTA/wm_http_fwup.c b/sdk/Src/App/OTA/wm_http_fwup.c index 4e7f0db..a6da5de 100644 --- a/sdk/Src/App/OTA/wm_http_fwup.c +++ b/sdk/Src/App/OTA/wm_http_fwup.c @@ -147,6 +147,20 @@ int http_fwup(HTTPParameters ClientParams) now_state = SHUTDOWN_LINK; break; } + if (nSize == 0) { + /* ninlight: a zero-length read is not a fwup rejection - it's what + * HTTPClient.c's HTTPIntrnRecv()/nSocketEvents<0 paths report for a server + * that closed mid-body or a transient socket hiccup (WLAN dropout), while + * still returning HTTP_CLIENT_SUCCESS/HTTP_CLIENT_EOS. Passing an empty + * packet into socket_fwup_recv() would turn it into the exact same ERR_ABRT + * as a genuine checksum/trailer rejection (wm_socket_fwup.c: datalen==0 goes + * to the same "goto err" as a failed tls_fwup_request_sync()) - the QUIT_OTA- + * on-ERR_ABRT fix below must not see this case, or a routine WLAN dropout + * would abort the update instead of resuming via SHUTDOWN_LINK/PREPARE_PACKET + * like before. See sdk/PATCHES.md point 1 (Fix-Runde 3). */ + now_state = SHUTDOWN_LINK; + break; + } #if USE_BREAK_POINT_RESUME == 0 if(breakFlag == 1){ breakLen += nSize; @@ -181,15 +195,19 @@ int http_fwup(HTTPParameters ClientParams) // Send received data to fwup thread and deal with socket issues. s8 ret = socket_fwup_recv(0, p, ERR_OK); if(ret != ERR_OK){ - /* ninlight: socket_fwup_recv() rejected this chunk (the fwup task hit a - * checksum/trailer mismatch or another fatal error, see wm_fwup.c) - recvLen - * never advances past this point, so going through SHUTDOWN_LINK used to start - * a full Range-resume retry (up to NL_OTA_MAX_RETRY_ROUNDS rounds) whose data - * the fwup task would just keep discarding. Bail out to QUIT_OTA directly so - * the caller gets the real error immediately instead of after minutes. Force - * nRetCode nonzero so the socket_fwup_err() cleanup below still runs. See - * sdk/PATCHES.md point 1. */ - nRetCode = ret; + /* ninlight: nSize > 0 is guaranteed here (see the nSize==0 check above), so + * this ERR_ABRT is NOT the "empty packet" case - it means + * wm_socket_fwup.c's socket_fwup_recv() actually rejected non-empty data: + * either tls_fwup_enter() refused the session (e.g. still busy from a + * previous attempt) on the first chunk, or tls_fwup_request_sync() reported a + * checksum/trailer/hash mismatch on a later one (see wm_fwup.c). Either way + * recvLen never advances past this point, so going through SHUTDOWN_LINK used + * to start a full Range-resume retry (up to NL_OTA_MAX_RETRY_ROUNDS rounds) + * whose data the fwup task would just keep discarding. Bail out to QUIT_OTA + * directly so the caller gets the real error immediately instead of after + * minutes. Force nRetCode nonzero so the socket_fwup_err() cleanup below + * still runs. See sdk/PATCHES.md point 1. */ + nRetCode = HTTP_CLIENT_ERROR_BAD_STATE; now_state = QUIT_OTA; break; }