Re: [edk2] [Patch] NetworkPkg: Avoid the indefinite wait case in HttpDxe

2016-04-27 Thread Fu, Siyuan 
Reviewed-by: Fu Siyuan <siyuan...@intel.com>

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Jiaxin Wu
> Sent: Tuesday, April 26, 2016 6:39 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting...@intel.com>; Zhang, Lubo <lubo.zh...@intel.com>; Fu,
> Siyuan <siyuan...@intel.com>
> Subject: [edk2] [Patch] NetworkPkg: Avoid the indefinite wait case in HttpDxe
> 
> Need the timer check to avoid the indefinite wait case
> in HttpDxe driver
> A.HTTP receive Header process in HttpTcpReceiveHeader();
> B.HTTP receive Body process in HttpTcpReceiveBody();
> 
> Cc: Hegde Nagaraj P <nagaraj-p.he...@hpe.com>
> Cc: El-Haj-Mahmoud Samer <samer.el-haj-mahm...@hpe.com>
> Cc: Ye Ting <ting...@intel.com>
> Cc: Fu Siyuan <siyuan...@intel.com>
> Cc: Zhang Lubo <lubo.zh...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu <jiaxin...@intel.com>
> ---
>  NetworkPkg/HttpDxe/HttpImpl.c  | 60
> --
>  NetworkPkg/HttpDxe/HttpProto.c | 59
> ++---
>  NetworkPkg/HttpDxe/HttpProto.h | 15 +++
>  3 files changed, 117 insertions(+), 17 deletions(-)
> 
> diff --git a/NetworkPkg/HttpDxe/HttpImpl.c
> b/NetworkPkg/HttpDxe/HttpImpl.c
> index 63b683e..fe2acbc 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -174,10 +174,11 @@ EfiHttpConfigure (
>  >IPv4Node,
>  HttpConfigData->AccessPoint.IPv4Node,
>  sizeof (HttpInstance->IPv4Node)
>  );
>  }
> +
>  //
>  // Creat Tcp child
>  //
>  Status = HttpInitProtocol (HttpInstance, 
> HttpInstance->LocalAddressIsIPv6);
>  if (EFI_ERROR (Status)) {
> @@ -894,11 +895,39 @@ HttpResponseWorker (
>  }
> 
>  HttpInstance->EndofHeader = 
>  HttpInstance->HttpHeaders = 
> 
> -Status = HttpTcpReceiveHeader (HttpInstance, ,
> );
> +
> +if (HttpInstance->TimeoutEvent == NULL) {
> +  //
> +  // Create TimeoutEvent for response
> +  //
> +  Status = gBS->CreateEvent (
> +  EVT_TIMER,
> +  TPL_CALLBACK,
> +  NULL,
> +  NULL,
> +  >TimeoutEvent
> +  );
> +  if (EFI_ERROR (Status)) {
> +goto Error;
> +  }
> +}
> +
> +//
> +// Start the timer, and wait Timeout seconds to receive the header 
> packet.
> +//
> +Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative,
> HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND);
> +if (EFI_ERROR (Status)) {
> +  goto Error;
> +}
> +
> +Status = HttpTcpReceiveHeader (HttpInstance, ,
> , HttpInstance->TimeoutEvent);
> +
> +gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0);
> +
>  if (EFI_ERROR (Status)) {
>goto Error;
>  }
> 
>  ASSERT (HttpHeaders != NULL);
> @@ -1095,14 +1124,41 @@ HttpResponseWorker (
>  }
>}
> 
>ASSERT (HttpInstance->MsgParser != NULL);
> 
> +  if (HttpInstance->TimeoutEvent == NULL) {
> +//
> +// Create TimeoutEvent for response
> +//
> +Status = gBS->CreateEvent (
> +EVT_TIMER,
> +TPL_CALLBACK,
> +NULL,
> +NULL,
> +>TimeoutEvent
> +);
> +if (EFI_ERROR (Status)) {
> +  goto Error;
> +}
> +  }
> +
> +  //
> +  // Start the timer, and wait Timeout seconds to receive the body packet.
> +  //
> +  Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative,
> HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND);
> +  if (EFI_ERROR (Status)) {
> +goto Error;
> +  }
> +
>//
>// We still need receive more data when there is no cache data and
> MsgParser is not NULL;
>//
> -  Status = HttpTcpReceiveBody (Wrap, HttpMsg);
> +  Status = HttpTcpReceiveBody (Wrap, HttpMsg, HttpInstance-
> >TimeoutEvent);
> +
> +  gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0);
> +
>if (EFI_ERROR (Status)) {
>  goto Error;
>}
> 
>return Status;
> diff --git a/NetworkPkg/HttpDxe/HttpProto.c
> b/NetworkPkg/HttpDxe/HttpProto.c
> index 156f138..eb2af7f 100644
> --- a/NetworkPkg/HttpDxe/HttpProto.c
> +++ b/NetworkPkg/HttpDxe/HttpProto.c
> @@ -813,10 +813,15 @@ HttpCleanProtocol (
>  {
>HttpCloseConnection (HttpInstance

Re: [edk2] [Patch] NetworkPkg: Avoid the indefinite wait case in HttpDxe

2016-04-26 Thread Hegde, Nagaraj P 
Reviewed-by: Hegde Nagaraj P 

-Original Message-
From: Jiaxin Wu [mailto:jiaxin...@intel.com] 
Sent: Tuesday, April 26, 2016 4:09 PM
To: edk2-devel@lists.01.org
Cc: Hegde, Nagaraj P ; El-Haj-Mahmoud, Samer 
; Ye Ting ; Fu Siyuan 
; Zhang Lubo 
Subject: [Patch] NetworkPkg: Avoid the indefinite wait case in HttpDxe

Need the timer check to avoid the indefinite wait case in HttpDxe driver A.HTTP 
receive Header process in HttpTcpReceiveHeader(); B.HTTP receive Body process 
in HttpTcpReceiveBody();

Cc: Hegde Nagaraj P 
Cc: El-Haj-Mahmoud Samer 
Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Zhang Lubo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/HttpDxe/HttpImpl.c  | 60 --
 NetworkPkg/HttpDxe/HttpProto.c | 59 ++---
 NetworkPkg/HttpDxe/HttpProto.h | 15 +++
 3 files changed, 117 insertions(+), 17 deletions(-)

diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c 
index 63b683e..fe2acbc 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -174,10 +174,11 @@ EfiHttpConfigure (
 >IPv4Node,
 HttpConfigData->AccessPoint.IPv4Node,
 sizeof (HttpInstance->IPv4Node)
 );
 }
+
 //
 // Creat Tcp child
 //
 Status = HttpInitProtocol (HttpInstance, HttpInstance->LocalAddressIsIPv6);
 if (EFI_ERROR (Status)) {
@@ -894,11 +895,39 @@ HttpResponseWorker (
 }   
 
 HttpInstance->EndofHeader = 
 HttpInstance->HttpHeaders = 
 
-Status = HttpTcpReceiveHeader (HttpInstance, , );
+
+if (HttpInstance->TimeoutEvent == NULL) {
+  //
+  // Create TimeoutEvent for response
+  //
+  Status = gBS->CreateEvent (
+  EVT_TIMER,
+  TPL_CALLBACK,
+  NULL,
+  NULL,
+  >TimeoutEvent
+  );
+  if (EFI_ERROR (Status)) {
+goto Error;
+  }
+}
+
+//
+// Start the timer, and wait Timeout seconds to receive the header packet.
+//
+Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, 
HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND);
+if (EFI_ERROR (Status)) {
+  goto Error;
+}
+
+Status = HttpTcpReceiveHeader (HttpInstance, , 
+ , HttpInstance->TimeoutEvent);
+
+gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0);
+
 if (EFI_ERROR (Status)) {
   goto Error;
 }
 
 ASSERT (HttpHeaders != NULL);
@@ -1095,14 +1124,41 @@ HttpResponseWorker (
 }
   }
 
   ASSERT (HttpInstance->MsgParser != NULL);
 
