[Patch 0/3] Fix the series issues in UefiPxeBcDxe.

[Patch 0/3] Fix the series issues in UefiPxeBcDxe.

[Jiaxin Wu]

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>

Jiaxin Wu (3):
  NetworkPkg/UefiPxeBcDxe: Fix Pxe.Dhcp() return status code.
  NetworkPkg/UefiPxeBcDxe: Correct the handle for PXE Base Code Callback
    Protocol.
  NetworkPkg/UefiPxeBcDxe: Allow the NULL configuration for
    NewStationIP/NewSubnetMask

 NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c    |  8 ++++----
 NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c   |  5 +++++
 NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c    |  2 +-
 NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c | 24 ++++++++++++++----------
 NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h |  4 ++--
 5 files changed, 26 insertions(+), 17 deletions(-)

-- 
1.9.5.msysgit.1

[Patch 1/3] NetworkPkg/UefiPxeBcDxe: Fix Pxe.Dhcp() return status code.

[Jiaxin Wu]

According UEFI Spec, if valid PXE offer is not received, Pxe.Dhcp()
should return EFI_NO_RESPONSE, but currently, EFI_TIMEOUT is returned
from Pxe.Dhcp().

This patch is to fix the above issue.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c  | 2 +-
 NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
index fc50a82..0bb1d66 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
@@ -1240,11 +1240,11 @@ ON_EXIT:
   } else if (Status == EFI_OUT_OF_RESOURCES) {
     AsciiPrint ("\n  PXE-E09: Could not allocate I/O buffers.\n");
   } else if (Status == EFI_NO_MEDIA) {
     AsciiPrint ("\n  PXE-E12: Could not detect network connection.\n");
   } else if (Status == EFI_NO_RESPONSE) {
-    AsciiPrint ("\n  PXE-E16: No offer received.\n");
+    AsciiPrint ("\n  PXE-E16: No valid offer received.\n");
   } else if (Status == EFI_TIMEOUT) {
     AsciiPrint ("\n  PXE-E18: Server response timeout.\n");
   } else if (Status == EFI_ABORTED) {
     AsciiPrint ("\n  PXE-E21: Remote boot cancelled.\n");
   } else if (Status == EFI_ICMP_ERROR) {
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c
index 97829e9..101c658 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c
@@ -1705,10 +1705,15 @@ PxeBcDhcp4Dora (
   Status = Dhcp4->Start (Dhcp4, NULL);
   if (EFI_ERROR (Status) && Status != EFI_ALREADY_STARTED) {
     if (Status == EFI_ICMP_ERROR) {
       PxeMode->IcmpErrorReceived = TRUE;
     }
+
+    if (Status == EFI_TIMEOUT && Private->OfferNum > 0) {
+      Status = EFI_NO_RESPONSE;
+    }
+    
     goto ON_EXIT;
   }
 
   //
   // Get the acquired IPv4 address and store them.
-- 
1.9.5.msysgit.1

[Patch 2/3] NetworkPkg/UefiPxeBcDxe: Correct the handle for PXE Base Code Callback Protocol.

[Jiaxin Wu]

According UEFI Spec:
The PXE Base Code Callback Protocol must be on the same handle as the PXE
Base Code Protocol.

But current implementation doesn't follow that. This patch is fix that issue.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c | 6 +++---
 NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
index 0bb1d66..1f8895f 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c
@@ -992,11 +992,11 @@ PxeBcInstallCallback (
   //
   // Check whether PxeBaseCodeCallbackProtocol already installed.
   //
   PxeBc  = &Private->PxeBc;
   Status = gBS->HandleProtocol (
-                  Private->Controller,
+                  Private->Mode.UsingIpv6 ? Private->Ip6Nic->Controller : Private->Ip4Nic->Controller,
                   &gEfiPxeBaseCodeCallbackProtocolGuid,
                   (VOID **) &Private->PxeBcCallback
                   );
   if (Status == EFI_UNSUPPORTED) {
 
@@ -1008,11 +1008,11 @@ PxeBcInstallCallback (
 
     //
     // Install a default callback if user didn't offer one.
     //
     Status = gBS->InstallProtocolInterface (
-                    &Private->Controller,
+                    Private->Mode.UsingIpv6 ? &Private->Ip6Nic->Controller : &Private->Ip4Nic->Controller,
                     &gEfiPxeBaseCodeCallbackProtocolGuid,
                     EFI_NATIVE_INTERFACE,
                     &Private->LoadFileCallback
                     );
 
@@ -1052,11 +1052,11 @@ PxeBcUninstallCallback (
     NewMakeCallback = FALSE;
 
     PxeBc->SetParameters (PxeBc, NULL, NULL, NULL, NULL, &NewMakeCallback);
 
     gBS->UninstallProtocolInterface (
-          Private->Controller,
+          Private->Mode.UsingIpv6 ? Private->Ip6Nic->Controller : Private->Ip4Nic->Controller,
           &gEfiPxeBaseCodeCallbackProtocolGuid,
           &Private->LoadFileCallback
           );
   }
 }
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
index ab9e494..1fc26c5 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
@@ -1923,11 +1923,11 @@ EfiPxeBcSetParameters (
     if (*NewMakeCallback) {
       //
       // Update the previous PxeBcCallback protocol.
       //
       Status = gBS->HandleProtocol (
-                      Private->Controller,
+                      Mode->UsingIpv6 ? Private->Ip6Nic->Controller : Private->Ip4Nic->Controller,
                       &gEfiPxeBaseCodeCallbackProtocolGuid,
                       (VOID **) &Private->PxeBcCallback
                       );
 
       if (EFI_ERROR (Status) || (Private->PxeBcCallback->Callback == NULL)) {
-- 
1.9.5.msysgit.1

[Patch 3/3] NetworkPkg/UefiPxeBcDxe: Allow the NULL configuration for NewStationIP/NewSubnetMask

[Jiaxin Wu]

According the UEFI Spec for PxeBc.SetStationIP():
If NewStationIP is NULL, then the current IP address will not be modified.
...
If NewSubnetMask is NULL, then the current subnet mask will not be modified.

Currently, EfiPxeBcSetStationIP() doesn't comply with UEFI Spec. This patch is
to fix the issue.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c | 24 ++++++++++++++----------
 NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h |  4 ++--
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
index 538cb59..52f1e92 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c
@@ -28,26 +28,26 @@
 
 **/
 EFI_STATUS
 PxeBcFlushStationIp (
   PXEBC_PRIVATE_DATA       *Private,
-  EFI_IP_ADDRESS           *StationIp,
+  EFI_IP_ADDRESS           *StationIp,     OPTIONAL
   EFI_IP_ADDRESS           *SubnetMask     OPTIONAL
   )
 {
   EFI_PXE_BASE_CODE_MODE   *Mode;
   EFI_STATUS               Status;
 
-  ASSERT (StationIp != NULL);
-
   Mode   = Private->PxeBc.Mode;
   Status = EFI_SUCCESS;
 
   if (Mode->UsingIpv6) {
 
-    CopyMem (&Private->Udp6CfgData.StationAddress, StationIp, sizeof (EFI_IPv6_ADDRESS));
-    CopyMem (&Private->Ip6CfgData.StationAddress, StationIp, sizeof (EFI_IPv6_ADDRESS));
+    if (StationIp != NULL) {
+      CopyMem (&Private->Udp6CfgData.StationAddress, StationIp, sizeof (EFI_IPv6_ADDRESS));
+      CopyMem (&Private->Ip6CfgData.StationAddress, StationIp, sizeof (EFI_IPv6_ADDRESS));
+    }
 
     //
     // Reconfigure the Ip6 instance to capture background ICMP6 packets with new station Ip address.
     //
     Private->Ip6->Cancel (Private->Ip6, &Private->Icmp6Token);
@@ -58,15 +58,19 @@ PxeBcFlushStationIp (
       goto ON_EXIT;
     }
 
     Status = Private->Ip6->Receive (Private->Ip6, &Private->Icmp6Token);
   } else {
-    ASSERT (SubnetMask != NULL);
-    CopyMem (&Private->Udp4CfgData.StationAddress, StationIp, sizeof (EFI_IPv4_ADDRESS));
-    CopyMem (&Private->Udp4CfgData.SubnetMask, SubnetMask, sizeof (EFI_IPv4_ADDRESS));
-    CopyMem (&Private->Ip4CfgData.StationAddress, StationIp, sizeof (EFI_IPv4_ADDRESS));
-    CopyMem (&Private->Ip4CfgData.SubnetMask, SubnetMask, sizeof (EFI_IPv4_ADDRESS));
+    if (StationIp != NULL) {
+      CopyMem (&Private->Udp4CfgData.StationAddress, StationIp, sizeof (EFI_IPv4_ADDRESS));
+      CopyMem (&Private->Ip4CfgData.StationAddress, StationIp, sizeof (EFI_IPv4_ADDRESS));
+    }
+    
+    if (SubnetMask != NULL) {
+      CopyMem (&Private->Udp4CfgData.SubnetMask, SubnetMask, sizeof (EFI_IPv4_ADDRESS));
+      CopyMem (&Private->Ip4CfgData.SubnetMask, SubnetMask, sizeof (EFI_IPv4_ADDRESS));
+    }
 
     //
     // Reconfigure the Ip4 instance to capture background ICMP packets with new station Ip address.
     //
     Private->Ip4->Cancel (Private->Ip4, &Private->IcmpToken);
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h
index b8519ae..17bee5c 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h
@@ -1,9 +1,9 @@
 /** @file
   Support functions declaration for UefiPxeBc Driver.
 
-  Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2007 - 2017, 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.
@@ -38,11 +38,11 @@
 
 **/
 EFI_STATUS
 PxeBcFlushStationIp (
   PXEBC_PRIVATE_DATA       *Private,
-  EFI_IP_ADDRESS           *StationIp,
+  EFI_IP_ADDRESS           *StationIp,     OPTIONAL
   EFI_IP_ADDRESS           *SubnetMask     OPTIONAL
   );
 
 
 /**
-- 
1.9.5.msysgit.1

Re: [Patch 0/3] Fix the series issues in UefiPxeBcDxe.

[Wu, Jiaxin]

Hi Siyuan and Ting,

Those issues are exposed by SCT manual test, please help to review them.

Thanks,
Jiaxin



> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jiaxin Wu
> Sent: Thursday, December 14, 2017 1:50 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> Subject: [edk2] [Patch 0/3] Fix the series issues in UefiPxeBcDxe.
> 
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> 
> Jiaxin Wu (3):
>   NetworkPkg/UefiPxeBcDxe: Fix Pxe.Dhcp() return status code.
>   NetworkPkg/UefiPxeBcDxe: Correct the handle for PXE Base Code Callback
>     Protocol.
>   NetworkPkg/UefiPxeBcDxe: Allow the NULL configuration for
>     NewStationIP/NewSubnetMask
> 
>  NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c    |  8 ++++----
>  NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c   |  5 +++++
>  NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c    |  2 +-
>  NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c | 24 ++++++++++++++----------
>  NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h |  4 ++--
>  5 files changed, 26 insertions(+), 17 deletions(-)
> 
> --
> 1.9.5.msysgit.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [Patch 0/3] Fix the series issues in UefiPxeBcDxe.

[Fu, Siyuan]

Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>

> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Thursday, December 14, 2017 1:50 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> Subject: [Patch 0/3] Fix the series issues in UefiPxeBcDxe.
> 
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> 
> Jiaxin Wu (3):
>   NetworkPkg/UefiPxeBcDxe: Fix Pxe.Dhcp() return status code.
>   NetworkPkg/UefiPxeBcDxe: Correct the handle for PXE Base Code Callback
>     Protocol.
>   NetworkPkg/UefiPxeBcDxe: Allow the NULL configuration for
>     NewStationIP/NewSubnetMask
> 
>  NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c    |  8 ++++----
>  NetworkPkg/UefiPxeBcDxe/PxeBcDhcp4.c   |  5 +++++
>  NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c    |  2 +-
>  NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c | 24 ++++++++++++++----------
>  NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h |  4 ++--
>  5 files changed, 26 insertions(+), 17 deletions(-)
> 
> --
> 1.9.5.msysgit.1