[edk2-devel] [PATCH] RedfishPkg: Remove the global variables related to Discover Token functionality

Igor Kulchytskyy via groups.io posted 1 patch 1 year ago
Failed in applying to current master (apply log)
RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c | 139 ++++++++------------
RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c           |  87 ++++++++----
RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h      |  28 +++-
3 files changed, 140 insertions(+), 114 deletions(-)
[edk2-devel] [PATCH] RedfishPkg: Remove the global variables related to Discover Token functionality
Posted by Igor Kulchytskyy via groups.io 1 year ago
gRedfishDiscoveredToken may be allocated several times,
if multiple NIC installed on the system.
To avoid this issue Discover Token related global variables
replaced with the local variables.

Cc: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Signed-off-by: Igor Kulchytskyy <igork@ami.com>
---
 RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c | 139 ++++++++------------
 RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c           |  87 ++++++++----
 RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h      |  28 +++-
 3 files changed, 140 insertions(+), 114 deletions(-)

diff --git a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
index 993ad33..8f85491 100644
--- a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
+++ b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
@@ -22,12 +22,6 @@ EFI_HANDLE                     gEfiRedfishDiscoverControllerHandle = NULL;
 EFI_REDFISH_DISCOVER_PROTOCOL  *gEfiRedfishDiscoverProtocol        = NULL;
 BOOLEAN                        gRedfishDiscoverActivated           = FALSE;
 BOOLEAN                        gRedfishServiceDiscovered           = FALSE;
-//
-// Network interfaces discovered by EFI Redfish Discover Protocol.
-//
-UINTN                                   gNumberOfNetworkInterfaces;
-EFI_REDFISH_DISCOVER_NETWORK_INTERFACE  *gNetworkInterfaceInstances = NULL;
-EFI_REDFISH_DISCOVERED_TOKEN            *gRedfishDiscoveredToken    = NULL;

 ///
 /// Driver Binding Protocol instance
@@ -58,13 +52,6 @@ RedfishConfigStopRedfishDiscovery (
       gBS->CloseEvent (gEfiRedfishDiscoverProtocolEvent);
     }

-    //
-    // Stop Redfish service discovery.
-    //
-    gEfiRedfishDiscoverProtocol->AbortAcquireRedfishService (
-                                   gEfiRedfishDiscoverProtocol,
-                                   gNetworkInterfaceInstances
-                                   );
     gEfiRedfishDiscoverControllerHandle = NULL;
     gEfiRedfishDiscoverProtocol         = NULL;
     gRedfishDiscoverActivated           = FALSE;