+  if (HttpInstance->TimeoutEvent == NULL) {
+//
+// Create TimeoutEvent for response
+//
+Status = gBS->CreateEvent (
+EVT_TIMER,
+TPL_CALLBACK,
+NULL,
+NULL,
+>TimeoutEvent
+);
+if (EFI_ERROR (Status)) {
+  goto Error;
+}
+  }
+
+  //
+  // Start the timer, and wait Timeout seconds to receive the body packet.
+  //
+  Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, 
+ HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND);  if (EFI_ERROR (Status)) {
+goto Error;
+  }
+
   //
   // We still need receive more data when there is no cache data and MsgParser 
is not NULL;
   //
-  Status = HttpTcpReceiveBody (Wrap, HttpMsg);
+  Status = HttpTcpReceiveBody (Wrap, HttpMsg, 
+ HttpInstance->TimeoutEvent);
+
+  gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0);
+
   if (EFI_ERROR (Status)) {
 goto Error;
   }
 
   return Status;
diff --git a/NetworkPkg/HttpDxe/HttpProto.c b/NetworkPkg/HttpDxe/HttpProto.c 
index 156f138..eb2af7f 100644
--- a/NetworkPkg/HttpDxe/HttpProto.c
+++ b/NetworkPkg/HttpDxe/HttpProto.c
@@ -813,10 +813,15 @@ HttpCleanProtocol (  {
   HttpCloseConnection (HttpInstance);
   
   HttpCloseTcpConnCloseEvent (HttpInstance);
 
+  if (HttpInstance->TimeoutEvent != NULL) {
+gBS->CloseEvent (HttpInstance->TimeoutEvent);
+HttpInstance->TimeoutEvent = NULL;
+  }
+
   if (HttpInstance->CacheBody != NULL) {
 FreePool (HttpInstance->CacheBody);
 HttpInstance->CacheBody = NULL;
 HttpInstance->NextMsg   = NULL;
   }
@@ -1537,20 +1542,22 @@ HttpTcpReceive (
   Receive the HTTP header by processing the associated HTTP token.
 
   @param[in]   HttpInstance The HTTP instance private data.
   @param[in, out]  SizeofHeadersThe HTTP header length.
   @param[in, out]  BufferSize   The size of buffer to cacahe the header 
message.
+  @param[in]   Timeout  The time to wait for receiving the header 
packet.

[edk2] [Patch] NetworkPkg: Avoid the indefinite wait case in HttpDxe

2016-04-26 Thread Jiaxin Wu 
Need the timer check to avoid the indefinite wait case
in HttpDxe driver
A.HTTP receive Header process in HttpTcpReceiveHeader();
B.HTTP receive Body process in HttpTcpReceiveBody();

Cc: Hegde Nagaraj P 
Cc: El-Haj-Mahmoud Samer 
Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Zhang Lubo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/HttpDxe/HttpImpl.c  | 60 --
 NetworkPkg/HttpDxe/HttpProto.c | 59 ++---
 NetworkPkg/HttpDxe/HttpProto.h | 15 +++
 3 files changed, 117 insertions(+), 17 deletions(-)

diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index 63b683e..fe2acbc 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -174,10 +174,11 @@ EfiHttpConfigure (
 >IPv4Node,
 HttpConfigData->AccessPoint.IPv4Node,
 sizeof (HttpInstance->IPv4Node)
 );
 }
+
 //
 // Creat Tcp child
 //
 Status = HttpInitProtocol (HttpInstance, HttpInstance->LocalAddressIsIPv6);
 if (EFI_ERROR (Status)) {
@@ -894,11 +895,39 @@ HttpResponseWorker (
 }   
 
 HttpInstance->EndofHeader = 
 HttpInstance->HttpHeaders = 
 
-Status = HttpTcpReceiveHeader (HttpInstance, , );
+
+if (HttpInstance->TimeoutEvent == NULL) {
+  //
+  // Create TimeoutEvent for response
+  //
+  Status = gBS->CreateEvent (
+  EVT_TIMER,
+  TPL_CALLBACK,
+  NULL,
+  NULL,
+  >TimeoutEvent
+  );
+  if (EFI_ERROR (Status)) {
+goto Error;
+  }
+}
+
+//
+// Start the timer, and wait Timeout seconds to receive the header packet.
+//
+Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, 
HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND);
+if (EFI_ERROR (Status)) {
+  goto Error;
+}
+
+Status = HttpTcpReceiveHeader (HttpInstance, , , 
HttpInstance->TimeoutEvent);
+
+gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0);
+
 if (EFI_ERROR (Status)) {
   goto Error;
 }
 
 ASSERT (HttpHeaders != NULL);
@@ -1095,14 +1124,41 @@ HttpResponseWorker (
 }
   }
 
   ASSERT (HttpInstance->MsgParser != NULL);
 
