[edk2] [Patch] NetworkPkg/Ip6Dxe: Fix the IPv6 PXE boot option goes missing issue

Jiaxin Wu posted 1 patch 6 years, 8 months ago
Failed in applying to current master (apply log)
NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c | 72 ++++++++++++++++++++++++++++-----------
1 file changed, 53 insertions(+), 19 deletions(-)
[edk2] [Patch] NetworkPkg/Ip6Dxe: Fix the IPv6 PXE boot option goes missing issue
Posted by Jiaxin Wu 6 years, 8 months ago
This patch is to fix the potential issue recorded at Bugzilla 636:
https://bugzilla.tianocore.org/show_bug.cgi?id=636

The issue is caused by the IPv6 policy switching after PXEv6 boot. When IP
policy is changing, the IPv6 children used by PXE.UdpRead() will be destroyed.
Then, PXE Stop() function is called to uninstall the devicePath protocol,
which leads to the IPv6 PXE boot option goes missing.

Through the above analysis, the IP driver should take the responsibility for
the upper layer network stacks recovery by using ConnectController().

Cc: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com>
Cc: Subramanian Sriram <sriram-s@hpe.com>
Cc: Ni Ruiyu <ruiyu.ni@intel.com>
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/Ip6Dxe/Ip6ConfigImpl.c | 72 ++++++++++++++++++++++++++++-----------
 1 file changed, 53 insertions(+), 19 deletions(-)

diff --git a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
index 7c7acc7..f170716 100644
--- a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
+++ b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
@@ -45,16 +45,21 @@ VOID
 Ip6ConfigOnPolicyChanged (
   IN IP6_SERVICE            *IpSb,
   IN EFI_IP6_CONFIG_POLICY  NewPolicy
   )
 {
-  LIST_ENTRY      *Entry;
-  LIST_ENTRY      *Entry2;
-  LIST_ENTRY      *Next;
-  IP6_INTERFACE   *IpIf;
-  IP6_DAD_ENTRY   *DadEntry;
-  IP6_DELAY_JOIN_LIST       *DelayNode;
+  LIST_ENTRY          *Entry;
+  LIST_ENTRY          *Entry2;
+  LIST_ENTRY          *Next;
+  IP6_INTERFACE       *IpIf;
+  IP6_DAD_ENTRY       *DadEntry;
+  IP6_DELAY_JOIN_LIST *DelayNode;
+  IP6_ADDRESS_INFO    *AddrInfo;
+  IP6_PROTOCOL        *Instance;
+  BOOLEAN             Recovery;
+
+  Recovery = FALSE;
   
   //
   // Currently there are only two policies: Manual and Automatic. Regardless of
   // what transition is going on, i.e., Manual -> Automatic and Automatic ->
   // Manual, we have to free default router list, on-link prefix list, autonomous
@@ -78,22 +83,52 @@ Ip6ConfigOnPolicyChanged (
       IP6_LINK_LOCAL_PREFIX_LENGTH,
       &IpSb->LinkLocalAddr
       );
   }
 
