From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by mx.groups.io with SMTP id smtpd.web11.59.1627490972291615859 for ; Wed, 28 Jul 2021 09:49:32 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: linux.intel.com, ip: 192.55.52.43, mailfrom: maciej.rabeda@linux.intel.com) X-IronPort-AV: E=McAfee;i="6200,9189,10059"; a="298288183" X-IronPort-AV: E=Sophos;i="5.84,276,1620716400"; d="scan'208";a="298288183" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Jul 2021 09:49:31 -0700 X-IronPort-AV: E=Sophos;i="5.84,276,1620716400"; d="scan'208";a="517606689" Received: from lzerbibm-mobl1.ger.corp.intel.com (HELO [10.214.252.131]) ([10.214.252.131]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Jul 2021 09:49:29 -0700 Subject: Re: [edk2-devel] [Patch V3] NetworkPkg: Making the HTTP IO timeout value programmable with PCD From: "Maciej Rabeda" To: devel@edk2.groups.io, heng.luo@intel.com Cc: Zachary Clark-Williams , Jiaxin Wu , Siyuan Fu Reply-To: devel@edk2.groups.io, maciej.rabeda@linux.intel.com References: <20210728121613.2328-1-heng.luo@intel.com> <1695F9210ED1203C.29235@groups.io> Message-ID: <96517ffa-7731-267b-ef8e-a4a39c684bff@linux.intel.com> Date: Wed, 28 Jul 2021 18:49:25 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <1695F9210ED1203C.29235@groups.io> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: pl Change merged: https://github.com/tianocore/edk2/pull/1840 Corrections applied: 1. NetworkPkg.dsc: [PdcsDynamic] -> [PdcsDynamicDefault] 2. NetworkPkg.dsc: PcdHttpTimeout -> PcdHttpIoTimeout On 28-Jul-21 15:59, Maciej Rabeda wrote: > Reviewed-by: Maciej Rabeda > > On 28-Jul-21 14:16, Heng Luo wrote: >> From: Zachary Clark-Williams >> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3507 >> >> HTTP boot has a default set forced timeout value of 5 seconds >> for getting the recovery image from a remote source. >> This change allows the HTTP boot flow to get the IO timeout value >> from the PcdHttpIoTimeout. >> PcdHttpIoTimeout value is set in platform code. >> >> Signed-off-by: Zachary Clark-Williams >> Cc: Maciej Rabeda >> Cc: Jiaxin Wu >> Cc: Siyuan Fu >> --- >>   NetworkPkg/HttpBootDxe/HttpBootClient.c | 12 +++++++++--- >>   NetworkPkg/HttpBootDxe/HttpBootClient.h |  7 +------ >>   NetworkPkg/HttpBootDxe/HttpBootDxe.inf  |  3 ++- >>   NetworkPkg/HttpDxe/HttpDxe.inf          |  3 ++- >>   NetworkPkg/HttpDxe/HttpImpl.c           | 17 ++++++++++++++--- >>   NetworkPkg/HttpDxe/HttpProto.h          |  3 +-- >>   NetworkPkg/NetworkPkg.dec               |  8 ++++---- >>   NetworkPkg/NetworkPkg.dsc               |  3 +++ >>   NetworkPkg/NetworkPkg.uni               |  8 +++++++- >>   9 files changed, 43 insertions(+), 21 deletions(-) >> >> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c >> b/NetworkPkg/HttpBootDxe/HttpBootClient.c >> index 8f21f7766e..aa0a153019 100644 >> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.c >> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c >> @@ -1,7 +1,7 @@ >>   /** @file >>     Implementation of the boot file download function. >>   -Copyright (c) 2015 - 2018, Intel Corporation. All rights >> reserved.
>> +Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.
>>   (C) Copyright 2016 Hewlett Packard Enterprise Development LP
>>   SPDX-License-Identifier: BSD-2-Clause-Patent >>   @@ -596,19 +596,25 @@ HttpBootCreateHttpIo ( >>     HTTP_IO_CONFIG_DATA          ConfigData; >>     EFI_STATUS                   Status; >>     EFI_HANDLE                   ImageHandle; >> +  UINT32                       TimeoutValue; >>       ASSERT (Private != NULL); >>   +  // >> +  // Get HTTP timeout value >> +  // >> +  TimeoutValue = PcdGet32 (PcdHttpIoTimeout); >> + >>     ZeroMem (&ConfigData, sizeof (HTTP_IO_CONFIG_DATA)); >>     if (!Private->UsingIpv6) { >>       ConfigData.Config4.HttpVersion    = HttpVersion11; >> -    ConfigData.Config4.RequestTimeOut = HTTP_BOOT_REQUEST_TIMEOUT; >> +    ConfigData.Config4.RequestTimeOut = TimeoutValue; >>       IP4_COPY_ADDRESS (&ConfigData.Config4.LocalIp, >> &Private->StationIp.v4); >>       IP4_COPY_ADDRESS (&ConfigData.Config4.SubnetMask, >> &Private->SubnetMask.v4); >>       ImageHandle = Private->Ip4Nic->ImageHandle; >>     } else { >>       ConfigData.Config6.HttpVersion    = HttpVersion11; >> -    ConfigData.Config6.RequestTimeOut = HTTP_BOOT_REQUEST_TIMEOUT; >> +    ConfigData.Config6.RequestTimeOut = TimeoutValue; >>       IP6_COPY_ADDRESS (&ConfigData.Config6.LocalIp, >> &Private->StationIp.v6); >>       ImageHandle = Private->Ip6Nic->ImageHandle; >>     } >> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.h >> b/NetworkPkg/HttpBootDxe/HttpBootClient.h >> index 971b2dc800..3a98f0f557 100644 >> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.h >> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.h >> @@ -1,7 +1,7 @@ >>   /** @file >>     Declaration of the boot file download function. >>   -Copyright (c) 2015 - 2018, Intel Corporation. All rights >> reserved.
>> +Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.
>>   (C) Copyright 2016 Hewlett Packard Enterprise Development LP
>>   SPDX-License-Identifier: BSD-2-Clause-Patent >>   @@ -10,12 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >>   #ifndef __EFI_HTTP_BOOT_HTTP_H__ >>   #define __EFI_HTTP_BOOT_HTTP_H__ >>   -#define HTTP_BOOT_REQUEST_TIMEOUT            5000      // 5 >> seconds in uints of millisecond. >> -#define HTTP_BOOT_RESPONSE_TIMEOUT           5000      // 5 seconds >> in uints of millisecond. >>   #define HTTP_BOOT_BLOCK_SIZE                 1500 >> - >> - >> - >>   #define HTTP_USER_AGENT_EFI_HTTP_BOOT "UefiHttpBoot/1.0" >>     // >> diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.inf >> b/NetworkPkg/HttpBootDxe/HttpBootDxe.inf >> index a27a561722..cffa642a4b 100644 >> --- a/NetworkPkg/HttpBootDxe/HttpBootDxe.inf >> +++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.inf >> @@ -1,7 +1,7 @@ >>   ## @file >>   #  This modules produce the Load File Protocol for UEFI HTTP boot. >>   # >> -#  Copyright (c) 2015 - 2018, Intel Corporation. All rights >> reserved.
>> +#  Copyright (c) 2015 - 2021, Intel Corporation. All rights >> reserved.
>>   #  (C) Copyright 2020 Hewlett-Packard Development Company, L.P.
>>   #  SPDX-License-Identifier: BSD-2-Clause-Patent >>   # >> @@ -96,6 +96,7 @@ >>     [Pcd] >>     gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections ## CONSUMES >> +  gEfiNetworkPkgTokenSpaceGuid.PcdHttpIoTimeout              ## >> CONSUMES >>     [UserExtensions.TianoCore."ExtraFiles"] >>     HttpBootDxeExtra.uni >> diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf >> b/NetworkPkg/HttpDxe/HttpDxe.inf >> index 35fe31af18..8dd3838793 100644 >> --- a/NetworkPkg/HttpDxe/HttpDxe.inf >> +++ b/NetworkPkg/HttpDxe/HttpDxe.inf >> @@ -1,7 +1,7 @@ >>   ## @file >>   #  Implementation of EFI HTTP protocol interfaces. >>   # >> -#  Copyright (c) 2015 - 2018, Intel Corporation. All rights >> reserved.
>> +#  Copyright (c) 2015 - 2021, Intel Corporation. All rights >> reserved.
>>   # >>   #  SPDX-License-Identifier: BSD-2-Clause-Patent >>   # >> @@ -73,6 +73,7 @@ >>     [Pcd] >>     gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections ## CONSUMES >> +  gEfiNetworkPkgTokenSpaceGuid.PcdHttpIoTimeout              ## >> CONSUMES >>     [UserExtensions.TianoCore."ExtraFiles"] >>     HttpDxeExtra.uni >> diff --git a/NetworkPkg/HttpDxe/HttpImpl.c >> b/NetworkPkg/HttpDxe/HttpImpl.c >> index 5a6ecbc9d9..8790e9b4e0 100644 >> --- a/NetworkPkg/HttpDxe/HttpImpl.c >> +++ b/NetworkPkg/HttpDxe/HttpImpl.c >> @@ -1,7 +1,7 @@ >>   /** @file >>     Implementation of EFI_HTTP_PROTOCOL protocol interfaces. >>   -  Copyright (c) 2015 - 2018, Intel Corporation. All rights >> reserved.
>> +  Copyright (c) 2015 - 2021, Intel Corporation. All rights >> reserved.
>>     (C) Copyright 2015-2016 Hewlett Packard Enterprise Development >> LP
>>       SPDX-License-Identifier: BSD-2-Clause-Patent >> @@ -983,6 +983,7 @@ HttpResponseWorker ( >>     HTTP_TOKEN_WRAP               *ValueInItem; >>     UINTN                         HdrLen; >>     NET_FRAGMENT                  Fragment; >> +  UINT32                        TimeoutValue; >>       if (Wrap == NULL || Wrap->HttpInstance == NULL) { >>       return EFI_INVALID_PARAMETER; >> @@ -1052,10 +1053,15 @@ HttpResponseWorker ( >>         } >>       } >>   +    // >> +    // Get HTTP timeout value >> +    // >> +    TimeoutValue = PcdGet32 (PcdHttpIoTimeout); >> + >>       // >>       // Start the timer, and wait Timeout seconds to receive the >> header packet. >>       // >> -    Status = gBS->SetTimer (HttpInstance->TimeoutEvent, >> TimerRelative, HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND); >> +    Status = gBS->SetTimer (HttpInstance->TimeoutEvent, >> TimerRelative, TimeoutValue * TICKS_PER_MS); >>       if (EFI_ERROR (Status)) { >>         goto Error; >>       } >> @@ -1329,10 +1335,15 @@ HttpResponseWorker ( >>         } >>       } >>   +    // >> +    // Get HTTP timeout value >> +    // >> +    TimeoutValue = PcdGet32 (PcdHttpIoTimeout); >> + >>       // >>       // Start the timer, and wait Timeout seconds to receive the >> body packet. >>       // >> -    Status = gBS->SetTimer (HttpInstance->TimeoutEvent, >> TimerRelative, HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND); >> +    Status = gBS->SetTimer (HttpInstance->TimeoutEvent, >> TimerRelative, TimeoutValue * TICKS_PER_MS); >>       if (EFI_ERROR (Status)) { >>         goto Error2; >>       } >> diff --git a/NetworkPkg/HttpDxe/HttpProto.h >> b/NetworkPkg/HttpDxe/HttpProto.h >> index 00ba26aca4..6b3e49090e 100644 >> --- a/NetworkPkg/HttpDxe/HttpProto.h >> +++ b/NetworkPkg/HttpDxe/HttpProto.h >> @@ -1,7 +1,7 @@ >>   /** @file >>     The header files of miscellaneous routines for HttpDxe driver. >>   -Copyright (c) 2015 - 2018, Intel Corporation. All rights >> reserved.
>> +Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.
>>   (C) Copyright 2016 Hewlett Packard Enterprise Development LP
>>   SPDX-License-Identifier: BSD-2-Clause-Patent >>   @@ -41,7 +41,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >>   #define HTTP_BUFFER_SIZE_DEAULT      65535 >>   #define HTTP_MAX_SYN_BACK_LOG        5 >>   #define HTTP_CONNECTION_TIMEOUT      60 >> -#define HTTP_RESPONSE_TIMEOUT        5 >>   #define HTTP_DATA_RETRIES            12 >>   #define HTTP_FIN_TIMEOUT             2 >>   #define HTTP_KEEP_ALIVE_PROBES       6 >> diff --git a/NetworkPkg/NetworkPkg.dec b/NetworkPkg/NetworkPkg.dec >> index b81f10ef6e..3e1f5c101d 100644 >> --- a/NetworkPkg/NetworkPkg.dec >> +++ b/NetworkPkg/NetworkPkg.dec >> @@ -97,10 +97,6 @@ >>     # @Prompt Max size of total HTTP chunk transfer. the default >> value is 12MB. >> gEfiNetworkPkgTokenSpaceGuid.PcdMaxHttpChunkTransfer|0x0C00000|UINT32|0x0000000E >>   -  ## The Timeout value of HTTP IO. >> -  # @Prompt The Timeout value of HTTP Io. Default value is 5000. >> - gEfiNetworkPkgTokenSpaceGuid.PcdHttpIoTimeout|5000|UINT32|0x0000000F >> - >>   [PcdsFixedAtBuild, PcdsPatchableInModule] >>     ## Indicates whether HTTP connections (i.e., unsecured) are >> permitted or not. >>     # TRUE  - HTTP connections are allowed. Both the "https://" and >> "http://" URI schemes are permitted. >> @@ -160,5 +156,9 @@ >>     # 0x00 = PXE Disabled >> gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01|UINT8|0x1000000a >>   +  ## The Timeout value of HTTP IO. >> +  # @Prompt The Timeout value of HTTP Io. Default value is 5000. >> + gEfiNetworkPkgTokenSpaceGuid.PcdHttpIoTimeout|5000|UINT32|0x0000000F >> + >>   [UserExtensions.TianoCore."ExtraFiles"] >>     NetworkPkgExtra.uni >> diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc >> index 5e6619ad85..04685a844d 100644 >> --- a/NetworkPkg/NetworkPkg.dsc >> +++ b/NetworkPkg/NetworkPkg.dsc >> @@ -87,6 +87,9 @@ >>     gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2f >>     gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80000000 >>   +[PcdsDynamic] >> +  gEfiNetworkPkgTokenSpaceGuid.PcdHttpTimeout|5000 >> + >> ################################################################################################### >>   # >>   # Components Section - list of the modules and components that will >> be processed by compilation >> diff --git a/NetworkPkg/NetworkPkg.uni b/NetworkPkg/NetworkPkg.uni >> index 328d8cb54a..6d0fa67c6f 100644 >> --- a/NetworkPkg/NetworkPkg.uni >> +++ b/NetworkPkg/NetworkPkg.uni >> @@ -3,7 +3,7 @@ >>   // >>   // This package provides network modules that conform to UEFI 2.4 >> specification. >>   // >> -// Copyright (c) 2009 - 2019, Intel Corporation. All rights >> reserved.
>> +// Copyright (c) 2009 - 2021, Intel Corporation. All rights >> reserved.
>>   // >>   // SPDX-License-Identifier: BSD-2-Clause-Patent >>   // >> @@ -105,3 +105,9 @@ >>   #string STR_gEfiNetworkPkgTokenSpaceGuid_PcdTftpBlockSize_HELP >> #language en-US "This setting can override the default TFTP block >> size. A value of 0 computes " >> "the default from MTU information. A non-zero value will be used as >> block size " >> "in bytes." >> + >> +#string STR_gEfiNetworkPkgTokenSpaceGuid_PcdHttpIoTimeout_PROMPT >> #language en-US "HTTP Boot Image Request and Response Timeout" >> + >> +#string STR_gEfiNetworkPkgTokenSpaceGuid_PcdHttpIoTimeout_HELP >> #language en-US "This value is used to configure the request and >> response timeout when getting " >> + "the recovery image from the remote source during an HTTP recovery >> boot." >> + "The default value set is 5 seconds." > > > > > >