+  if (HttpInstance->TimeoutEvent == NULL) {
+//
+// Create TimeoutEvent for response
+//
+Status = gBS->CreateEvent (
+EVT_TIMER,
+TPL_CALLBACK,
+NULL,
+NULL,
+>TimeoutEvent
+);
+if (EFI_ERROR (Status)) {
+  goto Error;
+}
+  }
+
+  //
+  // Start the timer, and wait Timeout seconds to receive the body packet.
+  //
+  Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, 
HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND);
+  if (EFI_ERROR (Status)) {
+goto Error;
+  }
+
   //
   // We still need receive more data when there is no cache data and MsgParser 
is not NULL;
   //
-  Status = HttpTcpReceiveBody (Wrap, HttpMsg);
+  Status = HttpTcpReceiveBody (Wrap, HttpMsg, HttpInstance->TimeoutEvent);
+
+  gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0);
+
   if (EFI_ERROR (Status)) {
 goto Error;
   }
 
   return Status;
diff --git a/NetworkPkg/HttpDxe/HttpProto.c b/NetworkPkg/HttpDxe/HttpProto.c
index 156f138..eb2af7f 100644
--- a/NetworkPkg/HttpDxe/HttpProto.c
+++ b/NetworkPkg/HttpDxe/HttpProto.c
@@ -813,10 +813,15 @@ HttpCleanProtocol (
 {
   HttpCloseConnection (HttpInstance);
   
   HttpCloseTcpConnCloseEvent (HttpInstance);
 
+  if (HttpInstance->TimeoutEvent != NULL) {
+gBS->CloseEvent (HttpInstance->TimeoutEvent);
+HttpInstance->TimeoutEvent = NULL;
+  }
+
   if (HttpInstance->CacheBody != NULL) {
 FreePool (HttpInstance->CacheBody);
 HttpInstance->CacheBody = NULL;
 HttpInstance->NextMsg   = NULL;
   }
@@ -1537,20 +1542,22 @@ HttpTcpReceive (
   Receive the HTTP header by processing the associated HTTP token.
 
   @param[in]   HttpInstance The HTTP instance private data.
   @param[in, out]  SizeofHeadersThe HTTP header length.
   @param[in, out]  BufferSize   The size of buffer to cacahe the header 
message.
+  @param[in]   Timeout  The time to wait for receiving the header 
packet.
   
   @retval EFI_SUCCESS   The HTTP header is received.   
   
   @retval OthersOther errors as indicated.
 
 **/
 EFI_STATUS
 HttpTcpReceiveHeader (
   IN  HTTP_PROTOCOL *HttpInstance,
   IN  OUT UINTN *SizeofHeaders,
-  IN  OUT UINTN *BufferSize
+  IN  OUT UINTN *BufferSize,
+  IN  EFI_EVENT Timeout
   )
 {
   EFI_STATUSStatus;