public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/6] Add DSC/FDF include segment files for network stack
@ 2018-11-21  5:28 Fu Siyuan
  2018-11-21  5:28 ` [PATCH 1/6] NetworkPkg: Add DSC/FDF include segment files to NetworkPkg Fu Siyuan
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Fu Siyuan @ 2018-11-21  5:28 UTC (permalink / raw)
  To: edk2-devel
  Cc: Jiaxin Wu, Ting Ye, Ruiyu Ni, Hao Wu, Laszlo Ersek,
	Ard Biesheuvel, Julien Grall, Jordan Justen, Andrew Fish,
	Anthony Perard, David Wei, Mang Guo

There is a patch to remove the redudant IP4 only iSCSI/PXE/TCP drivers
from MdeModulePkg, which has been reviewed before edk2-stable201811 tag.
And we also have plan to move all network related libraries/modules to
NetworkPkg. In order to make these change more smoothly, 2 configuration
fragment files are provided for platform to enable the network stack
support, without directly reference the INF module path.

Patch 1/6 adds 2 centralized dsc/fdf include files to NetworkPkg, with
a set of flags for feature set enable/disable.
Patch 2~6 updates edk2 platform dsc/fdf files to use the new include
files, instead of reference the module INF.


Fu Siyuan (6):
  NetworkPkg: Add DSC/FDF include segment files to NetworkPkg.
  Nt32Pkg: Update DSC/FDF to use NetworkPkg's include fragment file.
  ArmVirtPkg: Update DSC/FDF to use NetworkPkg's include fragment file.
  EmulatorPkg: Update DSC/FDF to use NetworkPkg's include fragment file.
  OvmfPkg: Update DSC/FDF to use NetworkPkg's include fragment file.
  Vlv2TbltDevicePkg: Update DSC/FDF to use NetworkPkg's include fragment
    file.

 ArmVirtPkg/ArmVirt.dsc.inc              |  12 --
 ArmVirtPkg/ArmVirtQemu.dsc              |  44 +----
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc    |  28 +--
 ArmVirtPkg/ArmVirtQemuKernel.dsc        |  38 +---
 EmulatorPkg/EmulatorPkg.dsc             |  32 ++-
 EmulatorPkg/EmulatorPkg.fdf             |  10 +-
 NetworkPkg/Network.dsc.inc              | 203 ++++++++++++++++++++
 NetworkPkg/Network.fdf.inc              |  81 ++++++++
 Nt32Pkg/Nt32Pkg.dsc                     |  71 +------
 Nt32Pkg/Nt32Pkg.fdf                     |  27 +--
 OvmfPkg/OvmfPkgIa32.dsc                 |  52 ++---
 OvmfPkg/OvmfPkgIa32.fdf                 |  25 +--
 OvmfPkg/OvmfPkgIa32X64.dsc              |  53 ++---
 OvmfPkg/OvmfPkgIa32X64.fdf              |  25 +--
 OvmfPkg/OvmfPkgX64.dsc                  |  52 ++---
 OvmfPkg/OvmfPkgX64.fdf                  |  25 +--
 Vlv2TbltDevicePkg/PlatformPkg.fdf       |  25 +--
 Vlv2TbltDevicePkg/PlatformPkgConfig.dsc |   3 +
 Vlv2TbltDevicePkg/PlatformPkgGcc.fdf    |  25 +--
 Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc |  42 +---
 Vlv2TbltDevicePkg/PlatformPkgIA32.dsc   |  42 +---
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc    |  42 +---
 22 files changed, 378 insertions(+), 579 deletions(-)
 create mode 100644 NetworkPkg/Network.dsc.inc
 create mode 100644 NetworkPkg/Network.fdf.inc

Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Ting Ye <ting.ye@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Julien Grall <julien.grall@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: David Wei <david.wei@intel.com>
Cc: Mang Guo <mang.guo@intel.com>
-- 
2.19.1.windows.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/6] NetworkPkg: Add DSC/FDF include segment files to NetworkPkg.
  2018-11-21  5:28 [PATCH 0/6] Add DSC/FDF include segment files for network stack Fu Siyuan
@ 2018-11-21  5:28 ` Fu Siyuan
  2018-11-21 10:46   ` Laszlo Ersek
  2018-11-21  5:28 ` [PATCH 2/6] Nt32Pkg: Update DSC/FDF to use NetworkPkg's include fragment file Fu Siyuan
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Fu Siyuan @ 2018-11-21  5:28 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jiaxin Wu, Ting Ye

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
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
+#
+# 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
+  #
+  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" 
+  #       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.  
+  # 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
+
+!ifndef NETWORK_ISCSI_ENABLE
+  #
+  # This flag is to enable or disable iSCSI feature.
+  # 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
+
+[PcdsFixedAtBuild]
+!if $(NETWORK_ALLOW_HTTP_CONNECTIONS) == TRUE
+  gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE
+!endif
+
+[Components]
+!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
+
+  !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
+
+  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
+
+  !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
+#
+# 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
+  !endif
+  
+  INF  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
+  INF  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
+  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
-- 
2.19.1.windows.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/6] Nt32Pkg: Update DSC/FDF to use NetworkPkg's include fragment file.
  2018-11-21  5:28 [PATCH 0/6] Add DSC/FDF include segment files for network stack Fu Siyuan
  2018-11-21  5:28 ` [PATCH 1/6] NetworkPkg: Add DSC/FDF include segment files to NetworkPkg Fu Siyuan
@ 2018-11-21  5:28 ` Fu Siyuan
  2018-11-21  5:28 ` [PATCH 3/6] ArmVirtPkg: " Fu Siyuan
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Fu Siyuan @ 2018-11-21  5:28 UTC (permalink / raw)
  To: edk2-devel; +Cc: Ruiyu Ni, Hao Wu

This patch updates the platform DSC/FDF files to use the include fragment
files provided by NetworkPkg.
The feature enabling flags in [Defines] section have be updated to use the
NetworkPkg's terms, and the value have been overridden with the original
default value on this platform.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
---
 Nt32Pkg/Nt32Pkg.dsc | 71 ++------------------
 Nt32Pkg/Nt32Pkg.fdf | 27 +-------
 2 files changed, 8 insertions(+), 90 deletions(-)

diff --git a/Nt32Pkg/Nt32Pkg.dsc b/Nt32Pkg/Nt32Pkg.dsc
index 4dbde0cc45b6..8cd29b27b0a2 100644
--- a/Nt32Pkg/Nt32Pkg.dsc
+++ b/Nt32Pkg/Nt32Pkg.dsc
@@ -49,34 +49,15 @@ [Defines]
   #       located in CryptoPkg\Library\OpensslLib to enable the OpenSSL building first.
   #
   DEFINE SECURE_BOOT_ENABLE      = FALSE
-  
-  #
-  # 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" 
-  #       located in CryptoPkg\Library\OpensslLib to enable the OpenSSL building first.
-  #
-  DEFINE TLS_ENABLE = FALSE
-  
-  #
-  # Indicates whether HTTP connections (i.e., unsecured) are permitted or not.
-  # -D FLAG=VALUE
-  #
-  # Note: If 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 ALLOW_HTTP_CONNECTIONS = TRUE
 
   #
-  # This flag is to enable or disable IPv6 network stack.
-  # These can be changed on the command line.
-  # -D FLAG=VALUE
+  # SnpNt32Dxe.inf will be used.
   #
+  DEFINE NETWORK_SNP_ENABLE = FALSE
   DEFINE NETWORK_IP6_ENABLE = FALSE
+  DEFINE NETWORK_TLS_ENABLE = FALSE
+  DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = TRUE
+!include NetworkPkg/Network.dsc.inc
 
 ################################################################################
 #
@@ -139,12 +120,6 @@ [LibraryClasses]
   #
   UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.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
-  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   GenericBdsLib|IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
   CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
@@ -276,11 +251,6 @@ [PcdsFixedAtBuild]
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
 
-!if $(ALLOW_HTTP_CONNECTIONS) == TRUE
-  gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE
-!endif
-
-
 !if $(SECURE_BOOT_ENABLE) == TRUE
   # override the default values from SecurityPkg to ensure images from all sources are verified in secure boot
   gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04
@@ -454,39 +424,10 @@ [Components]
   MdeModulePkg/Application/HelloWorld/HelloWorld.inf
 
   #
-  # Network stack drivers
+  # Network SNP drivers
   # To test network drivers, need network Io driver(SnpNt32Io.dll), please refer to NETWORK-IO Subproject.
   #
-  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-  MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
-  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
-  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
-  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
-  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
-  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
-  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-  NetworkPkg/TcpDxe/TcpDxe.inf
-  NetworkPkg/IScsiDxe/IScsiDxe.inf
   Nt32Pkg/SnpNt32Dxe/SnpNt32Dxe.inf
-
-!if $(NETWORK_IP6_ENABLE) == TRUE
-  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
-  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
-  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
-  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
-!endif
-
-  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
-  NetworkPkg/DnsDxe/DnsDxe.inf
-  NetworkPkg/HttpDxe/HttpDxe.inf
-  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
-   
-!if $(TLS_ENABLE) == TRUE
-  NetworkPkg/TlsDxe/TlsDxe.inf
-  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
-!endif
-
   MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
   MdeModulePkg/Application/UiApp/UiApp.inf{
     <LibraryClasses>
diff --git a/Nt32Pkg/Nt32Pkg.fdf b/Nt32Pkg/Nt32Pkg.fdf
index b65c95201b36..3e7b4477f753 100644
--- a/Nt32Pkg/Nt32Pkg.fdf
+++ b/Nt32Pkg/Nt32Pkg.fdf
@@ -249,32 +249,9 @@ [FV.FvRecovery]
 INF  MdeModulePkg/Universal/PlatformDriOverrideDxe/PlatformDriOverrideDxe.inf
 INF  MdeModulePkg/Universal/DriverSampleDxe/DriverSampleDxe.inf
 
-INF  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-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/MnpDxe/MnpDxe.inf
-INF  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-INF  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
-INF  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
-INF  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-INF  NetworkPkg/TcpDxe/TcpDxe.inf
-INF  NetworkPkg/IScsiDxe/IScsiDxe.inf
 INF  Nt32Pkg/SnpNt32Dxe/SnpNt32Dxe.inf
-!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
-INF  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
-INF  NetworkPkg/DnsDxe/DnsDxe.inf
-INF  NetworkPkg/HttpDxe/HttpDxe.inf
-INF  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
-!if $(TLS_ENABLE) == TRUE
-INF  NetworkPkg/TlsDxe/TlsDxe.inf
-INF  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
-!endif
+!include NetworkPkg/Network.fdf.inc
+
 INF  MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuApp.inf
 ################################################################################
 #
-- 
2.19.1.windows.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/6] ArmVirtPkg: Update DSC/FDF to use NetworkPkg's include fragment file.
  2018-11-21  5:28 [PATCH 0/6] Add DSC/FDF include segment files for network stack Fu Siyuan
  2018-11-21  5:28 ` [PATCH 1/6] NetworkPkg: Add DSC/FDF include segment files to NetworkPkg Fu Siyuan
  2018-11-21  5:28 ` [PATCH 2/6] Nt32Pkg: Update DSC/FDF to use NetworkPkg's include fragment file Fu Siyuan
@ 2018-11-21  5:28 ` Fu Siyuan
  2018-11-21 11:26   ` Laszlo Ersek
  2018-11-21  5:28 ` [PATCH 4/6] EmulatorPkg: " Fu Siyuan
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Fu Siyuan @ 2018-11-21  5:28 UTC (permalink / raw)
  To: edk2-devel; +Cc: Laszlo Ersek, Ard Biesheuvel, Julien Grall