@@ -318,36 +305,39 @@ RedfishServiceDiscoveredCallback (
   EFI_REDFISH_DISCOVERED_TOKEN     *RedfishDiscoveredToken;
   EFI_REDFISH_DISCOVERED_INSTANCE  *RedfishInstance;

-  if (gRedfishServiceDiscovered) {
-    //
-    // Only support one Redfish service on platform.
-    //
-    return;
-  }
-
   RedfishDiscoveredToken = (EFI_REDFISH_DISCOVERED_TOKEN *)Context;
-  RedfishInstance        = RedfishDiscoveredToken->DiscoverList.RedfishInstances;
+  gBS->CloseEvent (RedfishDiscoveredToken->Event);
+
   //
-  // Only pick up the first found Redfish service.
+  // Only support one Redfish service on platform.
   //
-  if (RedfishInstance->Status == EFI_SUCCESS) {
-    gRedfishConfigData.RedfishServiceInfo.RedfishServiceRestExHandle = RedfishInstance->Information.RedfishRestExHandle;
-    gRedfishConfigData.RedfishServiceInfo.RedfishServiceVersion      = RedfishInstance->Information.RedfishVersion;
-    gRedfishConfigData.RedfishServiceInfo.RedfishServiceLocation     = RedfishInstance->Information.Location;
-    gRedfishConfigData.RedfishServiceInfo.RedfishServiceUuid         = RedfishInstance->Information.Uuid;
-    gRedfishConfigData.RedfishServiceInfo.RedfishServiceOs           = RedfishInstance->Information.Os;
-    gRedfishConfigData.RedfishServiceInfo.RedfishServiceOsVersion    = RedfishInstance->Information.OsVersion;
-    gRedfishConfigData.RedfishServiceInfo.RedfishServiceProduct      = RedfishInstance->Information.Product;
-    gRedfishConfigData.RedfishServiceInfo.RedfishServiceProductVer   = RedfishInstance->Information.ProductVer;
-    gRedfishConfigData.RedfishServiceInfo.RedfishServiceUseHttps     = RedfishInstance->Information.UseHttps;
-    gRedfishServiceDiscovered                                        = TRUE;
+  if (!gRedfishServiceDiscovered) {
+    RedfishInstance        = RedfishDiscoveredToken->DiscoverList.RedfishInstances;
+    //
+    // Only pick up the first found Redfish service.
+    //
+    if (RedfishInstance->Status == EFI_SUCCESS) {
+      gRedfishConfigData.RedfishServiceInfo.RedfishServiceRestExHandle = RedfishInstance->Information.RedfishRestExHandle;
+      gRedfishConfigData.RedfishServiceInfo.RedfishServiceVersion      = RedfishInstance->Information.RedfishVersion;
+      gRedfishConfigData.RedfishServiceInfo.RedfishServiceLocation     = RedfishInstance->Information.Location;
+      gRedfishConfigData.RedfishServiceInfo.RedfishServiceUuid         = RedfishInstance->Information.Uuid;
+      gRedfishConfigData.RedfishServiceInfo.RedfishServiceOs           = RedfishInstance->Information.Os;
+      gRedfishConfigData.RedfishServiceInfo.RedfishServiceOsVersion    = RedfishInstance->Information.OsVersion;
+      gRedfishConfigData.RedfishServiceInfo.RedfishServiceProduct      = RedfishInstance->Information.Product;
+      gRedfishConfigData.RedfishServiceInfo.RedfishServiceProductVer   = RedfishInstance->Information.ProductVer;
+      gRedfishConfigData.RedfishServiceInfo.RedfishServiceUseHttps     = RedfishInstance->Information.UseHttps;
+      gRedfishServiceDiscovered                                        = TRUE;
+    }
+
+    //
+    // Invoke RedfishConfigHandlerInstalledCallback to execute
+    // the initialization of Redfish Configure Handler instance.
+    //
+    RedfishConfigHandlerInstalledCallback (gRedfishConfigData.Event, &gRedfishConfigData);
   }

-  //
-  // Invoke RedfishConfigHandlerInstalledCallback to execute
-  // the initialization of Redfish Configure Handler instance.
-  //
-  RedfishConfigHandlerInstalledCallback (gRedfishConfigData.Event, &gRedfishConfigData);
+  FreePool(RedfishDiscoveredToken);
+
 }

 /**
@@ -371,6 +361,7 @@ RedfishDiscoverProtocolInstalled (
   UINTN                                   NetworkInterfaceIndex;
   EFI_REDFISH_DISCOVER_NETWORK_INTERFACE  *ThisNetworkInterface;
   EFI_REDFISH_DISCOVERED_TOKEN            *ThisRedfishDiscoveredToken;
+  UINTN                                   NumberOfNetworkInterfaces;

   DEBUG ((DEBUG_INFO, "%a: New network interface is installed on system by EFI Redfish discover driver.\n", __func__));

@@ -408,36 +399,31 @@ RedfishDiscoverProtocolInstalled (
     }
   }

-  //
-  // Check the new found network interface.
-  //
-  if (gNetworkInterfaceInstances != NULL) {
-    FreePool (gNetworkInterfaceInstances);
-  }

   Status = gEfiRedfishDiscoverProtocol->GetNetworkInterfaceList (
                                           gEfiRedfishDiscoverProtocol,
                                           gRedfishConfigData.Image,
-                                          &gNumberOfNetworkInterfaces,
-                                          &gNetworkInterfaceInstances
+                                          &NumberOfNetworkInterfaces,
+                                          &ThisNetworkInterface
                                           );
-  if (EFI_ERROR (Status) || (gNumberOfNetworkInterfaces == 0)) {
+  if (EFI_ERROR (Status) || (NumberOfNetworkInterfaces == 0)) {
     DEBUG ((DEBUG_ERROR, "%a: No network interfaces found on the handle.\n", __func__));
     return;
   }

-  gRedfishDiscoveredToken = AllocateZeroPool (gNumberOfNetworkInterfaces * sizeof (EFI_REDFISH_DISCOVERED_TOKEN));
-  if (gRedfishDiscoveredToken == NULL) {
+  //
+  // Loop to discover Redfish service on each network interface.
+  //
+  for (NetworkInterfaceIndex = 0; NetworkInterfaceIndex < NumberOfNetworkInterfaces; NetworkInterfaceIndex++) {
+
+    ThisRedfishDiscoveredToken = (EFI_REDFISH_DISCOVERED_TOKEN *)AllocateZeroPool(sizeof (EFI_REDFISH_DISCOVERED_TOKEN));
+    if (ThisRedfishDiscoveredToken == NULL) {
     DEBUG ((DEBUG_ERROR, "%a: Not enough memory for EFI_REDFISH_DISCOVERED_TOKEN.\n", __func__));
     return;
   }

-  ThisNetworkInterface       = gNetworkInterfaceInstances;
-  ThisRedfishDiscoveredToken = gRedfishDiscoveredToken;
-  //
-  // Loop to discover Redfish service on each network interface.
-  //
-  for (NetworkInterfaceIndex = 0; NetworkInterfaceIndex < gNumberOfNetworkInterfaces; NetworkInterfaceIndex++) {
+    ThisRedfishDiscoveredToken->Signature = REDFISH_DISCOVER_TOKEN_SIGNATURE;
+
     //
     // Initial this Redfish Discovered Token
     //
@@ -449,13 +435,11 @@ RedfishDiscoverProtocolInstalled (
                     &ThisRedfishDiscoveredToken->Event
                     );
     if (EFI_ERROR (Status)) {
+      FreePool(ThisRedfishDiscoveredToken);
       DEBUG ((DEBUG_ERROR, "%a: Failed to create event for Redfish discovered token.\n", __func__));
-      goto ErrorReturn;
+      return;
     }

-    ThisRedfishDiscoveredToken->Signature                         = REDFISH_DISCOVER_TOKEN_SIGNATURE;
-    ThisRedfishDiscoveredToken->DiscoverList.NumberOfServiceFound = 0;
-    ThisRedfishDiscoveredToken->DiscoverList.RedfishInstances     = NULL;
     //
     // Acquire for Redfish service which is reported by
     // Redfish Host Interface.
@@ -467,21 +451,23 @@ RedfishDiscoverProtocolInstalled (
                                             EFI_REDFISH_DISCOVER_HOST_INTERFACE,
                                             ThisRedfishDiscoveredToken
                                             );
-    ThisNetworkInterface++;
-    ThisRedfishDiscoveredToken++;
-  }

-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a: Acquire Redfish service fail.\n", __func__));
-    goto ErrorReturn;
+    //
+    // Free Redfish Discovered Token if Discover Instance was not created and
+    // Redfish Service Discovered Callback event was not triggered.
+    //
+    if(ThisRedfishDiscoveredToken->DiscoverList.NumberOfServiceFound == 0 ||
+        EFI_ERROR(ThisRedfishDiscoveredToken->DiscoverList.RedfishInstances->Status)){
+      gBS->CloseEvent (ThisRedfishDiscoveredToken->Event);
+      DEBUG ((DEBUG_ERROR, "%a: Free Redfish discovered token - %x.\n",
+          __func__, ThisRedfishDiscoveredToken));
+      FreePool(ThisRedfishDiscoveredToken);
+    }
+    ThisNetworkInterface++;
   }

   return;

-ErrorReturn:
-  if (gRedfishDiscoveredToken != NULL) {
-    FreePool (gRedfishDiscoveredToken);
-  }
 }

 /**
@@ -498,25 +484,10 @@ RedfishConfigHandlerDriverUnload (
   IN EFI_HANDLE  ImageHandle
   )
 {
-  EFI_REDFISH_DISCOVERED_TOKEN  *ThisRedfishDiscoveredToken;
-  UINTN                         NumberOfNetworkInterfacesIndex;

   RedfishConfigDriverCommonUnload (ImageHandle);

   RedfishConfigStopRedfishDiscovery ();
-  if (gRedfishDiscoveredToken != NULL) {
-    ThisRedfishDiscoveredToken = gRedfishDiscoveredToken;
-    for (NumberOfNetworkInterfacesIndex = 0; NumberOfNetworkInterfacesIndex < gNumberOfNetworkInterfaces; NumberOfNetworkInterfacesIndex++) {
-      if (ThisRedfishDiscoveredToken->Event != NULL) {
-        gBS->CloseEvent (ThisRedfishDiscoveredToken->Event);
-      }
-
-      FreePool (ThisRedfishDiscoveredToken);
-      ThisRedfishDiscoveredToken++;
-    }
-
-    gRedfishDiscoveredToken = NULL;
-  }

   return EFI_SUCCESS;
 }
diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
index 583c6f7..a6c8d60 100644
--- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
+++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
@@ -1095,8 +1095,10 @@ RedfishServiceGetNetworkInterface (
 {
   EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL  *ThisNetworkInterfaceIntn;
   EFI_REDFISH_DISCOVER_NETWORK_INTERFACE           *ThisNetworkInterface;
+  EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL   *RestExInstance;

-  if ((NetworkIntfInstances == NULL) || (NumberOfNetworkIntfs == NULL) || (ImageHandle == NULL)) {
+  if ((This == NULL) || (NetworkIntfInstances == NULL) || (NumberOfNetworkIntfs == NULL) ||
+      (ImageHandle == NULL)) {
     return EFI_INVALID_PARAMETER;
   }

@@ -1107,14 +1109,30 @@ RedfishServiceGetNetworkInterface (
     return EFI_NOT_FOUND;
   }

+  RestExInstance = EFI_REDFISH_DISOVER_DATA_FROM_DISCOVER_PROTOCOL(This);
+
+  //
+  // Check the new found network interface.
+  //
+  if (RestExInstance->NetworkInterfaceInstances != NULL) {
+    FreePool (RestExInstance->NetworkInterfaceInstances);
+    RestExInstance->NetworkInterfaceInstances = NULL;
+  }
+
   ThisNetworkInterface = (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE *)AllocateZeroPool (sizeof (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE) * mNumNetworkInterface);
   if (ThisNetworkInterface == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }

   *NetworkIntfInstances    = ThisNetworkInterface;
+
+
+  RestExInstance->NetworkInterfaceInstances = ThisNetworkInterface;
+  RestExInstance->NumberOfNetworkInterfaces = 0;
+
   ThisNetworkInterfaceIntn = (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetFirstNode (&mEfiRedfishDiscoverNetworkInterface);
   while (TRUE) {
+
     ThisNetworkInterface->IsIpv6 = FALSE;
     if (CheckIsIpVersion6 (ThisNetworkInterfaceIntn)) {
       ThisNetworkInterface->IsIpv6 = TRUE;
@@ -1130,7 +1148,7 @@ RedfishServiceGetNetworkInterface (

     ThisNetworkInterface->SubnetPrefixLength = ThisNetworkInterfaceIntn->SubnetPrefixLength;
     ThisNetworkInterface->VlanId             = ThisNetworkInterfaceIntn->VlanId;
-    (*NumberOfNetworkIntfs)++;
+    RestExInstance->NumberOfNetworkInterfaces++;
     if (IsNodeAtEnd (&mEfiRedfishDiscoverNetworkInterface, &ThisNetworkInterfaceIntn->Entry)) {
       break;
     }
@@ -1139,6 +1157,8 @@ RedfishServiceGetNetworkInterface (
     ThisNetworkInterface++;
   }

+  *NumberOfNetworkIntfs = RestExInstance->NumberOfNetworkInterfaces;
+
   return EFI_SUCCESS;
 }

@@ -1178,7 +1198,6 @@ RedfishServiceAcquireService (
 {
   EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE         *Instance;
   EFI_STATUS                                       Status1;
-  EFI_STATUS                                       Status2;
   BOOLEAN                                          NewInstance;
   UINTN                                            NumNetworkInterfaces;
   UINTN                                            NetworkInterfacesIndex;
@@ -1215,7 +1234,6 @@ RedfishServiceAcquireService (

   for (NetworkInterfacesIndex = 0; NetworkInterfacesIndex < NumNetworkInterfaces; NetworkInterfacesIndex++) {
     Status1     = EFI_SUCCESS;
-    Status2     = EFI_SUCCESS;
     NewInstance = FALSE;
     Instance    = GetInstanceByOwner (ImageHandle, TargetNetworkInterfaceInternal, Flags & ~EFI_REDFISH_DISCOVER_VALIDATION); // Check if we can re-use previous instance.
     if (Instance == NULL) {
@@ -1223,6 +1241,7 @@ RedfishServiceAcquireService (
       Instance = (EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE *)AllocateZeroPool (sizeof (EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE));
       if (Instance == NULL) {
         DEBUG ((DEBUG_ERROR, "%a:Memory allocation fail.\n", __func__));
+        return EFI_OUT_OF_RESOURCES;
       }

       InitializeListHead (&Instance->Entry);
@@ -1258,9 +1277,11 @@ RedfishServiceAcquireService (
       DEBUG ((DEBUG_ERROR, "%a:Redfish service discovery through SSDP is not supported\n", __func__));
       return EFI_UNSUPPORTED;
     } else {
-      if (EFI_ERROR (Status1) && EFI_ERROR (Status2)) {
-        FreePool ((VOID *)Instance);
-        DEBUG ((DEBUG_ERROR, "%a:Something wrong on Redfish service discovery Status1=%x, Status2=%x.\n", __func__, Status1, Status2));
+      if (EFI_ERROR (Status1)) {
+        if (NewInstance) {
+          FreePool ((VOID *)Instance);
+        }
+        DEBUG ((DEBUG_ERROR, "%a:Something wrong on Redfish service discovery Status1=%r.\n", __func__, Status1));
       } else {
         if (NewInstance) {
           InsertTailList (&mRedfishDiscoverList, &Instance->Entry);
@@ -1387,13 +1408,6 @@ ReleaseNext:;
   }
 }

-EFI_REDFISH_DISCOVER_PROTOCOL  mRedfishDiscover = {
-  RedfishServiceGetNetworkInterface,
-  RedfishServiceAcquireService,
-  RedfishServiceAbortAcquire,
-  RedfishServiceReleaseService
-};
-
 /**
   This function create an EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL for the
   given network interface.
@@ -1713,12 +1727,20 @@ BuildupNetworkInterface (

           NewNetworkInterfaceInstalled                       = FALSE;
           NetworkInterface->EfiRedfishDiscoverProtocolHandle = NULL;
-          Status                                             = gBS->InstallProtocolInterface (
-                                                                      &NetworkInterface->EfiRedfishDiscoverProtocolHandle,
-                                                                      &gEfiRedfishDiscoverProtocolGuid,
-                                                                      EFI_NATIVE_INTERFACE,
-                                                                      (VOID *)&mRedfishDiscover
-                                                                      );
+
+          RestExInstance->Signature = EFI_REDFISH_DISCOVER_DATA_SIGNATURE;
+
+          RestExInstance->RedfishDiscoverProtocol.GetNetworkInterfaceList = RedfishServiceGetNetworkInterface;
+          RestExInstance->RedfishDiscoverProtocol.AcquireRedfishService = RedfishServiceAcquireService;
+          RestExInstance->RedfishDiscoverProtocol.AbortAcquireRedfishService = RedfishServiceAbortAcquire;
+          RestExInstance->RedfishDiscoverProtocol.ReleaseRedfishService = RedfishServiceReleaseService;
+
+          Status = gBS->InstallProtocolInterface (
+                          &NetworkInterface->EfiRedfishDiscoverProtocolHandle,
+                          &gEfiRedfishDiscoverProtocolGuid,
+                          EFI_NATIVE_INTERFACE,
+                          (VOID *)&RestExInstance->RedfishDiscoverProtocol
+                          );
           if (EFI_ERROR (Status)) {
             DEBUG ((DEBUG_ERROR, "%a: Fail to install EFI_REDFISH_DISCOVER_PROTOCOL\n", __func__));
           }
@@ -1815,6 +1837,7 @@ StopServiceOnNetworkInterface (
   EFI_HANDLE                                       DiscoverProtocolHandle;
   EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL  *ThisNetworkInterface;
   EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL   *RestExInstance;
+  EFI_REDFISH_DISCOVER_PROTOCOL                    *RedfishDiscoverProtocol;

   for (Index = 0; Index < (sizeof (gRequiredProtocol) / sizeof (REDFISH_DISCOVER_REQUIRED_PROTOCOL)); Index++) {
     Status = gBS->HandleProtocol (
@@ -1854,12 +1877,30 @@ StopServiceOnNetworkInterface (
             // client which uses .EFI Redfish discover protocol.
             //
             if (DiscoverProtocolHandle != NULL) {
-              gBS->DisconnectController (DiscoverProtocolHandle, NULL, NULL);
-              Status = gBS->UninstallProtocolInterface (
+
+              Status = gBS->HandleProtocol (
                               DiscoverProtocolHandle,
                               &gEfiRedfishDiscoverProtocolGuid,
-                              (VOID *)&mRedfishDiscover
+                              (VOID **) &RedfishDiscoverProtocol
                               );
+              if (!EFI_ERROR (Status)) {
+                RestExInstance = EFI_REDFISH_DISOVER_DATA_FROM_DISCOVER_PROTOCOL(RedfishDiscoverProtocol);
+                //
+                // Stop Redfish service discovery.
+                //
+                RedfishDiscoverProtocol->AbortAcquireRedfishService (
+                                           RedfishDiscoverProtocol,
+                                           RestExInstance->NetworkInterfaceInstances
+                                           );
+
+
+                gBS->DisconnectController (DiscoverProtocolHandle, NULL, NULL);
+                Status = gBS->UninstallProtocolInterface (
+                                DiscoverProtocolHandle,
+                                &gEfiRedfishDiscoverProtocolGuid,
+                                (VOID *)&RestExInstance->RedfishDiscoverProtocol
+                                );
+              }
             }

             return Status;
diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
index 2704cd9..ddb5f2e 100644
--- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
+++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
@@ -118,16 +118,30 @@ typedef struct {
 } EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL;

 //
+// Redfish Discover Instance signature
+//
+
+#define EFI_REDFISH_DISCOVER_DATA_SIGNATURE   SIGNATURE_32 ('E', 'R', 'D', 'D')
+
+#define EFI_REDFISH_DISOVER_DATA_FROM_DISCOVER_PROTOCOL(a) \
+  CR (a, EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL, RedfishDiscoverProtocol, EFI_REDFISH_DISCOVER_DATA_SIGNATURE)
+
+//
 // Internal structure used to maintain REST EX properties.
 //
 typedef struct {
-  LIST_ENTRY              Entry;                      ///< Link list entry.
-  EFI_HANDLE              OpenDriverAgentHandle;      ///< The agent to open network protocol.
-  EFI_HANDLE              OpenDriverControllerHandle; ///< The controller handle to open network protocol.
-  EFI_HANDLE              RestExChildHandle;          ///< The child handle created through REST EX Service Protocol.
-  EFI_HANDLE              RestExControllerHandle;     ///< The controller handle which provide REST EX protocol.
-  EFI_REST_EX_PROTOCOL    *RestExProtocolInterface;   ///< Pointer to EFI_REST_EX_PROTOCOL.
-  UINT32                  RestExId;                   ///< The identifier installed on REST EX controller handle.
+  LIST_ENTRY                              Entry;                      ///< Link list entry.
+  UINT32                                  Signature;                  ///< Instance signature.
+  EFI_HANDLE                              OpenDriverAgentHandle;      ///< The agent to open network protocol.
+  EFI_HANDLE                              OpenDriverControllerHandle; ///< The controller handle to open network protocol.
+  EFI_HANDLE                              RestExChildHandle;          ///< The child handle created through REST EX Service Protocol.
+  EFI_HANDLE                              RestExControllerHandle;     ///< The controller handle which provide REST EX protocol.
+  EFI_REST_EX_PROTOCOL                    *RestExProtocolInterface;   ///< Pointer to EFI_REST_EX_PROTOCOL.
+  UINT32                                  RestExId;                   ///< The identifier installed on REST EX controller handle.
+  UINTN                                   NumberOfNetworkInterfaces;  ///< Number of network interfaces can do Redfish service discovery.
+  EFI_REDFISH_DISCOVER_NETWORK_INTERFACE  *NetworkInterfaceInstances; ///< Network interface instances. It's an array of instances. The number of entries
+                                                                      ///< in array is indicated by NumberOfNetworkInterfaces.
+  EFI_REDFISH_DISCOVER_PROTOCOL           RedfishDiscoverProtocol;    ///< EFI_REDFISH_DISCOVER_PROTOCOL protocol.
 } EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL;

 /**
--
2.6.1.windows.1
-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103223): https://edk2.groups.io/g/devel/message/103223
Mute This Topic: https://groups.io/mt/98368593/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] RedfishPkg: Remove the global variables related to Discover Token functionality
Posted by Chang, Abner via groups.io 1 year ago
[AMD Official Use Only - General]

Hi Igor,
I have no problem with this change, however some upstream practices here,

- Please shorten the subject to <= 76
- Each line in the commit message should be <=76
You can run Patchcheck.py (here: BaseTools\Scripts) before you sending out the patches.

Please run UncrustiyCheck for each source code you modified, . pytool/Plugin/UncrustifyCheck/ and used for beautifying the source code.
You can get the binary from here: https://sourceforge.net/projects/uncrustify/files.

Please resend the patch with above issue fixed.

BTW, the patch you sent to group.io still has a problem. The white space of each blank line in the patch file were gone (you can take a look at your message sent to group.io). This leads to a failure of patch apply. I recover those white space in the blank lines and the patch can be applied. However, I can do this just for a short patch. 😊 Please figure it out at your end.  That's fine if this problem still exist on your resent patch in case you need some time to figure out the email problem.

Thanks
Abner

> -----Original Message-----
> From: Igor Kulchytskyy <igork@ami.com>
> Sent: Wednesday, April 19, 2023 11:04 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; Nickle Wang
> <nicklew@nvidia.com>
> Subject: [PATCH] RedfishPkg: Remove the global variables related to
> Discover Token functionality
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> gRedfishDiscoveredToken may be allocated several times, if multiple NIC
> installed on the system.
> To avoid this issue Discover Token related global variables replaced with the
> local variables.
> 
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Signed-off-by: Igor Kulchytskyy <igork@ami.com>
> ---
>  RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c | 139
> ++++++++------------
>  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c           |  87 ++++++++-
> ---
>  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h      |  28 +++-
>  3 files changed, 140 insertions(+), 114 deletions(-)
> 
> diff --git a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> index 993ad33..8f85491 100644
> --- a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> +++ b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> @@ -22,12 +22,6 @@ EFI_HANDLE
> gEfiRedfishDiscoverControllerHandle = NULL;
>  EFI_REDFISH_DISCOVER_PROTOCOL  *gEfiRedfishDiscoverProtocol        =
> NULL;
>  BOOLEAN                        gRedfishDiscoverActivated           = FALSE;
>  BOOLEAN                        gRedfishServiceDiscovered           = FALSE;
> -//
> -// Network interfaces discovered by EFI Redfish Discover Protocol.
> -//
> -UINTN                                   gNumberOfNetworkInterfaces;
> -EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> *gNetworkInterfaceInstances = NULL;
> -EFI_REDFISH_DISCOVERED_TOKEN            *gRedfishDiscoveredToken    =
> NULL;
> 
>  ///
>  /// Driver Binding Protocol instance
> @@ -58,13 +52,6 @@ RedfishConfigStopRedfishDiscovery (
>        gBS->CloseEvent (gEfiRedfishDiscoverProtocolEvent);
>      }
> 
> -    //
> -    // Stop Redfish service discovery.
> -    //
> -    gEfiRedfishDiscoverProtocol->AbortAcquireRedfishService (
> -                                   gEfiRedfishDiscoverProtocol,
> -                                   gNetworkInterfaceInstances
> -                                   );
>      gEfiRedfishDiscoverControllerHandle = NULL;
>      gEfiRedfishDiscoverProtocol         = NULL;
>      gRedfishDiscoverActivated           = FALSE;
> @@ -318,36 +305,39 @@ RedfishServiceDiscoveredCallback (
>    EFI_REDFISH_DISCOVERED_TOKEN     *RedfishDiscoveredToken;
>    EFI_REDFISH_DISCOVERED_INSTANCE  *RedfishInstance;
> 
> -  if (gRedfishServiceDiscovered) {
> -    //
> -    // Only support one Redfish service on platform.
> -    //
> -    return;
> -  }
> -
>    RedfishDiscoveredToken = (EFI_REDFISH_DISCOVERED_TOKEN *)Context;
> -  RedfishInstance        = RedfishDiscoveredToken-
> >DiscoverList.RedfishInstances;
> +  gBS->CloseEvent (RedfishDiscoveredToken->Event);
> +
>    //
> -  // Only pick up the first found Redfish service.
> +  // Only support one Redfish service on platform.
>    //
> -  if (RedfishInstance->Status == EFI_SUCCESS) {
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceRestExHandle =
> RedfishInstance->Information.RedfishRestExHandle;
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceVersion      =
> RedfishInstance->Information.RedfishVersion;
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceLocation     =
> RedfishInstance->Information.Location;
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceUuid         =
> RedfishInstance->Information.Uuid;
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceOs           =
> RedfishInstance->Information.Os;
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceOsVersion    =
> RedfishInstance->Information.OsVersion;
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceProduct      =
> RedfishInstance->Information.Product;
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceProductVer   =
> RedfishInstance->Information.ProductVer;
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceUseHttps     =
> RedfishInstance->Information.UseHttps;
> -    gRedfishServiceDiscovered                                        = TRUE;
> +  if (!gRedfishServiceDiscovered) {
> +    RedfishInstance        = RedfishDiscoveredToken-
> >DiscoverList.RedfishInstances;
> +    //
> +    // Only pick up the first found Redfish service.
> +    //
> +    if (RedfishInstance->Status == EFI_SUCCESS) {
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceRestExHandle =
> RedfishInstance->Information.RedfishRestExHandle;
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceVersion      =
> RedfishInstance->Information.RedfishVersion;
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceLocation     =
> RedfishInstance->Information.Location;
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceUuid         =
> RedfishInstance->Information.Uuid;
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceOs           =
> RedfishInstance->Information.Os;
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceOsVersion    =
> RedfishInstance->Information.OsVersion;
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceProduct      =
> RedfishInstance->Information.Product;
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceProductVer   =
> RedfishInstance->Information.ProductVer;
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceUseHttps     =
> RedfishInstance->Information.UseHttps;
> +      gRedfishServiceDiscovered                                        = TRUE;
> +    }
> +
> +    //
> +    // Invoke RedfishConfigHandlerInstalledCallback to execute
> +    // the initialization of Redfish Configure Handler instance.
> +    //
> +    RedfishConfigHandlerInstalledCallback (gRedfishConfigData.Event,
> + &gRedfishConfigData);
>    }
> 
> -  //
> -  // Invoke RedfishConfigHandlerInstalledCallback to execute
> -  // the initialization of Redfish Configure Handler instance.
> -  //
> -  RedfishConfigHandlerInstalledCallback (gRedfishConfigData.Event,
> &gRedfishConfigData);
> +  FreePool(RedfishDiscoveredToken);
> +
>  }
> 
>  /**
> @@ -371,6 +361,7 @@ RedfishDiscoverProtocolInstalled (
>    UINTN                                   NetworkInterfaceIndex;
>    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE  *ThisNetworkInterface;
>    EFI_REDFISH_DISCOVERED_TOKEN            *ThisRedfishDiscoveredToken;
> +  UINTN                                   NumberOfNetworkInterfaces;
> 
>    DEBUG ((DEBUG_INFO, "%a: New network interface is installed on system
> by EFI Redfish discover driver.\n", __func__));
> 
> @@ -408,36 +399,31 @@ RedfishDiscoverProtocolInstalled (
>      }
>    }
> 
> -  //
> -  // Check the new found network interface.
> -  //
> -  if (gNetworkInterfaceInstances != NULL) {
> -    FreePool (gNetworkInterfaceInstances);
> -  }
> 
>    Status = gEfiRedfishDiscoverProtocol->GetNetworkInterfaceList (
>                                            gEfiRedfishDiscoverProtocol,
>                                            gRedfishConfigData.Image,
> -                                          &gNumberOfNetworkInterfaces,
> -                                          &gNetworkInterfaceInstances
> +                                          &NumberOfNetworkInterfaces,
> +                                          &ThisNetworkInterface
>                                            );
> -  if (EFI_ERROR (Status) || (gNumberOfNetworkInterfaces == 0)) {
> +  if (EFI_ERROR (Status) || (NumberOfNetworkInterfaces == 0)) {
>      DEBUG ((DEBUG_ERROR, "%a: No network interfaces found on the
> handle.\n", __func__));
>      return;
>    }
> 
> -  gRedfishDiscoveredToken = AllocateZeroPool
> (gNumberOfNetworkInterfaces * sizeof
> (EFI_REDFISH_DISCOVERED_TOKEN));
> -  if (gRedfishDiscoveredToken == NULL) {
> +  //
> +  // Loop to discover Redfish service on each network interface.
> +  //
> +  for (NetworkInterfaceIndex = 0; NetworkInterfaceIndex <
> + NumberOfNetworkInterfaces; NetworkInterfaceIndex++) {
> +
> +    ThisRedfishDiscoveredToken = (EFI_REDFISH_DISCOVERED_TOKEN
> *)AllocateZeroPool(sizeof (EFI_REDFISH_DISCOVERED_TOKEN));
> +    if (ThisRedfishDiscoveredToken == NULL) {
>      DEBUG ((DEBUG_ERROR, "%a: Not enough memory for
> EFI_REDFISH_DISCOVERED_TOKEN.\n", __func__));
>      return;
>    }
> 
> -  ThisNetworkInterface       = gNetworkInterfaceInstances;
> -  ThisRedfishDiscoveredToken = gRedfishDiscoveredToken;
> -  //
> -  // Loop to discover Redfish service on each network interface.
> -  //
> -  for (NetworkInterfaceIndex = 0; NetworkInterfaceIndex <
> gNumberOfNetworkInterfaces; NetworkInterfaceIndex++) {
> +    ThisRedfishDiscoveredToken->Signature =
> + REDFISH_DISCOVER_TOKEN_SIGNATURE;
> +
>      //
>      // Initial this Redfish Discovered Token
>      //
> @@ -449,13 +435,11 @@ RedfishDiscoverProtocolInstalled (
>                      &ThisRedfishDiscoveredToken->Event
>                      );
>      if (EFI_ERROR (Status)) {
> +      FreePool(ThisRedfishDiscoveredToken);
>        DEBUG ((DEBUG_ERROR, "%a: Failed to create event for Redfish
> discovered token.\n", __func__));
> -      goto ErrorReturn;
> +      return;
>      }
> 
> -    ThisRedfishDiscoveredToken->Signature                         =
> REDFISH_DISCOVER_TOKEN_SIGNATURE;
> -    ThisRedfishDiscoveredToken->DiscoverList.NumberOfServiceFound = 0;
> -    ThisRedfishDiscoveredToken->DiscoverList.RedfishInstances     = NULL;
>      //
>      // Acquire for Redfish service which is reported by
>      // Redfish Host Interface.
> @@ -467,21 +451,23 @@ RedfishDiscoverProtocolInstalled (
>                                              EFI_REDFISH_DISCOVER_HOST_INTERFACE,
>                                              ThisRedfishDiscoveredToken
>                                              );
> -    ThisNetworkInterface++;
> -    ThisRedfishDiscoveredToken++;
> -  }
> 
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "%a: Acquire Redfish service fail.\n", __func__));
> -    goto ErrorReturn;
> +    //
> +    // Free Redfish Discovered Token if Discover Instance was not created
> and
> +    // Redfish Service Discovered Callback event was not triggered.
> +    //
> +    if(ThisRedfishDiscoveredToken->DiscoverList.NumberOfServiceFound ==
> 0 ||
> +        EFI_ERROR(ThisRedfishDiscoveredToken-
> >DiscoverList.RedfishInstances->Status)){
> +      gBS->CloseEvent (ThisRedfishDiscoveredToken->Event);
> +      DEBUG ((DEBUG_ERROR, "%a: Free Redfish discovered token - %x.\n",
> +          __func__, ThisRedfishDiscoveredToken));
> +      FreePool(ThisRedfishDiscoveredToken);
> +    }
> +    ThisNetworkInterface++;
>    }
> 
>    return;
> 
> -ErrorReturn:
> -  if (gRedfishDiscoveredToken != NULL) {
> -    FreePool (gRedfishDiscoveredToken);
> -  }
>  }
> 
>  /**
> @@ -498,25 +484,10 @@ RedfishConfigHandlerDriverUnload (
>    IN EFI_HANDLE  ImageHandle
>    )
>  {
> -  EFI_REDFISH_DISCOVERED_TOKEN  *ThisRedfishDiscoveredToken;
> -  UINTN                         NumberOfNetworkInterfacesIndex;
> 
>    RedfishConfigDriverCommonUnload (ImageHandle);
> 
>    RedfishConfigStopRedfishDiscovery ();
> -  if (gRedfishDiscoveredToken != NULL) {
> -    ThisRedfishDiscoveredToken = gRedfishDiscoveredToken;
> -    for (NumberOfNetworkInterfacesIndex = 0;
> NumberOfNetworkInterfacesIndex < gNumberOfNetworkInterfaces;
> NumberOfNetworkInterfacesIndex++) {
> -      if (ThisRedfishDiscoveredToken->Event != NULL) {
> -        gBS->CloseEvent (ThisRedfishDiscoveredToken->Event);
> -      }
> -
> -      FreePool (ThisRedfishDiscoveredToken);
> -      ThisRedfishDiscoveredToken++;
> -    }
> -
> -    gRedfishDiscoveredToken = NULL;
> -  }
> 
>    return EFI_SUCCESS;
>  }
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 583c6f7..a6c8d60 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -1095,8 +1095,10 @@ RedfishServiceGetNetworkInterface (  {
>    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> *ThisNetworkInterfaceIntn;
>    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> *ThisNetworkInterface;
> +  EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL   *RestExInstance;
> 
> -  if ((NetworkIntfInstances == NULL) || (NumberOfNetworkIntfs == NULL)
> || (ImageHandle == NULL)) {
> +  if ((This == NULL) || (NetworkIntfInstances == NULL) ||
> (NumberOfNetworkIntfs == NULL) ||
> +      (ImageHandle == NULL)) {
>      return EFI_INVALID_PARAMETER;
>    }
> 
> @@ -1107,14 +1109,30 @@ RedfishServiceGetNetworkInterface (
>      return EFI_NOT_FOUND;
>    }
> 
> +  RestExInstance =
> + EFI_REDFISH_DISOVER_DATA_FROM_DISCOVER_PROTOCOL(This);
> +
> +  //
> +  // Check the new found network interface.
> +  //
> +  if (RestExInstance->NetworkInterfaceInstances != NULL) {
> +    FreePool (RestExInstance->NetworkInterfaceInstances);
> +    RestExInstance->NetworkInterfaceInstances = NULL;  }
> +
>    ThisNetworkInterface = (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> *)AllocateZeroPool (sizeof (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE)
> * mNumNetworkInterface);
>    if (ThisNetworkInterface == NULL) {
>      return EFI_OUT_OF_RESOURCES;
>    }
> 
>    *NetworkIntfInstances    = ThisNetworkInterface;
> +
> +
> +  RestExInstance->NetworkInterfaceInstances = ThisNetworkInterface;
> + RestExInstance->NumberOfNetworkInterfaces = 0;
> +
>    ThisNetworkInterfaceIntn =
> (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetFirstNode
> (&mEfiRedfishDiscoverNetworkInterface);
>    while (TRUE) {
> +
>      ThisNetworkInterface->IsIpv6 = FALSE;
>      if (CheckIsIpVersion6 (ThisNetworkInterfaceIntn)) {
>        ThisNetworkInterface->IsIpv6 = TRUE; @@ -1130,7 +1148,7 @@
> RedfishServiceGetNetworkInterface (
> 
>      ThisNetworkInterface->SubnetPrefixLength = ThisNetworkInterfaceIntn-
> >SubnetPrefixLength;
>      ThisNetworkInterface->VlanId             = ThisNetworkInterfaceIntn->VlanId;
> -    (*NumberOfNetworkIntfs)++;
> +    RestExInstance->NumberOfNetworkInterfaces++;
>      if (IsNodeAtEnd (&mEfiRedfishDiscoverNetworkInterface,
> &ThisNetworkInterfaceIntn->Entry)) {
>        break;
>      }
> @@ -1139,6 +1157,8 @@ RedfishServiceGetNetworkInterface (
>      ThisNetworkInterface++;
>    }
> 
> +  *NumberOfNetworkIntfs = RestExInstance-
> >NumberOfNetworkInterfaces;
> +
>    return EFI_SUCCESS;
>  }
> 
> @@ -1178,7 +1198,6 @@ RedfishServiceAcquireService (  {
>    EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE         *Instance;
>    EFI_STATUS                                       Status1;
> -  EFI_STATUS                                       Status2;
>    BOOLEAN                                          NewInstance;
>    UINTN                                            NumNetworkInterfaces;
>    UINTN                                            NetworkInterfacesIndex;
> @@ -1215,7 +1234,6 @@ RedfishServiceAcquireService (
> 
>    for (NetworkInterfacesIndex = 0; NetworkInterfacesIndex <
> NumNetworkInterfaces; NetworkInterfacesIndex++) {
>      Status1     = EFI_SUCCESS;
> -    Status2     = EFI_SUCCESS;
>      NewInstance = FALSE;
>      Instance    = GetInstanceByOwner (ImageHandle,
> TargetNetworkInterfaceInternal, Flags &
> ~EFI_REDFISH_DISCOVER_VALIDATION); // Check if we can re-use previous
> instance.
>      if (Instance == NULL) {
> @@ -1223,6 +1241,7 @@ RedfishServiceAcquireService (
>        Instance = (EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE
> *)AllocateZeroPool (sizeof
> (EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE));
>        if (Instance == NULL) {
>          DEBUG ((DEBUG_ERROR, "%a:Memory allocation fail.\n", __func__));
> +        return EFI_OUT_OF_RESOURCES;
>        }
> 
>        InitializeListHead (&Instance->Entry); @@ -1258,9 +1277,11 @@
> RedfishServiceAcquireService (
>        DEBUG ((DEBUG_ERROR, "%a:Redfish service discovery through SSDP is
> not supported\n", __func__));
>        return EFI_UNSUPPORTED;
>      } else {
> -      if (EFI_ERROR (Status1) && EFI_ERROR (Status2)) {
> -        FreePool ((VOID *)Instance);
> -        DEBUG ((DEBUG_ERROR, "%a:Something wrong on Redfish service
> discovery Status1=%x, Status2=%x.\n", __func__, Status1, Status2));
> +      if (EFI_ERROR (Status1)) {
> +        if (NewInstance) {
> +          FreePool ((VOID *)Instance);
> +        }
> +        DEBUG ((DEBUG_ERROR, "%a:Something wrong on Redfish service
> + discovery Status1=%r.\n", __func__, Status1));
>        } else {
>          if (NewInstance) {
>            InsertTailList (&mRedfishDiscoverList, &Instance->Entry); @@ -1387,13
> +1408,6 @@ ReleaseNext:;
>    }
>  }
> 
> -EFI_REDFISH_DISCOVER_PROTOCOL  mRedfishDiscover = {
> -  RedfishServiceGetNetworkInterface,
> -  RedfishServiceAcquireService,
> -  RedfishServiceAbortAcquire,
> -  RedfishServiceReleaseService
> -};
> -
>  /**
>    This function create an
> EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL for the
>    given network interface.
> @@ -1713,12 +1727,20 @@ BuildupNetworkInterface (
> 
>            NewNetworkInterfaceInstalled                       = FALSE;
>            NetworkInterface->EfiRedfishDiscoverProtocolHandle = NULL;
> -          Status                                             = gBS->InstallProtocolInterface (
> -                                                                      &NetworkInterface-
> >EfiRedfishDiscoverProtocolHandle,
> -                                                                      &gEfiRedfishDiscoverProtocolGuid,
> -                                                                      EFI_NATIVE_INTERFACE,
> -                                                                      (VOID *)&mRedfishDiscover
> -                                                                      );
> +
> +          RestExInstance->Signature =
> + EFI_REDFISH_DISCOVER_DATA_SIGNATURE;
> +
> +          RestExInstance->RedfishDiscoverProtocol.GetNetworkInterfaceList =
> RedfishServiceGetNetworkInterface;
> +          RestExInstance->RedfishDiscoverProtocol.AcquireRedfishService =
> RedfishServiceAcquireService;
> +          RestExInstance->RedfishDiscoverProtocol.AbortAcquireRedfishService
> = RedfishServiceAbortAcquire;
> +          RestExInstance->RedfishDiscoverProtocol.ReleaseRedfishService
> + = RedfishServiceReleaseService;
> +
> +          Status = gBS->InstallProtocolInterface (
> +                          &NetworkInterface->EfiRedfishDiscoverProtocolHandle,
> +                          &gEfiRedfishDiscoverProtocolGuid,
> +                          EFI_NATIVE_INTERFACE,
> +                          (VOID *)&RestExInstance->RedfishDiscoverProtocol
> +                          );
>            if (EFI_ERROR (Status)) {
>              DEBUG ((DEBUG_ERROR, "%a: Fail to install
> EFI_REDFISH_DISCOVER_PROTOCOL\n", __func__));
>            }
> @@ -1815,6 +1837,7 @@ StopServiceOnNetworkInterface (
>    EFI_HANDLE                                       DiscoverProtocolHandle;
>    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> *ThisNetworkInterface;
>    EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL   *RestExInstance;
> +  EFI_REDFISH_DISCOVER_PROTOCOL                    *RedfishDiscoverProtocol;
> 
>    for (Index = 0; Index < (sizeof (gRequiredProtocol) / sizeof
> (REDFISH_DISCOVER_REQUIRED_PROTOCOL)); Index++) {
>      Status = gBS->HandleProtocol (
> @@ -1854,12 +1877,30 @@ StopServiceOnNetworkInterface (
>              // client which uses .EFI Redfish discover protocol.
>              //
>              if (DiscoverProtocolHandle != NULL) {
> -              gBS->DisconnectController (DiscoverProtocolHandle, NULL, NULL);
> -              Status = gBS->UninstallProtocolInterface (
> +
> +              Status = gBS->HandleProtocol (
>                                DiscoverProtocolHandle,
>                                &gEfiRedfishDiscoverProtocolGuid,
> -                              (VOID *)&mRedfishDiscover
> +                              (VOID **) &RedfishDiscoverProtocol
>                                );
> +              if (!EFI_ERROR (Status)) {
> +                RestExInstance =
> EFI_REDFISH_DISOVER_DATA_FROM_DISCOVER_PROTOCOL(RedfishDiscove
> rProtocol);
> +                //
> +                // Stop Redfish service discovery.
> +                //
> +                RedfishDiscoverProtocol->AbortAcquireRedfishService (
> +                                           RedfishDiscoverProtocol,
> +                                           RestExInstance->NetworkInterfaceInstances
> +                                           );
> +
> +
> +                gBS->DisconnectController (DiscoverProtocolHandle, NULL, NULL);
> +                Status = gBS->UninstallProtocolInterface (
> +                                DiscoverProtocolHandle,
> +                                &gEfiRedfishDiscoverProtocolGuid,
> +                                (VOID *)&RestExInstance->RedfishDiscoverProtocol
> +                                );
> +              }
>              }
> 
>              return Status;
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> index 2704cd9..ddb5f2e 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> @@ -118,16 +118,30 @@ typedef struct {
>  } EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL;
> 
>  //
> +// Redfish Discover Instance signature
> +//
> +
> +#define EFI_REDFISH_DISCOVER_DATA_SIGNATURE   SIGNATURE_32 ('E',
> 'R', 'D', 'D')
> +
> +#define EFI_REDFISH_DISOVER_DATA_FROM_DISCOVER_PROTOCOL(a) \
> +  CR (a, EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL,
> +RedfishDiscoverProtocol, EFI_REDFISH_DISCOVER_DATA_SIGNATURE)
> +
> +//
>  // Internal structure used to maintain REST EX properties.
>  //
>  typedef struct {
> -  LIST_ENTRY              Entry;                      ///< Link list entry.
> -  EFI_HANDLE              OpenDriverAgentHandle;      ///< The agent to open
> network protocol.
> -  EFI_HANDLE              OpenDriverControllerHandle; ///< The controller handle
> to open network protocol.
> -  EFI_HANDLE              RestExChildHandle;          ///< The child handle created
> through REST EX Service Protocol.
> -  EFI_HANDLE              RestExControllerHandle;     ///< The controller handle
> which provide REST EX protocol.
> -  EFI_REST_EX_PROTOCOL    *RestExProtocolInterface;   ///< Pointer to
> EFI_REST_EX_PROTOCOL.
> -  UINT32                  RestExId;                   ///< The identifier installed on REST EX
> controller handle.
> +  LIST_ENTRY                              Entry;                      ///< Link list entry.
> +  UINT32                                  Signature;                  ///< Instance signature.
> +  EFI_HANDLE                              OpenDriverAgentHandle;      ///< The agent to
> open network protocol.
> +  EFI_HANDLE                              OpenDriverControllerHandle; ///< The
> controller handle to open network protocol.
> +  EFI_HANDLE                              RestExChildHandle;          ///< The child handle
> created through REST EX Service Protocol.
> +  EFI_HANDLE                              RestExControllerHandle;     ///< The controller
> handle which provide REST EX protocol.
> +  EFI_REST_EX_PROTOCOL                    *RestExProtocolInterface;   ///< Pointer
> to EFI_REST_EX_PROTOCOL.
> +  UINT32                                  RestExId;                   ///< The identifier installed on
> REST EX controller handle.
> +  UINTN                                   NumberOfNetworkInterfaces;  ///< Number of
> network interfaces can do Redfish service discovery.
> +  EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> *NetworkInterfaceInstances; ///< Network interface instances. It's an array
> of instances. The number of entries
> +                                                                      ///< in array is indicated by
> NumberOfNetworkInterfaces.
> +  EFI_REDFISH_DISCOVER_PROTOCOL           RedfishDiscoverProtocol;    ///<
> EFI_REDFISH_DISCOVER_PROTOCOL protocol.
>  } EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL;
> 
>  /**
> --
> 2.6.1.windows.1
> -The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI). This communication is intended
> to be read only by the individual or entity to whom it is addressed or by their
> designee. If the reader of this message is not the intended recipient, you are
> on notice that any distribution of this message, in any form, is strictly
> prohibited. Please promptly notify the sender by reply e-mail or by
> telephone at 770-246-8600, and then delete or destroy all copies of the
> transmission.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103266): https://edk2.groups.io/g/devel/message/103266
Mute This Topic: https://groups.io/mt/98368593/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] RedfishPkg: Remove the global variables related to Discover Token functionality
Posted by Igor Kulchytskyy via groups.io 1 year ago
Hi Abner,
I did what you asked to do - Patchcheck.py and UncrustiyCheck (got binary).
Going to send PATCH V2.
But I'm not sure what happened with white space. Going to check with MIS.
Thank you,
Igor

-----Original Message-----
From: Chang, Abner <Abner.Chang@amd.com>
Sent: Wednesday, April 19, 2023 9:00 PM
To: Igor Kulchytskyy <igork@ami.com>; devel@edk2.groups.io
Cc: Nickle Wang <nicklew@nvidia.com>
Subject: [EXTERNAL] RE: [PATCH] RedfishPkg: Remove the global variables related to Discover Token functionality


**CAUTION: The e-mail below is from an external source. Please exercise caution before opening attachments, clicking links, or following guidance.**

[AMD Official Use Only - General]

Hi Igor,
I have no problem with this change, however some upstream practices here,

- Please shorten the subject to <= 76
- Each line in the commit message should be <=76
You can run Patchcheck.py (here: BaseTools\Scripts) before you sending out the patches.

Please run UncrustiyCheck for each source code you modified, . pytool/Plugin/UncrustifyCheck/ and used for beautifying the source code.
You can get the binary from here: https://sourceforge.net/projects/uncrustify/files.

Please resend the patch with above issue fixed.

BTW, the patch you sent to group.io still has a problem. The white space of each blank line in the patch file were gone (you can take a look at your message sent to group.io). This leads to a failure of patch apply. I recover those white space in the blank lines and the patch can be applied. However, I can do this just for a short patch. 😊 Please figure it out at your end.  That's fine if this problem still exist on your resent patch in case you need some time to figure out the email problem.

Thanks
Abner

> -----Original Message-----
> From: Igor Kulchytskyy <igork@ami.com>
> Sent: Wednesday, April 19, 2023 11:04 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; Nickle Wang
> <nicklew@nvidia.com>
> Subject: [PATCH] RedfishPkg: Remove the global variables related to
> Discover Token functionality
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> gRedfishDiscoveredToken may be allocated several times, if multiple NIC
> installed on the system.
> To avoid this issue Discover Token related global variables replaced with the
> local variables.
>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Signed-off-by: Igor Kulchytskyy <igork@ami.com>
> ---
>  RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c | 139
> ++++++++------------
>  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c           |  87 ++++++++-
> ---
>  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h      |  28 +++-
>  3 files changed, 140 insertions(+), 114 deletions(-)
>
> diff --git a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> index 993ad33..8f85491 100644
> --- a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> +++ b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> @@ -22,12 +22,6 @@ EFI_HANDLE
> gEfiRedfishDiscoverControllerHandle = NULL;
>  EFI_REDFISH_DISCOVER_PROTOCOL  *gEfiRedfishDiscoverProtocol        =
> NULL;
>  BOOLEAN                        gRedfishDiscoverActivated           = FALSE;
>  BOOLEAN                        gRedfishServiceDiscovered           = FALSE;
> -//
> -// Network interfaces discovered by EFI Redfish Discover Protocol.
> -//
> -UINTN                                   gNumberOfNetworkInterfaces;
> -EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> *gNetworkInterfaceInstances = NULL;
> -EFI_REDFISH_DISCOVERED_TOKEN            *gRedfishDiscoveredToken    =
> NULL;
>
>  ///
>  /// Driver Binding Protocol instance
> @@ -58,13 +52,6 @@ RedfishConfigStopRedfishDiscovery (
>        gBS->CloseEvent (gEfiRedfishDiscoverProtocolEvent);
>      }
>
> -    //
> -    // Stop Redfish service discovery.
> -    //
> -    gEfiRedfishDiscoverProtocol->AbortAcquireRedfishService (
> -                                   gEfiRedfishDiscoverProtocol,
> -                                   gNetworkInterfaceInstances
> -                                   );
>      gEfiRedfishDiscoverControllerHandle = NULL;
>      gEfiRedfishDiscoverProtocol         = NULL;
>      gRedfishDiscoverActivated           = FALSE;
> @@ -318,36 +305,39 @@ RedfishServiceDiscoveredCallback (
>    EFI_REDFISH_DISCOVERED_TOKEN     *RedfishDiscoveredToken;
>    EFI_REDFISH_DISCOVERED_INSTANCE  *RedfishInstance;
>
> -  if (gRedfishServiceDiscovered) {
> -    //
> -    // Only support one Redfish service on platform.
> -    //
> -    return;
> -  }
> -
>    RedfishDiscoveredToken = (EFI_REDFISH_DISCOVERED_TOKEN *)Context;
> -  RedfishInstance        = RedfishDiscoveredToken-
> >DiscoverList.RedfishInstances;
> +  gBS->CloseEvent (RedfishDiscoveredToken->Event);
> +
>    //
> -  // Only pick up the first found Redfish service.
> +  // Only support one Redfish service on platform.
>    //
> -  if (RedfishInstance->Status == EFI_SUCCESS) {
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceRestExHandle =
> RedfishInstance->Information.RedfishRestExHandle;
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceVersion      =
> RedfishInstance->Information.RedfishVersion;
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceLocation     =
> RedfishInstance->Information.Location;
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceUuid         =
> RedfishInstance->Information.Uuid;
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceOs           =
> RedfishInstance->Information.Os;
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceOsVersion    =
> RedfishInstance->Information.OsVersion;
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceProduct      =
> RedfishInstance->Information.Product;
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceProductVer   =
> RedfishInstance->Information.ProductVer;
> -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceUseHttps     =
> RedfishInstance->Information.UseHttps;
> -    gRedfishServiceDiscovered                                        = TRUE;
> +  if (!gRedfishServiceDiscovered) {
> +    RedfishInstance        = RedfishDiscoveredToken-
> >DiscoverList.RedfishInstances;
> +    //
> +    // Only pick up the first found Redfish service.
> +    //
> +    if (RedfishInstance->Status == EFI_SUCCESS) {
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceRestExHandle =
> RedfishInstance->Information.RedfishRestExHandle;
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceVersion      =
> RedfishInstance->Information.RedfishVersion;
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceLocation     =
> RedfishInstance->Information.Location;
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceUuid         =
> RedfishInstance->Information.Uuid;
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceOs           =
> RedfishInstance->Information.Os;
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceOsVersion    =
> RedfishInstance->Information.OsVersion;
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceProduct      =
> RedfishInstance->Information.Product;
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceProductVer   =
> RedfishInstance->Information.ProductVer;
> +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceUseHttps     =
> RedfishInstance->Information.UseHttps;
> +      gRedfishServiceDiscovered                                        = TRUE;
> +    }
> +
> +    //
> +    // Invoke RedfishConfigHandlerInstalledCallback to execute
> +    // the initialization of Redfish Configure Handler instance.
> +    //
> +    RedfishConfigHandlerInstalledCallback (gRedfishConfigData.Event,
> + &gRedfishConfigData);
>    }
>
> -  //
> -  // Invoke RedfishConfigHandlerInstalledCallback to execute
> -  // the initialization of Redfish Configure Handler instance.
> -  //
> -  RedfishConfigHandlerInstalledCallback (gRedfishConfigData.Event,
> &gRedfishConfigData);
> +  FreePool(RedfishDiscoveredToken);
> +
>  }
>
>  /**
> @@ -371,6 +361,7 @@ RedfishDiscoverProtocolInstalled (
>    UINTN                                   NetworkInterfaceIndex;
>    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE  *ThisNetworkInterface;
>    EFI_REDFISH_DISCOVERED_TOKEN            *ThisRedfishDiscoveredToken;
> +  UINTN                                   NumberOfNetworkInterfaces;
>
>    DEBUG ((DEBUG_INFO, "%a: New network interface is installed on system
> by EFI Redfish discover driver.\n", __func__));
>
> @@ -408,36 +399,31 @@ RedfishDiscoverProtocolInstalled (
>      }
>    }
>
> -  //
> -  // Check the new found network interface.
> -  //
> -  if (gNetworkInterfaceInstances != NULL) {
> -    FreePool (gNetworkInterfaceInstances);
> -  }
>
>    Status = gEfiRedfishDiscoverProtocol->GetNetworkInterfaceList (
>                                            gEfiRedfishDiscoverProtocol,
>                                            gRedfishConfigData.Image,
> -                                          &gNumberOfNetworkInterfaces,
> -                                          &gNetworkInterfaceInstances
> +                                          &NumberOfNetworkInterfaces,
> +                                          &ThisNetworkInterface
>                                            );
> -  if (EFI_ERROR (Status) || (gNumberOfNetworkInterfaces == 0)) {
> +  if (EFI_ERROR (Status) || (NumberOfNetworkInterfaces == 0)) {
>      DEBUG ((DEBUG_ERROR, "%a: No network interfaces found on the
> handle.\n", __func__));
>      return;
>    }
>
> -  gRedfishDiscoveredToken = AllocateZeroPool
> (gNumberOfNetworkInterfaces * sizeof
> (EFI_REDFISH_DISCOVERED_TOKEN));
> -  if (gRedfishDiscoveredToken == NULL) {
> +  //
> +  // Loop to discover Redfish service on each network interface.
> +  //
> +  for (NetworkInterfaceIndex = 0; NetworkInterfaceIndex <
> + NumberOfNetworkInterfaces; NetworkInterfaceIndex++) {
> +
> +    ThisRedfishDiscoveredToken = (EFI_REDFISH_DISCOVERED_TOKEN
> *)AllocateZeroPool(sizeof (EFI_REDFISH_DISCOVERED_TOKEN));
> +    if (ThisRedfishDiscoveredToken == NULL) {
>      DEBUG ((DEBUG_ERROR, "%a: Not enough memory for
> EFI_REDFISH_DISCOVERED_TOKEN.\n", __func__));
>      return;
>    }
>
> -  ThisNetworkInterface       = gNetworkInterfaceInstances;
> -  ThisRedfishDiscoveredToken = gRedfishDiscoveredToken;
> -  //
> -  // Loop to discover Redfish service on each network interface.
> -  //
> -  for (NetworkInterfaceIndex = 0; NetworkInterfaceIndex <
> gNumberOfNetworkInterfaces; NetworkInterfaceIndex++) {
> +    ThisRedfishDiscoveredToken->Signature =
> + REDFISH_DISCOVER_TOKEN_SIGNATURE;
> +
>      //
>      // Initial this Redfish Discovered Token
>      //
> @@ -449,13 +435,11 @@ RedfishDiscoverProtocolInstalled (
>                      &ThisRedfishDiscoveredToken->Event
>                      );
>      if (EFI_ERROR (Status)) {
> +      FreePool(ThisRedfishDiscoveredToken);
>        DEBUG ((DEBUG_ERROR, "%a: Failed to create event for Redfish
> discovered token.\n", __func__));
> -      goto ErrorReturn;
> +      return;
>      }
>
> -    ThisRedfishDiscoveredToken->Signature                         =
> REDFISH_DISCOVER_TOKEN_SIGNATURE;
> -    ThisRedfishDiscoveredToken->DiscoverList.NumberOfServiceFound = 0;
> -    ThisRedfishDiscoveredToken->DiscoverList.RedfishInstances     = NULL;
>      //
>      // Acquire for Redfish service which is reported by
>      // Redfish Host Interface.
> @@ -467,21 +451,23 @@ RedfishDiscoverProtocolInstalled (
>                                              EFI_REDFISH_DISCOVER_HOST_INTERFACE,
>                                              ThisRedfishDiscoveredToken
>                                              );
> -    ThisNetworkInterface++;
> -    ThisRedfishDiscoveredToken++;
> -  }
>
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "%a: Acquire Redfish service fail.\n", __func__));
> -    goto ErrorReturn;
> +    //
> +    // Free Redfish Discovered Token if Discover Instance was not created
> and
> +    // Redfish Service Discovered Callback event was not triggered.
> +    //
> +    if(ThisRedfishDiscoveredToken->DiscoverList.NumberOfServiceFound ==
> 0 ||
> +        EFI_ERROR(ThisRedfishDiscoveredToken-
> >DiscoverList.RedfishInstances->Status)){
> +      gBS->CloseEvent (ThisRedfishDiscoveredToken->Event);
> +      DEBUG ((DEBUG_ERROR, "%a: Free Redfish discovered token - %x.\n",
> +          __func__, ThisRedfishDiscoveredToken));
> +      FreePool(ThisRedfishDiscoveredToken);
> +    }
> +    ThisNetworkInterface++;
>    }
>
>    return;
>
> -ErrorReturn:
> -  if (gRedfishDiscoveredToken != NULL) {
> -    FreePool (gRedfishDiscoveredToken);
> -  }
>  }
>
>  /**
> @@ -498,25 +484,10 @@ RedfishConfigHandlerDriverUnload (
>    IN EFI_HANDLE  ImageHandle
>    )
>  {
> -  EFI_REDFISH_DISCOVERED_TOKEN  *ThisRedfishDiscoveredToken;
> -  UINTN                         NumberOfNetworkInterfacesIndex;
>
>    RedfishConfigDriverCommonUnload (ImageHandle);
>
>    RedfishConfigStopRedfishDiscovery ();
> -  if (gRedfishDiscoveredToken != NULL) {
> -    ThisRedfishDiscoveredToken = gRedfishDiscoveredToken;
> -    for (NumberOfNetworkInterfacesIndex = 0;
> NumberOfNetworkInterfacesIndex < gNumberOfNetworkInterfaces;
> NumberOfNetworkInterfacesIndex++) {
> -      if (ThisRedfishDiscoveredToken->Event != NULL) {
> -        gBS->CloseEvent (ThisRedfishDiscoveredToken->Event);
> -      }
> -
> -      FreePool (ThisRedfishDiscoveredToken);
> -      ThisRedfishDiscoveredToken++;
> -    }
> -
> -    gRedfishDiscoveredToken = NULL;
> -  }
>
>    return EFI_SUCCESS;
>  }
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 583c6f7..a6c8d60 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -1095,8 +1095,10 @@ RedfishServiceGetNetworkInterface (  {
>    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> *ThisNetworkInterfaceIntn;
>    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> *ThisNetworkInterface;
> +  EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL   *RestExInstance;
>
> -  if ((NetworkIntfInstances == NULL) || (NumberOfNetworkIntfs == NULL)
> || (ImageHandle == NULL)) {
> +  if ((This == NULL) || (NetworkIntfInstances == NULL) ||
> (NumberOfNetworkIntfs == NULL) ||
> +      (ImageHandle == NULL)) {
>      return EFI_INVALID_PARAMETER;
>    }
>
> @@ -1107,14 +1109,30 @@ RedfishServiceGetNetworkInterface (
>      return EFI_NOT_FOUND;
>    }
>
> +  RestExInstance =
> + EFI_REDFISH_DISOVER_DATA_FROM_DISCOVER_PROTOCOL(This);
> +
> +  //
> +  // Check the new found network interface.
> +  //
> +  if (RestExInstance->NetworkInterfaceInstances != NULL) {
> +    FreePool (RestExInstance->NetworkInterfaceInstances);
> +    RestExInstance->NetworkInterfaceInstances = NULL;  }
> +
>    ThisNetworkInterface = (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> *)AllocateZeroPool (sizeof (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE)
> * mNumNetworkInterface);
>    if (ThisNetworkInterface == NULL) {
>      return EFI_OUT_OF_RESOURCES;
>    }
>
>    *NetworkIntfInstances    = ThisNetworkInterface;
> +
> +
> +  RestExInstance->NetworkInterfaceInstances = ThisNetworkInterface;
> + RestExInstance->NumberOfNetworkInterfaces = 0;
> +
>    ThisNetworkInterfaceIntn =
> (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetFirstNode
> (&mEfiRedfishDiscoverNetworkInterface);
>    while (TRUE) {
> +
>      ThisNetworkInterface->IsIpv6 = FALSE;
>      if (CheckIsIpVersion6 (ThisNetworkInterfaceIntn)) {
>        ThisNetworkInterface->IsIpv6 = TRUE; @@ -1130,7 +1148,7 @@
> RedfishServiceGetNetworkInterface (
>
>      ThisNetworkInterface->SubnetPrefixLength = ThisNetworkInterfaceIntn-
> >SubnetPrefixLength;
>      ThisNetworkInterface->VlanId             = ThisNetworkInterfaceIntn->VlanId;
> -    (*NumberOfNetworkIntfs)++;
> +    RestExInstance->NumberOfNetworkInterfaces++;
>      if (IsNodeAtEnd (&mEfiRedfishDiscoverNetworkInterface,
> &ThisNetworkInterfaceIntn->Entry)) {
>        break;
>      }
> @@ -1139,6 +1157,8 @@ RedfishServiceGetNetworkInterface (
>      ThisNetworkInterface++;
>    }
>
> +  *NumberOfNetworkIntfs = RestExInstance-
> >NumberOfNetworkInterfaces;
> +
>    return EFI_SUCCESS;
>  }
>
> @@ -1178,7 +1198,6 @@ RedfishServiceAcquireService (  {
>    EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE         *Instance;
>    EFI_STATUS                                       Status1;
> -  EFI_STATUS                                       Status2;
>    BOOLEAN                                          NewInstance;
>    UINTN                                            NumNetworkInterfaces;
>    UINTN                                            NetworkInterfacesIndex;
> @@ -1215,7 +1234,6 @@ RedfishServiceAcquireService (
>
>    for (NetworkInterfacesIndex = 0; NetworkInterfacesIndex <
> NumNetworkInterfaces; NetworkInterfacesIndex++) {
>      Status1     = EFI_SUCCESS;
> -    Status2     = EFI_SUCCESS;
>      NewInstance = FALSE;
>      Instance    = GetInstanceByOwner (ImageHandle,
> TargetNetworkInterfaceInternal, Flags &
> ~EFI_REDFISH_DISCOVER_VALIDATION); // Check if we can re-use previous
> instance.
>      if (Instance == NULL) {
> @@ -1223,6 +1241,7 @@ RedfishServiceAcquireService (
>        Instance = (EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE
> *)AllocateZeroPool (sizeof
> (EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE));
>        if (Instance == NULL) {
>          DEBUG ((DEBUG_ERROR, "%a:Memory allocation fail.\n", __func__));
> +        return EFI_OUT_OF_RESOURCES;
>        }
>
>        InitializeListHead (&Instance->Entry); @@ -1258,9 +1277,11 @@
> RedfishServiceAcquireService (
>        DEBUG ((DEBUG_ERROR, "%a:Redfish service discovery through SSDP is
> not supported\n", __func__));
>        return EFI_UNSUPPORTED;
>      } else {
> -      if (EFI_ERROR (Status1) && EFI_ERROR (Status2)) {
> -        FreePool ((VOID *)Instance);
> -        DEBUG ((DEBUG_ERROR, "%a:Something wrong on Redfish service
> discovery Status1=%x, Status2=%x.\n", __func__, Status1, Status2));
> +      if (EFI_ERROR (Status1)) {
> +        if (NewInstance) {
> +          FreePool ((VOID *)Instance);
> +        }
> +        DEBUG ((DEBUG_ERROR, "%a:Something wrong on Redfish service
> + discovery Status1=%r.\n", __func__, Status1));
>        } else {
>          if (NewInstance) {
>            InsertTailList (&mRedfishDiscoverList, &Instance->Entry); @@ -1387,13
> +1408,6 @@ ReleaseNext:;
>    }
>  }
>
> -EFI_REDFISH_DISCOVER_PROTOCOL  mRedfishDiscover = {
> -  RedfishServiceGetNetworkInterface,
> -  RedfishServiceAcquireService,
> -  RedfishServiceAbortAcquire,
> -  RedfishServiceReleaseService
> -};
> -
>  /**
>    This function create an
> EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL for the
>    given network interface.
> @@ -1713,12 +1727,20 @@ BuildupNetworkInterface (
>
>            NewNetworkInterfaceInstalled                       = FALSE;
>            NetworkInterface->EfiRedfishDiscoverProtocolHandle = NULL;
> -          Status                                             = gBS->InstallProtocolInterface (
> -                                                                      &NetworkInterface-
> >EfiRedfishDiscoverProtocolHandle,
> -                                                                      &gEfiRedfishDiscoverProtocolGuid,
> -                                                                      EFI_NATIVE_INTERFACE,
> -                                                                      (VOID *)&mRedfishDiscover
> -                                                                      );
> +
> +          RestExInstance->Signature =
> + EFI_REDFISH_DISCOVER_DATA_SIGNATURE;
> +
> +          RestExInstance->RedfishDiscoverProtocol.GetNetworkInterfaceList =
> RedfishServiceGetNetworkInterface;
> +          RestExInstance->RedfishDiscoverProtocol.AcquireRedfishService =
> RedfishServiceAcquireService;
> +          RestExInstance->RedfishDiscoverProtocol.AbortAcquireRedfishService
> = RedfishServiceAbortAcquire;
> +          RestExInstance->RedfishDiscoverProtocol.ReleaseRedfishService
> + = RedfishServiceReleaseService;
> +
> +          Status = gBS->InstallProtocolInterface (
> +                          &NetworkInterface->EfiRedfishDiscoverProtocolHandle,
> +                          &gEfiRedfishDiscoverProtocolGuid,
> +                          EFI_NATIVE_INTERFACE,
> +                          (VOID *)&RestExInstance->RedfishDiscoverProtocol
> +                          );
>            if (EFI_ERROR (Status)) {
>              DEBUG ((DEBUG_ERROR, "%a: Fail to install
> EFI_REDFISH_DISCOVER_PROTOCOL\n", __func__));
>            }
> @@ -1815,6 +1837,7 @@ StopServiceOnNetworkInterface (
>    EFI_HANDLE                                       DiscoverProtocolHandle;
>    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> *ThisNetworkInterface;
>    EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL   *RestExInstance;
> +  EFI_REDFISH_DISCOVER_PROTOCOL                    *RedfishDiscoverProtocol;
>
>    for (Index = 0; Index < (sizeof (gRequiredProtocol) / sizeof
> (REDFISH_DISCOVER_REQUIRED_PROTOCOL)); Index++) {
>      Status = gBS->HandleProtocol (
> @@ -1854,12 +1877,30 @@ StopServiceOnNetworkInterface (
>              // client which uses .EFI Redfish discover protocol.
>              //
>              if (DiscoverProtocolHandle != NULL) {
> -              gBS->DisconnectController (DiscoverProtocolHandle, NULL, NULL);
> -              Status = gBS->UninstallProtocolInterface (
> +
> +              Status = gBS->HandleProtocol (
>                                DiscoverProtocolHandle,
>                                &gEfiRedfishDiscoverProtocolGuid,
> -                              (VOID *)&mRedfishDiscover
> +                              (VOID **) &RedfishDiscoverProtocol
>                                );
> +              if (!EFI_ERROR (Status)) {
> +                RestExInstance =
> EFI_REDFISH_DISOVER_DATA_FROM_DISCOVER_PROTOCOL(RedfishDiscove
> rProtocol);
> +                //
> +                // Stop Redfish service discovery.
> +                //
> +                RedfishDiscoverProtocol->AbortAcquireRedfishService (
> +                                           RedfishDiscoverProtocol,
> +                                           RestExInstance->NetworkInterfaceInstances
> +                                           );
> +
> +
> +                gBS->DisconnectController (DiscoverProtocolHandle, NULL, NULL);
> +                Status = gBS->UninstallProtocolInterface (
> +                                DiscoverProtocolHandle,
> +                                &gEfiRedfishDiscoverProtocolGuid,
> +                                (VOID *)&RestExInstance->RedfishDiscoverProtocol
> +                                );
> +              }
>              }
>
>              return Status;
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> index 2704cd9..ddb5f2e 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> @@ -118,16 +118,30 @@ typedef struct {
>  } EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL;
>
>  //
> +// Redfish Discover Instance signature
> +//
> +
> +#define EFI_REDFISH_DISCOVER_DATA_SIGNATURE   SIGNATURE_32 ('E',
> 'R', 'D', 'D')
> +
> +#define EFI_REDFISH_DISOVER_DATA_FROM_DISCOVER_PROTOCOL(a) \
> +  CR (a, EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL,
> +RedfishDiscoverProtocol, EFI_REDFISH_DISCOVER_DATA_SIGNATURE)
> +
> +//
>  // Internal structure used to maintain REST EX properties.
>  //
>  typedef struct {
> -  LIST_ENTRY              Entry;                      ///< Link list entry.
> -  EFI_HANDLE              OpenDriverAgentHandle;      ///< The agent to open
> network protocol.
> -  EFI_HANDLE              OpenDriverControllerHandle; ///< The controller handle
> to open network protocol.
> -  EFI_HANDLE              RestExChildHandle;          ///< The child handle created
> through REST EX Service Protocol.
> -  EFI_HANDLE              RestExControllerHandle;     ///< The controller handle
> which provide REST EX protocol.
> -  EFI_REST_EX_PROTOCOL    *RestExProtocolInterface;   ///< Pointer to
> EFI_REST_EX_PROTOCOL.
> -  UINT32                  RestExId;                   ///< The identifier installed on REST EX
> controller handle.
> +  LIST_ENTRY                              Entry;                      ///< Link list entry.
> +  UINT32                                  Signature;                  ///< Instance signature.
> +  EFI_HANDLE                              OpenDriverAgentHandle;      ///< The agent to
> open network protocol.
> +  EFI_HANDLE                              OpenDriverControllerHandle; ///< The
> controller handle to open network protocol.
> +  EFI_HANDLE                              RestExChildHandle;          ///< The child handle
> created through REST EX Service Protocol.
> +  EFI_HANDLE                              RestExControllerHandle;     ///< The controller
> handle which provide REST EX protocol.
> +  EFI_REST_EX_PROTOCOL                    *RestExProtocolInterface;   ///< Pointer
> to EFI_REST_EX_PROTOCOL.
> +  UINT32                                  RestExId;                   ///< The identifier installed on
> REST EX controller handle.
> +  UINTN                                   NumberOfNetworkInterfaces;  ///< Number of
> network interfaces can do Redfish service discovery.
> +  EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> *NetworkInterfaceInstances; ///< Network interface instances. It's an array
> of instances. The number of entries
> +                                                                      ///< in array is indicated by
> NumberOfNetworkInterfaces.
> +  EFI_REDFISH_DISCOVER_PROTOCOL           RedfishDiscoverProtocol;    ///<
> EFI_REDFISH_DISCOVER_PROTOCOL protocol.
>  } EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL;
>
>  /**
> --
> 2.6.1.windows.1
> -The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI). This communication is intended
> to be read only by the individual or entity to whom it is addressed or by their
> designee. If the reader of this message is not the intended recipient, you are
> on notice that any distribution of this message, in any form, is strictly
> prohibited. Please promptly notify the sender by reply e-mail or by
> telephone at 770-246-8600, and then delete or destroy all copies of the
> transmission.
-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103320): https://edk2.groups.io/g/devel/message/103320
Mute This Topic: https://groups.io/mt/98368593/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] RedfishPkg: Remove the global variables related to Discover Token functionality
Posted by Chang, Abner via groups.io 1 year ago
[AMD Official Use Only - General]

Thanks Igor,
You probably can ask community if any one has the similar issue on patch email format.

I already gave R-B to this change, please create a PR against to edk2 and let me know the PR#. I will push it once it passes CI.
Thanks
Abner

> -----Original Message-----
> From: Igor Kulchytskyy <igork@ami.com>
> Sent: Friday, April 21, 2023 2:46 AM
> To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
> Cc: Nickle Wang <nicklew@nvidia.com>
> Subject: RE: [EXTERNAL] RE: [PATCH] RedfishPkg: Remove the global
> variables related to Discover Token functionality
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> Hi Abner,
> I did what you asked to do - Patchcheck.py and UncrustiyCheck (got binary).
> Going to send PATCH V2.
> But I'm not sure what happened with white space. Going to check with MIS.
> Thank you,
> Igor
> 
> -----Original Message-----
> From: Chang, Abner <Abner.Chang@amd.com>
> Sent: Wednesday, April 19, 2023 9:00 PM
> To: Igor Kulchytskyy <igork@ami.com>; devel@edk2.groups.io
> Cc: Nickle Wang <nicklew@nvidia.com>
> Subject: [EXTERNAL] RE: [PATCH] RedfishPkg: Remove the global variables
> related to Discover Token functionality
> 
> 
> **CAUTION: The e-mail below is from an external source. Please exercise
> caution before opening attachments, clicking links, or following guidance.**
> 
> [AMD Official Use Only - General]
> 
> Hi Igor,
> I have no problem with this change, however some upstream practices here,
> 
> - Please shorten the subject to <= 76
> - Each line in the commit message should be <=76 You can run Patchcheck.py
> (here: BaseTools\Scripts) before you sending out the patches.
> 
> Please run UncrustiyCheck for each source code you modified, .
> pytool/Plugin/UncrustifyCheck/ and used for beautifying the source code.
> You can get the binary from here:
> https://sourceforge.net/projects/uncrustify/files.
> 
> Please resend the patch with above issue fixed.
> 
> BTW, the patch you sent to group.io still has a problem. The white space of
> each blank line in the patch file were gone (you can take a look at your
> message sent to group.io). This leads to a failure of patch apply. I recover
> those white space in the blank lines and the patch can be applied. However, I
> can do this just for a short patch. 😊 Please figure it out at your end.  That's
> fine if this problem still exist on your resent patch in case you need some
> time to figure out the email problem.
> 
> Thanks
> Abner
> 
> > -----Original Message-----
> > From: Igor Kulchytskyy <igork@ami.com>
> > Sent: Wednesday, April 19, 2023 11:04 PM
> > To: devel@edk2.groups.io
> > Cc: Chang, Abner <Abner.Chang@amd.com>; Nickle Wang
> > <nicklew@nvidia.com>
> > Subject: [PATCH] RedfishPkg: Remove the global variables related to
> > Discover Token functionality
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > gRedfishDiscoveredToken may be allocated several times, if multiple
> > NIC installed on the system.
> > To avoid this issue Discover Token related global variables replaced
> > with the local variables.
> >
> > Cc: Abner Chang <abner.chang@amd.com>
> > Cc: Nickle Wang <nicklew@nvidia.com>
> > Signed-off-by: Igor Kulchytskyy <igork@ami.com>
> > ---
> >  RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c | 139
> > ++++++++------------
> >  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c           |  87
> ++++++++-
> > ---
> >  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h      |  28 +++-
> >  3 files changed, 140 insertions(+), 114 deletions(-)
> >
> > diff --git
> > a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> > b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> > index 993ad33..8f85491 100644
> > --- a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> > +++ b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> > @@ -22,12 +22,6 @@ EFI_HANDLE
> > gEfiRedfishDiscoverControllerHandle = NULL;
> >  EFI_REDFISH_DISCOVER_PROTOCOL  *gEfiRedfishDiscoverProtocol        =
> > NULL;
> >  BOOLEAN                        gRedfishDiscoverActivated           = FALSE;
> >  BOOLEAN                        gRedfishServiceDiscovered           = FALSE;
> > -//
> > -// Network interfaces discovered by EFI Redfish Discover Protocol.
> > -//
> > -UINTN                                   gNumberOfNetworkInterfaces;
> > -EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> > *gNetworkInterfaceInstances = NULL;
> > -EFI_REDFISH_DISCOVERED_TOKEN            *gRedfishDiscoveredToken    =
> > NULL;
> >
> >  ///
> >  /// Driver Binding Protocol instance
> > @@ -58,13 +52,6 @@ RedfishConfigStopRedfishDiscovery (
> >        gBS->CloseEvent (gEfiRedfishDiscoverProtocolEvent);
> >      }
> >
> > -    //
> > -    // Stop Redfish service discovery.
> > -    //
> > -    gEfiRedfishDiscoverProtocol->AbortAcquireRedfishService (
> > -                                   gEfiRedfishDiscoverProtocol,
> > -                                   gNetworkInterfaceInstances
> > -                                   );
> >      gEfiRedfishDiscoverControllerHandle = NULL;
> >      gEfiRedfishDiscoverProtocol         = NULL;
> >      gRedfishDiscoverActivated           = FALSE;
> > @@ -318,36 +305,39 @@ RedfishServiceDiscoveredCallback (
> >    EFI_REDFISH_DISCOVERED_TOKEN     *RedfishDiscoveredToken;
> >    EFI_REDFISH_DISCOVERED_INSTANCE  *RedfishInstance;
> >
> > -  if (gRedfishServiceDiscovered) {
> > -    //
> > -    // Only support one Redfish service on platform.
> > -    //
> > -    return;
> > -  }
> > -
> >    RedfishDiscoveredToken = (EFI_REDFISH_DISCOVERED_TOKEN *)Context;
> > -  RedfishInstance        = RedfishDiscoveredToken-
> > >DiscoverList.RedfishInstances;
> > +  gBS->CloseEvent (RedfishDiscoveredToken->Event);
> > +
> >    //
> > -  // Only pick up the first found Redfish service.
> > +  // Only support one Redfish service on platform.
> >    //
> > -  if (RedfishInstance->Status == EFI_SUCCESS) {
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceRestExHandle =
> > RedfishInstance->Information.RedfishRestExHandle;
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceVersion      =
> > RedfishInstance->Information.RedfishVersion;
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceLocation     =
> > RedfishInstance->Information.Location;
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceUuid         =
> > RedfishInstance->Information.Uuid;
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceOs           =
> > RedfishInstance->Information.Os;
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceOsVersion    =
> > RedfishInstance->Information.OsVersion;
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceProduct      =
> > RedfishInstance->Information.Product;
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceProductVer   =
> > RedfishInstance->Information.ProductVer;
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceUseHttps     =
> > RedfishInstance->Information.UseHttps;
> > -    gRedfishServiceDiscovered                                        = TRUE;
> > +  if (!gRedfishServiceDiscovered) {
> > +    RedfishInstance        = RedfishDiscoveredToken-
> > >DiscoverList.RedfishInstances;
> > +    //
> > +    // Only pick up the first found Redfish service.
> > +    //
> > +    if (RedfishInstance->Status == EFI_SUCCESS) {
> > +
> > + gRedfishConfigData.RedfishServiceInfo.RedfishServiceRestExHandle =
> > RedfishInstance->Information.RedfishRestExHandle;
> > +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceVersion      =
> > RedfishInstance->Information.RedfishVersion;
> > +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceLocation     =
> > RedfishInstance->Information.Location;
> > +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceUuid         =
> > RedfishInstance->Information.Uuid;
> > +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceOs           =
> > RedfishInstance->Information.Os;
> > +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceOsVersion    =
> > RedfishInstance->Information.OsVersion;
> > +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceProduct      =
> > RedfishInstance->Information.Product;
> > +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceProductVer   =
> > RedfishInstance->Information.ProductVer;
> > +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceUseHttps     =
> > RedfishInstance->Information.UseHttps;
> > +      gRedfishServiceDiscovered                                        = TRUE;
> > +    }
> > +
> > +    //
> > +    // Invoke RedfishConfigHandlerInstalledCallback to execute
> > +    // the initialization of Redfish Configure Handler instance.
> > +    //
> > +    RedfishConfigHandlerInstalledCallback (gRedfishConfigData.Event,
> > + &gRedfishConfigData);
> >    }
> >
> > -  //
> > -  // Invoke RedfishConfigHandlerInstalledCallback to execute
> > -  // the initialization of Redfish Configure Handler instance.
> > -  //
> > -  RedfishConfigHandlerInstalledCallback (gRedfishConfigData.Event,
> > &gRedfishConfigData);
> > +  FreePool(RedfishDiscoveredToken);
> > +
> >  }
> >
> >  /**
> > @@ -371,6 +361,7 @@ RedfishDiscoverProtocolInstalled (
> >    UINTN                                   NetworkInterfaceIndex;
> >    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE  *ThisNetworkInterface;
> >    EFI_REDFISH_DISCOVERED_TOKEN            *ThisRedfishDiscoveredToken;
> > +  UINTN                                   NumberOfNetworkInterfaces;
> >
> >    DEBUG ((DEBUG_INFO, "%a: New network interface is installed on
> > system by EFI Redfish discover driver.\n", __func__));
> >
> > @@ -408,36 +399,31 @@ RedfishDiscoverProtocolInstalled (
> >      }
> >    }
> >
> > -  //
> > -  // Check the new found network interface.
> > -  //
> > -  if (gNetworkInterfaceInstances != NULL) {
> > -    FreePool (gNetworkInterfaceInstances);
> > -  }
> >
> >    Status = gEfiRedfishDiscoverProtocol->GetNetworkInterfaceList (
> >                                            gEfiRedfishDiscoverProtocol,
> >                                            gRedfishConfigData.Image,
> > -                                          &gNumberOfNetworkInterfaces,
> > -                                          &gNetworkInterfaceInstances
> > +                                          &NumberOfNetworkInterfaces,
> > +                                          &ThisNetworkInterface
> >                                            );
> > -  if (EFI_ERROR (Status) || (gNumberOfNetworkInterfaces == 0)) {
> > +  if (EFI_ERROR (Status) || (NumberOfNetworkInterfaces == 0)) {
> >      DEBUG ((DEBUG_ERROR, "%a: No network interfaces found on the
> > handle.\n", __func__));
> >      return;
> >    }
> >
> > -  gRedfishDiscoveredToken = AllocateZeroPool
> > (gNumberOfNetworkInterfaces * sizeof
> (EFI_REDFISH_DISCOVERED_TOKEN));
> > -  if (gRedfishDiscoveredToken == NULL) {
> > +  //
> > +  // Loop to discover Redfish service on each network interface.
> > +  //
> > +  for (NetworkInterfaceIndex = 0; NetworkInterfaceIndex <
> > + NumberOfNetworkInterfaces; NetworkInterfaceIndex++) {
> > +
> > +    ThisRedfishDiscoveredToken = (EFI_REDFISH_DISCOVERED_TOKEN
> > *)AllocateZeroPool(sizeof (EFI_REDFISH_DISCOVERED_TOKEN));
> > +    if (ThisRedfishDiscoveredToken == NULL) {
> >      DEBUG ((DEBUG_ERROR, "%a: Not enough memory for
> > EFI_REDFISH_DISCOVERED_TOKEN.\n", __func__));
> >      return;
> >    }
> >
> > -  ThisNetworkInterface       = gNetworkInterfaceInstances;
> > -  ThisRedfishDiscoveredToken = gRedfishDiscoveredToken;
> > -  //
> > -  // Loop to discover Redfish service on each network interface.
> > -  //
> > -  for (NetworkInterfaceIndex = 0; NetworkInterfaceIndex <
> > gNumberOfNetworkInterfaces; NetworkInterfaceIndex++) {
> > +    ThisRedfishDiscoveredToken->Signature =
> > + REDFISH_DISCOVER_TOKEN_SIGNATURE;
> > +
> >      //
> >      // Initial this Redfish Discovered Token
> >      //
> > @@ -449,13 +435,11 @@ RedfishDiscoverProtocolInstalled (
> >                      &ThisRedfishDiscoveredToken->Event
> >                      );
> >      if (EFI_ERROR (Status)) {
> > +      FreePool(ThisRedfishDiscoveredToken);
> >        DEBUG ((DEBUG_ERROR, "%a: Failed to create event for Redfish
> > discovered token.\n", __func__));
> > -      goto ErrorReturn;
> > +      return;
> >      }
> >
> > -    ThisRedfishDiscoveredToken->Signature                         =
> > REDFISH_DISCOVER_TOKEN_SIGNATURE;
> > -    ThisRedfishDiscoveredToken->DiscoverList.NumberOfServiceFound = 0;
> > -    ThisRedfishDiscoveredToken->DiscoverList.RedfishInstances     = NULL;
> >      //
> >      // Acquire for Redfish service which is reported by
> >      // Redfish Host Interface.
> > @@ -467,21 +451,23 @@ RedfishDiscoverProtocolInstalled (
> >                                              EFI_REDFISH_DISCOVER_HOST_INTERFACE,
> >                                              ThisRedfishDiscoveredToken
> >                                              );
> > -    ThisNetworkInterface++;
> > -    ThisRedfishDiscoveredToken++;
> > -  }
> >
> > -  if (EFI_ERROR (Status)) {
> > -    DEBUG ((DEBUG_ERROR, "%a: Acquire Redfish service fail.\n",
> __func__));
> > -    goto ErrorReturn;
> > +    //
> > +    // Free Redfish Discovered Token if Discover Instance was not
> > + created
> > and
> > +    // Redfish Service Discovered Callback event was not triggered.
> > +    //
> > +    if(ThisRedfishDiscoveredToken->DiscoverList.NumberOfServiceFound
> > + ==
> > 0 ||
> > +        EFI_ERROR(ThisRedfishDiscoveredToken-
> > >DiscoverList.RedfishInstances->Status)){
> > +      gBS->CloseEvent (ThisRedfishDiscoveredToken->Event);
> > +      DEBUG ((DEBUG_ERROR, "%a: Free Redfish discovered token - %x.\n",
> > +          __func__, ThisRedfishDiscoveredToken));
> > +      FreePool(ThisRedfishDiscoveredToken);
> > +    }
> > +    ThisNetworkInterface++;
> >    }
> >
> >    return;
> >
> > -ErrorReturn:
> > -  if (gRedfishDiscoveredToken != NULL) {
> > -    FreePool (gRedfishDiscoveredToken);
> > -  }
> >  }
> >
> >  /**
> > @@ -498,25 +484,10 @@ RedfishConfigHandlerDriverUnload (
> >    IN EFI_HANDLE  ImageHandle
> >    )
> >  {
> > -  EFI_REDFISH_DISCOVERED_TOKEN  *ThisRedfishDiscoveredToken;
> > -  UINTN                         NumberOfNetworkInterfacesIndex;
> >
> >    RedfishConfigDriverCommonUnload (ImageHandle);
> >
> >    RedfishConfigStopRedfishDiscovery ();
> > -  if (gRedfishDiscoveredToken != NULL) {
> > -    ThisRedfishDiscoveredToken = gRedfishDiscoveredToken;
> > -    for (NumberOfNetworkInterfacesIndex = 0;
> > NumberOfNetworkInterfacesIndex < gNumberOfNetworkInterfaces;
> > NumberOfNetworkInterfacesIndex++) {
> > -      if (ThisRedfishDiscoveredToken->Event != NULL) {
> > -        gBS->CloseEvent (ThisRedfishDiscoveredToken->Event);
> > -      }
> > -
> > -      FreePool (ThisRedfishDiscoveredToken);
> > -      ThisRedfishDiscoveredToken++;
> > -    }
> > -
> > -    gRedfishDiscoveredToken = NULL;
> > -  }
> >
> >    return EFI_SUCCESS;
> >  }
> > diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > index 583c6f7..a6c8d60 100644
> > --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > @@ -1095,8 +1095,10 @@ RedfishServiceGetNetworkInterface (  {
> >    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> > *ThisNetworkInterfaceIntn;
> >    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> > *ThisNetworkInterface;
> > +  EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL
> *RestExInstance;
> >
> > -  if ((NetworkIntfInstances == NULL) || (NumberOfNetworkIntfs ==
> > NULL)
> > || (ImageHandle == NULL)) {
> > +  if ((This == NULL) || (NetworkIntfInstances == NULL) ||
> > (NumberOfNetworkIntfs == NULL) ||
> > +      (ImageHandle == NULL)) {
> >      return EFI_INVALID_PARAMETER;
> >    }
> >
> > @@ -1107,14 +1109,30 @@ RedfishServiceGetNetworkInterface (
> >      return EFI_NOT_FOUND;
> >    }
> >
> > +  RestExInstance =
> > + EFI_REDFISH_DISOVER_DATA_FROM_DISCOVER_PROTOCOL(This);
> > +
> > +  //
> > +  // Check the new found network interface.
> > +  //
> > +  if (RestExInstance->NetworkInterfaceInstances != NULL) {
> > +    FreePool (RestExInstance->NetworkInterfaceInstances);
> > +    RestExInstance->NetworkInterfaceInstances = NULL;  }
> > +
> >    ThisNetworkInterface = (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> > *)AllocateZeroPool (sizeof
> (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE)
> > * mNumNetworkInterface);
> >    if (ThisNetworkInterface == NULL) {
> >      return EFI_OUT_OF_RESOURCES;
> >    }
> >
> >    *NetworkIntfInstances    = ThisNetworkInterface;
> > +
> > +
> > +  RestExInstance->NetworkInterfaceInstances = ThisNetworkInterface;
> > + RestExInstance->NumberOfNetworkInterfaces = 0;
> > +
> >    ThisNetworkInterfaceIntn =
> > (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> *)GetFirstNode
> > (&mEfiRedfishDiscoverNetworkInterface);
> >    while (TRUE) {
> > +
> >      ThisNetworkInterface->IsIpv6 = FALSE;
> >      if (CheckIsIpVersion6 (ThisNetworkInterfaceIntn)) {
> >        ThisNetworkInterface->IsIpv6 = TRUE; @@ -1130,7 +1148,7 @@
> > RedfishServiceGetNetworkInterface (
> >
> >      ThisNetworkInterface->SubnetPrefixLength =
> > ThisNetworkInterfaceIntn-
> > >SubnetPrefixLength;
> >      ThisNetworkInterface->VlanId             = ThisNetworkInterfaceIntn-
> >VlanId;
> > -    (*NumberOfNetworkIntfs)++;
> > +    RestExInstance->NumberOfNetworkInterfaces++;
> >      if (IsNodeAtEnd (&mEfiRedfishDiscoverNetworkInterface,
> > &ThisNetworkInterfaceIntn->Entry)) {
> >        break;
> >      }
> > @@ -1139,6 +1157,8 @@ RedfishServiceGetNetworkInterface (
> >      ThisNetworkInterface++;
> >    }
> >
> > +  *NumberOfNetworkIntfs = RestExInstance-
> > >NumberOfNetworkInterfaces;
> > +
> >    return EFI_SUCCESS;
> >  }
> >
> > @@ -1178,7 +1198,6 @@ RedfishServiceAcquireService (  {
> >    EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE         *Instance;
> >    EFI_STATUS                                       Status1;
> > -  EFI_STATUS                                       Status2;
> >    BOOLEAN                                          NewInstance;
> >    UINTN                                            NumNetworkInterfaces;
> >    UINTN                                            NetworkInterfacesIndex;
> > @@ -1215,7 +1234,6 @@ RedfishServiceAcquireService (
> >
> >    for (NetworkInterfacesIndex = 0; NetworkInterfacesIndex <
> > NumNetworkInterfaces; NetworkInterfacesIndex++) {
> >      Status1     = EFI_SUCCESS;
> > -    Status2     = EFI_SUCCESS;
> >      NewInstance = FALSE;
> >      Instance    = GetInstanceByOwner (ImageHandle,
> > TargetNetworkInterfaceInternal, Flags &
> > ~EFI_REDFISH_DISCOVER_VALIDATION); // Check if we can re-use
> previous
> > instance.
> >      if (Instance == NULL) {
> > @@ -1223,6 +1241,7 @@ RedfishServiceAcquireService (
> >        Instance = (EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE
> > *)AllocateZeroPool (sizeof
> > (EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE));
> >        if (Instance == NULL) {
> >          DEBUG ((DEBUG_ERROR, "%a:Memory allocation fail.\n",
> > __func__));
> > +        return EFI_OUT_OF_RESOURCES;
> >        }
> >
> >        InitializeListHead (&Instance->Entry); @@ -1258,9 +1277,11 @@
> > RedfishServiceAcquireService (
> >        DEBUG ((DEBUG_ERROR, "%a:Redfish service discovery through SSDP
> > is not supported\n", __func__));
> >        return EFI_UNSUPPORTED;
> >      } else {
> > -      if (EFI_ERROR (Status1) && EFI_ERROR (Status2)) {
> > -        FreePool ((VOID *)Instance);
> > -        DEBUG ((DEBUG_ERROR, "%a:Something wrong on Redfish service
> > discovery Status1=%x, Status2=%x.\n", __func__, Status1, Status2));
> > +      if (EFI_ERROR (Status1)) {
> > +        if (NewInstance) {
> > +          FreePool ((VOID *)Instance);
> > +        }
> > +        DEBUG ((DEBUG_ERROR, "%a:Something wrong on Redfish service
> > + discovery Status1=%r.\n", __func__, Status1));
> >        } else {
> >          if (NewInstance) {
> >            InsertTailList (&mRedfishDiscoverList, &Instance->Entry);
> > @@ -1387,13
> > +1408,6 @@ ReleaseNext:;
> >    }
> >  }
> >
> > -EFI_REDFISH_DISCOVER_PROTOCOL  mRedfishDiscover = {
> > -  RedfishServiceGetNetworkInterface,
> > -  RedfishServiceAcquireService,
> > -  RedfishServiceAbortAcquire,
> > -  RedfishServiceReleaseService
> > -};
> > -
> >  /**
> >    This function create an
> > EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL for the
> >    given network interface.
> > @@ -1713,12 +1727,20 @@ BuildupNetworkInterface (
> >
> >            NewNetworkInterfaceInstalled                       = FALSE;
> >            NetworkInterface->EfiRedfishDiscoverProtocolHandle = NULL;
> > -          Status                                             = gBS->InstallProtocolInterface (
> > -                                                                      &NetworkInterface-
> > >EfiRedfishDiscoverProtocolHandle,
> > -                                                                      &gEfiRedfishDiscoverProtocolGuid,
> > -                                                                      EFI_NATIVE_INTERFACE,
> > -                                                                      (VOID *)&mRedfishDiscover
> > -                                                                      );
> > +
> > +          RestExInstance->Signature =
> > + EFI_REDFISH_DISCOVER_DATA_SIGNATURE;
> > +
> > +
> > + RestExInstance->RedfishDiscoverProtocol.GetNetworkInterfaceList =
> > RedfishServiceGetNetworkInterface;
> > +
> > + RestExInstance->RedfishDiscoverProtocol.AcquireRedfishService =
> > RedfishServiceAcquireService;
> > +
> > + RestExInstance->RedfishDiscoverProtocol.AbortAcquireRedfishService
> > = RedfishServiceAbortAcquire;
> > +
> > + RestExInstance->RedfishDiscoverProtocol.ReleaseRedfishService
> > + = RedfishServiceReleaseService;
> > +
> > +          Status = gBS->InstallProtocolInterface (
> > +                          &NetworkInterface->EfiRedfishDiscoverProtocolHandle,
> > +                          &gEfiRedfishDiscoverProtocolGuid,
> > +                          EFI_NATIVE_INTERFACE,
> > +                          (VOID *)&RestExInstance->RedfishDiscoverProtocol
> > +                          );
> >            if (EFI_ERROR (Status)) {
> >              DEBUG ((DEBUG_ERROR, "%a: Fail to install
> > EFI_REDFISH_DISCOVER_PROTOCOL\n", __func__));
> >            }
> > @@ -1815,6 +1837,7 @@ StopServiceOnNetworkInterface (
> >    EFI_HANDLE                                       DiscoverProtocolHandle;
> >    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> > *ThisNetworkInterface;
> >    EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL
> *RestExInstance;
> > +  EFI_REDFISH_DISCOVER_PROTOCOL                    *RedfishDiscoverProtocol;
> >
> >    for (Index = 0; Index < (sizeof (gRequiredProtocol) / sizeof
> > (REDFISH_DISCOVER_REQUIRED_PROTOCOL)); Index++) {
> >      Status = gBS->HandleProtocol (
> > @@ -1854,12 +1877,30 @@ StopServiceOnNetworkInterface (
> >              // client which uses .EFI Redfish discover protocol.
> >              //
> >              if (DiscoverProtocolHandle != NULL) {
> > -              gBS->DisconnectController (DiscoverProtocolHandle, NULL, NULL);
> > -              Status = gBS->UninstallProtocolInterface (
> > +
> > +              Status = gBS->HandleProtocol (
> >                                DiscoverProtocolHandle,
> >                                &gEfiRedfishDiscoverProtocolGuid,
> > -                              (VOID *)&mRedfishDiscover
> > +                              (VOID **) &RedfishDiscoverProtocol
> >                                );
> > +              if (!EFI_ERROR (Status)) {
> > +                RestExInstance =
> >
> EFI_REDFISH_DISOVER_DATA_FROM_DISCOVER_PROTOCOL(RedfishDiscove
> > rProtocol);
> > +                //
> > +                // Stop Redfish service discovery.
> > +                //
> > +                RedfishDiscoverProtocol->AbortAcquireRedfishService (
> > +                                           RedfishDiscoverProtocol,
> > +                                           RestExInstance->NetworkInterfaceInstances
> > +                                           );
> > +
> > +
> > +                gBS->DisconnectController (DiscoverProtocolHandle, NULL, NULL);
> > +                Status = gBS->UninstallProtocolInterface (
> > +                                DiscoverProtocolHandle,
> > +                                &gEfiRedfishDiscoverProtocolGuid,
> > +                                (VOID *)&RestExInstance->RedfishDiscoverProtocol
> > +                                );
> > +              }
> >              }
> >
> >              return Status;
> > diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> > b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> > index 2704cd9..ddb5f2e 100644
> > --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> > +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> > @@ -118,16 +118,30 @@ typedef struct {  }
> > EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL;
> >
> >  //
> > +// Redfish Discover Instance signature //
> > +
> > +#define EFI_REDFISH_DISCOVER_DATA_SIGNATURE   SIGNATURE_32 ('E',
> > 'R', 'D', 'D')
> > +
> > +#define EFI_REDFISH_DISOVER_DATA_FROM_DISCOVER_PROTOCOL(a) \
> > +  CR (a, EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL,
> > +RedfishDiscoverProtocol, EFI_REDFISH_DISCOVER_DATA_SIGNATURE)
> > +
> > +//
> >  // Internal structure used to maintain REST EX properties.
> >  //
> >  typedef struct {
> > -  LIST_ENTRY              Entry;                      ///< Link list entry.
> > -  EFI_HANDLE              OpenDriverAgentHandle;      ///< The agent to open
> > network protocol.
> > -  EFI_HANDLE              OpenDriverControllerHandle; ///< The controller
> handle
> > to open network protocol.
> > -  EFI_HANDLE              RestExChildHandle;          ///< The child handle created
> > through REST EX Service Protocol.
> > -  EFI_HANDLE              RestExControllerHandle;     ///< The controller handle
> > which provide REST EX protocol.
> > -  EFI_REST_EX_PROTOCOL    *RestExProtocolInterface;   ///< Pointer to
> > EFI_REST_EX_PROTOCOL.
> > -  UINT32                  RestExId;                   ///< The identifier installed on REST EX
> > controller handle.
> > +  LIST_ENTRY                              Entry;                      ///< Link list entry.
> > +  UINT32                                  Signature;                  ///< Instance signature.
> > +  EFI_HANDLE                              OpenDriverAgentHandle;      ///< The agent to
> > open network protocol.
> > +  EFI_HANDLE                              OpenDriverControllerHandle; ///< The
> > controller handle to open network protocol.
> > +  EFI_HANDLE                              RestExChildHandle;          ///< The child handle
> > created through REST EX Service Protocol.
> > +  EFI_HANDLE                              RestExControllerHandle;     ///< The controller
> > handle which provide REST EX protocol.
> > +  EFI_REST_EX_PROTOCOL                    *RestExProtocolInterface;   ///<
> Pointer
> > to EFI_REST_EX_PROTOCOL.
> > +  UINT32                                  RestExId;                   ///< The identifier installed on
> > REST EX controller handle.
> > +  UINTN                                   NumberOfNetworkInterfaces;  ///< Number of
> > network interfaces can do Redfish service discovery.
> > +  EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> > *NetworkInterfaceInstances; ///< Network interface instances. It's an
> > array of instances. The number of entries
> > +
> > + ///< in array is indicated by
> > NumberOfNetworkInterfaces.
> > +  EFI_REDFISH_DISCOVER_PROTOCOL           RedfishDiscoverProtocol;
> ///<
> > EFI_REDFISH_DISCOVER_PROTOCOL protocol.
> >  } EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL;
> >
> >  /**
> > --
> > 2.6.1.windows.1
> > -The information contained in this message may be confidential and
> > proprietary to American Megatrends (AMI). This communication is
> > intended to be read only by the individual or entity to whom it is
> > addressed or by their designee. If the reader of this message is not
> > the intended recipient, you are on notice that any distribution of
> > this message, in any form, is strictly prohibited. Please promptly
> > notify the sender by reply e-mail or by telephone at 770-246-8600, and
> > then delete or destroy all copies of the transmission.
> -The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI). This communication is intended
> to be read only by the individual or entity to whom it is addressed or by their
> designee. If the reader of this message is not the intended recipient, you are
> on notice that any distribution of this message, in any form, is strictly
> prohibited. Please promptly notify the sender by reply e-mail or by
> telephone at 770-246-8600, and then delete or destroy all copies of the
> transmission.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103326): https://edk2.groups.io/g/devel/message/103326
Mute This Topic: https://groups.io/mt/98368593/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] RedfishPkg: Remove the global variables related to Discover Token functionality
Posted by Igor Kulchytskyy via groups.io 1 year ago
Hi Abner,
I created PR.
https://github.com/tianocore/edk2/pull/4297
Since it is my first PR created, I'm not sure if everything done correctly.
Please double check everything.
Thank you,
Igor

-----Original Message-----
From: Chang, Abner <Abner.Chang@amd.com>
Sent: Thursday, April 20, 2023 8:13 PM
To: Igor Kulchytskyy <igork@ami.com>; devel@edk2.groups.io
Cc: Nickle Wang <nicklew@nvidia.com>
Subject: RE: [EXTERNAL] RE: [PATCH] RedfishPkg: Remove the global variables related to Discover Token functionality

[AMD Official Use Only - General]

Thanks Igor,
You probably can ask community if any one has the similar issue on patch email format.

I already gave R-B to this change, please create a PR against to edk2 and let me know the PR#. I will push it once it passes CI.
Thanks
Abner

> -----Original Message-----
> From: Igor Kulchytskyy <igork@ami.com>
> Sent: Friday, April 21, 2023 2:46 AM
> To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
> Cc: Nickle Wang <nicklew@nvidia.com>
> Subject: RE: [EXTERNAL] RE: [PATCH] RedfishPkg: Remove the global
> variables related to Discover Token functionality
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> Hi Abner,
> I did what you asked to do - Patchcheck.py and UncrustiyCheck (got binary).
> Going to send PATCH V2.
> But I'm not sure what happened with white space. Going to check with MIS.
> Thank you,
> Igor
>
> -----Original Message-----
> From: Chang, Abner <Abner.Chang@amd.com>
> Sent: Wednesday, April 19, 2023 9:00 PM
> To: Igor Kulchytskyy <igork@ami.com>; devel@edk2.groups.io
> Cc: Nickle Wang <nicklew@nvidia.com>
> Subject: [EXTERNAL] RE: [PATCH] RedfishPkg: Remove the global variables
> related to Discover Token functionality
>
>
> **CAUTION: The e-mail below is from an external source. Please exercise
> caution before opening attachments, clicking links, or following guidance.**
>
> [AMD Official Use Only - General]
>
> Hi Igor,
> I have no problem with this change, however some upstream practices here,
>
> - Please shorten the subject to <= 76
> - Each line in the commit message should be <=76 You can run Patchcheck.py
> (here: BaseTools\Scripts) before you sending out the patches.
>
> Please run UncrustiyCheck for each source code you modified, .
> pytool/Plugin/UncrustifyCheck/ and used for beautifying the source code.
> You can get the binary from here:
> https://sourceforge.net/projects/uncrustify/files.
>
> Please resend the patch with above issue fixed.
>
> BTW, the patch you sent to group.io still has a problem. The white space of
> each blank line in the patch file were gone (you can take a look at your
> message sent to group.io). This leads to a failure of patch apply. I recover
> those white space in the blank lines and the patch can be applied. However, I
> can do this just for a short patch. 😊 Please figure it out at your end.  That's
> fine if this problem still exist on your resent patch in case you need some
> time to figure out the email problem.
>
> Thanks
> Abner
>
> > -----Original Message-----
> > From: Igor Kulchytskyy <igork@ami.com>
> > Sent: Wednesday, April 19, 2023 11:04 PM
> > To: devel@edk2.groups.io
> > Cc: Chang, Abner <Abner.Chang@amd.com>; Nickle Wang
> > <nicklew@nvidia.com>
> > Subject: [PATCH] RedfishPkg: Remove the global variables related to
> > Discover Token functionality
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > gRedfishDiscoveredToken may be allocated several times, if multiple
> > NIC installed on the system.
> > To avoid this issue Discover Token related global variables replaced
> > with the local variables.
> >
> > Cc: Abner Chang <abner.chang@amd.com>
> > Cc: Nickle Wang <nicklew@nvidia.com>
> > Signed-off-by: Igor Kulchytskyy <igork@ami.com>
> > ---
> >  RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c | 139
> > ++++++++------------
> >  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c           |  87
> ++++++++-
> > ---
> >  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h      |  28 +++-
> >  3 files changed, 140 insertions(+), 114 deletions(-)
> >
> > diff --git
> > a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> > b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> > index 993ad33..8f85491 100644
> > --- a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> > +++ b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> > @@ -22,12 +22,6 @@ EFI_HANDLE
> > gEfiRedfishDiscoverControllerHandle = NULL;
> >  EFI_REDFISH_DISCOVER_PROTOCOL  *gEfiRedfishDiscoverProtocol        =
> > NULL;
> >  BOOLEAN                        gRedfishDiscoverActivated           = FALSE;
> >  BOOLEAN                        gRedfishServiceDiscovered           = FALSE;
> > -//
> > -// Network interfaces discovered by EFI Redfish Discover Protocol.
> > -//
> > -UINTN                                   gNumberOfNetworkInterfaces;
> > -EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> > *gNetworkInterfaceInstances = NULL;
> > -EFI_REDFISH_DISCOVERED_TOKEN            *gRedfishDiscoveredToken    =
> > NULL;
> >
> >  ///
> >  /// Driver Binding Protocol instance
> > @@ -58,13 +52,6 @@ RedfishConfigStopRedfishDiscovery (
> >        gBS->CloseEvent (gEfiRedfishDiscoverProtocolEvent);
> >      }
> >
> > -    //
> > -    // Stop Redfish service discovery.
> > -    //
> > -    gEfiRedfishDiscoverProtocol->AbortAcquireRedfishService (
> > -                                   gEfiRedfishDiscoverProtocol,
> > -                                   gNetworkInterfaceInstances
> > -                                   );
> >      gEfiRedfishDiscoverControllerHandle = NULL;
> >      gEfiRedfishDiscoverProtocol         = NULL;
> >      gRedfishDiscoverActivated           = FALSE;
> > @@ -318,36 +305,39 @@ RedfishServiceDiscoveredCallback (
> >    EFI_REDFISH_DISCOVERED_TOKEN     *RedfishDiscoveredToken;
> >    EFI_REDFISH_DISCOVERED_INSTANCE  *RedfishInstance;
> >
> > -  if (gRedfishServiceDiscovered) {
> > -    //
> > -    // Only support one Redfish service on platform.
> > -    //
> > -    return;
> > -  }
> > -
> >    RedfishDiscoveredToken = (EFI_REDFISH_DISCOVERED_TOKEN *)Context;
> > -  RedfishInstance        = RedfishDiscoveredToken-
> > >DiscoverList.RedfishInstances;
> > +  gBS->CloseEvent (RedfishDiscoveredToken->Event);
> > +
> >    //
> > -  // Only pick up the first found Redfish service.
> > +  // Only support one Redfish service on platform.
> >    //
> > -  if (RedfishInstance->Status == EFI_SUCCESS) {
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceRestExHandle =
> > RedfishInstance->Information.RedfishRestExHandle;
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceVersion      =
> > RedfishInstance->Information.RedfishVersion;
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceLocation     =
> > RedfishInstance->Information.Location;
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceUuid         =
> > RedfishInstance->Information.Uuid;
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceOs           =
> > RedfishInstance->Information.Os;
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceOsVersion    =
> > RedfishInstance->Information.OsVersion;
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceProduct      =
> > RedfishInstance->Information.Product;
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceProductVer   =
> > RedfishInstance->Information.ProductVer;
> > -    gRedfishConfigData.RedfishServiceInfo.RedfishServiceUseHttps     =
> > RedfishInstance->Information.UseHttps;
> > -    gRedfishServiceDiscovered                                        = TRUE;
> > +  if (!gRedfishServiceDiscovered) {
> > +    RedfishInstance        = RedfishDiscoveredToken-
> > >DiscoverList.RedfishInstances;
> > +    //
> > +    // Only pick up the first found Redfish service.
> > +    //
> > +    if (RedfishInstance->Status == EFI_SUCCESS) {
> > +
> > + gRedfishConfigData.RedfishServiceInfo.RedfishServiceRestExHandle =
> > RedfishInstance->Information.RedfishRestExHandle;
> > +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceVersion      =
> > RedfishInstance->Information.RedfishVersion;
> > +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceLocation     =
> > RedfishInstance->Information.Location;
> > +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceUuid         =
> > RedfishInstance->Information.Uuid;
> > +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceOs           =
> > RedfishInstance->Information.Os;
> > +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceOsVersion    =
> > RedfishInstance->Information.OsVersion;
> > +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceProduct      =
> > RedfishInstance->Information.Product;
> > +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceProductVer   =
> > RedfishInstance->Information.ProductVer;
> > +      gRedfishConfigData.RedfishServiceInfo.RedfishServiceUseHttps     =
> > RedfishInstance->Information.UseHttps;
> > +      gRedfishServiceDiscovered                                        = TRUE;
> > +    }
> > +
> > +    //
> > +    // Invoke RedfishConfigHandlerInstalledCallback to execute
> > +    // the initialization of Redfish Configure Handler instance.
> > +    //
> > +    RedfishConfigHandlerInstalledCallback (gRedfishConfigData.Event,
> > + &gRedfishConfigData);
> >    }
> >
> > -  //
> > -  // Invoke RedfishConfigHandlerInstalledCallback to execute
> > -  // the initialization of Redfish Configure Handler instance.
> > -  //
> > -  RedfishConfigHandlerInstalledCallback (gRedfishConfigData.Event,
> > &gRedfishConfigData);
> > +  FreePool(RedfishDiscoveredToken);
> > +
> >  }
> >
> >  /**
> > @@ -371,6 +361,7 @@ RedfishDiscoverProtocolInstalled (
> >    UINTN                                   NetworkInterfaceIndex;
> >    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE  *ThisNetworkInterface;
> >    EFI_REDFISH_DISCOVERED_TOKEN            *ThisRedfishDiscoveredToken;
> > +  UINTN                                   NumberOfNetworkInterfaces;
> >
> >    DEBUG ((DEBUG_INFO, "%a: New network interface is installed on
> > system by EFI Redfish discover driver.\n", __func__));
> >
> > @@ -408,36 +399,31 @@ RedfishDiscoverProtocolInstalled (
> >      }
> >    }
> >
> > -  //
> > -  // Check the new found network interface.
> > -  //
> > -  if (gNetworkInterfaceInstances != NULL) {
> > -    FreePool (gNetworkInterfaceInstances);
> > -  }
> >
> >    Status = gEfiRedfishDiscoverProtocol->GetNetworkInterfaceList (
> >                                            gEfiRedfishDiscoverProtocol,
> >                                            gRedfishConfigData.Image,
> > -                                          &gNumberOfNetworkInterfaces,
> > -                                          &gNetworkInterfaceInstances
> > +                                          &NumberOfNetworkInterfaces,
> > +                                          &ThisNetworkInterface
> >                                            );
> > -  if (EFI_ERROR (Status) || (gNumberOfNetworkInterfaces == 0)) {
> > +  if (EFI_ERROR (Status) || (NumberOfNetworkInterfaces == 0)) {
> >      DEBUG ((DEBUG_ERROR, "%a: No network interfaces found on the
> > handle.\n", __func__));
> >      return;
> >    }
> >
> > -  gRedfishDiscoveredToken = AllocateZeroPool
> > (gNumberOfNetworkInterfaces * sizeof
> (EFI_REDFISH_DISCOVERED_TOKEN));
> > -  if (gRedfishDiscoveredToken == NULL) {
> > +  //
> > +  // Loop to discover Redfish service on each network interface.
> > +  //
> > +  for (NetworkInterfaceIndex = 0; NetworkInterfaceIndex <
> > + NumberOfNetworkInterfaces; NetworkInterfaceIndex++) {
> > +
> > +    ThisRedfishDiscoveredToken = (EFI_REDFISH_DISCOVERED_TOKEN
> > *)AllocateZeroPool(sizeof (EFI_REDFISH_DISCOVERED_TOKEN));
> > +    if (ThisRedfishDiscoveredToken == NULL) {
> >      DEBUG ((DEBUG_ERROR, "%a: Not enough memory for
> > EFI_REDFISH_DISCOVERED_TOKEN.\n", __func__));
> >      return;
> >    }
> >
> > -  ThisNetworkInterface       = gNetworkInterfaceInstances;
> > -  ThisRedfishDiscoveredToken = gRedfishDiscoveredToken;
> > -  //
> > -  // Loop to discover Redfish service on each network interface.
> > -  //
> > -  for (NetworkInterfaceIndex = 0; NetworkInterfaceIndex <
> > gNumberOfNetworkInterfaces; NetworkInterfaceIndex++) {
> > +    ThisRedfishDiscoveredToken->Signature =
> > + REDFISH_DISCOVER_TOKEN_SIGNATURE;
> > +
> >      //
> >      // Initial this Redfish Discovered Token
> >      //
> > @@ -449,13 +435,11 @@ RedfishDiscoverProtocolInstalled (
> >                      &ThisRedfishDiscoveredToken->Event
> >                      );
> >      if (EFI_ERROR (Status)) {
> > +      FreePool(ThisRedfishDiscoveredToken);
> >        DEBUG ((DEBUG_ERROR, "%a: Failed to create event for Redfish
> > discovered token.\n", __func__));
> > -      goto ErrorReturn;
> > +      return;
> >      }
> >
> > -    ThisRedfishDiscoveredToken->Signature                         =
> > REDFISH_DISCOVER_TOKEN_SIGNATURE;
> > -    ThisRedfishDiscoveredToken->DiscoverList.NumberOfServiceFound = 0;
> > -    ThisRedfishDiscoveredToken->DiscoverList.RedfishInstances     = NULL;
> >      //
> >      // Acquire for Redfish service which is reported by
> >      // Redfish Host Interface.
> > @@ -467,21 +451,23 @@ RedfishDiscoverProtocolInstalled (
> >                                              EFI_REDFISH_DISCOVER_HOST_INTERFACE,
> >                                              ThisRedfishDiscoveredToken
> >                                              );
> > -    ThisNetworkInterface++;
> > -    ThisRedfishDiscoveredToken++;
> > -  }
> >
> > -  if (EFI_ERROR (Status)) {
> > -    DEBUG ((DEBUG_ERROR, "%a: Acquire Redfish service fail.\n",
> __func__));
> > -    goto ErrorReturn;
> > +    //
> > +    // Free Redfish Discovered Token if Discover Instance was not
> > + created
> > and
> > +    // Redfish Service Discovered Callback event was not triggered.
> > +    //
> > +    if(ThisRedfishDiscoveredToken->DiscoverList.NumberOfServiceFound
> > + ==
> > 0 ||
> > +        EFI_ERROR(ThisRedfishDiscoveredToken-
> > >DiscoverList.RedfishInstances->Status)){
> > +      gBS->CloseEvent (ThisRedfishDiscoveredToken->Event);
> > +      DEBUG ((DEBUG_ERROR, "%a: Free Redfish discovered token - %x.\n",
> > +          __func__, ThisRedfishDiscoveredToken));
> > +      FreePool(ThisRedfishDiscoveredToken);
> > +    }
> > +    ThisNetworkInterface++;
> >    }
> >
> >    return;
> >
> > -ErrorReturn:
> > -  if (gRedfishDiscoveredToken != NULL) {
> > -    FreePool (gRedfishDiscoveredToken);
> > -  }
> >  }
> >
> >  /**
> > @@ -498,25 +484,10 @@ RedfishConfigHandlerDriverUnload (
> >    IN EFI_HANDLE  ImageHandle
> >    )
> >  {
> > -  EFI_REDFISH_DISCOVERED_TOKEN  *ThisRedfishDiscoveredToken;
> > -  UINTN                         NumberOfNetworkInterfacesIndex;
> >
> >    RedfishConfigDriverCommonUnload (ImageHandle);
> >
> >    RedfishConfigStopRedfishDiscovery ();
> > -  if (gRedfishDiscoveredToken != NULL) {
> > -    ThisRedfishDiscoveredToken = gRedfishDiscoveredToken;
> > -    for (NumberOfNetworkInterfacesIndex = 0;
> > NumberOfNetworkInterfacesIndex < gNumberOfNetworkInterfaces;
> > NumberOfNetworkInterfacesIndex++) {
> > -      if (ThisRedfishDiscoveredToken->Event != NULL) {
> > -        gBS->CloseEvent (ThisRedfishDiscoveredToken->Event);
> > -      }
> > -
> > -      FreePool (ThisRedfishDiscoveredToken);
> > -      ThisRedfishDiscoveredToken++;
> > -    }
> > -
> > -    gRedfishDiscoveredToken = NULL;
> > -  }
> >
> >    return EFI_SUCCESS;
> >  }
> > diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > index 583c6f7..a6c8d60 100644
> > --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > @@ -1095,8 +1095,10 @@ RedfishServiceGetNetworkInterface (  {
> >    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> > *ThisNetworkInterfaceIntn;
> >    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> > *ThisNetworkInterface;
> > +  EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL
> *RestExInstance;
> >
> > -  if ((NetworkIntfInstances == NULL) || (NumberOfNetworkIntfs ==
> > NULL)
> > || (ImageHandle == NULL)) {
> > +  if ((This == NULL) || (NetworkIntfInstances == NULL) ||
> > (NumberOfNetworkIntfs == NULL) ||
> > +      (ImageHandle == NULL)) {
> >      return EFI_INVALID_PARAMETER;
> >    }
> >
> > @@ -1107,14 +1109,30 @@ RedfishServiceGetNetworkInterface (
> >      return EFI_NOT_FOUND;
> >    }
> >
> > +  RestExInstance =
> > + EFI_REDFISH_DISOVER_DATA_FROM_DISCOVER_PROTOCOL(This);
> > +
> > +  //
> > +  // Check the new found network interface.
> > +  //
> > +  if (RestExInstance->NetworkInterfaceInstances != NULL) {
> > +    FreePool (RestExInstance->NetworkInterfaceInstances);
> > +    RestExInstance->NetworkInterfaceInstances = NULL;  }
> > +
> >    ThisNetworkInterface = (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> > *)AllocateZeroPool (sizeof
> (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE)
> > * mNumNetworkInterface);
> >    if (ThisNetworkInterface == NULL) {
> >      return EFI_OUT_OF_RESOURCES;
> >    }
> >
> >    *NetworkIntfInstances    = ThisNetworkInterface;
> > +
> > +
> > +  RestExInstance->NetworkInterfaceInstances = ThisNetworkInterface;
> > + RestExInstance->NumberOfNetworkInterfaces = 0;
> > +
> >    ThisNetworkInterfaceIntn =
> > (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> *)GetFirstNode
> > (&mEfiRedfishDiscoverNetworkInterface);
> >    while (TRUE) {
> > +
> >      ThisNetworkInterface->IsIpv6 = FALSE;
> >      if (CheckIsIpVersion6 (ThisNetworkInterfaceIntn)) {
> >        ThisNetworkInterface->IsIpv6 = TRUE; @@ -1130,7 +1148,7 @@
> > RedfishServiceGetNetworkInterface (
> >
> >      ThisNetworkInterface->SubnetPrefixLength =
> > ThisNetworkInterfaceIntn-
> > >SubnetPrefixLength;
> >      ThisNetworkInterface->VlanId             = ThisNetworkInterfaceIntn-
> >VlanId;
> > -    (*NumberOfNetworkIntfs)++;
> > +    RestExInstance->NumberOfNetworkInterfaces++;
> >      if (IsNodeAtEnd (&mEfiRedfishDiscoverNetworkInterface,
> > &ThisNetworkInterfaceIntn->Entry)) {
> >        break;
> >      }
> > @@ -1139,6 +1157,8 @@ RedfishServiceGetNetworkInterface (
> >      ThisNetworkInterface++;
> >    }
> >
> > +  *NumberOfNetworkIntfs = RestExInstance-
> > >NumberOfNetworkInterfaces;
> > +
> >    return EFI_SUCCESS;
> >  }
> >
> > @@ -1178,7 +1198,6 @@ RedfishServiceAcquireService (  {
> >    EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE         *Instance;
> >    EFI_STATUS                                       Status1;
> > -  EFI_STATUS                                       Status2;
> >    BOOLEAN                                          NewInstance;
> >    UINTN                                            NumNetworkInterfaces;
> >    UINTN                                            NetworkInterfacesIndex;
> > @@ -1215,7 +1234,6 @@ RedfishServiceAcquireService (
> >
> >    for (NetworkInterfacesIndex = 0; NetworkInterfacesIndex <
> > NumNetworkInterfaces; NetworkInterfacesIndex++) {
> >      Status1     = EFI_SUCCESS;
> > -    Status2     = EFI_SUCCESS;
> >      NewInstance = FALSE;
> >      Instance    = GetInstanceByOwner (ImageHandle,
> > TargetNetworkInterfaceInternal, Flags &
> > ~EFI_REDFISH_DISCOVER_VALIDATION); // Check if we can re-use
> previous
> > instance.
> >      if (Instance == NULL) {
> > @@ -1223,6 +1241,7 @@ RedfishServiceAcquireService (
> >        Instance = (EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE
> > *)AllocateZeroPool (sizeof
> > (EFI_REDFISH_DISCOVERED_INTERNAL_INSTANCE));
> >        if (Instance == NULL) {
> >          DEBUG ((DEBUG_ERROR, "%a:Memory allocation fail.\n",
> > __func__));
> > +        return EFI_OUT_OF_RESOURCES;
> >        }
> >
> >        InitializeListHead (&Instance->Entry); @@ -1258,9 +1277,11 @@
> > RedfishServiceAcquireService (
> >        DEBUG ((DEBUG_ERROR, "%a:Redfish service discovery through SSDP
> > is not supported\n", __func__));
> >        return EFI_UNSUPPORTED;
> >      } else {
> > -      if (EFI_ERROR (Status1) && EFI_ERROR (Status2)) {
> > -        FreePool ((VOID *)Instance);
> > -        DEBUG ((DEBUG_ERROR, "%a:Something wrong on Redfish service
> > discovery Status1=%x, Status2=%x.\n", __func__, Status1, Status2));
> > +      if (EFI_ERROR (Status1)) {
> > +        if (NewInstance) {
> > +          FreePool ((VOID *)Instance);
> > +        }
> > +        DEBUG ((DEBUG_ERROR, "%a:Something wrong on Redfish service
> > + discovery Status1=%r.\n", __func__, Status1));
> >        } else {
> >          if (NewInstance) {
> >            InsertTailList (&mRedfishDiscoverList, &Instance->Entry);
> > @@ -1387,13
> > +1408,6 @@ ReleaseNext:;
> >    }
> >  }
> >
> > -EFI_REDFISH_DISCOVER_PROTOCOL  mRedfishDiscover = {
> > -  RedfishServiceGetNetworkInterface,
> > -  RedfishServiceAcquireService,
> > -  RedfishServiceAbortAcquire,
> > -  RedfishServiceReleaseService
> > -};
> > -
> >  /**
> >    This function create an
> > EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL for the
> >    given network interface.
> > @@ -1713,12 +1727,20 @@ BuildupNetworkInterface (
> >
> >            NewNetworkInterfaceInstalled                       = FALSE;
> >            NetworkInterface->EfiRedfishDiscoverProtocolHandle = NULL;
> > -          Status                                             = gBS->InstallProtocolInterface (
> > -                                                                      &NetworkInterface-
> > >EfiRedfishDiscoverProtocolHandle,
> > -                                                                      &gEfiRedfishDiscoverProtocolGuid,
> > -                                                                      EFI_NATIVE_INTERFACE,
> > -                                                                      (VOID *)&mRedfishDiscover
> > -                                                                      );
> > +
> > +          RestExInstance->Signature =
> > + EFI_REDFISH_DISCOVER_DATA_SIGNATURE;
> > +
> > +
> > + RestExInstance->RedfishDiscoverProtocol.GetNetworkInterfaceList =
> > RedfishServiceGetNetworkInterface;
> > +
> > + RestExInstance->RedfishDiscoverProtocol.AcquireRedfishService =
> > RedfishServiceAcquireService;
> > +
> > + RestExInstance->RedfishDiscoverProtocol.AbortAcquireRedfishService
> > = RedfishServiceAbortAcquire;
> > +
> > + RestExInstance->RedfishDiscoverProtocol.ReleaseRedfishService
> > + = RedfishServiceReleaseService;
> > +
> > +          Status = gBS->InstallProtocolInterface (
> > +                          &NetworkInterface->EfiRedfishDiscoverProtocolHandle,
> > +                          &gEfiRedfishDiscoverProtocolGuid,
> > +                          EFI_NATIVE_INTERFACE,
> > +                          (VOID *)&RestExInstance->RedfishDiscoverProtocol
> > +                          );
> >            if (EFI_ERROR (Status)) {
> >              DEBUG ((DEBUG_ERROR, "%a: Fail to install
> > EFI_REDFISH_DISCOVER_PROTOCOL\n", __func__));
> >            }
> > @@ -1815,6 +1837,7 @@ StopServiceOnNetworkInterface (
> >    EFI_HANDLE                                       DiscoverProtocolHandle;
> >    EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> > *ThisNetworkInterface;
> >    EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL
> *RestExInstance;
> > +  EFI_REDFISH_DISCOVER_PROTOCOL                    *RedfishDiscoverProtocol;
> >
> >    for (Index = 0; Index < (sizeof (gRequiredProtocol) / sizeof
> > (REDFISH_DISCOVER_REQUIRED_PROTOCOL)); Index++) {
> >      Status = gBS->HandleProtocol (
> > @@ -1854,12 +1877,30 @@ StopServiceOnNetworkInterface (
> >              // client which uses .EFI Redfish discover protocol.
> >              //
> >              if (DiscoverProtocolHandle != NULL) {
> > -              gBS->DisconnectController (DiscoverProtocolHandle, NULL, NULL);
> > -              Status = gBS->UninstallProtocolInterface (
> > +
> > +              Status = gBS->HandleProtocol (
> >                                DiscoverProtocolHandle,
> >                                &gEfiRedfishDiscoverProtocolGuid,
> > -                              (VOID *)&mRedfishDiscover
> > +                              (VOID **) &RedfishDiscoverProtocol
> >                                );
> > +              if (!EFI_ERROR (Status)) {
> > +                RestExInstance =
> >
> EFI_REDFISH_DISOVER_DATA_FROM_DISCOVER_PROTOCOL(RedfishDiscove
> > rProtocol);
> > +                //
> > +                // Stop Redfish service discovery.
> > +                //
> > +                RedfishDiscoverProtocol->AbortAcquireRedfishService (
> > +                                           RedfishDiscoverProtocol,
> > +                                           RestExInstance->NetworkInterfaceInstances
> > +                                           );
> > +
> > +
> > +                gBS->DisconnectController (DiscoverProtocolHandle, NULL, NULL);
> > +                Status = gBS->UninstallProtocolInterface (
> > +                                DiscoverProtocolHandle,
> > +                                &gEfiRedfishDiscoverProtocolGuid,
> > +                                (VOID *)&RestExInstance->RedfishDiscoverProtocol
> > +                                );
> > +              }
> >              }
> >
> >              return Status;
> > diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> > b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> > index 2704cd9..ddb5f2e 100644
> > --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> > +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
> > @@ -118,16 +118,30 @@ typedef struct {  }
> > EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL;
> >
> >  //
> > +// Redfish Discover Instance signature //
> > +
> > +#define EFI_REDFISH_DISCOVER_DATA_SIGNATURE   SIGNATURE_32 ('E',
> > 'R', 'D', 'D')
> > +
> > +#define EFI_REDFISH_DISOVER_DATA_FROM_DISCOVER_PROTOCOL(a) \
> > +  CR (a, EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL,
> > +RedfishDiscoverProtocol, EFI_REDFISH_DISCOVER_DATA_SIGNATURE)
> > +
> > +//
> >  // Internal structure used to maintain REST EX properties.
> >  //
> >  typedef struct {
> > -  LIST_ENTRY              Entry;                      ///< Link list entry.
> > -  EFI_HANDLE              OpenDriverAgentHandle;      ///< The agent to open
> > network protocol.
> > -  EFI_HANDLE              OpenDriverControllerHandle; ///< The controller
> handle
> > to open network protocol.
> > -  EFI_HANDLE              RestExChildHandle;          ///< The child handle created
> > through REST EX Service Protocol.
> > -  EFI_HANDLE              RestExControllerHandle;     ///< The controller handle
> > which provide REST EX protocol.
> > -  EFI_REST_EX_PROTOCOL    *RestExProtocolInterface;   ///< Pointer to
> > EFI_REST_EX_PROTOCOL.
> > -  UINT32                  RestExId;                   ///< The identifier installed on REST EX
> > controller handle.
> > +  LIST_ENTRY                              Entry;                      ///< Link list entry.
> > +  UINT32                                  Signature;                  ///< Instance signature.
> > +  EFI_HANDLE                              OpenDriverAgentHandle;      ///< The agent to
> > open network protocol.
> > +  EFI_HANDLE                              OpenDriverControllerHandle; ///< The
> > controller handle to open network protocol.
> > +  EFI_HANDLE                              RestExChildHandle;          ///< The child handle
> > created through REST EX Service Protocol.
> > +  EFI_HANDLE                              RestExControllerHandle;     ///< The controller
> > handle which provide REST EX protocol.
> > +  EFI_REST_EX_PROTOCOL                    *RestExProtocolInterface;   ///<
> Pointer
> > to EFI_REST_EX_PROTOCOL.
> > +  UINT32                                  RestExId;                   ///< The identifier installed on
> > REST EX controller handle.
> > +  UINTN                                   NumberOfNetworkInterfaces;  ///< Number of
> > network interfaces can do Redfish service discovery.
> > +  EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> > *NetworkInterfaceInstances; ///< Network interface instances. It's an
> > array of instances. The number of entries
> > +
> > + ///< in array is indicated by
> > NumberOfNetworkInterfaces.
> > +  EFI_REDFISH_DISCOVER_PROTOCOL           RedfishDiscoverProtocol;
> ///<
> > EFI_REDFISH_DISCOVER_PROTOCOL protocol.
> >  } EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL;
> >
> >  /**
> > --
> > 2.6.1.windows.1
> > -The information contained in this message may be confidential and
> > proprietary to American Megatrends (AMI). This communication is
> > intended to be read only by the individual or entity to whom it is
> > addressed or by their designee. If the reader of this message is not
> > the intended recipient, you are on notice that any distribution of
> > this message, in any form, is strictly prohibited. Please promptly
> > notify the sender by reply e-mail or by telephone at 770-246-8600, and
> > then delete or destroy all copies of the transmission.
> -The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI). This communication is intended
> to be read only by the individual or entity to whom it is addressed or by their
> designee. If the reader of this message is not the intended recipient, you are
> on notice that any distribution of this message, in any form, is strictly
> prohibited. Please promptly notify the sender by reply e-mail or by
> telephone at 770-246-8600, and then delete or destroy all copies of the
> transmission.
-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103428): https://edk2.groups.io/g/devel/message/103428
Mute This Topic: https://groups.io/mt/98368593/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-