Re: [PATCH 1/6] NetworkPkg: Add DSC/FDF include segment files to NetworkPkg.

["Fu, Siyuan" <siyuan.fu@intel.com>]

Hi, Laszlo

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Wednesday, November 21, 2018 6:47 PM
> To: Fu, Siyuan <siyuan.fu@intel.com>
> Cc: edk2-devel@lists.01.org; Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin
> <jiaxin.wu@intel.com>
> Subject: Re: [edk2] [PATCH 1/6] NetworkPkg: Add DSC/FDF include segment
> files to NetworkPkg.
> 
> On 11/21/18 06:28, Fu Siyuan wrote:
> > The "Network.dsc.inc" and "Network.fdf.inc" are added for platform owner
> to
> > easily enable/disable network stack support on their platform, by adding
> >   !include NetworkPkg/Network.dsc.inc
> > and
> >   !include NetworkPkg/Network.fdf.inc
> > to their platform DSC/FDF files.
> >
> > A set of flags can be changed before the include line or in build
> command
> > line ("-D FLAG=VALUE") to enable or disable related feature set.
> >
> > The default value of these flags are:
> >   DEFINE NETWORK_ENABLE     = TRUE
> >   DEFINE NETWORK_SNP_ENABLE = TRUE
> >   DEFINE NETWORK_IP4_ENABLE = TRUE
> >   DEFINE NETWORK_IP6_ENABLE = TRUE
> >   DEFINE NETWORK_TLS_ENABLE = TRUE
> >   DEFINE NETWORK_HTTP_BOOT_ENABLE       = TRUE
> >   DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE
> >   DEFINE NETWORK_IPSEC_ENABLE = TRUE
> >   DEFINE NETWORK_ISCSI_ENABLE = TRUE
> >   DEFINE NETWORK_VLAN_ENABLE  = TRUE
> 
> (1) This table is inconsistent with regard to alignment. In some cases,
> there are attempts to align the equal signs, and the assigned values
> (such as NETWORK_ENABLE and NETWORK_ALLOW_HTTP_CONNECTIONS), however, as
> a whole, the table is inconsistent. Please align all the equal signs and
> the assigned values to the longest macro name, namely
> NETWORK_ALLOW_HTTP_CONNECTIONS.

Agree, I Will fix this in v2 patch.

> 
> > Detail description of each flag is in Network.dsc.inc file.
> >
> > Related BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1293
> >
> > Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> > Cc: Ting Ye <ting.ye@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> > ---
> >  NetworkPkg/Network.dsc.inc | 203 ++++++++++++++++++++
> >  NetworkPkg/Network.fdf.inc |  81 ++++++++
> >  2 files changed, 284 insertions(+)
> >
> > diff --git a/NetworkPkg/Network.dsc.inc b/NetworkPkg/Network.dsc.inc
> > new file mode 100644
> > index 000000000000..50cf93ba816a
> > --- /dev/null
> > +++ b/NetworkPkg/Network.dsc.inc
> > @@ -0,0 +1,203 @@
> > +## @file
> > +# Network DSC include file for All Architectures.
> > +#
> > +# This file can be included to a platform DSC by using "!include
> NetworkPkg/Network.dsc.inc"
> > +# to add edk2 network stack drivers.
> > +# Below flags can be changed on the command line to enable or disable
> related feature
> > +# support.
> > +#   -D FLAG=VALUE
> > +# The default value of these flags are:
> > +#   DEFINE NETWORK_ENABLE     = TRUE
> > +#   DEFINE NETWORK_SNP_ENABLE = TRUE
> > +#   DEFINE NETWORK_IP4_ENABLE = TRUE
> > +#   DEFINE NETWORK_IP6_ENABLE = TRUE
> > +#   DEFINE NETWORK_TLS_ENABLE = TRUE
> > +#   DEFINE NETWORK_HTTP_BOOT_ENABLE       = TRUE
> > +#   DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE
> > +#   DEFINE NETWORK_IPSEC_ENABLE = TRUE
> > +#   DEFINE NETWORK_ISCSI_ENABLE = TRUE
> > +#   DEFINE NETWORK_VLAN_ENABLE  = TRUE
> 
> (2) Same as (1).
> 
> > +#
> > +# Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
> > +#
> > +#    This program and the accompanying materials
> > +#    are licensed and made available under the terms and conditions of
> the BSD License
> > +#    which accompanies this distribution. The full text of the license
> may be found at
> > +#    http://opensource.org/licenses/bsd-license.php
> > +#
> > +#    THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > +#    WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS
> OR IMPLIED.
> > +#
> > +##
> > +
> > +[Defines]
> > +!ifndef NETWORK_ENABLE
> > +  #
> > +  # This flag is to enable or disable the whole network stack.
> > +  # These can be changed on the command line.
> > +  # -D FLAG=VALUE
> 
> (3) I suggest dropping the statement "These can be changed on the
> command line".
> 
> I also suggest dropping the generic "-D FLAG=VALUE" line.
> 
> Both of those apply to all settings, and they are well explained in the
> general description near the top.

You are correct. These lines were added before I wrote the file header, they are redundant
now since the file header already describe it. Will remove it in V2 patch.

> 
> > +  #
> > +  DEFINE NETWORK_ENABLE = TRUE
> > +!endif
> > +
> > +!ifndef NETWORK_SNP_ENABLE
> > +  #
> > +  # This flag is to include the common SNP driver or not.
> > +  # These can be changed on the command line.
> > +  # -D FLAG=VALUE
> > +  #
> > +  DEFINE NETWORK_SNP_ENABLE = TRUE
> > +!endif
> > +
> > +!ifndef NETWORK_IP4_ENABLE
> > +  #
> > +  # This flag is to enable or disable IPv4 network stack.
> > +  # These can be changed on the command line.
> > +  # -D FLAG=VALUE
> > +  #
> > +  DEFINE NETWORK_IP4_ENABLE = TRUE
> > +!endif
> > +
> > +!ifndef NETWORK_IP6_ENABLE
> > +  #
> > +  # This flag is to enable or disable IPv6 network stack.
> > +  # These can be changed on the command line.
> > +  # -D FLAG=VALUE
> > +  #
> > +  DEFINE NETWORK_IP6_ENABLE = TRUE
> > +!endif
> > +
> > +!ifndef NETWORK_TLS_ENABLE
> > +  #
> > +  # This flag is to enable or disable TLS feature.
> > +  # These can be changed on the command line.
> > +  # -D FLAG=VALUE
> > +  #
> > +  # Note: TLS feature highly depends on the OpenSSL building. To enable
> this
> > +  #       feature, please follow the instructions found in the file
> "Patch-HOWTO.txt"
> 
> (4) The file is now called "OpenSSL-HOWTO.txt".
> 
> (5) Please strip all space characters directly preceding CRLFs. (There
> are multiple instances in this patch.)
> 
> > +  #       located in CryptoPkg\Library\OpensslLib to enable the OpenSSL
> building first.
> > +  #
> > +  DEFINE NETWORK_TLS_ENABLE = TRUE
> > +!endif
> > +
> > +!ifndef NETWORK_HTTP_BOOT_ENABLE
> > +  #
> > +  # This flag is to enable or disable HTTP(s) boot feature.
> 
> (6) I think we should spell it as HTTP(S), upper-case "S". "HTTPs" looks
> strange.
> 
> > +  # These can be changed on the command line.
> > +  # -D FLAG=VALUE
> > +  #
> > +  DEFINE NETWORK_HTTP_BOOT_ENABLE = TRUE
> > +!endif
> > +
> > +!ifndef NETWORK_ALLOW_HTTP_CONNECTIONS
> > +  #
> > +  # Indicates whether HTTP connections (i.e., unsecured) are permitted
> or not.
> > +  # -D FLAG=VALUE
> > +  #
> > +  # Note: If NETWORK_ALLOW_HTTP_CONNECTIONS is TRUE, HTTP connections
> are allowed.
> > +  #       Both the "https://" and "http://" URI schemes are permitted.
> Otherwise, HTTP
> > +  #       connections are denied. Only the "https://" URI scheme is
> permitted.
> > +  #
> > +  DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE
> > +!endif
> > +
> > +!ifndef NETWORK_IPSEC_ENABLE
> > +  #
> > +  # This flag is to enable or disable IPsec feature.
> > +  # These can be changed on the command line.
> > +  # -D FLAG=VALUE
> > +  #
> > +  DEFINE NETWORK_IPSEC_ENABLE = TRUE
> > +!endif
> 
> (7) If IPSEC requires OpenSSL, please spell that out here.
> 
> > +
> > +!ifndef NETWORK_ISCSI_ENABLE
> > +  #
> > +  # This flag is to enable or disable iSCSI feature.
> 
> (8) Please make a note here too about the OpenSSL dependency.
> 
> (9) Regarding OpenSSL dependencies, I see that lower down, this patch
> resolves some library classes to specific library instances. I guess
> that's OK, given that a platform is never expected to resolve these
> library classes to different (possibly platform-specific) library
> instances.
> 
> However, OpenSSL sits somewhere in the middle. It is true that a
> platform is not expected provide its own OpensslLib instance. On the
> other hand, the platform *should* make a choice between "OpensslLib.inf"
> and "OpensslLibCrypto.inf". And, in my opinion, this DSC include file
> should support the platform owner in making that choice.
> 
> I can see multiple options here.
> 
> (9a) Dependent on various NETWORK_* flags, resolve the OpensslLib class
> globally, in this DSC include file.
> 
> However, I think this isn't flexible enough, for all platform needs.
> 
> (9b) Do not resolve OpensslLib globally, but make sure OpensslLib is
> resolved for *individual* NetworkPkg modules on a case-by-case basis
> (using the INF-scoped <LibraryClasses> syntax in this DSC include).
> 
> This approach would mean that TLS drivers get "OpensslLib.inf" hooked
> into them, while IScsiDxe only gets "OpensslLibCrypto.inf". Ultimately,
> if someone looks at a build report file, they should be able to decide
> quickly whether the full-blown "OpensslLib.inf" lib instance was used at
> all.
> 
> (9c) Don't resolve OpensslLib at all, but precisely document the lib
> instance expectations near the NETWORK_* flags. I.e., wherever you
> mention OpenSSL as a dependency (NETWORK_TLS_ENABLE,
> NETWORK_ISCSI_ENABLE, and possibly NETWORK_IPSEC_ENABLE), please also
> explain the minimum required OpenSSL lib instance.