This patch updates the platform DSC/FDF files to use the include fragment
files provided by NetworkPkg.
The feature enabling flags in [Defines] section have been updated to use
the NetworkPkg's terms, and the value has been overridden with the original
default value on this platform.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Julien Grall <julien.grall@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
---
 ArmVirtPkg/ArmVirt.dsc.inc           | 12 ------
 ArmVirtPkg/ArmVirtQemu.dsc           | 44 ++++----------------
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 28 +------------
 ArmVirtPkg/ArmVirtQemuKernel.dsc     | 38 +++--------------
 4 files changed, 14 insertions(+), 108 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 70a0ac4d786c..24a064198b5e 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -75,18 +75,6 @@ [LibraryClasses.common]
   # use the accelerated BaseMemoryLibOptDxe by default, overrides for SEC/PEI below
   BaseMemoryLib|MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
 
-  # Networking Requirements
-  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
-  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
-  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
-  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
-!if $(NETWORK_IP6_ENABLE) == TRUE
-  TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
-!endif
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
-!endif
-
   #
   # It is not possible to prevent the ARM compiler from inserting calls to intrinsic functions.
   # This library provides the instrinsic functions such a compiler may generate calls to.
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 885c6b14b844..5a1b6b0f0519 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -35,9 +35,14 @@ [Defines]
   #
   DEFINE SECURE_BOOT_ENABLE      = FALSE
   DEFINE NETWORK_IP6_ENABLE      = FALSE
-  DEFINE HTTP_BOOT_ENABLE        = FALSE
-
+  DEFINE NETWORK_HTTP_BOOT_ENABLE= FALSE
+  DEFINE NETWORK_SNP_ENABLE      = FALSE
+  DEFINE NETWORK_TLS_ENABLE      = FALSE
+  DEFINE NETWORK_IPSEC_ENABLE    = FALSE
+  DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS  = TRUE
+  
 !include ArmVirtPkg/ArmVirt.dsc.inc
+!include NetworkPkg/Network.dsc.inc
 
 [LibraryClasses.common]
   ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
@@ -123,10 +128,6 @@ [PcdsFixedAtBuild.common]
   #
   gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|0
 
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE
-!endif
-
   # System Memory Base -- fixed at 0x4000_0000
   gArmTokenSpaceGuid.PcdSystemMemoryBase|0x40000000
 
@@ -335,37 +336,6 @@ [Components.common]
       NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
   }
 
-  #
-  # Networking stack
-  #
-  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-  MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
-  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
-  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
-  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
-  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
-  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
-!if $(NETWORK_IP6_ENABLE) == TRUE
-  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
-  NetworkPkg/TcpDxe/TcpDxe.inf
-  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
-  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
-  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
-  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-  NetworkPkg/IScsiDxe/IScsiDxe.inf
-!else
-  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
-  MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
-  MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
-!endif
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  NetworkPkg/DnsDxe/DnsDxe.inf
-  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
-  NetworkPkg/HttpDxe/HttpDxe.inf
-  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
-!endif
-
   #
   # SCSI Bus and Disk Driver
   #
diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
index a6390bd4b841..560651e49e0b 100644
--- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
+++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
@@ -118,33 +118,7 @@ [FV.FvMain]
   #
   # Networking stack
   #
