Re: [edk2-devel] [PATCH] Edk2\NetworkPkg\UefiPxeBcDxe : Open a method to Enabled/Disabled IPV6/IPV6 PXE Support.

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

Hi, Shengfeng

I gave 4 review comments to your V1 patch, but you only fixed the patch description.

Also, an updated patch should be prefixed with [PATCH v2]

BestRegards
Fu Siyuan


> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Xue,
> ShengfengX
> Sent: Monday, April 15, 2019 5:37 PM
> To: devel@edk2.groups.io
> Cc: Xue; Xue, ShengfengX <shengfengx.xue@intel.com>
> Subject: [edk2-devel] [PATCH] Edk2\NetworkPkg\UefiPxeBcDxe : Open a method to
> Enabled/Disabled IPV6/IPV6 PXE Support.
> 
> Please refer this link for defail.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1695
> 
> Some Platform setup need provide a item for user to Enabled/Disabled IPV4/IPV6
> PXE boot, but Origin UefiPxeBcDxe driver doesn't have such interface.
> So attached code review added two PCD to control IPV4/IPV6 PXE function in
> PxeBcSupported().
> Platform Developer will override this two PCD to Enabled/Disabled IPV4/IPV6
> PXE boot according to Setup value.
> attached code review have no side effect on current PXE function with default
> PCD value.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Xue, ShengfengX <shengfengx.xue@intel.com>
> Reviewed-by: Reviewer Name <devel@edk2.groups.io>
> ---
>  NetworkPkg/NetworkPkg.dec                | 12 +++++++++++-
>  NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c    | 12 +++++++++++-
>  NetworkPkg/UefiPxeBcDxe/PxeBcImpl.h      |  5 ++++-
>  NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf |  4 +++-
>  4 files changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/NetworkPkg/NetworkPkg.dec b/NetworkPkg/NetworkPkg.dec
> index 9b8ece4837..1a7b634631 100644
> --- a/NetworkPkg/NetworkPkg.dec
> +++ b/NetworkPkg/NetworkPkg.dec
> @@ -3,7 +3,7 @@
>  #
>  # This package provides network modules that conform to UEFI 2.4
> specification.
>  #
> -# Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
>  # (C) Copyright 2015-2017 Hewlett Packard Enterprise Development LP<BR>
>  #
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -117,5 +117,15 @@
>    # @Prompt Type Value of network boot policy used in iSCSI.
> 
> gEfiNetworkPkgTokenSpaceGuid.PcdIScsiAIPNetworkBootPolicy|0x08|UINT8|0x1000000
> 7
> 
> +  ## IPv4 PXE support
> +  # 0x00 = PXE Enabled
> +  # 0x01 = PXE Disabled
> +  gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport|0x00|UINT8|0x10000008
> +
> +  ## IPv6 PXE support
> +  # 0x00 = PXE Enabled
> +  # 0x01 = PXE Disabled
> +  gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x00|UINT8|0x10000009
> +
>  [UserExtensions.TianoCore."ExtraFiles"]
>    NetworkPkgExtra.uni
> diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
> b/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
> index 3c1d400d50..cac19242aa 100644
> --- a/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
> +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
> @@ -2,7 +2,7 @@
>    Driver Binding functions implementationfor for UefiPxeBc Driver.
> 
>    (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
> -  Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -1242,6 +1242,10 @@ PxeBcDriverEntryPoint (
>  {
>    EFI_STATUS  Status;
> 
> +  if ((PcdGet8(PcdIPv4PXESupport) == PXEDISABLED) &&
> (PcdGet8(PcdIPv6PXESupport) == PXEDISABLED)) {
> +    return EFI_UNSUPPORTED;
> +  }
> +
>    Status = EfiLibInstallDriverBindingComponentName2 (
>               ImageHandle,
>               SystemTable,
> @@ -1301,9 +1305,15 @@ PxeBcSupported (
>    EFI_GUID                        *MtftpServiceBindingGuid;
> 
>    if (IpVersion == IP_VERSION_4) {
> +    if (PcdGet8(PcdIPv4PXESupport) == PXEDISABLED) {
> +      return EFI_UNSUPPORTED;
> +    }
>      DhcpServiceBindingGuid  = &gEfiDhcp4ServiceBindingProtocolGuid;
>      MtftpServiceBindingGuid = &gEfiMtftp4ServiceBindingProtocolGuid;
>    } else {
> +    if (PcdGet8(PcdIPv6PXESupport) == PXEDISABLED) {
> +      return EFI_UNSUPPORTED;
> +    }
>      DhcpServiceBindingGuid  = &gEfiDhcp6ServiceBindingProtocolGuid;
>      MtftpServiceBindingGuid = &gEfiMtftp6ServiceBindingProtocolGuid;
>    }
> diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.h
> b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.h
> index f0db4a0c1a..ba870ddf5a 100644
> --- a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.h
> +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.h
> @@ -2,7 +2,7 @@
>    This EFI_PXE_BASE_CODE_PROTOCOL and EFI_LOAD_FILE_PROTOCOL.
>    interfaces declaration.
> 
> -  Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -78,6 +78,9 @@ typedef struct _PXEBC_VIRTUAL_NIC   PXEBC_VIRTUAL_NIC;
>  #define PXEBC_PRIVATE_DATA_FROM_ID(a)         CR (a, PXEBC_PRIVATE_DATA, Id,
> PXEBC_PRIVATE_DATA_SIGNATURE)
>  #define PXEBC_VIRTUAL_NIC_FROM_LOADFILE(a)    CR (a, PXEBC_VIRTUAL_NIC,
> LoadFile, PXEBC_VIRTUAL_NIC_SIGNATURE)
> 
> +#define PXEENABLED                            0x00
> +#define PXEDISABLED                           0x01
> +
>  typedef union {
>    PXEBC_DHCP4_PACKET_CACHE            Dhcp4;
>    PXEBC_DHCP6_PACKET_CACHE            Dhcp6;
> diff --git a/NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> b/NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> index fc31250dea..0341f1052a 100644
> --- a/NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> +++ b/NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
> @@ -5,7 +5,7 @@
>  #  PXE-compatible device for network access or booting. This driver supports
>  #  both IPv4 and IPv6 network stack.
>  #
> -#  Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -102,6 +102,8 @@
>  [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdTftpBlockSize      ## SOMETIMES_CONSUMES
>    gEfiNetworkPkgTokenSpaceGuid.PcdPxeTftpWindowSize    ## SOMETIMES_CONSUMES
> +  gEfiNetworkPkgTokenSpaceGuid.PcdIPv4PXESupport       ## CONSUMES
> +  gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport       ## CONSUMES
> 
>  [UserExtensions.TianoCore."ExtraFiles"]
>    UefiPxeBcDxeExtra.uni
> --
> 2.16.2.windows.1
> 
> 
>