I also considered this problem when creating this patch, about whether I
should resolve OpensslLib instance in NetworkPkg's include file.

Actually I hesitated between your option 9a and 9b and can't decide which
one is better, so I choose to only resolve network specific libraries and
leave all other library to platform owner. You are correct that I should
add more description about the dependency of Openssl, so the platform owner
could get enough knowledge to make right choice.

So I prefer your 9c solution, will add this in the v2 patch.

> 
> > +  # These can be changed on the command line.
> > +  # -D FLAG=VALUE
> > +  #
> > +  DEFINE NETWORK_ISCSI_ENABLE = TRUE
> > +!endif
> > +
> > +!ifndef NETWORK_VLAN_ENABLE
> > +  #
> > +  # This flag is to enable or disable VLAN feature.
> > +  # These can be changed on the command line.
> > +  # -D FLAG=VALUE
> > +  #
> > +  DEFINE NETWORK_VLAN_ENABLE = TRUE
> > +!endif
> > +
> > +[LibraryClasses.common.DXE_DRIVER, LibraryClasses.common.UEFI_DRIVER,
> LibraryClasses.common.UEFI_APPLICATION]
> > +  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
> > +  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
> > +  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
> > +  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
> > +  TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> > +  HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
> 
> (10) Does this DSC include file actually refer to any UEFI_APPLICATION?

No, will remove the "LibraryClasses.common.UEFI_APPLICATION" in v2 patch.

> 
> > +
> > +[PcdsFixedAtBuild]
> > +!if $(NETWORK_ALLOW_HTTP_CONNECTIONS) == TRUE
> > +  gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE
> > +!endif
> 
> (11) I'm not sure this is flexible enough.
> 
> First, in "OvmfPkg/OvmfPkgIa32X64.dsc", we set the PCD only under
> [PcdsFixedAtBuild.X64], not under [PcdsFixedAtBuild]. I agree that in
> practice, such a change shouldn't be a problem however.
> 
> Second, a more practical observation: NetworkPkg.dec declares this PCD
> not just as fixed, but also as patchable-in-module. As far as I
> understand, the above DSC include hunk will prevent platforms from using
> the PCD as patchable.
> 
> I think the most flexible option would be to simply remove the
> NETWORK_ALLOW_HTTP_CONNECTIONS build flag, from this patch, and to allow
> platforms to set the PCD however they want. A build macro ("-D") is not
> expressive enough for this. Also remember that "--pcd" can be passed on
> the build command line too, so not much usability/convenience is lost by
> removing NETWORK_ALLOW_HTTP_CONNECTIONS.

I'm OK to remove this flag.