-  INF MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-  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/MnpDxe/MnpDxe.inf
-  INF MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-  INF MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
-  INF MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
-!if $(NETWORK_IP6_ENABLE) == TRUE
-  INF NetworkPkg/Ip6Dxe/Ip6Dxe.inf
-  INF NetworkPkg/TcpDxe/TcpDxe.inf
-  INF NetworkPkg/Udp6Dxe/Udp6Dxe.inf
-  INF NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
-  INF NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
-  INF NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-  INF NetworkPkg/IScsiDxe/IScsiDxe.inf
-!else
-  INF MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
-  INF MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
-  INF MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
-!endif
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  INF NetworkPkg/DnsDxe/DnsDxe.inf
-  INF NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
-  INF NetworkPkg/HttpDxe/HttpDxe.inf
-  INF NetworkPkg/HttpBootDxe/HttpBootDxe.inf
-!endif
+!include NetworkPkg/Network.fdf.inc
 
   #
   # SCSI Bus and Disk Driver
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 434d6861a56f..a5c3e957ac82 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -35,9 +35,14 @@ [Defines]
   #
   DEFINE SECURE_BOOT_ENABLE      = FALSE
   DEFINE NETWORK_IP6_ENABLE      = FALSE
-  DEFINE HTTP_BOOT_ENABLE        = FALSE
+  DEFINE NETWORK_HTTP_BOOT_ENABLE        = FALSE
+  DEFINE NETWORK_SNP_ENABLE      = FALSE
+  DEFINE NETWORK_TLS_ENABLE              = FALSE
+  DEFINE NETWORK_IPSEC_ENABLE    = FALSE
+  DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS  = TRUE
 
 !include ArmVirtPkg/ArmVirt.dsc.inc
+!include NetworkPkg/Network.dsc.inc
 
 [LibraryClasses.common]
   ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
@@ -324,37 +329,6 @@ [Components.common]
       NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
   }
 
-  #
-  # Networking stack
-  #
-  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-  MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
-  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
-  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
-  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
-  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
-  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
-!if $(NETWORK_IP6_ENABLE) == TRUE
-  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
-  NetworkPkg/TcpDxe/TcpDxe.inf
-  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
-  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
-  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
-  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-  NetworkPkg/IScsiDxe/IScsiDxe.inf
-!else
-  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
-  MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
-  MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
-!endif
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  NetworkPkg/DnsDxe/DnsDxe.inf
-  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
-  NetworkPkg/HttpDxe/HttpDxe.inf
-  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
-!endif
-
   #
   # SCSI Bus and Disk Driver
   #
-- 
2.19.1.windows.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/6] EmulatorPkg: Update DSC/FDF to use NetworkPkg's include fragment file.
  2018-11-21  5:28 [PATCH 0/6] Add DSC/FDF include segment files for network stack Fu Siyuan
                   ` (2 preceding siblings ...)
  2018-11-21  5:28 ` [PATCH 3/6] ArmVirtPkg: " Fu Siyuan
@ 2018-11-21  5:28 ` Fu Siyuan
  2018-11-21  5:28 ` [PATCH 5/6] OvmfPkg: " Fu Siyuan
  2018-11-21  5:28 ` [PATCH 6/6] Vlv2TbltDevicePkg: " Fu Siyuan
  5 siblings, 0 replies; 13+ messages in thread
From: Fu Siyuan @ 2018-11-21  5:28 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jordan Justen, Andrew Fish, Ruiyu Ni

This patch updates the platform DSC/FDF files to use the include fragment
files provided by NetworkPkg.
The feature enabling flags in [Defines] section have been updated to use
the NetworkPkg's terms, and the value has been overridden with the original
default value on this platform.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
---
 EmulatorPkg/EmulatorPkg.dsc | 32 +++++++++-----------
 EmulatorPkg/EmulatorPkg.fdf | 10 +-----
 2 files changed, 16 insertions(+), 26 deletions(-)

diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
index 4097e1192ebc..3609bc06bb29 100644
--- a/EmulatorPkg/EmulatorPkg.dsc
+++ b/EmulatorPkg/EmulatorPkg.dsc
@@ -29,6 +29,21 @@ [Defines]
   SKUID_IDENTIFIER               = DEFAULT
   FLASH_DEFINITION               = EmulatorPkg/EmulatorPkg.fdf
 
+  #
+  # Network stack drivers
+  #
+  
+  #
+  # EmuSnpDxe.inf will be used.
+  #
+  DEFINE NETWORK_SNP_ENABLE       = FALSE
+  DEFINE NETWORK_IP6_ENABLE       = FALSE
+  DEFINE NETWORK_TLS_ENABLE       = FALSE
+  DEFINE NETWORK_HTTP_BOOT_ENABLE = FALSE
+  DEFINE NETWORK_IPSEC_ENABLE     = FALSE
+  DEFINE NETWORK_ISCSI_ENABLE     = FALSE
+!include NetworkPkg/Network.dsc.inc
+
 [SkuIds]
   0|DEFAULT
 
@@ -74,10 +89,6 @@ [LibraryClasses]
   # Generic Modules
   #
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
-  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
-  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
-  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
-  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
   OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
   FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
@@ -365,19 +376,6 @@ [Components]
 
   MdeModulePkg/Application/HelloWorld/HelloWorld.inf
 
-  #
-  # Network stack drivers
-  #
-  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-  MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
-  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
-  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
-  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
-  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
-  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
-  NetworkPkg/TcpDxe/TcpDxe.inf
-
   MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
   MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
   MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
diff --git a/EmulatorPkg/EmulatorPkg.fdf b/EmulatorPkg/EmulatorPkg.fdf
index 45697da25860..a8b46be66083 100644
--- a/EmulatorPkg/EmulatorPkg.fdf
+++ b/EmulatorPkg/EmulatorPkg.fdf
@@ -196,15 +196,7 @@ [FV.FvRecovery]
 !if $(NETWORK_SUPPORT)
 INF  EmulatorPkg/EmuSnpDxe/EmuSnpDxe.inf
 !endif
-INF  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-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/MnpDxe/MnpDxe.inf
-INF  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-INF  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
-INF  NetworkPkg/TcpDxe/TcpDxe.inf
-INF  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
+!include NetworkPkg/Network.fdf.inc
 
 INF FatPkg/EnhancedFatDxe/Fat.inf
 
-- 
2.19.1.windows.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/6] OvmfPkg: Update DSC/FDF to use NetworkPkg's include fragment file.
  2018-11-21  5:28 [PATCH 0/6] Add DSC/FDF include segment files for network stack Fu Siyuan
                   ` (3 preceding siblings ...)
  2018-11-21  5:28 ` [PATCH 4/6] EmulatorPkg: " Fu Siyuan
@ 2018-11-21  5:28 ` Fu Siyuan
  2018-11-21 11:07   ` Laszlo Ersek
  2018-11-21  5:28 ` [PATCH 6/6] Vlv2TbltDevicePkg: " Fu Siyuan
  5 siblings, 1 reply; 13+ messages in thread
From: Fu Siyuan @ 2018-11-21  5:28 UTC (permalink / raw)
  To: edk2-devel
  Cc: Jordan Justen, Laszlo Ersek, Ard Biesheuvel, Anthony Perard,
	Julien Grall

This patch updates the platform DSC/FDF files to use the include fragment
files provided by NetworkPkg.
The feature enabling flags in [Defines] section have been updated to use
the NetworkPkg's terms, and the value has been overridden with the original
default value on this platform.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien.grall@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
---
 OvmfPkg/OvmfPkgIa32.dsc    | 52 ++++---------------
 OvmfPkg/OvmfPkgIa32.fdf    | 25 +--------
 OvmfPkg/OvmfPkgIa32X64.dsc | 53 ++++----------------
 OvmfPkg/OvmfPkgIa32X64.fdf | 25 +--------
 OvmfPkg/OvmfPkgX64.dsc     | 52 ++++---------------
 OvmfPkg/OvmfPkgX64.fdf     | 25 +--------
 6 files changed, 36 insertions(+), 196 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index eccf34d3d1cb..5d6ea3e67001 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -35,12 +35,21 @@ [Defines]
   # -D FLAG=VALUE
   #
   DEFINE SECURE_BOOT_ENABLE      = FALSE
-  DEFINE NETWORK_IP6_ENABLE      = FALSE
-  DEFINE HTTP_BOOT_ENABLE        = FALSE
   DEFINE SMM_REQUIRE             = FALSE
   DEFINE TLS_ENABLE              = FALSE
   DEFINE TPM2_ENABLE             = FALSE
 
+  DEFINE NETWORK_IP6_ENABLE = FALSE
+  #
+  # TLS_ENABLE flag is used to control platform specific configuration for TLS support.
+  # NETWORK_TLS_ENABLE should always be set to FALSE.
+  #
+  DEFINE NETWORK_TLS_ENABLE = FALSE
+  DEFINE NETWORK_HTTP_BOOT_ENABLE       = FALSE
+  DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE
+  DEFINE NETWORK_IPSEC_ENABLE = FALSE
+!include NetworkPkg/Network.dsc.inc
+
   #
   # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to
   # one of the supported values, in place of any of the convenience macros, is
@@ -144,10 +153,6 @@ [LibraryClasses]
   FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
   UefiCpuLib|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
   SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
-  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
-  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
-  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
-  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
   UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
   SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
   QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
@@ -191,10 +196,6 @@ [LibraryClasses]
 
   TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
 
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
-!endif
-
 !if $(TLS_ENABLE) == TRUE
   TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
 !endif
@@ -504,10 +505,6 @@ [PcdsFixedAtBuild]
   gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
 !endif
 
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE
-!endif
-
   gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
 
 !if $(SMM_REQUIRE) == TRUE
@@ -774,33 +771,6 @@ [Components]
   MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
   MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 
-  #
-  # Network Support
-  #
-  MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
-  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
-  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-  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
-  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-  NetworkPkg/TcpDxe/TcpDxe.inf
-  NetworkPkg/IScsiDxe/IScsiDxe.inf
-!if $(NETWORK_IP6_ENABLE) == TRUE
-  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
-  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
-  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
-  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
-!endif
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  NetworkPkg/DnsDxe/DnsDxe.inf
-  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
-  NetworkPkg/HttpDxe/HttpDxe.inf
-  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
-!endif
 !if $(TLS_ENABLE) == TRUE
   NetworkPkg/TlsDxe/TlsDxe.inf
   NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf {
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index f7f9ab06bb5a..185b5a385ee3 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -296,30 +296,7 @@ [FV.DXEFV]
 #
 # Network modules
 #
-  INF  MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
-  INF  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-  INF  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
-  INF  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-  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/Mtftp4Dxe/Mtftp4Dxe.inf
-  INF  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
-  INF  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-  INF  NetworkPkg/TcpDxe/TcpDxe.inf
-  INF  NetworkPkg/IScsiDxe/IScsiDxe.inf
-!if $(NETWORK_IP6_ENABLE) == TRUE
-  INF  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
-  INF  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
-  INF  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
-  INF  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
-!endif
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  INF  NetworkPkg/DnsDxe/DnsDxe.inf
-  INF  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
-  INF  NetworkPkg/HttpDxe/HttpDxe.inf
-  INF  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
-!endif
+!include NetworkPkg/Network.fdf.inc
 !if $(TLS_ENABLE) == TRUE
   INF  NetworkPkg/TlsDxe/TlsDxe.inf
   INF  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 4ac4faf5dc18..2b2d68562d7d 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -35,12 +35,21 @@ [Defines]
   # -D FLAG=VALUE
   #
   DEFINE SECURE_BOOT_ENABLE      = FALSE
-  DEFINE NETWORK_IP6_ENABLE      = FALSE
-  DEFINE HTTP_BOOT_ENABLE        = FALSE
   DEFINE SMM_REQUIRE             = FALSE
   DEFINE TLS_ENABLE              = FALSE
   DEFINE TPM2_ENABLE             = FALSE
 
+  DEFINE NETWORK_IP6_ENABLE = FALSE
+  #
+  # TLS_ENABLE flag is used to control platform specific configuration for TLS support.
+  # NETWORK_TLS_ENABLE should always be set to FALSE.
+  #
+  DEFINE NETWORK_TLS_ENABLE = FALSE
+  DEFINE NETWORK_HTTP_BOOT_ENABLE       = FALSE
+  DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE
+  DEFINE NETWORK_IPSEC_ENABLE = FALSE
+!include NetworkPkg/Network.dsc.inc
+
   #
   # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to
   # one of the supported values, in place of any of the convenience macros, is
@@ -149,10 +158,6 @@ [LibraryClasses]
   FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
   UefiCpuLib|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
   SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
-  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
-  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
-  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
-  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
   UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
   SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
   QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
@@ -196,10 +201,6 @@ [LibraryClasses]
 
   TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
 
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
-!endif
-
 !if $(TLS_ENABLE) == TRUE
   TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
 !endif
@@ -509,11 +510,6 @@ [PcdsFixedAtBuild]
   gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
 !endif
 
-[PcdsFixedAtBuild.X64]
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE
-!endif
-
   gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
 
 !if $(SMM_REQUIRE) == TRUE
@@ -783,33 +779,6 @@ [Components.X64]
   MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
   MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 
-  #
-  # Network Support
-  #
-  MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
-  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
-  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-  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
-  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-  NetworkPkg/TcpDxe/TcpDxe.inf
-  NetworkPkg/IScsiDxe/IScsiDxe.inf
-!if $(NETWORK_IP6_ENABLE) == TRUE
-  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
-  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
-  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
-  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
-!endif
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  NetworkPkg/DnsDxe/DnsDxe.inf
-  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
-  NetworkPkg/HttpDxe/HttpDxe.inf
-  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
-!endif
 !if $(TLS_ENABLE) == TRUE
   NetworkPkg/TlsDxe/TlsDxe.inf
   NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf {
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 067ea1993eaa..078cf82a3183 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -302,30 +302,7 @@ [FV.DXEFV]
     SECTION PE32 = Intel3.5/EFIX64/E3522X2.EFI
   }
 !endif
-  INF  MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
-  INF  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-  INF  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
-  INF  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-  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/Mtftp4Dxe/Mtftp4Dxe.inf
-  INF  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
-  INF  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-  INF  NetworkPkg/TcpDxe/TcpDxe.inf
-  INF  NetworkPkg/IScsiDxe/IScsiDxe.inf
-!if $(NETWORK_IP6_ENABLE) == TRUE
-  INF  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
-  INF  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
-  INF  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
-  INF  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
-!endif
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  INF  NetworkPkg/DnsDxe/DnsDxe.inf
-  INF  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
-  INF  NetworkPkg/HttpDxe/HttpDxe.inf
-  INF  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
-!endif
+!include NetworkPkg/Network.fdf.inc
 !if $(TLS_ENABLE) == TRUE
   INF  NetworkPkg/TlsDxe/TlsDxe.inf
   INF  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index ecd5db416c47..7fb1f7fdd233 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -35,12 +35,21 @@ [Defines]
   # -D FLAG=VALUE
   #
   DEFINE SECURE_BOOT_ENABLE      = FALSE
-  DEFINE NETWORK_IP6_ENABLE      = FALSE
-  DEFINE HTTP_BOOT_ENABLE        = FALSE
   DEFINE SMM_REQUIRE             = FALSE
   DEFINE TLS_ENABLE              = FALSE
   DEFINE TPM2_ENABLE             = FALSE
 
+  DEFINE NETWORK_IP6_ENABLE = FALSE
+  #
+  # TLS_ENABLE flag is used to control platform specific configuration for TLS support.
+  # NETWORK_TLS_ENABLE should always be set to FALSE.
+  #
+  DEFINE NETWORK_TLS_ENABLE = FALSE
+  DEFINE NETWORK_HTTP_BOOT_ENABLE       = FALSE
+  DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE
+  DEFINE NETWORK_IPSEC_ENABLE = FALSE
+!include NetworkPkg/Network.dsc.inc
+
   #
   # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to
   # one of the supported values, in place of any of the convenience macros, is
@@ -149,10 +158,6 @@ [LibraryClasses]
   FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
   UefiCpuLib|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
   SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
-  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
-  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
-  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
-  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
   UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
   SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
   QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
@@ -196,10 +201,6 @@ [LibraryClasses]
 
   TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
 
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
-!endif
-
 !if $(TLS_ENABLE) == TRUE
   TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
 !endif
@@ -509,10 +510,6 @@ [PcdsFixedAtBuild]
   gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
 !endif
 
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE
-!endif
-
   gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 0x7C, 0x3E, 0x9E, 0x1C, 0x4F, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
 
 !if $(SMM_REQUIRE) == TRUE
@@ -781,33 +778,6 @@ [Components]
   MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
   MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 
-  #
-  # Network Support
-  #
-  MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
-  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
-  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-  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
-  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-  NetworkPkg/TcpDxe/TcpDxe.inf
-  NetworkPkg/IScsiDxe/IScsiDxe.inf
-!if $(NETWORK_IP6_ENABLE) == TRUE
-  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
-  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
-  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
-  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
-!endif
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  NetworkPkg/DnsDxe/DnsDxe.inf
-  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
-  NetworkPkg/HttpDxe/HttpDxe.inf
-  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
-!endif
 !if $(TLS_ENABLE) == TRUE
   NetworkPkg/TlsDxe/TlsDxe.inf
   NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf {
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 067ea1993eaa..078cf82a3183 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -302,30 +302,7 @@ [FV.DXEFV]
     SECTION PE32 = Intel3.5/EFIX64/E3522X2.EFI
   }
 !endif
-  INF  MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
-  INF  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-  INF  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
-  INF  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-  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/Mtftp4Dxe/Mtftp4Dxe.inf
-  INF  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
-  INF  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-  INF  NetworkPkg/TcpDxe/TcpDxe.inf
-  INF  NetworkPkg/IScsiDxe/IScsiDxe.inf
-!if $(NETWORK_IP6_ENABLE) == TRUE
-  INF  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
-  INF  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
-  INF  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
-  INF  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
-!endif
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  INF  NetworkPkg/DnsDxe/DnsDxe.inf
-  INF  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
-  INF  NetworkPkg/HttpDxe/HttpDxe.inf
-  INF  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
-!endif
+!include NetworkPkg/Network.fdf.inc
 !if $(TLS_ENABLE) == TRUE
   INF  NetworkPkg/TlsDxe/TlsDxe.inf
   INF  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
-- 
2.19.1.windows.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 6/6] Vlv2TbltDevicePkg: Update DSC/FDF to use NetworkPkg's include fragment file.
  2018-11-21  5:28 [PATCH 0/6] Add DSC/FDF include segment files for network stack Fu Siyuan
                   ` (4 preceding siblings ...)
  2018-11-21  5:28 ` [PATCH 5/6] OvmfPkg: " Fu Siyuan
