public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2][PATCH 1/1] NetworkPkg/HttpBootDxe: handle servers which may FIN after file sizing in HttpBootLoadFile
@ 2020-05-22 19:46 Andrei Warkentin
  2020-05-25 12:49 ` [edk2-devel] " Maciej Rabeda
  0 siblings, 1 reply; 4+ messages in thread
From: Andrei Warkentin @ 2020-05-22 19:46 UTC (permalink / raw)
  To: devel; +Cc: maciej.rabeda, jiaxin.wu, siyuan.fu

This is v2 with Maciej Rabeda's feedback.

Python http.server seems to FIN after the first HEAD request to size
the loaded file is completed and ACKed. What happens next is interesting.
On low latency connections, the GET request to download may get sent
after the server sends the FIN but before the client has a chance to
process it. The net result is:
- Server ignores GET
- HttpBootLoadFile returns EFI_CONNECTION_FIN. Boot fails.

In the other case, client handles the FIN before attempting the GET,
so there's a proper three-way handshake as part of GET.

The solution is to retry HttpBootLoadFile 2 times if it returns
EFI_CONNECTION_FIN. This is because HttpBootLoadFile may issue up to
3 requests: HEAD/GET to get size and final GET to load. Some servers
may send a FIN after each request. The first request (HEAD) is not
supposed to fail this way, so only the two subsequent GET request
may result in a FIN.

Fixes https://bugzilla.tianocore.org/show_bug.cgi?id=2720

This what the boot looks like now (when using that Python server):
>>Start HTTP Boot over IPv4....
  Station IP address is 10.0.1.186

  URI: http://10.0.1.57/bootaa64.efi

 Error: Server has terminated the connection.

  URI: http://10.0.1.57/bootaa64.efi
  File Size: 179160 Bytes
  Downloading...100%

Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Andrei Warkentin <andrey.warkentin@gmail.com>
---
 NetworkPkg/HttpBootDxe/HttpBootImpl.c | 35 +++++++++++++++++++-
 NetworkPkg/HttpDxe/HttpImpl.c         |  5 +++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootImpl.c b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
index 4a51f35cdd..1a251b4347 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
@@ -288,6 +288,7 @@ HttpBootDhcp (
   @retval EFI_NOT_STARTED             The driver is in stopped state.
   @retval EFI_BUFFER_TOO_SMALL        The BufferSize is too small to read the boot file. BufferSize has
                                       been updated with the size needed to complete the request.
+  @retval EFI_CONNECTION_FIN          Server had closed the connection while we were waiting downloading.
   @retval EFI_DEVICE_ERROR            An unexpected network error occurred.
   @retval Others                      Other errors as indicated.
 
@@ -408,6 +409,8 @@ ON_EXIT:
       AsciiPrint ("\n  Error: Server response timeout.\n");
     } else if (Status == EFI_ABORTED) {
       AsciiPrint ("\n  Error: Remote boot cancelled.\n");
+    } else if (Status == EFI_CONNECTION_FIN) {
+      AsciiPrint ("\n Error: Server has terminated the connection.\n");
     } else if (Status != EFI_BUFFER_TOO_SMALL) {
       AsciiPrint ("\n  Error: Unexpected network error.\n");
     }
@@ -553,6 +556,7 @@ HttpBootDxeLoadFile (
   BOOLEAN                       UsingIpv6;
   EFI_STATUS                    Status;
   HTTP_BOOT_IMAGE_TYPE          ImageType;
+  UINTN                         MaxTries;
 
   if (This == NULL || BufferSize == NULL || FilePath == NULL) {
     return EFI_INVALID_PARAMETER;
@@ -598,13 +602,42 @@ HttpBootDxeLoadFile (
   // Load the boot file.
   //
   ImageType = ImageTypeMax;
-  Status = HttpBootLoadFile (Private, BufferSize, Buffer, &ImageType);
+  //
+  // HttpBootLoadFile may issue up to 2 requests: HEAD/GET to get
+  // size and final GET to load. Some servers may send a FIN after
+  // each request. The first request (HEAD) is not supposed to
+  // fail this way, so only the two possible GETs need the special
+  // handling.
+  //
+  MaxTries = 2;
+  do {
+    Status = HttpBootLoadFile (Private, BufferSize, Buffer, &ImageType);
+    if (Status == EFI_CONNECTION_FIN) {
+      if (Private->HttpCreated) {
+        //
+        // Tear down HTTP/TCP state entirely. Http->Configure (NULL) is not
+        // sufficient (EFI_ACCESS_DENIED from TCP stack on subsequent
+        // HttpBootLoadFile.
+        //
+        HttpIoDestroyIo (&Private->HttpIo);
+        Private->HttpCreated = FALSE;
+      }
+    }
+  } while (MaxTries-- && Status == EFI_CONNECTION_FIN);
+
   if (EFI_ERROR (Status)) {
     if (Status == EFI_BUFFER_TOO_SMALL && (ImageType == ImageTypeVirtualCd || ImageType == ImageTypeVirtualDisk)) {
       Status = EFI_WARN_FILE_SYSTEM;
     } else if (Status != EFI_BUFFER_TOO_SMALL) {
       HttpBootStop (Private);
     }
+    if (Status == EFI_CONNECTION_FIN) {
+      //
+      // EFI_CONNECTION_FIN is not an expected error for EFI_LOAD_FILE_PROTOCOL.LoadFile(), so
+      // map it to closest matching error. Note: already logged the error in HttpBootLoadFile.
+      //
+      Status = EFI_ABORTED;
+    }
     return Status;
   }
 
diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index 5a6ecbc9d9..34a33b09f7 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -959,6 +959,8 @@ HttpBodyParserCallback (
   @retval EFI_OUT_OF_RESOURCES    Failed to complete the operation due to lack of resources.
   @retval EFI_NOT_READY           Can't find a corresponding Tx4Token/Tx6Token or
                                   the EFI_HTTP_UTILITIES_PROTOCOL is not available.
+  @retval EFI_CONNECTION_FIN      Server had closed the connection while we were waiting for
+                                  a response.
 
 **/
 EFI_STATUS
@@ -1528,6 +1530,9 @@ Error:
   @retval EFI_OUT_OF_RESOURCES    Could not allocate enough system resources.
   @retval EFI_ACCESS_DENIED       An open TCP connection is not present with the host
                                   specified by response URL.
+
+  @retval EFI_CONNECTION_FIN      Server had closed the connection while we were waiting for
+                                  a response.
 **/
 EFI_STATUS
 EFIAPI
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-26 10:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-22 19:46 [edk2][PATCH 1/1] NetworkPkg/HttpBootDxe: handle servers which may FIN after file sizing in HttpBootLoadFile Andrei Warkentin
2020-05-25 12:49 ` [edk2-devel] " Maciej Rabeda
2020-05-26  6:13   ` Andrei Warkentin
2020-05-26 10:59     ` Maciej Rabeda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox