From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by mx.groups.io with SMTP id smtpd.web08.5170.1627480760500042449 for ; Wed, 28 Jul 2021 06:59:20 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: linux.intel.com, ip: 192.55.52.136, mailfrom: maciej.rabeda@linux.intel.com) X-IronPort-AV: E=McAfee;i="6200,9189,10058"; a="192262035" X-IronPort-AV: E=Sophos;i="5.84,276,1620716400"; d="scan'208";a="192262035" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Jul 2021 06:59:19 -0700 X-IronPort-AV: E=Sophos;i="5.84,276,1620716400"; d="scan'208";a="437778787" Received: from mrabeda-mobl.ger.corp.intel.com (HELO [10.102.8.134]) ([10.102.8.134]) by fmsmga007-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Jul 2021 06:59:18 -0700 Subject: Re: [edk2-devel] [Patch V3] NetworkPkg: Making the HTTP IO timeout value programmable with PCD To: devel@edk2.groups.io, heng.luo@intel.com Cc: Zachary Clark-Williams , Jiaxin Wu , Siyuan Fu References: <20210728121613.2328-1-heng.luo@intel.com> From: "Maciej Rabeda" Message-ID: <978cbcdf-3ad2-26e3-aed1-1c1eb746d99e@linux.intel.com> Date: Wed, 28 Jul 2021 15:59:17 +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: <20210728121613.2328-1-heng.luo@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: pl 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."