@ 2018-11-21  5:28 ` Fu Siyuan
  5 siblings, 0 replies; 13+ messages in thread
From: Fu Siyuan @ 2018-11-21  5:28 UTC (permalink / raw)
  To: edk2-devel; +Cc: David Wei, Mang Guo

This patch updates the platform DSC/FDF files to use the include fragment
files provided by NetworkPkg.
The feature enabling flags in [Defines] section have been updated to use
the NetworkPkg's terms, and the value has been overridden with the original
default value on this platform.

Cc: David Wei <david.wei@intel.com>
Cc: Mang Guo <mang.guo@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
---
 Vlv2TbltDevicePkg/PlatformPkg.fdf       | 25 +-----------
 Vlv2TbltDevicePkg/PlatformPkgConfig.dsc |  3 ++
 Vlv2TbltDevicePkg/PlatformPkgGcc.fdf    | 25 +-----------
 Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 42 +++-----------------
 Vlv2TbltDevicePkg/PlatformPkgIA32.dsc   | 42 +++-----------------
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc    | 42 +++-----------------
 6 files changed, 20 insertions(+), 159 deletions(-)

diff --git a/Vlv2TbltDevicePkg/PlatformPkg.fdf b/Vlv2TbltDevicePkg/PlatformPkg.fdf
index f976b9fa4057..e281d1407fc2 100644
--- a/Vlv2TbltDevicePkg/PlatformPkg.fdf
+++ b/Vlv2TbltDevicePkg/PlatformPkg.fdf
@@ -728,35 +728,12 @@ [FV.FVMAIN]
 #
 # Network Modules
 #
+!include NetworkPkg/Network.fdf.inc
 !if $(NETWORK_ENABLE) == TRUE
   FILE DRIVER = 22DE1691-D65D-456a-993E-A253DD1F308C {
     SECTION PE32 = Vlv2MiscBinariesPkg/UNDI/RtkUndiDxe/$(DXE_ARCHITECTURE)/RtkUndiDxe.efi
     SECTION UI = "UNDI"
   }
-  INF  MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
-  INF  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-  INF  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
-  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
-  INF  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-  INF  NetworkPkg/TcpDxe/TcpDxe.inf
-  INF  NetworkPkg/IScsiDxe/IScsiDxe.inf
-  !if $(NETWORK_IP6_ENABLE) == TRUE
-  INF  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
-  INF  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
-  INF  NetworkPkg/IpSecDxe/IpSecDxe.inf
-  INF  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
-  INF  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.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
 
 !if $(CAPSULE_ENABLE)
diff --git a/Vlv2TbltDevicePkg/PlatformPkgConfig.dsc b/Vlv2TbltDevicePkg/PlatformPkgConfig.dsc
index 672853dda6a6..63a5b00b43bd 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgConfig.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgConfig.dsc
@@ -85,6 +85,9 @@
 DEFINE NETWORK_IP6_ENABLE = TRUE
 DEFINE NETWORK_ISCSI_ENABLE = FALSE
 DEFINE NETWORK_VLAN_ENABLE = FALSE
+DEFINE NETWORK_TLS_ENABLE = FALSE
+DEFINE NETWORK_HTTP_BOOT_ENABLE       = FALSE
+DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE
 
 DEFINE SATA_ENABLE       = TRUE
 DEFINE PCIESC_ENABLE     = TRUE
diff --git a/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf b/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
index 50032f7d5f19..6ea0ec92d4ae 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
+++ b/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
@@ -684,35 +684,12 @@ [FV.FVMAIN]
 #
 # Network Modules
 #
+!include NetworkPkg/Network.fdf.inc
 !if $(NETWORK_ENABLE) == TRUE
   FILE DRIVER = 22DE1691-D65D-456a-993E-A253DD1F308C {
     SECTION PE32 = Vlv2MiscBinariesPkg/UNDI/RtkUndiDxe/$(DXE_ARCHITECTURE)/RtkUndiDxe.efi
     SECTION UI = "UNDI"
   }
-  INF  MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
-  INF  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-  INF  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
-  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
-  INF  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-  INF  NetworkPkg/TcpDxe/TcpDxe.inf
-  INF  NetworkPkg/IScsiDxe/IScsiDxe.inf
-  !if $(NETWORK_IP6_ENABLE) == TRUE
-  INF  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
-  INF  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
-  INF  NetworkPkg/IpSecDxe/IpSecDxe.inf
-  INF  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
-  INF  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.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
 
 !if $(CAPSULE_ENABLE)
diff --git a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
index f8ad29df5986..219ddd1646e5 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
@@ -56,6 +56,11 @@ [Defines]
   !include $(PLATFORM_PACKAGE)/AutoPlatformCFG.txt
   !include $(PLATFORM_PACKAGE)/PlatformPkgConfig.dsc
 
+  #
+  # Network Stacks
+  #
+  !include NetworkPkg/Network.dsc.inc
+  
 !if $(X64_CONFIG) == TRUE
   DEFINE      DXE_ARCHITECTURE        = X64
   DEFINE      EDK_DXE_ARCHITECTURE    = X64
@@ -173,13 +178,6 @@ [LibraryClasses.common]
 !if $(SCSI_ENABLE) == TRUE
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
 !endif
-!if $(NETWORK_ENABLE) == TRUE
-  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
-  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
-  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
-  TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
-  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
-!endif
 !if $(S3_ENABLE) == TRUE
   S3Lib|IntelFrameworkModulePkg/Library/PeiS3Lib/PeiS3Lib.inf
 !endif
@@ -1528,41 +1526,11 @@ [Components.X64]
 
 
 !if $(NETWORK_ENABLE) == TRUE
-  !if $(NETWORK_ISCSI_ENABLE) == TRUE
-    NetworkPkg/IScsiDxe/IScsiDxe.inf
-  !endif
-  !if $(NETWORK_VLAN_ENABLE) == TRUE
-    MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-  !endif
   !if $(CSM_ENABLE) == TRUE
     IntelFrameworkModulePkg/Csm/BiosThunk/Snp16Dxe/Snp16Dxe.inf
   !endif
 !endif
 
-!if $(NETWORK_ENABLE) == TRUE
-  #
-  # UEFI network modules
-  #
-    MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-    MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
-
-    MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
-    MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
-    MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
-    MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
-    MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
-    NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-    NetworkPkg/TcpDxe/TcpDxe.inf
-    MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
-    !if $(NETWORK_IP6_ENABLE) == TRUE
-      NetworkPkg/Ip6Dxe/Ip6Dxe.inf
-      NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
-      NetworkPkg/IpSecDxe/IpSecDxe.inf
-      NetworkPkg/Udp6Dxe/Udp6Dxe.inf
-      NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
-    !endif
-!endif
-
 !if $(CAPSULE_ENABLE) || $(MICOCODE_CAPSULE_ENABLE)
   MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmpDxe.inf
   MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
diff --git a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
index ca3b2ff90287..715a51a2ec08 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
@@ -56,6 +56,11 @@ [Defines]
   !include $(PLATFORM_PACKAGE)/AutoPlatformCFG.txt
   !include $(PLATFORM_PACKAGE)/PlatformPkgConfig.dsc
 
+  #
+  # Network Stacks
+  #
+  !include NetworkPkg/Network.dsc.inc
+  
 !if $(X64_CONFIG) == TRUE
   DEFINE      DXE_ARCHITECTURE        = X64
   DEFINE      EDK_DXE_ARCHITECTURE    = X64
@@ -173,13 +178,6 @@ [LibraryClasses.common]
 !if $(SCSI_ENABLE) == TRUE
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
 !endif
-!if $(NETWORK_ENABLE) == TRUE
-  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
-  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
-  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
-  TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
-  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
-!endif
 !if $(S3_ENABLE) == TRUE
   S3Lib|IntelFrameworkModulePkg/Library/PeiS3Lib/PeiS3Lib.inf
 !endif
@@ -1516,41 +1514,11 @@ [Components.IA32]
 
 
 !if $(NETWORK_ENABLE) == TRUE
-  !if $(NETWORK_ISCSI_ENABLE) == TRUE
-    NetworkPkg/IScsiDxe/IScsiDxe.inf
-  !endif
-  !if $(NETWORK_VLAN_ENABLE) == TRUE
-    MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-  !endif
   !if $(CSM_ENABLE) == TRUE
     IntelFrameworkModulePkg/Csm/BiosThunk/Snp16Dxe/Snp16Dxe.inf
   !endif
 !endif
 
-!if $(NETWORK_ENABLE) == TRUE
-  #
-  # UEFI network modules
-  #
-    MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-    MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
-
-    MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
-    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
-    NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-    NetworkPkg/TcpDxe/TcpDxe.inf
-    !if $(NETWORK_IP6_ENABLE) == TRUE
-      NetworkPkg/Ip6Dxe/Ip6Dxe.inf
-      NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
-      NetworkPkg/IpSecDxe/IpSecDxe.inf
-      NetworkPkg/Udp6Dxe/Udp6Dxe.inf
-      NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
-    !endif
-!endif
-
 !if $(CAPSULE_ENABLE) || $(MICOCODE_CAPSULE_ENABLE)
   MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmpDxe.inf
   MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
diff --git a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
index 81f36bd73b28..0393e2ace3aa 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
@@ -56,6 +56,11 @@ [Defines]
   !include $(PLATFORM_PACKAGE)/AutoPlatformCFG.txt
   !include $(PLATFORM_PACKAGE)/PlatformPkgConfig.dsc
 
+  #
+  # Network Stacks
+  #
+  !include NetworkPkg/Network.dsc.inc
+  
 !if $(X64_CONFIG) == TRUE
   DEFINE      DXE_ARCHITECTURE        = X64
   DEFINE      EDK_DXE_ARCHITECTURE    = X64
@@ -173,13 +178,6 @@ [LibraryClasses.common]
 !if $(SCSI_ENABLE) == TRUE
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
 !endif
-!if $(NETWORK_ENABLE) == TRUE
-  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
-  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
-  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
-  TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
-  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
-!endif
 !if $(S3_ENABLE) == TRUE
   S3Lib|IntelFrameworkModulePkg/Library/PeiS3Lib/PeiS3Lib.inf
 !endif
@@ -1528,41 +1526,11 @@ [Components.X64]
 
 
 !if $(NETWORK_ENABLE) == TRUE
-  !if $(NETWORK_ISCSI_ENABLE) == TRUE
-    NetworkPkg/IScsiDxe/IScsiDxe.inf
-  !endif
-  !if $(NETWORK_VLAN_ENABLE) == TRUE
-    MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
-  !endif
   !if $(CSM_ENABLE) == TRUE
     IntelFrameworkModulePkg/Csm/BiosThunk/Snp16Dxe/Snp16Dxe.inf
   !endif
 !endif
 
-!if $(NETWORK_ENABLE) == TRUE
-  #
-  # UEFI network modules
-  #
-    MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
-    MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
-
-    MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
-    MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
-    MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
-    MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
-    MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
-    NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
-    NetworkPkg/TcpDxe/TcpDxe.inf
-    MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
-    !if $(NETWORK_IP6_ENABLE) == TRUE
-      NetworkPkg/Ip6Dxe/Ip6Dxe.inf
-      NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
-      NetworkPkg/IpSecDxe/IpSecDxe.inf
-      NetworkPkg/Udp6Dxe/Udp6Dxe.inf
-      NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
-    !endif
-!endif
-
 !if $(CAPSULE_ENABLE) || $(MICOCODE_CAPSULE_ENABLE)
   MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmpDxe.inf
   MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
-- 
2.19.1.windows.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/6] NetworkPkg: Add DSC/FDF include segment files to NetworkPkg.
  2018-11-21  5:28 ` [PATCH 1/6] NetworkPkg: Add DSC/FDF include segment files to NetworkPkg Fu Siyuan
