From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Tue, 07 May 2019 11:27:48 -0700 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EDF023079B6C; Tue, 7 May 2019 18:27:47 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-152.rdu2.redhat.com [10.10.120.152]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3B48160BF4; Tue, 7 May 2019 18:27:46 +0000 (UTC) Subject: Re: [edk2-devel] [Patch v5 2/3] NetworkPkg: Add DSC/FDF include segment files to NetworkPkg. To: devel@edk2.groups.io, liming.gao@intel.com Cc: Jiaxin Wu , Ting Ye , Fu Siyuan References: <20190507142705.20092-1-liming.gao@intel.com> <20190507142705.20092-3-liming.gao@intel.com> From: "Laszlo Ersek" Message-ID: <5cc12fc5-451a-2924-5179-73c1f192681a@redhat.com> Date: Tue, 7 May 2019 20:27:45 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190507142705.20092-3-liming.gao@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.41]); Tue, 07 May 2019 18:27:48 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 05/07/19 16:27, Liming Gao wrote: > This patch provides a set of include segment files for platform owner to > easily enable/disable network stack support on their platform. > > For DSC, there are: > - a "NetworkDefines.dsc.inc" for the [Defines] section(s), > - a "NetworkLibs.dsc.inc" for the [LibraryClasses*] section(s), > - a "NetworkPcds.dsc.inc" for the [Pcds*] section(s), > - a "NetworkComponents.dsc.inc" for the [Components*] section(s). > For FDF, there is: > - a "Network.fdf.inc" for the [Fv*] section(s). > > These files can be added to the platform DSC/FDF file by using > !include NetworkPkg/xxx > where "xxx" is the *.inc file name. > > A platform DSC file can diverge from the defaults in > "NetworkDefines.dsc.inc" by setting the individual DEFINEs before > including "NetworkDefines.dsc.inc". > And, build command line ("-D FLAG=VALUE") can be used to enable or > disable related feature set, please check "NetworkDefines.dsc.inc" > for a detail description of each flag. > > The default value of these flags are: > DEFINE NETWORK_ENABLE = TRUE > DEFINE NETWORK_SNP_ENABLE = TRUE > DEFINE NETWORK_IP4_ENABLE = TRUE > DEFINE NETWORK_IP6_ENABLE = TRUE > DEFINE NETWORK_TLS_ENABLE = TRUE > DEFINE NETWORK_HTTP_BOOT_ENABLE = TRUE > DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE > DEFINE NETWORK_ISCSI_ENABLE = TRUE > DEFINE NETWORK_VLAN_ENABLE = TRUE > > Related BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1293 > > Cc: Jiaxin Wu > Cc: Ting Ye > Signed-off-by: Fu Siyuan > --- > NetworkPkg/Network.fdf.inc | 60 ++++++++++++++++++ > NetworkPkg/NetworkComponents.dsc.inc | 61 ++++++++++++++++++ > NetworkPkg/NetworkDefines.dsc.inc | 118 +++++++++++++++++++++++++++++++++++ > NetworkPkg/NetworkLibs.dsc.inc | 20 ++++++ > NetworkPkg/NetworkPcds.dsc.inc | 16 +++++ > 5 files changed, 275 insertions(+) > create mode 100644 NetworkPkg/Network.fdf.inc > create mode 100644 NetworkPkg/NetworkComponents.dsc.inc > create mode 100644 NetworkPkg/NetworkDefines.dsc.inc > create mode 100644 NetworkPkg/NetworkLibs.dsc.inc > create mode 100644 NetworkPkg/NetworkPcds.dsc.inc > > diff --git a/NetworkPkg/Network.fdf.inc b/NetworkPkg/Network.fdf.inc > new file mode 100644 > index 0000000000..92e2c56cb8 > --- /dev/null > +++ b/NetworkPkg/Network.fdf.inc > @@ -0,0 +1,60 @@ > +## @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 EDKII network stack drivers > +# according to the value of flags described in "NetworkPkg/Network.dsc.inc". This addresses my comment (4) from the v3 review, thanks. > +# > +# Copyright (c) 2019, Intel Corporation. All rights reserved.
Copyright year updated (5), thanks. > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > + > +!if $(NETWORK_ENABLE) == TRUE > + INF MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf > + > + !if $(NETWORK_SNP_ENABLE) == TRUE > + INF MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf > + !endif > + > + !if $(NETWORK_VLAN_ENABLE) == TRUE > + INF MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf > + !endif > + > + INF MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf > + > + !if $(NETWORK_IP4_ENABLE) == TRUE > + INF MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf I now understand that ArpDxe is IPv4 only. Answers my question (7). > + INF MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf > + INF MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf > + INF MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf > + INF MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf > + !endif > + > + !if $(NETWORK_IP6_ENABLE) == TRUE > + INF NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf > + INF NetworkPkg/Ip6Dxe/Ip6Dxe.inf > + INF NetworkPkg/Udp6Dxe/Udp6Dxe.inf > + INF NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf > + !endif > + > + INF NetworkPkg/TcpDxe/TcpDxe.inf > + INF NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf > + > + !if $(NETWORK_TLS_ENABLE) == TRUE > + INF NetworkPkg/TlsDxe/TlsDxe.inf > + INF NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf > + !endif This will be good for OVMF too, based on . > + > + !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_ISCSI_ENABLE) == TRUE > + INF NetworkPkg/IScsiDxe/IScsiDxe.inf > + !endif Addresses my request (6). > + > +!endif > diff --git a/NetworkPkg/NetworkComponents.dsc.inc b/NetworkPkg/NetworkComponents.dsc.inc > new file mode 100644 > index 0000000000..aede5ea8be > --- /dev/null > +++ b/NetworkPkg/NetworkComponents.dsc.inc > @@ -0,0 +1,61 @@ > +## @file > +# Network DSC include file for [Components*] section of all Architectures. > +# > +# This file can be included to the [Components*] section(s) of a platform DSC file > +# by using "!include NetworkPkg/NetworkComponents.dsc.inc" to specify the INF files > +# of EDKII network drivers according to the value of flags described in > +# "NetworkDefines.dsc.inc". > +# > +# Copyright (c) 2019, Intel Corporation. All rights reserved.
> +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +!if $(NETWORK_ENABLE) == TRUE > + MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf > + > + !if $(NETWORK_SNP_ENABLE) == TRUE > + MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf > + !endif > + > + !if $(NETWORK_VLAN_ENABLE) == TRUE > + MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf > + !endif > + > + MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf > + > + !if $(NETWORK_IP4_ENABLE) == TRUE > + MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf > + MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf > + MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf > + MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf > + MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf > + !endif > + > + !if $(NETWORK_IP6_ENABLE) == TRUE > + NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf > + NetworkPkg/Ip6Dxe/Ip6Dxe.inf > + NetworkPkg/Udp6Dxe/Udp6Dxe.inf > + NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf > + !endif > + > + NetworkPkg/TcpDxe/TcpDxe.inf > + NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf > + > + !if $(NETWORK_TLS_ENABLE) == TRUE > + NetworkPkg/TlsDxe/TlsDxe.inf > + NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf > + !endif > + > + !if $(NETWORK_HTTP_BOOT_ENABLE) == TRUE > + NetworkPkg/DnsDxe/DnsDxe.inf > + NetworkPkg/HttpDxe/HttpDxe.inf > + NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf > + NetworkPkg/HttpBootDxe/HttpBootDxe.inf > + !endif > + > + !if $(NETWORK_ISCSI_ENABLE) == TRUE > + NetworkPkg/IScsiDxe/IScsiDxe.inf > + !endif > +!endif > diff --git a/NetworkPkg/NetworkDefines.dsc.inc b/NetworkPkg/NetworkDefines.dsc.inc > new file mode 100644 > index 0000000000..a442d1b157 > --- /dev/null > +++ b/NetworkPkg/NetworkDefines.dsc.inc > @@ -0,0 +1,118 @@ > +## @file > +# Network DSC include file for [Defines] section of all Architectures. > +# > +# This file can be included to the [Defines] section of a platform DSC file by > +# using "!include NetworkPkg/NetworkDefines.dsc.inc" to set default value of > +# flags if they are not defined somewhere else, and also check the value to see > +# if there is any conflict. > +# > +# These flags can be defined before the !include line, or changed on the command > +# line to enable or disable related feature support. > +# -D FLAG=VALUE > +# The default value of these flags are: > +# DEFINE NETWORK_ENABLE = TRUE > +# DEFINE NETWORK_SNP_ENABLE = TRUE > +# DEFINE NETWORK_IP4_ENABLE = TRUE > +# DEFINE NETWORK_IP6_ENABLE = TRUE > +# DEFINE NETWORK_TLS_ENABLE = TRUE > +# DEFINE NETWORK_HTTP_BOOT_ENABLE = TRUE > +# DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE > +# DEFINE NETWORK_ISCSI_ENABLE = TRUE > +# DEFINE NETWORK_VLAN_ENABLE = TRUE > +# > +# Copyright (c) 2019, Intel Corporation. All rights reserved.
> +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +!ifndef NETWORK_ENABLE > + # > + # This flag is to enable or disable the whole network stack. > + # > + DEFINE NETWORK_ENABLE = TRUE > +!endif > + > +!ifndef NETWORK_SNP_ENABLE > + # > + # This flag is to include the common SNP driver or not. > + # > + DEFINE NETWORK_SNP_ENABLE = TRUE > +!endif > + > +!ifndef NETWORK_VLAN_ENABLE > + # > + # This flag is to enable or disable VLAN feature. > + # > + DEFINE NETWORK_VLAN_ENABLE = TRUE > +!endif > + > +!ifndef NETWORK_IP4_ENABLE > + # > + # This flag is to enable or disable IPv4 network stack. > + # > + DEFINE NETWORK_IP4_ENABLE = TRUE > +!endif > + > +!ifndef NETWORK_IP6_ENABLE > + # > + # This flag is to enable or disable IPv6 network stack. > + # > + DEFINE NETWORK_IP6_ENABLE = TRUE > +!endif > + > +!ifndef NETWORK_TLS_ENABLE > + # > + # This flag is to enable or disable TLS feature. > + # > + # Note: This feature depends on the OpenSSL building. To enable this feature, please > + # follow the instructions found in the file "OpenSSL-HOWTO.txt" located in > + # CryptoPkg\Library\OpensslLib to enable the OpenSSL building first. > + # The OpensslLib.inf library instance should be used since libssl is required. > + # > + DEFINE NETWORK_TLS_ENABLE = TRUE > +!endif > + > +!ifndef NETWORK_HTTP_BOOT_ENABLE > + # > + # This flag is to enable or disable HTTP(S) boot feature. > + # > + DEFINE NETWORK_HTTP_BOOT_ENABLE = TRUE > +!endif > + > +!ifndef NETWORK_ALLOW_HTTP_CONNECTIONS > + # > + # Indicates whether HTTP connections (i.e., unsecured) are permitted or not. > + # > + # Note: If NETWORK_ALLOW_HTTP_CONNECTIONS is TRUE, HTTP connections are allowed. > + # Both the "https://" and "http://" URI schemes are permitted. Otherwise, HTTP > + # connections are denied. Only the "https://" URI scheme is permitted. > + # > + DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE > +!endif > + > +!ifndef NETWORK_ISCSI_ENABLE > + # > + # This flag is to enable or disable iSCSI feature. > + # > + # Note: This feature depends on the OpenSSL building. To enable this feature, please > + # follow the instructions found in the file "OpenSSL-HOWTO.txt" located in > + # CryptoPkg\Library\OpensslLib to enable the OpenSSL building first. > + # Both OpensslLib.inf and OpensslLibCrypto.inf library instance can be used > + # since libssl is not required for iSCSI. > + # > + DEFINE NETWORK_ISCSI_ENABLE = TRUE > +!endif > + > +!if $(NETWORK_ENABLE) == TRUE > + # > + # Check the flags to see if there is any conflict. > + # > + !if ($(NETWORK_IP4_ENABLE) == FALSE) AND ($(NETWORK_IP6_ENABLE) == FALSE) > + !error "Must enable at least IP4 or IP6 stack if NETWORK_ENABLE is set to TRUE!" > + !endif > + > + !if ($(NETWORK_HTTP_BOOT_ENABLE) == TRUE) AND ($(NETWORK_TLS_ENABLE) == FALSE) AND ($(NETWORK_ALLOW_HTTP_CONNECTIONS) == FALSE) > + !error "Must enable TLS to support HTTPS, or allow unsecured HTTP connection, if NETWORK_HTTP_BOOT_ENABLE is set to TRUE!" > + !endif My question (8) concerned this sanity check. Given that allows platforms to set NETWORK_TLS_ENABLE=TRUE *even if* they need to hook special overrides into the TLS drivers, I now see that this sanity check is correct. So my question (8) does not apply any longer. Regarding my question (9), PLATFORMX64_ENABLE has been moved to patch 3/3, and I now understand why it is required. OK. > +!endif > diff --git a/NetworkPkg/NetworkLibs.dsc.inc b/NetworkPkg/NetworkLibs.dsc.inc > new file mode 100644 > index 0000000000..a23f982381 > --- /dev/null > +++ b/NetworkPkg/NetworkLibs.dsc.inc > @@ -0,0 +1,20 @@ > +## @file > +# Network DSC include file for [LibraryClasses*] section of all Architectures. > +# > +# This file can be included to the [LibraryClasses*] section(s) of a platform DSC file > +# by using "!include NetworkPkg/NetworkLibs.dsc.inc" to specify the library instances > +# of EDKII network library classes. > +# > +# Copyright (c) 2019, Intel Corporation. All rights reserved.
> +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > + DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf > + NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf > + IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf > + UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf > + TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf > + # HttpLib is used for Http Boot > + HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf Thanks for the comment, addresses (10). I've also read Siyuan's comment about "TlsLib", and I understand the platform will have to spell out the resolution for that, going forward too. That answers my question (11). Reviewed-by: Laszlo Ersek Thanks! Laszlo > diff --git a/NetworkPkg/NetworkPcds.dsc.inc b/NetworkPkg/NetworkPcds.dsc.inc > new file mode 100644 > index 0000000000..f874b382ef > --- /dev/null > +++ b/NetworkPkg/NetworkPcds.dsc.inc > @@ -0,0 +1,16 @@ > +## @file > +# Network DSC include file for [Pcds*] section of all Architectures. > +# > +# This file can be included to the [Pcds*] section(s) of a platform DSC file > +# by using "!include NetworkPkg/NetworkPcds.dsc.inc" to specify PCD settings > +# according to the value of flags described in "NetworkDefines.dsc.inc". > +# > +# Copyright (c) 2019, Intel Corporation. All rights reserved.
> +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +!if $(NETWORK_ALLOW_HTTP_CONNECTIONS) == TRUE > + gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE > +!endif >