Re: [edk2-devel] [PATCH v3] NetworkPkg: Add NETWORK_HTTP_ENABLE macro

["Maciej Rabeda" <maciej.rabeda@linux.intel.com>]

Merged:
PR: https://github.com/tianocore/edk2/pull/1172
Commit: 
https://github.com/tianocore/edk2/commit/126115a9fb3f89f8609336c87aa82fe7da19a9aa

On 19-Nov-20 13:18, Laszlo Ersek wrote:
> On 11/19/20 02:58, Abner Chang wrote:
>> BZ:2917
>>
>> Add NETWORK_HTTP_ENABLE macro and separate HttpDxe
>> and HttpUtilitiesDxe drivers from
>> HTTP_NETWORK_HTTP_BOOT_ENABLE macro.
>>
>> Current NETWORK_HTTP_BOOT_ENABLE macro is defined to enable HTTP
>> boot feature in POST, this macro is not only enabling HTTP Boot
>> related modules but also enabling other generic HTTP modules
>> such as HttpDxe, HttpUtilitiesDxe and DnsDxe.
>> These HTTP base drivers would not be only used by HTTP boot
>> when we introduce the use case of Redfish implementation over
>> HTTP to edk2.
>> We should have a dedicate macro to enable generic HTTP functions
>> on Network stack and additionally provide NETWORK_HTTP_BOOT_ENABLE
>> for HTTP boot functionality for the use case that platform doesn't
>> require HTTP boot.
>>
>> Signed-off-by: Abner Chang <abner.chang@hpe.com>
>> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
>> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
>> Cc: Siyuan Fu <siyuan.fu@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Nickle Wang <nickle.wang@hpe.com>
>> Cc: Peter O'Hanley <peter.ohanley@hpe.com>
>> ---
>>   NetworkPkg/Network.fdf.inc           |  5 ++++-
>>   NetworkPkg/NetworkComponents.dsc.inc |  5 ++++-
>>   NetworkPkg/NetworkDefines.dsc.inc    | 19 +++++++++++++++++--
>>   NetworkPkg/NetworkPkg.ci.yaml        |  2 ++
>>   4 files changed, 27 insertions(+), 4 deletions(-)
>>
>> diff --git a/NetworkPkg/Network.fdf.inc b/NetworkPkg/Network.fdf.inc
>> index 803a0d64fd..8a662ad1de 100644
>> --- a/NetworkPkg/Network.fdf.inc
>> +++ b/NetworkPkg/Network.fdf.inc
>> @@ -46,10 +46,13 @@
>>       INF  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
>>     !endif
>>   
>> -  !if $(NETWORK_HTTP_BOOT_ENABLE) == TRUE
>> +  !if ($(NETWORK_HTTP_BOOT_ENABLE) == TRUE) OR ($(NETWORK_HTTP_ENABLE) == TRUE)
>>       INF  NetworkPkg/DnsDxe/DnsDxe.inf
>>       INF  NetworkPkg/HttpDxe/HttpDxe.inf
>>       INF  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
>> +  !endif
>> +
>> +  !if $(NETWORK_HTTP_BOOT_ENABLE) == TRUE
>>       INF  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
>>     !endif
>>   
>> diff --git a/NetworkPkg/NetworkComponents.dsc.inc b/NetworkPkg/NetworkComponents.dsc.inc
>> index 40cb8ee18e..21cb62082f 100644
>> --- a/NetworkPkg/NetworkComponents.dsc.inc
>> +++ b/NetworkPkg/NetworkComponents.dsc.inc
>> @@ -48,10 +48,13 @@
>>       NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
>>     !endif
>>   
>> -  !if $(NETWORK_HTTP_BOOT_ENABLE) == TRUE
>> +  !if ($(NETWORK_HTTP_BOOT_ENABLE) == TRUE) OR ($(NETWORK_HTTP_ENABLE) == TRUE)
>>       NetworkPkg/DnsDxe/DnsDxe.inf
>>       NetworkPkg/HttpDxe/HttpDxe.inf
>>       NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
>> +  !endif
>> +
>> +  !if $(NETWORK_HTTP_BOOT_ENABLE) == TRUE
>>       NetworkPkg/HttpBootDxe/HttpBootDxe.inf
>>     !endif
>>   
>> diff --git a/NetworkPkg/NetworkDefines.dsc.inc b/NetworkPkg/NetworkDefines.dsc.inc
>> index 18921d81f6..54deb6342a 100644
>> --- a/NetworkPkg/NetworkDefines.dsc.inc
>> +++ b/NetworkPkg/NetworkDefines.dsc.inc
>> @@ -15,12 +15,14 @@
>>   #   DEFINE NETWORK_IP4_ENABLE             = TRUE
>>   #   DEFINE NETWORK_IP6_ENABLE             = TRUE
>>   #   DEFINE NETWORK_TLS_ENABLE             = TRUE
>> +#   DEFINE NETWORK_HTTP_ENABLE            = FALSE
>>   #   DEFINE NETWORK_HTTP_BOOT_ENABLE       = TRUE
>>   #   DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE
>>   #   DEFINE NETWORK_ISCSI_ENABLE           = FALSE
>>   #   DEFINE NETWORK_VLAN_ENABLE            = TRUE
>>   #
>>   # Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
>> +# (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
>>   #
>>   #    SPDX-License-Identifier: BSD-2-Clause-Patent
>>   #
>> @@ -73,10 +75,21 @@
>>     DEFINE NETWORK_TLS_ENABLE = TRUE
>>   !endif
>>   
>> +!ifndef NETWORK_HTTP_ENABLE
>> +  #
>> +  # This flag is to enable or disable HTTP(S) feature.
>> +  # The default is set to FALSE to not affecting the existing
>> +  # platforms.
>> +  # NETWORK_HTTP_ENABLE set to FALSE is not affecting NETWORK_HTTP_BOOT_ENABLE
>> +  # when NETWORK_HTTP_BOOT_ENABLE is TRUE.
>> +  DEFINE NETWORK_HTTP_ENABLE = FALSE
>> +!endif
>> +
>>   !ifndef NETWORK_HTTP_BOOT_ENABLE
>>     #
>>     # This flag is to enable or disable HTTP(S) boot feature.
>>     #
>> +  #
> (1) This new empty line does not seem useful.
>
> I think the maintainer that merges this patch can remove the empty line;
> no need to post v4 just because of it.
>
>>     DEFINE NETWORK_HTTP_BOOT_ENABLE = TRUE
>>   !endif
>>   
>> @@ -112,7 +125,9 @@
>>       !error "Must enable at least IP4 or IP6 stack if NETWORK_ENABLE is set to TRUE!"
>>     !endif
>>   
>> -  !if ($(NETWORK_HTTP_BOOT_ENABLE) == TRUE) AND ($(NETWORK_TLS_ENABLE) == FALSE) AND ($(NETWORK_ALLOW_HTTP_CONNECTIONS) == FALSE)
>> -    !error "Must enable TLS to support HTTPS, or allow unsecured HTTP connection, if NETWORK_HTTP_BOOT_ENABLE is set to TRUE!"
>> +  !if ($(NETWORK_HTTP_BOOT_ENABLE) == TRUE) OR ($(NETWORK_HTTP_ENABLE) == TRUE)
>> +    !if ($(NETWORK_TLS_ENABLE) == FALSE) AND ($(NETWORK_ALLOW_HTTP_CONNECTIONS) == FALSE)
>> +      !error "Must enable TLS to support HTTPS, or allow unsecured HTTP connection, if NETWORK_HTTP_BOOT_ENABLE or NETWORK_HTTP_ENABLE is set to TRUE!"
>> +    !endif
>>     !endif
>>   !endif
>> diff --git a/NetworkPkg/NetworkPkg.ci.yaml b/NetworkPkg/NetworkPkg.ci.yaml
>> index 1a3ab71792..07dc7abd69 100644
>> --- a/NetworkPkg/NetworkPkg.ci.yaml
>> +++ b/NetworkPkg/NetworkPkg.ci.yaml
>> @@ -3,6 +3,7 @@
>>   #
>>   # Copyright (c) Microsoft Corporation
>>   # Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
>> +# (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
>>   # SPDX-License-Identifier: BSD-2-Clause-Patent
>>   ##
>>   {
>> @@ -71,6 +72,7 @@
>>           "BLD_*_NETWORK_IP4_ENABLE": "TRUE",
>>           "BLD_*_NETWORK_IP6_ENABLE": "TRUE",
>>           "BLD_*_NETWORK_TLS_ENABLE": "TRUE",
>> +        "BLD_*_NETWORK_HTTP_ENABLE": "FALSE",
>>           "BLD_*_NETWORK_HTTP_BOOT_ENABLE": "TRUE",
>>           "BLD_*_NETWORK_ISCSI_ENABLE": "TRUE",
>>       }
>>
> The patch should be merged after the edk2-stable202011 release.
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
> Thanks!
> Laszlo
>