@ 2018-11-21 10:46   ` Laszlo Ersek
  2018-11-21 10:56     ` Laszlo Ersek
  2018-11-21 11:53     ` Fu, Siyuan
  0 siblings, 2 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-11-21 10:46 UTC (permalink / raw)
  To: Fu Siyuan; +Cc: edk2-devel, Ting Ye, Jiaxin Wu

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.

> 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.

> +  #
> +  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.

> +  # 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?

> +
> +[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.

> +
> +[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?

> +!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

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

> +
> +  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.

> +
> +  !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.

> +#
> +# 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
> 

Thanks,
Laszlo


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/6] NetworkPkg: Add DSC/FDF include segment files to NetworkPkg.
  2018-11-21 10:46   ` Laszlo Ersek
@ 2018-11-21 10:56     ` Laszlo Ersek
  2018-11-21 11:53     ` Fu, Siyuan
  1 sibling, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-11-21 10:56 UTC (permalink / raw)
  To: Fu Siyuan; +Cc: edk2-devel, Ting Ye, Jiaxin Wu

On 11/21/18 11:46, Laszlo Ersek wrote:
> 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
> 
> [...]

I'd like to review the ArmVirtPkg and OvmfPkg patches in detail once the
NetworkPkg patch stabilizes.

In addition, I have some higher-level comments. This patch series
effectively renames a few preexistent build flags, such as
"HTTP_BOOT_ENABLE" to "NETWORK_HTTP_BOOT_ENABLE", "TLS_ENABLE" to
"NETWORK_TLS_ENABLE", and so on.