-  //
-  // All IPv6 children that use global unicast address as it's source address
-  // should be destryoed now. The survivers are those use the link-local address
-  // or the unspecified address as the source address.
-  // TODO: Conduct a check here.
-  Ip6RemoveAddr (
-    IpSb,
-    &IpSb->DefaultInterface->AddressList,
-    &IpSb->DefaultInterface->AddressCount,
-    NULL,
-    0
-    );
+  if (!IsListEmpty (&IpSb->DefaultInterface->AddressList) && IpSb->DefaultInterface->AddressCount > 0) {
+    //  
+    // If any IPv6 children (Instance) in configured state and use global unicast address, it will be 
+    // destroyed in Ip6RemoveAddr() function later. Then, the upper layer driver's Stop() function will be 
+    // called, which may break the upper layer network stacks. So, the driver should take the responsibility 
+    // for the recovery by using ConnectController() after Ip6RemoveAddr(). 
+    // Here, just check whether need to recover the upper layer network stacks later.
+    //
+    NET_LIST_FOR_EACH (Entry, &IpSb->DefaultInterface->AddressList) { 
+      AddrInfo = NET_LIST_USER_STRUCT_S (Entry, IP6_ADDRESS_INFO, Link, IP6_ADDR_INFO_SIGNATURE);
+      if (!IsListEmpty (&IpSb->Children)) {
+        NET_LIST_FOR_EACH (Entry2, &IpSb->Children) {
+          Instance = NET_LIST_USER_STRUCT_S (Entry2, IP6_PROTOCOL, Link, IP6_PROTOCOL_SIGNATURE);
+          if ((Instance->State == IP6_STATE_CONFIGED) && EFI_IP6_EQUAL (&Instance->ConfigData.StationAddress, &AddrInfo->Address)) {
+            Recovery = TRUE;
+            break;
+          }
+        }
+      }
+    }
+    
+    //
+    // All IPv6 children that use global unicast address as it's source address
+    // should be destroyed now. The survivers are those use the link-local address
+    // or the unspecified address as the source address.
+    // TODO: Conduct a check here.
+    Ip6RemoveAddr (
+      IpSb,
+      &IpSb->DefaultInterface->AddressList,
+      &IpSb->DefaultInterface->AddressCount,
+      NULL,
+      0
+      );
+    
+    if (IpSb->Controller != NULL && Recovery) {
+      //
+      // ConnectController() to recover the upper layer network stacks.
+      //
+      gBS->ConnectController (IpSb->Controller, NULL, NULL, TRUE);
+    }
+  }
+
 
   NET_LIST_FOR_EACH (Entry, &IpSb->Interfaces) {
     //
     // remove all pending delay node and DAD entries for the global addresses.
     //
@@ -128,11 +163,10 @@ Ip6ConfigOnPolicyChanged (
     //
     // delay 1 second
     //
     IpSb->Ticks                   = (UINT32) IP6_GET_TICKS (IP6_ONE_SECOND_IN_MS);
   }
-
 }
 
 /**
   The work function to trigger the DHCPv6 process to perform a stateful autoconfiguration.
 
-- 
1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] NetworkPkg/Ip6Dxe: Fix the IPv6 PXE boot option goes missing issue
Posted by Hegde, Nagaraj P 6 years, 8 months ago
Reviewed-by: Hegde, Nagaraj P <nagaraj-p.hegde@hpe.com>
Tested-by: Hegde, Nagaraj P <nagaraj-p.hegde@hpe.com>

-----Original Message-----
From: Jiaxin Wu [mailto:jiaxin.wu@intel.com] 
Sent: Sunday, July 30, 2017 7:37 PM
To: edk2-devel@lists.01.org
Cc: Hegde, Nagaraj P <nagaraj-p.hegde@hpe.com>; Subramanian, Sriram <sriram-s@hpe.com>; Ni Ruiyu <ruiyu.ni@intel.com>; Ye Ting <ting.ye@intel.com>; Fu Siyuan <siyuan.fu@intel.com>; Wu Jiaxin <jiaxin.wu@intel.com>
Subject: [Patch] NetworkPkg/Ip6Dxe: Fix the IPv6 PXE boot option goes missing issue

This patch is to fix the potential issue recorded at Bugzilla 636:
https://bugzilla.tianocore.org/show_bug.cgi?id=636

The issue is caused by the IPv6 policy switching after PXEv6 boot. When IP policy is changing, the IPv6 children used by PXE.UdpRead() will be destroyed.
Then, PXE Stop() function is called to uninstall the devicePath protocol, which leads to the IPv6 PXE boot option goes missing.

Through the above analysis, the IP driver should take the responsibility for the upper layer network stacks recovery by using ConnectController().

Cc: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com>
Cc: Subramanian Sriram <sriram-s@hpe.com>
Cc: Ni Ruiyu <ruiyu.ni@intel.com>
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/Ip6Dxe/Ip6ConfigImpl.c | 72 ++++++++++++++++++++++++++++-----------
 1 file changed, 53 insertions(+), 19 deletions(-)

diff --git a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
index 7c7acc7..f170716 100644
--- a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
+++ b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
@@ -45,16 +45,21 @@ VOID
 Ip6ConfigOnPolicyChanged (
   IN IP6_SERVICE            *IpSb,
   IN EFI_IP6_CONFIG_POLICY  NewPolicy
   )
 {
-  LIST_ENTRY      *Entry;
-  LIST_ENTRY      *Entry2;
-  LIST_ENTRY      *Next;
-  IP6_INTERFACE   *IpIf;
-  IP6_DAD_ENTRY   *DadEntry;
-  IP6_DELAY_JOIN_LIST       *DelayNode;
+  LIST_ENTRY          *Entry;
+  LIST_ENTRY          *Entry2;
+  LIST_ENTRY          *Next;
+  IP6_INTERFACE       *IpIf;
+  IP6_DAD_ENTRY       *DadEntry;
+  IP6_DELAY_JOIN_LIST *DelayNode;
+  IP6_ADDRESS_INFO    *AddrInfo;
+  IP6_PROTOCOL        *Instance;
+  BOOLEAN             Recovery;
+
+  Recovery = FALSE;
   
   //
   // Currently there are only two policies: Manual and Automatic. Regardless of
   // what transition is going on, i.e., Manual -> Automatic and Automatic ->
   // Manual, we have to free default router list, on-link prefix list, autonomous @@ -78,22 +83,52 @@ Ip6ConfigOnPolicyChanged (
       IP6_LINK_LOCAL_PREFIX_LENGTH,
       &IpSb->LinkLocalAddr
       );
   }
 
-  //
-  // All IPv6 children that use global unicast address as it's source address
-  // should be destryoed now. The survivers are those use the link-local address
-  // or the unspecified address as the source address.
-  // TODO: Conduct a check here.
-  Ip6RemoveAddr (
-    IpSb,
-    &IpSb->DefaultInterface->AddressList,
-    &IpSb->DefaultInterface->AddressCount,
-    NULL,
-    0
-    );
+  if (!IsListEmpty (&IpSb->DefaultInterface->AddressList) && IpSb->DefaultInterface->AddressCount > 0) {
+    //  
+    // If any IPv6 children (Instance) in configured state and use global unicast address, it will be 
+    // destroyed in Ip6RemoveAddr() function later. Then, the upper layer driver's Stop() function will be 
+    // called, which may break the upper layer network stacks. So, the driver should take the responsibility 
+    // for the recovery by using ConnectController() after Ip6RemoveAddr(). 
+    // Here, just check whether need to recover the upper layer network stacks later.
+    //
+    NET_LIST_FOR_EACH (Entry, &IpSb->DefaultInterface->AddressList) { 
+      AddrInfo = NET_LIST_USER_STRUCT_S (Entry, IP6_ADDRESS_INFO, Link, IP6_ADDR_INFO_SIGNATURE);
+      if (!IsListEmpty (&IpSb->Children)) {
+        NET_LIST_FOR_EACH (Entry2, &IpSb->Children) {
+          Instance = NET_LIST_USER_STRUCT_S (Entry2, IP6_PROTOCOL, Link, IP6_PROTOCOL_SIGNATURE);
+          if ((Instance->State == IP6_STATE_CONFIGED) && EFI_IP6_EQUAL (&Instance->ConfigData.StationAddress, &AddrInfo->Address)) {
+            Recovery = TRUE;
+            break;
+          }
+        }
+      }
+    }
+    
+    //
+    // All IPv6 children that use global unicast address as it's source address
+    // should be destroyed now. The survivers are those use the link-local address
+    // or the unspecified address as the source address.
+    // TODO: Conduct a check here.
+    Ip6RemoveAddr (
+      IpSb,
+      &IpSb->DefaultInterface->AddressList,
+      &IpSb->DefaultInterface->AddressCount,
+      NULL,
+      0
+      );
+    
+    if (IpSb->Controller != NULL && Recovery) {
+      //
+      // ConnectController() to recover the upper layer network stacks.
+      //
+      gBS->ConnectController (IpSb->Controller, NULL, NULL, TRUE);
+    }
+  }
+
 
   NET_LIST_FOR_EACH (Entry, &IpSb->Interfaces) {
     //
     // remove all pending delay node and DAD entries for the global addresses.
     //
@@ -128,11 +163,10 @@ Ip6ConfigOnPolicyChanged (
     //
     // delay 1 second
     //
     IpSb->Ticks                   = (UINT32) IP6_GET_TICKS (IP6_ONE_SECOND_IN_MS);
   }
-
 }
 
 /**
   The work function to trigger the DHCPv6 process to perform a stateful autoconfiguration.
 
--
1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] NetworkPkg/Ip6Dxe: Fix the IPv6 PXE boot option goes missing issue
Posted by Subramanian, Sriram 6 years, 8 months ago
Reviewed-by: Sriram Subramanian <sriram-s@hpe.com>

-----Original Message-----
From: Jiaxin Wu [mailto:jiaxin.wu@intel.com] 
Sent: Sunday, July 30, 2017 7:37 PM
To: edk2-devel@lists.01.org
Cc: Hegde, Nagaraj P <nagaraj-p.hegde@hpe.com>; Subramanian, Sriram <sriram-s@hpe.com>; Ni Ruiyu <ruiyu.ni@intel.com>; Ye Ting <ting.ye@intel.com>; Fu Siyuan <siyuan.fu@intel.com>; Wu Jiaxin <jiaxin.wu@intel.com>
Subject: [Patch] NetworkPkg/Ip6Dxe: Fix the IPv6 PXE boot option goes missing issue

This patch is to fix the potential issue recorded at Bugzilla 636:
https://bugzilla.tianocore.org/show_bug.cgi?id=636

The issue is caused by the IPv6 policy switching after PXEv6 boot. When IP
policy is changing, the IPv6 children used by PXE.UdpRead() will be destroyed.
Then, PXE Stop() function is called to uninstall the devicePath protocol,
which leads to the IPv6 PXE boot option goes missing.

Through the above analysis, the IP driver should take the responsibility for
the upper layer network stacks recovery by using ConnectController().

Cc: Hegde Nagaraj P <nagaraj-p.hegde@hpe.com>
Cc: Subramanian Sriram <sriram-s@hpe.com>
Cc: Ni Ruiyu <ruiyu.ni@intel.com>
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/Ip6Dxe/Ip6ConfigImpl.c | 72 ++++++++++++++++++++++++++++-----------
 1 file changed, 53 insertions(+), 19 deletions(-)

diff --git a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
index 7c7acc7..f170716 100644
--- a/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
+++ b/NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c
@@ -45,16 +45,21 @@ VOID
 Ip6ConfigOnPolicyChanged (
   IN IP6_SERVICE            *IpSb,
   IN EFI_IP6_CONFIG_POLICY  NewPolicy
   )
 {
-  LIST_ENTRY      *Entry;
-  LIST_ENTRY      *Entry2;
-  LIST_ENTRY      *Next;
-  IP6_INTERFACE   *IpIf;
-  IP6_DAD_ENTRY   *DadEntry;
-  IP6_DELAY_JOIN_LIST       *DelayNode;
+  LIST_ENTRY          *Entry;
+  LIST_ENTRY          *Entry2;
+  LIST_ENTRY          *Next;
+  IP6_INTERFACE       *IpIf;
+  IP6_DAD_ENTRY       *DadEntry;
+  IP6_DELAY_JOIN_LIST *DelayNode;
+  IP6_ADDRESS_INFO    *AddrInfo;
+  IP6_PROTOCOL        *Instance;
+  BOOLEAN             Recovery;
+
+  Recovery = FALSE;
   
   //
   // Currently there are only two policies: Manual and Automatic. Regardless of
   // what transition is going on, i.e., Manual -> Automatic and Automatic ->
   // Manual, we have to free default router list, on-link prefix list, autonomous
@@ -78,22 +83,52 @@ Ip6ConfigOnPolicyChanged (
       IP6_LINK_LOCAL_PREFIX_LENGTH,
       &IpSb->LinkLocalAddr
       );
   }
 
-  //
-  // All IPv6 children that use global unicast address as it's source address
-  // should be destryoed now. The survivers are those use the link-local address
-  // or the unspecified address as the source address.
-  // TODO: Conduct a check here.
-  Ip6RemoveAddr (
-    IpSb,
-    &IpSb->DefaultInterface->AddressList,
-    &IpSb->DefaultInterface->AddressCount,
-    NULL,
-    0
-    );
+  if (!IsListEmpty (&IpSb->DefaultInterface->AddressList) && IpSb->DefaultInterface->AddressCount > 0) {
+    //  
+    // If any IPv6 children (Instance) in configured state and use global unicast address, it will be 
+    // destroyed in Ip6RemoveAddr() function later. Then, the upper layer driver's Stop() function will be 
+    // called, which may break the upper layer network stacks. So, the driver should take the responsibility 
+    // for the recovery by using ConnectController() after Ip6RemoveAddr(). 
+    // Here, just check whether need to recover the upper layer network stacks later.
+    //
+    NET_LIST_FOR_EACH (Entry, &IpSb->DefaultInterface->AddressList) { 
+      AddrInfo = NET_LIST_USER_STRUCT_S (Entry, IP6_ADDRESS_INFO, Link, IP6_ADDR_INFO_SIGNATURE);
+      if (!IsListEmpty (&IpSb->Children)) {
+        NET_LIST_FOR_EACH (Entry2, &IpSb->Children) {
+          Instance = NET_LIST_USER_STRUCT_S (Entry2, IP6_PROTOCOL, Link, IP6_PROTOCOL_SIGNATURE);
+          if ((Instance->State == IP6_STATE_CONFIGED) && EFI_IP6_EQUAL (&Instance->ConfigData.StationAddress, &AddrInfo->Address)) {
+            Recovery = TRUE;
+            break;
+          }
+        }
+      }
+    }
+    
+    //
+    // All IPv6 children that use global unicast address as it's source address
+    // should be destroyed now. The survivers are those use the link-local address
+    // or the unspecified address as the source address.
+    // TODO: Conduct a check here.
+    Ip6RemoveAddr (
+      IpSb,
+      &IpSb->DefaultInterface->AddressList,
+      &IpSb->DefaultInterface->AddressCount,
+      NULL,
+      0
+      );
+    
+    if (IpSb->Controller != NULL && Recovery) {
+      //
+      // ConnectController() to recover the upper layer network stacks.
+      //
+      gBS->ConnectController (IpSb->Controller, NULL, NULL, TRUE);
+    }
+  }
+
 
   NET_LIST_FOR_EACH (Entry, &IpSb->Interfaces) {
     //
     // remove all pending delay node and DAD entries for the global addresses.
     //
@@ -128,11 +163,10 @@ Ip6ConfigOnPolicyChanged (
     //
     // delay 1 second
     //
     IpSb->Ticks                   = (UINT32) IP6_GET_TICKS (IP6_ONE_SECOND_IN_MS);
   }
-
 }
 
 /**
   The work function to trigger the DHCPv6 process to perform a stateful autoconfiguration.
 
-- 
1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel