Hi, Jiaxin

The Http->Response() is a unblocking interface, that the function return status 
should be SUCCESS once the token has been queued to the driver. Then if there 
is an error later, the Status in the token should be updated and Event will be 
signaled.
So I think the original code in HTTP driver does make sense, but the HTTP boot 
driver function has bug that, when download the message body, it should also 
check the Status in the token, not only the function.

Best Regards
Siyuan


> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Friday, May 20, 2016 3:24 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting...@intel.com>; Fu, Siyuan <siyuan...@intel.com>; Gary Lin
> <g...@suse.com>; Samer El-Haj-Mahmoud <el...@hpe.com>; Hegde Nagaraj
> P <nagaraj-p.he...@hpe.com>
> Subject: [Patch] NetworkPkg: Need update Http token status while timeout
> happened
> 
> Http token status should be updated to EFI_TIMEOUT while timeout
> happened by any abruptly interrupted (e.g. network disconnection,
> cable plug/unplug...). Otherwise, HttpBootDxe driver will continue
> treat it as no error happened, and its ReceivedSize will be updated
> to ContentLength directly. Moreover, If download image type is RAM
> Disk, the corresponding info will be registered to system.
> 
> Cc: Ye Ting <ting...@intel.com>
> Cc: Fu Siyuan <siyuan...@intel.com>
> Cc: Gary Lin <g...@suse.com>
> Cc: Samer El-Haj-Mahmoud <el...@hpe.com>
> Cc: Hegde Nagaraj P <nagaraj-p.he...@hpe.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu <jiaxin...@intel.com>
> ---
>  NetworkPkg/HttpDxe/HttpImpl.c  |  3 +++
>  NetworkPkg/HttpDxe/HttpProto.c | 16 +++++++++++-----
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/NetworkPkg/HttpDxe/HttpImpl.c
> b/NetworkPkg/HttpDxe/HttpImpl.c
> index f4ae28a..05a96e9 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -1259,10 +1259,13 @@ Exit:
> 
>  Error2:
>    NetMapInsertHead (&HttpInstance->TxTokens, ValueInItem->HttpToken,
> ValueInItem);
> 
>  Error:
> +
> +  HttpCloseConnection (HttpInstance);
> +
>    HttpTcpTokenCleanup (Wrap);
> 
>    if (HttpHeaders != NULL) {
>      FreePool (HttpHeaders);
>    }
> diff --git a/NetworkPkg/HttpDxe/HttpProto.c
> b/NetworkPkg/HttpDxe/HttpProto.c
> index afa7fe4..c3608c0 100644
> --- a/NetworkPkg/HttpDxe/HttpProto.c
> +++ b/NetworkPkg/HttpDxe/HttpProto.c
> @@ -1,9 +1,9 @@
>  /** @file
>    Miscellaneous routines for HttpDxe driver.
> 
> -Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
>  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be
> found at
>  http://opensource.org/licenses/bsd-license.php
> @@ -1605,10 +1605,11 @@ HttpTcpReceiveHeader (
>        while (!HttpInstance->IsRxDone && ((Timeout == NULL) || EFI_ERROR
> (gBS->CheckEvent (Timeout)))) {
>          Tcp4->Poll (Tcp4);
>        }
> 
>        if (!HttpInstance->IsRxDone) {
> +        Tcp4->Cancel (Tcp4, &Rx4Token->CompletionToken);
>          gBS->CloseEvent (Rx4Token->CompletionToken.Event);
>          Rx4Token->CompletionToken.Status = EFI_TIMEOUT;
>        }
> 
>        Status = Rx4Token->CompletionToken.Status;
> @@ -1671,10 +1672,11 @@ HttpTcpReceiveHeader (
>        while (!HttpInstance->IsRxDone && ((Timeout == NULL) || EFI_ERROR
> (gBS->CheckEvent (Timeout)))) {
>          Tcp6->Poll (Tcp6);
>        }
> 
>        if (!HttpInstance->IsRxDone) {
> +        Tcp6->Cancel (Tcp6, &Rx6Token->CompletionToken);
>          gBS->CloseEvent (Rx6Token->CompletionToken.Event);
>          Rx6Token->CompletionToken.Status = EFI_TIMEOUT;
>        }
> 
>        Status = Rx6Token->CompletionToken.Status;
> @@ -1745,11 +1747,12 @@ HttpTcpReceiveBody (
>    HTTP_PROTOCOL             *HttpInstance;
>    EFI_TCP6_PROTOCOL         *Tcp6;
>    EFI_TCP6_IO_TOKEN         *Rx6Token;
>    EFI_TCP4_PROTOCOL         *Tcp4;
>    EFI_TCP4_IO_TOKEN         *Rx4Token;
> -
> +
> +  Status = EFI_SUCCESS;
>    HttpInstance   = Wrap->HttpInstance;
>    Tcp4 = HttpInstance->Tcp4;
>    Tcp6 = HttpInstance->Tcp6;
>    Rx4Token       = NULL;
>    Rx6Token       = NULL;
> @@ -1776,14 +1779,15 @@ HttpTcpReceiveBody (
>      while (!Wrap->TcpWrap.IsRxDone && ((Timeout == NULL) || EFI_ERROR
> (gBS->CheckEvent (Timeout)))) {
>        Tcp6->Poll (Tcp6);
>      }
> 
>      if (!Wrap->TcpWrap.IsRxDone) {
> +      Tcp6->Cancel (Tcp6, &Rx6Token->CompletionToken);
>        gBS->CloseEvent (Rx6Token->CompletionToken.Event);
> +      Rx6Token->CompletionToken.Event = NULL;
>        Rx6Token->CompletionToken.Status = EFI_TIMEOUT;
>        Wrap->HttpToken->Status = Rx6Token->CompletionToken.Status;
> -      gBS->SignalEvent (Wrap->HttpToken->Event);
>      }
>    } else {
>      Rx4Token = &Wrap->TcpWrap.Rx4Token;
>      Rx4Token->Packet.RxData->DataLength = (UINT32) HttpMsg->BodyLength;
>      Rx4Token->Packet.RxData->FragmentTable[0].FragmentLength = (UINT32)
> HttpMsg->BodyLength;
> @@ -1799,19 +1803,21 @@ HttpTcpReceiveBody (
>      while (!Wrap->TcpWrap.IsRxDone && ((Timeout == NULL) || EFI_ERROR
> (gBS->CheckEvent (Timeout)))) {
>        Tcp4->Poll (Tcp4);
>      }
> 
>      if (!Wrap->TcpWrap.IsRxDone) {
> +      Tcp4->Cancel (Tcp4, &Rx4Token->CompletionToken);
>        gBS->CloseEvent (Rx4Token->CompletionToken.Event);
> +      Rx4Token->CompletionToken.Event = NULL;
>        Rx4Token->CompletionToken.Status = EFI_TIMEOUT;
>        Wrap->HttpToken->Status = Rx4Token->CompletionToken.Status;
> -      gBS->SignalEvent (Wrap->HttpToken->Event);
>      }
>    }
> 
> -  return EFI_SUCCESS;
> +  Status = Wrap->HttpToken->Status;
> 
> +  return Status;
>  }
> 
>  /**
>    Clean up Tcp Tokens while the Tcp transmission error occurs.
> 
> --
> 1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to