Copying my comments from Bugzilla:
I have gone through the Wireshark trace that you have provided. It seems
to be all clear now and the approach to the issue must change.
HttpBootDxe supports HTTP 1.1 and assumes that the HTTP session stays
persistent (single TCP connection for multiple requests instead of one
for each HTTP request/response pair).
https://en.wikipedia.org/wiki/HTTP_persistent_connection I am hesitant
to introduce HTTP/1.0 keep-alive additions to the header. Kindly request
more debugging on Python server side whether to confirm that
implementation you are using supports HTTP/1.1. From what I see in here:
https://github.com/python/cpython/blob/master/Lib/http/server.py
http.server has support for HTTP/1.1 and assumption on not closing the
connection. At the time of writing this comment, it is line 311: if
version_number >= (1, 1) and self.protocol_version >= "HTTP/1.1":
self.close_connection = False Putting the patch on hold (with trend for
rejecting with no support for HTTP/1.0 keep-alive). Additional comment:
https://github.com/python/cpython/blob/master/Lib/http/server.py Line
616: # The version of the HTTP protocol we support. # Set this to
HTTP/1.1 to enable automatic keepalive protocol_version = "HTTP/1.0"
Please modify it that variable to "HTTP/1.1". Your scenario should start
working. Please come back to me with confirmation.
Thanks,
Maciej
On 14-May-20 09:02, Andrei Warkentin wrote:
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
Signed-off-by: Andrei Warkentin <andrey.warken...@gmail.com>
---
NetworkPkg/HttpBootDxe/HttpBootImpl.c | 27 +++++++++++++++++++-
NetworkPkg/HttpDxe/HttpImpl.c | 5 ++++
2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/NetworkPkg/HttpBootDxe/HttpBootImpl.c
b/NetworkPkg/HttpBootDxe/HttpBootImpl.c
index 4a51f35cdd..2d74d5f293 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.
@@ -535,6 +536,7 @@ HttpBootStop (
@retval EFI_BUFFER_TOO_SMALL The BufferSize is too small to read the
current directory entry.
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 for a response.
**/
EFI_STATUS
@@ -553,6 +555,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,7 +601,29 @@ 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;
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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#60206): https://edk2.groups.io/g/devel/message/60206
Mute This Topic: https://groups.io/mt/74200034/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
