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

["Abner Chang" <abner.chang@hpe.com>]

[-- Attachment #1: Type: text/plain, Size: 9955 bytes --]

Hi Maciej,
I had two feedbacks to (2) and (5) in another thread, email attached.

Thanks
Abner

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Maciej Rabeda
> Sent: Monday, November 16, 2020 8:53 PM
> To: devel@edk2.groups.io; lersek@redhat.com; Chang, Abner (HPS SW/FW
> Technologist) <abner.chang@hpe.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>; Siyuan Fu <siyuan.fu@intel.com>;
> Wang, Nickle (HPS SW) <nickle.wang@hpe.com>; O'Hanley, Peter (EXL)
> <peter.ohanley@hpe.com>
> Subject: Re: [edk2-devel] [NETWORK_HTTP_ENABLE PATCH 1/1]
> NetworkPkg: Add NETWORK_HTTP_ENABLE macro
> 
> Hi Abner,
> 
> Agreeing with Laszlo - please pay attention to our comments. We spend
> some time going through the code to write those - for our mutual benefit.
> Please submit v2 with corrections addressing Laszlo's comments.
> 
> Thanks,
> Maciej
> 
> On 13-Nov-20 20:29, Laszlo Ersek wrote:
> > On 11/12/20 01:53, Chang, Abner (HPS SW/FW Technologist) wrote:
> >> Ok Laszlo,  I think you told me once before. :)
> >>
> >> BTW, do you have comment on this patch because you had ever put some
> opinions on BZ.
> > I think you must have missed my points (2) through (6).
> >
> > Thanks
> > Laszlo
> >
> >>> -----Original Message-----
> >>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf
> >>> Of Laszlo Ersek
> >>> Sent: Thursday, November 12, 2020 5:22 AM
> >>> To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>;
> >>> devel@edk2.groups.io
> >>> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Jiaxin Wu
> >>> <jiaxin.wu@intel.com>; Siyuan Fu <siyuan.fu@intel.com>; Wang, Nickle
> >>> (HPS
> >>> SW) <nickle.wang@hpe.com>; O'Hanley, Peter (EXL)
> >>> <peter.ohanley@hpe.com>
> >>> Subject: Re: [edk2-devel] [NETWORK_HTTP_ENABLE PATCH 1/1]
> >>> NetworkPkg: Add NETWORK_HTTP_ENABLE macro
> >>>
> >>> On 11/11/20 14:19, Abner Chang wrote:
> >>>> BZ:2917
> >>>>
> >>>> Add NETWORK_HTTP_ENABLE macro and separate HttpDxe and
> >>>> HttpUtilitiesDxe drivers from HTTP_NETWORK_HTTP_BOOT_ENABLE
> macro.
> >>>>
> >>>> 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    | 9 +++++++++
> >>>>   3 files changed, 17 insertions(+), 2 deletions(-)
> >>> (1) meta comment -- Abner, I've noticed that you keep placing
> >>> expressions related to the feature or bugfix at hand in the
> >>> bracketed subject prefix. For example, in the current case, it's
> "NETWORK_HTTP_ENABLE":
> >>>
> >>>    [NETWORK_HTTP_ENABLE PATCH 1/1] NetworkPkg: Add
> >>> NETWORK_HTTP_ENABLE macro
> >>>     ^^^^^^^^^^^^^^^^^^^
> >>>
> >>> Please stop doing this. It is incredibly distracting. The subject
> >>> prefix should contain the following elements:
> >>>
> >>> (a) If the patch is not for the main "edk2" repository, then the
> >>> repository
> >>> (project) identifier. For example "edk2-wiki",
> >>> "edk2-InfSpecification",  "edk2- platforms", and so on.
> >>>
> >>> (b) Either the word PATCH or the word RFC.
> >>>
> >>> (c) A version identifier. "v1" is usually not specified (except when
> >>> the submitter already expects having to send a v2). Sometimes the
> >>> version identifier takes the form of "v2 RESEND", when it's an
> >>> identical repost of v2, being reposted only because some people failed
> to receive v2 originally.
> >>>
> >>> (d) the patch number within a series (zero standing for the blurb,
> >>> and altogether omitted when the series consists of a single patch).
> >>>
> >>> In other words, everything we put in the subject prefix is *routing
> >>> information*. It's not *content*.
> >>>
> >>> Please stop putting content in the subject prefix.
> >>>
> >>>
> >>>> 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 a442d1b157..6f274582a8 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            = TRUE
> >>>>   #   DEFINE NETWORK_HTTP_BOOT_ENABLE       = TRUE
> >>> (2) I disagree; the default value for NETWORK_HTTP_ENABLE should be
> >>> FALSE.
> >>>
> >>> Existent platforms that consume "NetworkPkg/NetworkDefines.dsc.inc",
> >>> or the higher level "Network.dsc.inc", fall in one of the following
> categories:
> >>>
> >>> - They don't specify NETWORK_HTTP_BOOT_ENABLE at all. As a result,
> >>> they get the full HTTP stack.
> >>>
> >>> - They set NETWORK_HTTP_BOOT_ENABLE explicitly to TRUE. As a result,
> >>> they get the full HTTP stack.
> >>>
> >>> - They set NETWORK_HTTP_BOOT_ENABLE explicitly to FALSE. As a
> >>> result, they get *none* of the full HTTP stack. They don't get a
> >>> *subset* of the HTTP stack -- they get *none* of it.
> >>>
> >>> The last bullet explains why the NETWORK_HTTP_ENABLE default should
> >>> be FALSE.
> >>>
> >>>
> >>> The new scenario should only be active if a platform explicitly sets
> >>> *both* NETWORK_HTTP_ENABLE=TRUE *and*
> >>> NETWORK_HTTP_BOOT_ENABLE=FALSE.
> >>>
> >>>
> >>>>   #   DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE
> >>>>   #   DEFINE NETWORK_ISCSI_ENABLE           = TRUE
> >>>>   #   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,6 +75,13 @@
> >>>>     DEFINE NETWORK_TLS_ENABLE = TRUE
> >>>>   !endif
> >>>>
> >>>> +!ifndef NETWORK_HTTP_ENABLE
> >>>> +  #
> >>>> +  # This flag is to enable or disable HTTP(S) feature.
> >>>> +  #
> >>> (3) The documentation here must explain that NETWORK_HTTP_ENABLE
> is
> >>> ignored (it has no effect whatsoever) if NETWORK_HTTP_BOOT_ENABLE
> is
> >>> TRUE.
> >>>
> >>>> +  DEFINE NETWORK_HTTP_ENABLE = TRUE
> >>> (4) See (2), this should be FALSE.
> >>>
> >>>> +!endif
> >>>> +
> >>>>   !ifndef NETWORK_HTTP_BOOT_ENABLE
> >>>>     #
> >>>>     # This flag is to enable or disable HTTP(S) boot feature.
> >>>>
> >>> (5) The following condition should be updated too:
> >>>
> >>>    !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!"
> >>>    !endif
> >>>
> >>> That's because NETWORK_ALLOW_HTTP_CONNECTIONS controls
> >>> "PcdAllowHttpConnections", and this PCD is consumed by HttpDxe as
> >>> well, not just HttpBootDxe.
> >>>
> >>> Thus, the subcondition
> >>>
> >>>    ($(NETWORK_HTTP_BOOT_ENABLE) == TRUE)
> >>>
> >>> should be replaced by
> >>>
> >>>    (($(NETWORK_HTTP_BOOT_ENABLE) == TRUE) OR
> >>> ($(NETWORK_HTTP_ENABLE) == TRUE))
> >>>
> >>> because that condition describes whether HttpDxe will be included.
> >>>
> >>> Specifically, the following build config should be rejected:
> >>>
> >>>    NETWORK_HTTP_BOOT_ENABLE       = FALSE (manually set)
> >>>    NETWORK_HTTP_ENABLE            = TRUE  (manually set)
> >>>    NETWORK_TLS_ENABLE             = FALSE (manually set)
> >>>    NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE (default)
> >>>
> >>>
> >>> (6) Please update the !error message as well, accordingly:
> >>>
> >>>    ... if NETWORK_HTTP_BOOT_ENABLE or NETWORK_HTTP_ENABLE is
> set to
> >>> TRUE
> >>>
> >>> Thanks,
> >>> Laszlo
> >>>
> >>>
> >>>
> >>>
> >>>
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 