- Please grep the edk2 source tree for the original (disappearing)
flags, and update them as necessary, not just in DSC/FDF files, but also
in documentation. For example, "HTTP_BOOT_ENABLE" and "TLS_ENABLE" are
mentioned in "OvmfPkg/README".

- Please also grep the edk2 Wiki for the same (clone it locally and then
run "git grep"). For example, the
"Testing-SMM-with-QEMU,-KVM-and-libvirt.md" article also refers to
HTTP_BOOT_ENABLE and TLS_ENABLE.

- It is likely a good idea to list this change (that is,
<https://bugzilla.tianocore.org/show_bug.cgi?id=1293>), under
<https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning#proposed-features-1>.
I see that #1278 is listed already, but the build flag renaming is an
incompatible change, and it deserves a mention.

Thanks!
Laszlo



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 5/6] OvmfPkg: Update DSC/FDF to use NetworkPkg's include fragment file.
  2018-11-21  5:28 ` [PATCH 5/6] OvmfPkg: " Fu Siyuan
@ 2018-11-21 11:07   ` Laszlo Ersek
  0 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-11-21 11:07 UTC (permalink / raw)
  To: Fu Siyuan, edk2-devel; +Cc: Anthony Perard, Jordan Justen

As I said, I wouldn't like to review this patch in detail right now.
Just some light comments:

On 11/21/18 06:28, Fu Siyuan wrote:
> This patch updates the platform DSC/FDF files to use the include fragment
> files provided by NetworkPkg.
> The feature enabling flags in [Defines] section have been updated to use
> the NetworkPkg's terms, and the value has been overridden with the original
> default value on this platform.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien.grall@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc    | 52 ++++---------------
>  OvmfPkg/OvmfPkgIa32.fdf    | 25 +--------
>  OvmfPkg/OvmfPkgIa32X64.dsc | 53 ++++----------------
>  OvmfPkg/OvmfPkgIa32X64.fdf | 25 +--------
>  OvmfPkg/OvmfPkgX64.dsc     | 52 ++++---------------
>  OvmfPkg/OvmfPkgX64.fdf     | 25 +--------
>  6 files changed, 36 insertions(+), 196 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index eccf34d3d1cb..5d6ea3e67001 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -35,12 +35,21 @@ [Defines]
>    # -D FLAG=VALUE
>    #
>    DEFINE SECURE_BOOT_ENABLE      = FALSE
> -  DEFINE NETWORK_IP6_ENABLE      = FALSE
> -  DEFINE HTTP_BOOT_ENABLE        = FALSE
>    DEFINE SMM_REQUIRE             = FALSE
>    DEFINE TLS_ENABLE              = FALSE
>    DEFINE TPM2_ENABLE             = FALSE
>  
> +  DEFINE NETWORK_IP6_ENABLE = FALSE
> +  #
> +  # TLS_ENABLE flag is used to control platform specific configuration for TLS support.
> +  # NETWORK_TLS_ENABLE should always be set to FALSE.
> +  #
> +  DEFINE NETWORK_TLS_ENABLE = FALSE

(1) Ah, OK, I understand, so basically the suggestion is that OVMF not
make use of NETWORK_TLS_ENABLE, but continue using its own TLS_ENABLE
solution.

Hmmm. I wonder if that's helpful at all. To me it seems to increase the
confusion rather than decrease it.

I guess it can work, but then we should rename TLS_ENABLE to something
better, such as "PLATFORM_TLS_ENABLE". And this comment should be more
detailed *why* we do that. (We do that because we configure the CA
certificates and the cipher suites with a null class lib instance hooked
into TlsAuthConfigDxe, which downloads the necessary data from QEMU via
fw_cfg.)

> +  DEFINE NETWORK_HTTP_BOOT_ENABLE       = FALSE
> +  DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE

(2) This (i.e. NETWORK_ALLOW_HTTP_CONNECTIONS=FALSE) is wrong. We set
PcdAllowHttpConnections to TRUE on purpose. See commit 4b2fb7986d57
("OvmfPkg: Allow HTTP connections if HTTP Boot enabled", 2017-01-23).

More after you post v2, I think.

Thanks!
Laszlo


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/6] ArmVirtPkg: Update DSC/FDF to use NetworkPkg's include fragment file.
  2018-11-21  5:28 ` [PATCH 3/6] ArmVirtPkg: " Fu Siyuan
@ 2018-11-21 11:26   ` Laszlo Ersek
  0 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-11-21 11:26 UTC (permalink / raw)
  To: Fu Siyuan, edk2-devel

some light comments for now:

On 11/21/18 06:28, Fu Siyuan wrote:
> This patch updates the platform DSC/FDF files to use the include fragment
> files provided by NetworkPkg.
> The feature enabling flags in [Defines] section have been updated to use
> the NetworkPkg's terms, and the value has been overridden with the original
> default value on this platform.
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Julien Grall <julien.grall@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> ---
>  ArmVirtPkg/ArmVirt.dsc.inc           | 12 ------
>  ArmVirtPkg/ArmVirtQemu.dsc           | 44 ++++----------------
>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 28 +------------
>  ArmVirtPkg/ArmVirtQemuKernel.dsc     | 38 +++--------------
>  4 files changed, 14 insertions(+), 108 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 70a0ac4d786c..24a064198b5e 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -75,18 +75,6 @@ [LibraryClasses.common]
>    # use the accelerated BaseMemoryLibOptDxe by default, overrides for SEC/PEI below
>    BaseMemoryLib|MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
>  
> -  # Networking Requirements
> -  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
> -  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
> -  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
> -  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
> -!if $(NETWORK_IP6_ENABLE) == TRUE
> -  TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
> -!endif
> -!if $(HTTP_BOOT_ENABLE) == TRUE
> -  HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
> -!endif
> -
>    #
>    # It is not possible to prevent the ARM compiler from inserting calls to intrinsic functions.
>    # This library provides the instrinsic functions such a compiler may generate calls to.
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 885c6b14b844..5a1b6b0f0519 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -35,9 +35,14 @@ [Defines]
>    #
>    DEFINE SECURE_BOOT_ENABLE      = FALSE
>    DEFINE NETWORK_IP6_ENABLE      = FALSE
> -  DEFINE HTTP_BOOT_ENABLE        = FALSE
> -
> +  DEFINE NETWORK_HTTP_BOOT_ENABLE= FALSE
> +  DEFINE NETWORK_SNP_ENABLE      = FALSE
> +  DEFINE NETWORK_TLS_ENABLE      = FALSE
> +  DEFINE NETWORK_IPSEC_ENABLE    = FALSE
> +  DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS  = TRUE
> +  

(1) Please keep these nicely aligned (I should have mentioned the same
for OvmfPkg too).

