Re: [edk2-devel] [Patch v3 2/3] NetworkPkg: Add DSC/FDF include segment files to NetworkPkg.

["Liming Gao" <liming.gao@intel.com>]

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Monday, April 29, 2019 9:05 PM
> To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>
> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: Re: [edk2-devel] [Patch v3 2/3] NetworkPkg: Add DSC/FDF include segment files to NetworkPkg.
> 
> On 04/25/19 14:37, Liming Gao wrote:
> > This patch provides a set of include segment files for platform owner to
> > easily enable/disable network stack support on their platform.
> >
> > For DSC, there are:
> > - 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).
> > For FDF, there is:
> > - a "Network.fdf.inc" for the [Fv*] section(s).
> >
> > These files can be added to the platform DSC/FDF file by using
> >   !include NetworkPkg/xxx
> > where "xxx" is the *.inc file name.
> >
> > 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, please
> > check "NetworkDefines.dsc.inc" for a detail description of each flag.
> 
> (1) Please clarify this paragraph as follows:
> 
> A platform DSC file can diverge from the defaults in
> "NetworkDefines.dsc.inc" by setting the individual DEFINEs before
> including "NetworkDefines.dsc.inc".
> 
> (The current paragraph does say "before the include line", but it
> doesn't say *which* include line. I had to check
> "NetworkDefines.dsc.inc" to see that it (correctly) uses "!ifndef
> FLAG...", to understand the idea.)
> 
Agree. 
> >
> > 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_ISCSI_ENABLE           = TRUE
> >   DEFINE NETWORK_VLAN_ENABLE            = TRUE
> >   DEFINE PLATFORMX64_ENABLE             = FALSE
> 
> (2) The PLATFORMX64_ENABLE flag looks useless; nothing depends on it.
> 
> ... looking at the next patch (v3 3/3), it seems that PLATFORMX64_ENABLE
> has a use after all -- but it only matters for platforms that include
> the "high-level" include files.
> 
> Please modify the commit message on this patch (2/3), and also
> "NetworkDefines.dsc.inc", to explain PLATFORMX64_ENABLE. (I.e. both the
> purpose of the PLATFORMX64_ENABLE flag, and also the fact that it can be
> ignored by platforms that include the individual INC files.)
> 
> ... Actually, under point (9) below, I will suggest dropping
> PLATFORMX64_ENABLE altogether.
> 
I will move this flag into Patch (v3 3/3)
> >
> > Related BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1293
> 
> (3) Please include this BZ in the release planning wiki page; it is
> important to platforms:
> 
> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning
> 
Yes. I will try to catch 201905 release. I will update releasing planning and notes. 
> >
> > Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> > Cc: Ting Ye <ting.ye@intel.com>
> > Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> > ---
> >  NetworkPkg/Network.fdf.inc           |  56 ++++++++++++++++
> >  NetworkPkg/NetworkComponents.dsc.inc |  61 +++++++++++++++++
> >  NetworkPkg/NetworkDefines.dsc.inc    | 126 +++++++++++++++++++++++++++++++++++
> >  NetworkPkg/NetworkLibs.dsc.inc       |  19 ++++++
> >  NetworkPkg/NetworkPcds.dsc.inc       |  16 +++++
> >  5 files changed, 278 insertions(+)
> >  create mode 100644 NetworkPkg/Network.fdf.inc
> >  create mode 100644 NetworkPkg/NetworkComponents.dsc.inc
> >  create mode 100644 NetworkPkg/NetworkDefines.dsc.inc
> >  create mode 100644 NetworkPkg/NetworkLibs.dsc.inc
> >  create mode 100644 NetworkPkg/NetworkPcds.dsc.inc
> >
> > diff --git a/NetworkPkg/Network.fdf.inc b/NetworkPkg/Network.fdf.inc
> > new file mode 100644
> > index 0000000000..8518bad12c
> > --- /dev/null
> > +++ b/NetworkPkg/Network.fdf.inc
> > @@ -0,0 +1,56 @@
> > +## @file
> > +# Network FDF include file for All Architectures.
> > +#
> > +# This file can be included to a platform FDF by using "!include NetworkPkg/Network.fdf.inc"
> 
> (4) Not too important, but I suggest rewrapping all comments to 80 chars
> if possible.
> 
> > +# to add EDKII network stack drivers according to the value of flags described in
> > +# "NetworkPkg/Network.dsc.inc".
> > +#
> > +# Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
> 
> (5) We should probably say 2019, or 2018-2019 here. (The rest of the new
> files say 2019.)
> 
> > +#
> > +#    SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +
> > +!if $(NETWORK_ENABLE) == TRUE
> > +  INF  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
> > +
> > +  !if $(NETWORK_SNP_ENABLE) == TRUE
> > +    INF  MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
> > +  !endif
> > +
> > +  !if $(NETWORK_VLAN_ENABLE) == TRUE
> > +    INF  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
> > +  !endif
> > +
> > +  INF  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
> > +
> > +  !if $(NETWORK_IP4_ENABLE) == TRUE
> > +    INF  MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
> > +    INF  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
> > +    INF  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
> > +    INF  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
> > +    INF  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> > +  !endif
> > +
> > +  !if $(NETWORK_IP6_ENABLE) == TRUE
> > +    INF  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> > +    INF  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
> > +    INF  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> > +    INF  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> > +  !endif
> > +
> > +  INF  NetworkPkg/TcpDxe/TcpDxe.inf
> > +  INF  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > +
> > +  !if $(NETWORK_TLS_ENABLE) == TRUE
> > +    INF  NetworkPkg/TlsDxe/TlsDxe.inf
> > +    INF  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.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
> 
> (6) This doesn't cover NETWORK_ISCSI_ENABLE,
> "NetworkPkg/IScsiDxe/IScsiDxe.inf".
> 
> (The DSC file does.)
> 
> 
> (7) Is ArpDxe really specific to IPv4? I grepped the INF files for
> "gEfiArpProtocolGuid" and "gEfiArpServiceBindingProtocolGuid", and two
> drivers consume those:
> 
> - Ip4Dxe
> - UefiPxeBcDxe
> 
> The latter driver (UefiPxeBcDxe) is not specific to IPv4 vs. IPv6.
> Therefore, should we include ArpDxe simply under "NETWORK_ENABLE"?
> 
> (I see that, elsewhere, we have an !error if the platform or user tries
> to exclude both IPv4 and IPv6, and that's OK. However, if the platform
> or user picks IPv6 only, then the above will prevent them from having
> ArpDxe, and then PXE boot might not work.)
> 
> Hmmmm I could be wrong about this; in the UefiPxeBcDxe driver, the only
> references to the ARP protocol GUIDs are in the functions
> - PxeBcGetNicByIp4Children
> - PxeBcDestroyIp4Children
> - PxeBcCreateIp4Children
> 
> OK. I guess ARP is an IPv4-only requirement then.
> 
> 
> Otherwise, this FDF include looks good, checked against the OVMF FDF
> files, and from "ArmVirtQemuFvMain.fdf.inc". (ArmVirtXen.fdf doesn't
> include the network stack.)
> 
> > +
> > +!endif
> > diff --git a/NetworkPkg/NetworkComponents.dsc.inc b/NetworkPkg/NetworkComponents.dsc.inc
> > new file mode 100644
> > index 0000000000..aede5ea8be
> > --- /dev/null
> > +++ b/NetworkPkg/NetworkComponents.dsc.inc
> > @@ -0,0 +1,61 @@
> > +## @file
> > +# Network DSC include file for [Components*] section of all Architectures.
> > +#
> > +# This file can be included to the [Components*] section(s) of a platform DSC file
> > +# by using "!include NetworkPkg/NetworkComponents.dsc.inc" to specify the INF files
> > +# of EDKII network drivers according to the value of flags described in
> > +# "NetworkDefines.dsc.inc".
> > +#
> > +# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > +#
> > +#    SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +##
> > +
> > +!if $(NETWORK_ENABLE) == TRUE
> > +  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
> > +
> > +  !if $(NETWORK_SNP_ENABLE) == TRUE
> > +    MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
> > +  !endif
> > +
> > +  !if $(NETWORK_VLAN_ENABLE) == TRUE
> > +    MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
> > +  !endif
> > +
> > +  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.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/Udp4Dxe/Udp4Dxe.inf
> > +    MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> > +  !endif
> > +
> > +  !if $(NETWORK_IP6_ENABLE) == TRUE
> > +    NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> > +    NetworkPkg/Ip6Dxe/Ip6Dxe.inf
> > +    NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> > +    NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> > +  !endif
> > +
> > +  NetworkPkg/TcpDxe/TcpDxe.inf
> > +  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> > +
> > +  !if $(NETWORK_TLS_ENABLE) == TRUE
> > +    NetworkPkg/TlsDxe/TlsDxe.inf
> > +    NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.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_ISCSI_ENABLE) == TRUE
> > +    NetworkPkg/IScsiDxe/IScsiDxe.inf
> > +  !endif
> > +!endif
> 
> OK, this matches the FDF include file (except for IScsiDxe, but that's a
> problem I pointed out under (6)).
> 
> The NETWORK_TLS_ENABLE part looks good too. It's worth noting that it
> won't be suitable for OVMF, because OVMF hooks TlsAuthConfigLib into
> TlsAuthConfigDxe, for dynamically setting the variables
> "HttpTlsCipherList" and "TlsCaCertificate".
> 
> But, that's not a problem for this generic DSC include file. OVMF can
> simply set NETWORK_TLS_ENABLE to FALSE, and preserve its own (current)
> TLS_ENABLE build flag, and everything in the DSC/FDF that depends on
> that platform build flag.
> 
> > diff --git a/NetworkPkg/NetworkDefines.dsc.inc b/NetworkPkg/NetworkDefines.dsc.inc
> > new file mode 100644
> > index 0000000000..7a318c98ca
> > --- /dev/null
> > +++ b/NetworkPkg/NetworkDefines.dsc.inc
> > @@ -0,0 +1,126 @@
> > +## @file
> > +# Network DSC include file for [Defines] section of all Architectures.
> > +#
> > +# This file can be included to the [Defines] section of a platform DSC file by
> > +# using "!include NetworkPkg/NetworkDefines.dsc.inc" to set default value of
> > +# flags if they are not defined somewhere else, and also check the value to see
> > +# if there is any conflict.
> > +#
> > +# These flags can be defined before the !include line, or 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_ISCSI_ENABLE           = TRUE
> > +#   DEFINE NETWORK_VLAN_ENABLE            = TRUE
> > +#   DEFINE PLATFORMX64_ENABLE             = FALSE
> > +#
> > +# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > +#
> > +#    SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +##
> > +
> > +!ifndef NETWORK_ENABLE
> > +  #
> > +  # This flag is to enable or disable the whole network stack.
> > +  #
> > +  DEFINE NETWORK_ENABLE = TRUE
> > +!endif
> > +
> > +!ifndef NETWORK_SNP_ENABLE
> > +  #
> > +  # This flag is to include the common SNP driver or not.
> > +  #
> > +  DEFINE NETWORK_SNP_ENABLE = TRUE
> > +!endif
> > +
> > +!ifndef NETWORK_VLAN_ENABLE
> > +  #
> > +  # This flag is to enable or disable VLAN feature.
> > +  #
> > +  DEFINE NETWORK_VLAN_ENABLE = TRUE
> > +!endif
> > +
> > +!ifndef NETWORK_IP4_ENABLE
> > +  #
> > +  # This flag is to enable or disable IPv4 network stack.
> > +  #
> > +  DEFINE NETWORK_IP4_ENABLE = TRUE
> > +!endif
> > +
> > +!ifndef NETWORK_IP6_ENABLE
> > +  #
> > +  # This flag is to enable or disable IPv6 network stack.
> > +  #
> > +  DEFINE NETWORK_IP6_ENABLE = TRUE
> > +!endif
> > +
> > +!ifndef NETWORK_TLS_ENABLE
> > +  #
> > +  # This flag is to enable or disable TLS feature.
> > +  #
> > +  # Note: This feature depends on the OpenSSL building. To enable this feature, please
> > +  #       follow the instructions found in the file "OpenSSL-HOWTO.txt" located in
> > +  #       CryptoPkg\Library\OpensslLib to enable the OpenSSL building first.
> > +  #       The OpensslLib.inf library instance should be used since libssl is required.
> > +  #
> > +  DEFINE NETWORK_TLS_ENABLE = TRUE
> > +!endif
> > +
> > +!ifndef NETWORK_HTTP_BOOT_ENABLE
> > +  #
> > +  # This flag is to enable or disable HTTP(S) boot feature.
> > +  #
> > +  DEFINE NETWORK_HTTP_BOOT_ENABLE = TRUE
> > +!endif
> > +
> > +!ifndef NETWORK_ALLOW_HTTP_CONNECTIONS
> > +  #
> > +  # Indicates whether HTTP connections (i.e., unsecured) are permitted or not.
> > +  #
> > +  # 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_ISCSI_ENABLE
> > +  #
> > +  # This flag is to enable or disable iSCSI feature.
> > +  #
> > +  # Note: This feature depends on the OpenSSL building. To enable this feature, please
> > +  #       follow the instructions found in the file "OpenSSL-HOWTO.txt" located in
> > +  #       CryptoPkg\Library\OpensslLib to enable the OpenSSL building first.
> > +  #       Both OpensslLib.inf and OpensslLibCrypto.inf library instance can be used
> > +  #       since libssl is not required for iSCSI.
> > +  #
> > +  DEFINE NETWORK_ISCSI_ENABLE = TRUE
> > +!endif
> > +
> > +!if $(NETWORK_ENABLE) == TRUE
> > +  #
> > +  # Check the flags to see if there is any conflict.
> > +  #
> > +  !if ($(NETWORK_IP4_ENABLE) == FALSE) AND ($(NETWORK_IP6_ENABLE) == FALSE)
> > +    !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!"
> > +  !endif
> 
> In practice, this is fine, from an OvmfPkg and ArmVirtPkg perspective,
> so I'm not asking for the code to be changed. But, I think it's worth
> raising the following at least in theory:
> 
> Above I mentioned that platforms might want to set "NETWORK_TLS_ENABLE"
> to FALSE, but still include the TLS drivers with various library
> instances hooked into them. In that case, the sanity check above will
> force them, unjustifiedly, to permit unsecured HTTP connections, because
> it will think that TLS is absent.
> 
> (8) Therefore, should we introduce an "override" flag for the last
> sanity check above?
> 
> Again, this is not a practical concern for me -- both OvmfPkg and
> ArmVirtPkg enable unsecured HTTP connections already, so the above check
> will never fire in those platforms. I'm fine if we decide to address
> this "problem" first when an actual platform runs into it.
> 
> > +!endif
> > +
> > +!ifndef PLATFORMX64_ENABLE
> > +  #
> > +  # PLATFORMX64_ENABLE is set to TRUE when PEI is IA32 and DXE is X64 platform
> > +  #
> > +  DEFINE PLATFORMX64_ENABLE = FALSE
> > +!endif
> 
> (9) I commented on this flag under (2) above already. But, now that I'm
> reading the explanation too, and re-checking patch "v3 3/3", I think we
> should simply *eliminate* this define, and in patch 3, we should use:
> 
>   !if ("X64" in $(ARCH))
> 
>   [Components.X64]
>   !include NetworkPkg/NetworkComponents.dsc.inc
> 
>   !else
> 
>   [Components.IA32, Components.ARM, Components.AARCH64]
>   !include NetworkPkg/NetworkComponents.dsc.inc
> 
>   !endif
> 
> Because, the first branch will:
> 
> - cover IA32 PEI + X64 DXE
>   (include the network stack in DXE only)
> 
> - cover X64 PEI+DXE
>   (as if we had written just [Components]
> 
> and the second branch will:
> - cover IA32 PEI+DXE
> - cover ARM PEI+DXE
> - cover AARCH64 PEI+DXE
> - exclude EBC altogether
> 
> (In fact, if we were willing to ignore EBC, the second branch could
> simply say [Components].)
> 
> > diff --git a/NetworkPkg/NetworkLibs.dsc.inc b/NetworkPkg/NetworkLibs.dsc.inc
> > new file mode 100644
> > index 0000000000..dac6b37c6a
> > --- /dev/null
> > +++ b/NetworkPkg/NetworkLibs.dsc.inc
> > @@ -0,0 +1,19 @@
> > +## @file
> > +# Network DSC include file for [LibraryClasses*] section of all Architectures.
> > +#
> > +# This file can be included to the [LibraryClasses*] section(s) of a platform DSC file
> > +# by using "!include NetworkPkg/NetworkLibs.dsc.inc" to specify the library instances
> > +# of EDKII network library classes.
> > +#
> > +# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > +#
> > +#    SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +##
> > +
> > +  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) If we decide to provide all these lib class resolutions without
> checking the DEFINEs -- e.g., we decide to resolve HttpLib without
> checking HTTP_BOOT_ENABLE --, then that's fine, but IMO we should add a
> simple comment about it.
> 
> (11) I think the following lib class resolution is missing:
> 
>   TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
> 
> 
> Other than that, I agree this include file is good. The OpenSSL lib
> class resolution can depend on many factors, and the NetworkPkg aspects
> are already well explained in the DSC include file, near the
> NETWORK_TLS_ENABLE and NETWORK_ISCSI_ENABLE flags.
> 
> 
> 
> > diff --git a/NetworkPkg/NetworkPcds.dsc.inc b/NetworkPkg/NetworkPcds.dsc.inc
> > new file mode 100644
> > index 0000000000..f874b382ef
> > --- /dev/null
> > +++ b/NetworkPkg/NetworkPcds.dsc.inc
> > @@ -0,0 +1,16 @@
> > +## @file
> > +# Network DSC include file for [Pcds*] section of all Architectures.
> > +#
> > +# This file can be included to the [Pcds*] section(s) of a platform DSC file
> > +# by using "!include NetworkPkg/NetworkPcds.dsc.inc" to specify PCD settings
> > +# according to the value of flags described in "NetworkDefines.dsc.inc".
> > +#
> > +# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > +#
> > +#    SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +##
> > +
> > +!if $(NETWORK_ALLOW_HTTP_CONNECTIONS) == TRUE
> > +  gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE
> > +!endif
> >
> 
> This file looks good.
> 
> 
> The "v3 2/3" patch makes me happy -- thank you for providing the include
> snippets without section headers, letting the platform provide its own
> section headers!
> 
> Laszlo