[-- Attachment #2: Type: message/rfc822, Size: 14086 bytes --]

From: "Chang, Abner (HPS SW/FW Technologist)" <abner.chang@hpe.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>, "lersek@redhat.com" <lersek@redhat.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>, Jiaxin Wu <jiaxin.wu@intel.com>, Siyuan Fu <siyuan.fu@intel.com>, "Wang, Nickle (HPS SW)" <nickle.wang@hpe.com>, "O'Hanley, Peter (EXL)" <peter.ohanley@hpe.com>
Subject: RE: [edk2-devel] [NETWORK_HTTP_ENABLE PATCH 1/1] NetworkPkg: Add NETWORK_HTTP_ENABLE macro
Date: Mon, 16 Nov 2020 02:32:57 +0000
Message-ID: <AT5PR8401MB113709049775E0DFAF913F9AFFE30@AT5PR8401MB1137.NAMPRD84.PROD.OUTLOOK.COM>



> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Laszlo Ersek
> Sent: Thursday, November 12, 2020 5:22 AM
> To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>;
> devel@edk2.groups.io
> Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>; Jiaxin Wu
> <jiaxin.wu@intel.com>; Siyuan Fu <siyuan.fu@intel.com>; Wang, Nickle (HPS
> SW) <nickle.wang@hpe.com>; O'Hanley, Peter (EXL)
> <peter.ohanley@hpe.com>
> Subject: Re: [edk2-devel] [NETWORK_HTTP_ENABLE PATCH 1/1]
> NetworkPkg: Add NETWORK_HTTP_ENABLE macro
>
> On 11/11/20 14:19, Abner Chang wrote:
> > BZ:2917
> >
> > Add NETWORK_HTTP_ENABLE macro and separate HttpDxe and
> > HttpUtilitiesDxe drivers from HTTP_NETWORK_HTTP_BOOT_ENABLE macro.
> >
> > 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    | 9 +++++++++
> >  3 files changed, 17 insertions(+), 2 deletions(-)
>
> (1) meta comment -- Abner, I've noticed that you keep placing expressions
> related to the feature or bugfix at hand in the bracketed subject prefix. For
> example, in the current case, it's "NETWORK_HTTP_ENABLE":
>
>   [NETWORK_HTTP_ENABLE PATCH 1/1] NetworkPkg: Add
> NETWORK_HTTP_ENABLE macro
>    ^^^^^^^^^^^^^^^^^^^
>
> Please stop doing this. It is incredibly distracting. The subject prefix should
> contain the following elements:
>
> (a) If the patch is not for the main "edk2" repository, then the repository
> (project) identifier. For example "edk2-wiki", "edk2-InfSpecification",  "edk2-
> platforms", and so on.
>
> (b) Either the word PATCH or the word RFC.
>
> (c) A version identifier. "v1" is usually not specified (except when the
> submitter already expects having to send a v2). Sometimes the version
> identifier takes the form of "v2 RESEND", when it's an identical repost of v2,
> being reposted only because some people failed to receive v2 originally.
>
> (d) the patch number within a series (zero standing for the blurb, and
> altogether omitted when the series consists of a single patch).
>
> In other words, everything we put in the subject prefix is *routing
> information*. It's not *content*.
>
> Please stop putting content in the subject prefix.
>
>
> >
> > 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 a442d1b157..6f274582a8 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            = TRUE
> >  #   DEFINE NETWORK_HTTP_BOOT_ENABLE       = TRUE
>
> (2) I disagree; the default value for NETWORK_HTTP_ENABLE should be
> FALSE.
>
> Existent platforms that consume "NetworkPkg/NetworkDefines.dsc.inc", or
> the higher level "Network.dsc.inc", fall in one of the following categories:
>
> - They don't specify NETWORK_HTTP_BOOT_ENABLE at all. As a result, they
> get the full HTTP stack.
>
> - They set NETWORK_HTTP_BOOT_ENABLE explicitly to TRUE. As a result,
> they get the full HTTP stack.
>
> - They set NETWORK_HTTP_BOOT_ENABLE explicitly to FALSE. As a result,
> they get *none* of the full HTTP stack. They don't get a *subset* of the
> HTTP stack -- they get *none* of it.
>
> The last bullet explains why the NETWORK_HTTP_ENABLE default should be
> FALSE.
I don’t quite get the last scenario. If they set NETWORK_HTTP_BOOT_ENABLE to FALSE then NETWORK_HTTP_ENABLE is still TURE for other HTTP use cases.
They can set NETWORK_HTTP_ENABLE to FALSE explicitly if they don’t even need HTTP.

I think those network definitions were designed as default ON.

>
>
> The new scenario should only be active if a platform explicitly sets *both*
> NETWORK_HTTP_ENABLE=TRUE *and*
> NETWORK_HTTP_BOOT_ENABLE=FALSE.
>
>
> >  #   DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE
> >  #   DEFINE NETWORK_ISCSI_ENABLE           = TRUE
> >  #   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,6 +75,13 @@
> >    DEFINE NETWORK_TLS_ENABLE = TRUE
> >  !endif
> >
> > +!ifndef NETWORK_HTTP_ENABLE
> > +  #
> > +  # This flag is to enable or disable HTTP(S) feature.
> > +  #
>
> (3) The documentation here must explain that NETWORK_HTTP_ENABLE is
> ignored (it has no effect whatsoever) if NETWORK_HTTP_BOOT_ENABLE is
> TRUE.
>
> > +  DEFINE NETWORK_HTTP_ENABLE = TRUE
>
> (4) See (2), this should be FALSE.
>
> > +!endif
> > +
> >  !ifndef NETWORK_HTTP_BOOT_ENABLE
> >    #
> >    # This flag is to enable or disable HTTP(S) boot feature.
> >
>
> (5) The following condition should be updated too:
>
>   !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!"
>   !endif
>
> That's because NETWORK_ALLOW_HTTP_CONNECTIONS controls
> "PcdAllowHttpConnections", and this PCD is consumed by HttpDxe as well,
> not just HttpBootDxe.
>
> Thus, the subcondition
>
>   ($(NETWORK_HTTP_BOOT_ENABLE) == TRUE)
>
> should be replaced by
>
>   (($(NETWORK_HTTP_BOOT_ENABLE) == TRUE) OR
> ($(NETWORK_HTTP_ENABLE) == TRUE))
>
> because that condition describes whether HttpDxe will be included.
>
> Specifically, the following build config should be rejected:
>
>   NETWORK_HTTP_BOOT_ENABLE       = FALSE (manually set)
>   NETWORK_HTTP_ENABLE            = TRUE  (manually set)
>   NETWORK_TLS_ENABLE             = FALSE (manually set)
>   NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE (default)
What if the use case just requires HTTP Utility Protocol to produce and consume HTTP headers but not sending out through HTTP protocol, via in-band channel instead.  I don’t think we have to put the restrictions this one.

>
>
> (6) Please update the !error message as well, accordingly:
>
>   ... if NETWORK_HTTP_BOOT_ENABLE or NETWORK_HTTP_ENABLE is set to
> TRUE

We come back to other issues once we clarify (2).
Abner

>
> Thanks,
> Laszlo
>
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#67310):
> INVALID URI REMOVED
> 3A__edk2.groups.io_g_devel_message_67310&d=DwICaQ&c=C5b8zRQO1mi
> GmBeVZ2LFWg&r=_SN6FZBN4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=
> Ib1CTqs8b8Q8p88F7LG1uLdIQYnG9BIfCuJHCDkTgYc&s=QyXVzsrJKZAr18UQX6
> JIj6aZzvgMBOwtA4L5wIax2zE&e=
> Mute This Topic: INVALID URI REMOVED
> 3A__groups.io_mt_78182924_1772629&d=DwICaQ&c=C5b8zRQO1miGmBeV
> Z2LFWg&r=_SN6FZBN4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=Ib1CTq
> s8b8Q8p88F7LG1uLdIQYnG9BIfCuJHCDkTgYc&s=o-
> 1cFCK77J8Gj63hJCgd2a63V-5ev-iOD1pap20Q6tw&e=
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: INVALID URI REMOVED
> 3A__edk2.groups.io_g_devel_unsub&d=DwICaQ&c=C5b8zRQO1miGmBeVZ
> 2LFWg&r=_SN6FZBN4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=Ib1CTqs
> 8b8Q8p88F7LG1uLdIQYnG9BIfCuJHCDkTgYc&s=c-
> xilcssfThv299NcEoxuL2siiZ0wvOx_27uGB6N2go&e=  [abner.chang@hpe.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>