> 
> > +
> > +[Components]
> 
> (12) How is this going to work with multi-arch platform builds, such as
> "OvmfPkg/OvmfPkgIa32X64.dsc", where the PEI phase is 32-bit, and the DXE
> phase is 64-bit?
> 
> I don't think "OvmfPkgIa32X64.dsc" should build the networking modules
> for 32-bit too. They would never be included in the final flash device,
> so it's wasted compilation.
> 
> Can we introduce separate DSC include files (fragments) for each of the
> DSC file sections? That is, we could have:
> 
> - a "NetworkDefines.dsc.inc" for the [Defines] section(s),
> - a "NetworkLibs.dsc.inc" for the [LibraryClasses*] section(s),
> - a "NetworkPcds.dsc.inc" for the [Pcds*] section(s),
> - a "NetworkComponents.dsc.inc" for the [Components*] section(s).
> 
> Then the platform DSC would be responsible for spelling out the precise
> section header it wants, and then include the matching DSC include file
> right below that.
> 
> In other words, can we split this DSC include into multiple files, at
> the currently shown section headers, and remove the section headers
> themselves?

It's quite a good suggestion.

My initial intention is to make the include file as simple as possible,
to minimize the platform owner's work, so I just provide 1 include file
for DSC, and you are correct that it was done at the cost of losing
flexibility and wasting build time. 

Now I think even we have 4 separate DSC include files, it's still much
easier to organize than original 20 more INF, and with much more flexibility
to platform owner.

This could also solve the problem (11).

> 
> > +!if $(NETWORK_ENABLE) == TRUE
> > +  !if ($(NETWORK_IP4_ENABLE) == FALSE) AND ($(NETWORK_IP6_ENABLE) ==
> FALSE)
> > +    Must select at least IP4 or IP6 stack if NETWORK_ENABLE is set to
> TRUE.
> > +  !endif
> 
> (13) Did you mean to use the !error statement, introduced for:
> 
>   https://bugzilla.tianocore.org/show_bug.cgi?id=701
> 
> ?
> 
> (The question applies to the rest of the error messages below.)
> 
> > +
> > +  !if ($(NETWORK_HTTP_BOOT_ENABLE) == TRUE) AND ($(NETWORK_TLS_ENABLE)
> == FALSE) AND ($(NETWORK_ALLOW_HTTP_CONNECTIONS) == FALSE)
> > +    Must enable TLS to support HTTPs, or allow unsecured HTTP
> connection, if NETWORK_HTTP_BOOT_ENABLE is set to TRUE.
> > +  !endif

Yes and I'm not aware of this statement support, thanks.

> 
> (14) If you agree with the split I'm suggesting under (12): can we move
> these checks into the [Defines] fragment?

Sure

> 
> > +
> > +  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
> > +
> > +  !if $(NETWORK_SNP_ENABLE) == TRUE
> > +    MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
> > +  !endif
> > +
> > +  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
> > +  NetworkPkg/TcpDxe/TcpDxe.inf
> > +  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > +
> > +  !if $(NETWORK_IP4_ENABLE) == TRUE
> > +    MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
> > +    MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
> > +    MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
> > +    MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> > +    MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
> > +  !endif
> > +
> > +  !if $(NETWORK_IP6_ENABLE) == TRUE
> > +    NetworkPkg/Ip6Dxe/Ip6Dxe.inf
> > +    NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> > +    NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> > +    NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> > +  !endif
> > +
> > +  !if $(NETWORK_HTTP_BOOT_ENABLE) == TRUE
> > +    NetworkPkg/DnsDxe/DnsDxe.inf
> > +    NetworkPkg/HttpDxe/HttpDxe.inf
> > +    NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> > +    NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> > +  !endif
> > +
> > +  !if $(NETWORK_IPSEC_ENABLE) == TRUE
> > +    NetworkPkg/IpSecDxe/IpSecDxe.inf
> > +  !endif
> > +
> > +  !if $(NETWORK_TLS_ENABLE) == TRUE
> > +    NetworkPkg/TlsDxe/TlsDxe.inf
> > +    NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
> > +  !endif
> 
> (15) Unfortunately, this isn't flexible enough for OVMF. OVMF hooks
> 
>   OvmfPkg/Library/TlsAuthConfigLib/TlsAuthConfigLib.inf
> 
> into TlsAuthConfigDxe via NULL class resolution -- for setting up the CA
> certificates and cipher suites, in volatile UEFI variables, just in time.

You are correct, that's why I leave the original "TLS_ENABLE" flag and set
NETWORK_TLS_ENABLE to false in OVMF package's patch. If a platform want to
override a driver or library component, it should disable the relative
NETWORK_*** flag for the include file, and add the override component in
its DSC/FDF separately.

I haven't figure out a good solution except this method.


> 
> > +
> > +  !if $(NETWORK_ISCSI_ENABLE) == TRUE
> > +    NetworkPkg/IScsiDxe/IScsiDxe.inf
> > +  !endif
> > +
> > +  !if $(NETWORK_VLAN_ENABLE) == TRUE
> > +    MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
> > +  !endif
> > +
> > +!endif
> > diff --git a/NetworkPkg/Network.fdf.inc b/NetworkPkg/Network.fdf.inc
> > new file mode 100644
> > index 000000000000..481dbb368d23
> > --- /dev/null
> > +++ b/NetworkPkg/Network.fdf.inc
> > @@ -0,0 +1,81 @@
> > +## @file
> > +# Network FDF include file for All Architectures.
> > +#
> > +# This file can be included to a platform FDF by using "!include
> NetworkPkg/Network.fdf.inc"
> > +# to add edk2 network stack drivers.
> > +# Below flags can be changed on the command line to enable or disable
> related feature
> > +# support.
> > +#   -D FLAG=VALUE
> > +# The default value of these flags are:
> > +#   DEFINE NETWORK_ENABLE     = TRUE
> > +#   DEFINE NETWORK_SNP_ENABLE = TRUE
> > +#   DEFINE NETWORK_IP4_ENABLE = TRUE
> > +#   DEFINE NETWORK_IP6_ENABLE = TRUE
> > +#   DEFINE NETWORK_TLS_ENABLE = TRUE
> > +#   DEFINE NETWORK_HTTP_BOOT_ENABLE       = TRUE
> > +#   DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE
> > +#   DEFINE NETWORK_IPSEC_ENABLE = TRUE
> > +#   DEFINE NETWORK_ISCSI_ENABLE = TRUE
> > +#   DEFINE NETWORK_VLAN_ENABLE  = TRUE
> 
> (16) I suggest removing this duplicated documentation, and referring the
> reader to the DSC include files instead.

It's OK with me.

> 
> > +#
> > +# Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
> > +#
> > +#    This program and the accompanying materials
> > +#    are licensed and made available under the terms and conditions of
> the BSD License
> > +#    which accompanies this distribution. The full text of the license
> may be found at
> > +#    http://opensource.org/licenses/bsd-license.php
> > +#
> > +#    THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> > +#    WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS
> OR IMPLIED.
> > +#
> > +
> > +!if $(NETWORK_ENABLE) == TRUE
> > +  !if $(NETWORK_SNP_ENABLE) == TRUE
> > +    INF MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
> 
> (17) There is only one space character after "INF". It is not consistent
> with the rest (the rest uses two space characters).
> 
> > +  !endif
> > +
> > +  INF  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
> > +  INF  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
> 
> (18) The DSC include file references the module INF files in a different
> order: DpcDxe, SnpDxe, MnpDxe. I suggest sticking with the same order in
> the FDF file, for easier maintenance.
> 
> Please verify this (i.e. the listing order) for the rest of the drivers
> too.
> 
> > +  INF  NetworkPkg/TcpDxe/TcpDxe.inf
> > +  INF  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > +
> > +  !if $(NETWORK_IP4_ENABLE) == TRUE
> > +    INF  MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
> > +    INF  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
> > +    INF  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
> > +    INF  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
> > +    INF  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> > +  !endif
> > +
> > +  !if $(NETWORK_IP6_ENABLE) == TRUE
> > +    INF  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
> > +    INF  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> > +    INF  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> > +    INF  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> > +  !endif
> > +
> > +  !if $(NETWORK_HTTP_BOOT_ENABLE) == TRUE
> > +    INF  NetworkPkg/DnsDxe/DnsDxe.inf
> > +    INF  NetworkPkg/HttpDxe/HttpDxe.inf
> > +    INF  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> > +    INF  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> > +  !endif
> > +
> > +  !if $(NETWORK_IPSEC_ENABLE) == TRUE
> > +    INF  NetworkPkg/IpSecDxe/IpSecDxe.inf
> > +  !endif
> > +
> > +  !if $(NETWORK_TLS_ENABLE) == TRUE
> > +    INF  NetworkPkg/TlsDxe/TlsDxe.inf
> > +    INF  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
> > +  !endif
> > +
> > +  !if $(NETWORK_VLAN_ENABLE) == TRUE
> > +    INF  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
> > +  !endif
> > +
> > +  !if $(NETWORK_ISCSI_ENABLE) == TRUE
> > +    INF  NetworkPkg/IScsiDxe/IScsiDxe.inf
> > +  !endif
> > +
> > +!endif
> >

Finally, really thanks for your carefully and patiently review on this patch, and very good suggestions.

> 
> Thanks,
> Laszlo