>  !include ArmVirtPkg/ArmVirt.dsc.inc
> +!include NetworkPkg/Network.dsc.inc
>  
>  [LibraryClasses.common]
>    ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> @@ -123,10 +128,6 @@ [PcdsFixedAtBuild.common]
>    #
>    gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|0
>  
> -!if $(HTTP_BOOT_ENABLE) == TRUE
> -  gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE
> -!endif
> -
>    # System Memory Base -- fixed at 0x4000_0000
>    gArmTokenSpaceGuid.PcdSystemMemoryBase|0x40000000
>  
> @@ -335,37 +336,6 @@ [Components.common]
>        NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
>    }
>  
> -  #
> -  # Networking stack
> -  #
> -  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
> -  MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
> -  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
> -  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
> -  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
> -  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
> -  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> -  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
> -!if $(NETWORK_IP6_ENABLE) == TRUE
> -  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
> -  NetworkPkg/TcpDxe/TcpDxe.inf
> -  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> -  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> -  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> -  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> -  NetworkPkg/IScsiDxe/IScsiDxe.inf
> -!else
> -  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> -  MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> -  MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> -!endif
> -!if $(HTTP_BOOT_ENABLE) == TRUE
> -  NetworkPkg/DnsDxe/DnsDxe.inf
> -  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> -  NetworkPkg/HttpDxe/HttpDxe.inf
> -  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> -!endif
> -
>    #
>    # SCSI Bus and Disk Driver
>    #
> diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> index a6390bd4b841..560651e49e0b 100644
> --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> @@ -118,33 +118,7 @@ [FV.FvMain]
>    #
>    # Networking stack
>    #
> -  INF MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
> -  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/MnpDxe/MnpDxe.inf
> -  INF MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
> -  INF MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> -  INF MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
> -!if $(NETWORK_IP6_ENABLE) == TRUE
> -  INF NetworkPkg/Ip6Dxe/Ip6Dxe.inf
> -  INF NetworkPkg/TcpDxe/TcpDxe.inf
> -  INF NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> -  INF NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> -  INF NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> -  INF NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> -  INF NetworkPkg/IScsiDxe/IScsiDxe.inf
> -!else
> -  INF MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> -  INF MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> -  INF MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> -!endif
> -!if $(HTTP_BOOT_ENABLE) == TRUE
> -  INF NetworkPkg/DnsDxe/DnsDxe.inf
> -  INF NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> -  INF NetworkPkg/HttpDxe/HttpDxe.inf
> -  INF NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> -!endif
> +!include NetworkPkg/Network.fdf.inc
>  
>    #
>    # SCSI Bus and Disk Driver
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index 434d6861a56f..a5c3e957ac82 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -35,9 +35,14 @@ [Defines]
>    #
>    DEFINE SECURE_BOOT_ENABLE      = FALSE
>    DEFINE NETWORK_IP6_ENABLE      = FALSE
> -  DEFINE HTTP_BOOT_ENABLE        = FALSE
> +  DEFINE NETWORK_HTTP_BOOT_ENABLE        = FALSE
> +  DEFINE NETWORK_SNP_ENABLE      = FALSE
> +  DEFINE NETWORK_TLS_ENABLE              = FALSE
> +  DEFINE NETWORK_IPSEC_ENABLE    = FALSE
> +  DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS  = TRUE

(2) Same as (1).

>  
>  !include ArmVirtPkg/ArmVirt.dsc.inc
> +!include NetworkPkg/Network.dsc.inc
>  
>  [LibraryClasses.common]
>    ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> @@ -324,37 +329,6 @@ [Components.common]
>        NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
>    }
>  
> -  #
> -  # Networking stack
> -  #
> -  MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
> -  MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
> -  MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
> -  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
> -  MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
> -  MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
> -  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
> -  MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
> -!if $(NETWORK_IP6_ENABLE) == TRUE
> -  NetworkPkg/Ip6Dxe/Ip6Dxe.inf
> -  NetworkPkg/TcpDxe/TcpDxe.inf
> -  NetworkPkg/Udp6Dxe/Udp6Dxe.inf
> -  NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
> -  NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
> -  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> -  NetworkPkg/IScsiDxe/IScsiDxe.inf
> -!else
> -  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
> -  MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
> -  MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> -!endif
> -!if $(HTTP_BOOT_ENABLE) == TRUE
> -  NetworkPkg/DnsDxe/DnsDxe.inf
> -  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
> -  NetworkPkg/HttpDxe/HttpDxe.inf
> -  NetworkPkg/HttpBootDxe/HttpBootDxe.inf
> -!endif
> -
>    #
>    # SCSI Bus and Disk Driver
>    #
> 

(3) Whatever we end up doing about "PcdAllowHttpConnections", it should
be consistent between "ArmVirtQemu.dsc" and "ArmVirtQemuKernel.dsc". The
patch curently removes PcdAllowHttpConnections only from the former,
however.

Thanks
Laszlo


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/6] NetworkPkg: Add DSC/FDF include segment files to NetworkPkg.
  2018-11-21 10:46   ` Laszlo Ersek
  2018-11-21 10:56     ` Laszlo Ersek
@ 2018-11-21 11:53     ` Fu, Siyuan
  2018-11-21 15:32       ` Laszlo Ersek
  1 sibling, 1 reply; 13+ messages in thread
From: Fu, Siyuan @ 2018-11-21 11:53 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: edk2-devel@lists.01.org, Ye, Ting, Wu, Jiaxin

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/6] NetworkPkg: Add DSC/FDF include segment files to NetworkPkg.
  2018-11-21 11:53     ` Fu, Siyuan
@ 2018-11-21 15:32       ` Laszlo Ersek
  0 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2018-11-21 15:32 UTC (permalink / raw)
  To: Fu, Siyuan; +Cc: edk2-devel@lists.01.org, Ye, Ting, Wu, Jiaxin

On 11/21/18 12:53, Fu, Siyuan wrote:
>> -----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.

[...]

>>> +[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).

Right, this would restore flexibility to the PCD settings as well.

[...]

>>> +  !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.

(See also my OvmfPkg patch comments:)

I think this method can work well; the only thing we should be careful
about IMO is that the platform-specific flag should really be clear
about it being platform specific. Hence my earlier suggestion to rename
TLS_ENABLE in OVMF to PLATFORM_TLS_ENABLE.

Because, just "TLS_ENABLE" is a little bit ambiguous (to me anyway) in
whether it utilizes a pre-packaged core feature, or a platform-specific
inclusion of the feature.

Thank you!
Laszlo


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-11-21 15:41 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-21  5:28 [PATCH 0/6] Add DSC/FDF include segment files for network stack Fu Siyuan
2018-11-21  5:28 ` [PATCH 1/6] NetworkPkg: Add DSC/FDF include segment files to NetworkPkg Fu Siyuan
2018-11-21 10:46   ` Laszlo Ersek
2018-11-21 10:56     ` Laszlo Ersek
2018-11-21 11:53     ` Fu, Siyuan
2018-11-21 15:32       ` Laszlo Ersek
2018-11-21  5:28 ` [PATCH 2/6] Nt32Pkg: Update DSC/FDF to use NetworkPkg's include fragment file Fu Siyuan
2018-11-21  5:28 ` [PATCH 3/6] ArmVirtPkg: " Fu Siyuan
2018-11-21 11:26   ` Laszlo Ersek
2018-11-21  5:28 ` [PATCH 4/6] EmulatorPkg: " Fu Siyuan
2018-11-21  5:28 ` [PATCH 5/6] OvmfPkg: " Fu Siyuan
2018-11-21 11:07   ` Laszlo Ersek
2018-11-21  5:28 ` [PATCH 6/6] Vlv2TbltDevicePkg: " Fu Siyuan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox