public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/5] Adding necessary changes for RedfishPkg
@ 2023-04-14  8:18 Minh Nguyen
  2023-04-14  8:18 ` [PATCH 1/5] RedfishPkg: Correct variable type to prevent memory corruption Minh Nguyen
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Minh Nguyen @ 2023-04-14  8:18 UTC (permalink / raw)
  To: devel; +Cc: patches, abner.chang, nicklew, igork, nhi, tinhnguyen,
	Minh Nguyen

This patchset adds necessary changes for RedfishPkg to avoid 
some unexpected cases and fix compilation.

Nhi Pham (1):
  RedfishPkg: Add missing newline character

Vu Nguyen (4):
  RedfishPkg: Correct variable type to prevent memory corruption
  RedfishPkg: Prevent assertion of allocate zero length region
  RedfishPkg: Create RestEx child on selected interface
  RedfishPkg: Fix compile issue on Linux

 RedfishPkg/Include/Library/RestExLib.h                       |   3 +
 RedfishPkg/Include/Pcd/RestExServiceDevicePath.h             |   5 +-
 RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h            |   3 +-
 RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c               | 153 ++++++++------------
 RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c           |   6 +-
 RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c |   3 +-
 6 files changed, 74 insertions(+), 99 deletions(-)

-- 
2.39.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/5] RedfishPkg: Correct variable type to prevent memory corruption
  2023-04-14  8:18 [PATCH 0/5] Adding necessary changes for RedfishPkg Minh Nguyen
@ 2023-04-14  8:18 ` Minh Nguyen
  2023-04-14  8:52   ` Nickle Wang
  2023-04-14  8:19 ` [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero length region Minh Nguyen
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Minh Nguyen @ 2023-04-14  8:18 UTC (permalink / raw)
  To: devel
  Cc: patches, abner.chang, nicklew, igork, nhi, tinhnguyen, Vu Nguyen,
	Minh Nguyen

From: Vu Nguyen <vunguyen@os.amperecomputing.com>

Id will be casted by CoreOpenProtocol, declare this variable with a
wrong type might result in the corruption of other local variables.

Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
---
 RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h b/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h
index 4a9e9f7b81a4..6b94e5814c43 100644
--- a/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h
+++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h
@@ -3,6 +3,7 @@
 
   Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
   (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
+  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -84,7 +85,7 @@ struct _RESTEX_SERVICE {
   //
   EFI_HANDLE                      HttpChildHandle;
 
-  UINT32                          Id;
+  UINT32                          *Id;
 
   EFI_REST_EX_SERVICE_INFO        RestExServiceInfo;
 };
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero length region
  2023-04-14  8:18 [PATCH 0/5] Adding necessary changes for RedfishPkg Minh Nguyen
  2023-04-14  8:18 ` [PATCH 1/5] RedfishPkg: Correct variable type to prevent memory corruption Minh Nguyen
@ 2023-04-14  8:19 ` Minh Nguyen
  2023-04-14  9:03   ` Nickle Wang
  2023-04-14  8:19 ` [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface Minh Nguyen
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Minh Nguyen @ 2023-04-14  8:19 UTC (permalink / raw)
  To: devel
  Cc: patches, abner.chang, nicklew, igork, nhi, tinhnguyen, Vu Nguyen,
	Minh Nguyen

From: Vu Nguyen <vunguyen@os.amperecomputing.com>

This change also fix miscellaneous errors on condition checking and
debug argument missing.

Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
---
 RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
index 583c6f78e19c..29e7b0624ebf 100644
--- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
+++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
@@ -5,6 +5,7 @@
   (C) Copyright 2021 Hewlett Packard Enterprise Development LP<BR>
   Copyright (c) 2022, AMD Incorporated. All rights reserved.
   Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -272,7 +273,7 @@ Tcp6GetSubnetInfo (
   if (IpModedata.AddressCount == 0) {
     DEBUG ((DEBUG_INFO, "%a: No IPv6 address configured.\n", __func__));
     Instance->SubnetAddrInfoIPv6Number = 0;
-    return EFI_SUCCESS;
+    return EFI_NOT_FOUND;
   }
 
   if (Instance->SubnetAddrInfoIPv6 != NULL) {
@@ -926,7 +927,7 @@ AddAndSignalNewRedfishService (
     }
 
     Status = gBS->SignalEvent (Instance->DiscoverToken->Event);
-    if (!EFI_ERROR (Status)) {
+    if (EFI_ERROR (Status)) {
       DEBUG ((DEBUG_ERROR, "%a:No event to signal!\n", __func__));
     }
   }
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface
  2023-04-14  8:18 [PATCH 0/5] Adding necessary changes for RedfishPkg Minh Nguyen
  2023-04-14  8:18 ` [PATCH 1/5] RedfishPkg: Correct variable type to prevent memory corruption Minh Nguyen
  2023-04-14  8:19 ` [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero length region Minh Nguyen
@ 2023-04-14  8:19 ` Minh Nguyen
  2023-04-16  4:36   ` Chang, Abner
  2023-04-14  8:19 ` [PATCH 4/5] RedfishPkg: Add missing newline character Minh Nguyen
  2023-04-14  8:19 ` [PATCH 5/5] RedfishPkg: Fix compile issue on Linux Minh Nguyen
  4 siblings, 1 reply; 17+ messages in thread
From: Minh Nguyen @ 2023-04-14  8:19 UTC (permalink / raw)
  To: devel
  Cc: patches, abner.chang, nicklew, igork, nhi, tinhnguyen, Vu Nguyen,
	Minh Nguyen

From: Vu Nguyen <vunguyen@os.amperecomputing.com>

When a MAC address matching interface is found, a RestEx child will be
created to provide the Redfish communication on that interface.
Currently, It will try to locate all RestEx binding services and choose
the first satisfied instance without taking care about current selected
interface. This might raise an issue on the system with multiple network
devices that the RestEx child was installed on wrong interface.

Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
---
 RedfishPkg/Include/Library/RestExLib.h             |   3 +
 RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c     | 153 ++++++++------------
 RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c |   1 +
 3 files changed, 63 insertions(+), 94 deletions(-)

diff --git a/RedfishPkg/Include/Library/RestExLib.h b/RedfishPkg/Include/Library/RestExLib.h
index bc4e4ca6caa7..2c32c3684cf4 100644
--- a/RedfishPkg/Include/Library/RestExLib.h
+++ b/RedfishPkg/Include/Library/RestExLib.h
@@ -2,6 +2,7 @@
   This library provides help functions for REST EX Protocol.
 
   (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
+  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -20,6 +21,7 @@
   This function allows the caller to create child handle for specific
   REST server.
 
+  @param[in]  Controller           The controller handle used of selected interface.
   @param[in]  Image                The image handle used to open service.
   @param[in]  AccessMode           Access mode of REST server.
   @param[in]  ConfigType           Underlying configuration to communicate with REST server.
@@ -32,6 +34,7 @@
 **/
 EFI_STATUS
 RestExLibCreateChild (
+  IN EFI_HANDLE                       Controller,
   IN EFI_HANDLE                       Image,
   IN EFI_REST_EX_SERVICE_ACCESS_MODE  AccessMode,
   IN EFI_REST_EX_CONFIG_TYPE          ConfigType,
diff --git a/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c b/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
index d9acad24dec1..6002be93c54e 100644
--- a/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
+++ b/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
@@ -2,6 +2,7 @@
   This library provides help functions for REST EX Protocol.
 
   (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
+  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -9,6 +10,7 @@
 
 #include <Uefi.h>
 #include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/NetLib.h>
 #include <Library/UefiBootServicesTableLib.h>
@@ -21,6 +23,7 @@
   This function allows the caller to create child handle for specific
   REST server.
 
+  @param[in]  Controller           The controller handle used of selected interface.
   @param[in]  Image                The image handle used to open service.
   @param[in]  AccessMode           Access mode of REST server.
   @param[in]  ConfigType           Underlying configuration to communicate with REST server.
@@ -33,6 +36,7 @@
 **/
 EFI_STATUS
 RestExLibCreateChild (
+  IN EFI_HANDLE                       Controller,
   IN EFI_HANDLE                       Image,
   IN EFI_REST_EX_SERVICE_ACCESS_MODE  AccessMode,
   IN EFI_REST_EX_CONFIG_TYPE          ConfigType,
@@ -41,8 +45,6 @@ RestExLibCreateChild (
   )
 {
   EFI_STATUS                Status;
-  UINTN                     NoBuffer;
-  EFI_HANDLE                *Handle;
   EFI_HANDLE                ChildHandle;
   EFI_REST_EX_PROTOCOL      *RestEx;
   EFI_REST_EX_SERVICE_INFO  *RestExServiceInfo;
@@ -59,116 +61,79 @@ RestExLibCreateChild (
   }
 
   *ChildInstanceHandle = NULL;
-  //
-  // Locate all REST EX binding service.
-  //
-  Handle   = NULL;
-  NoBuffer = 0;
-  Status   = gBS->LocateHandleBuffer (
-                    ByProtocol,
-                    &gEfiRestExServiceBindingProtocolGuid,
-                    NULL,
-                    &NoBuffer,
-                    &Handle
-                    );
-  if (EFI_ERROR (Status) && (Status != EFI_BUFFER_TOO_SMALL)) {
+
+  ChildHandle = NULL;
+  Status = NetLibCreateServiceChild (
+             Controller,
+             Image,
+             &gEfiRestExServiceBindingProtocolGuid,
+             &ChildHandle
+             );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Failed to create service child - %r \n",
+      __FUNCTION__,
+      Status
+      ));
     return Status;
   }
-
-  Handle = (EFI_HANDLE *)AllocateZeroPool (sizeof (EFI_HANDLE) * NoBuffer);
-  if (Handle == NULL) {
-    return EFI_OUT_OF_RESOURCES;
-  }
-
-  Status = gBS->LocateHandleBuffer (
-                  ByProtocol,
-                  &gEfiRestExServiceBindingProtocolGuid,
+  Status = gBS->OpenProtocol (
+                  ChildHandle,
+                  &gEfiRestExProtocolGuid,
+                  (VOID **)&RestEx,
+                  Image,
                   NULL,
-                  &NoBuffer,
-                  &Handle
+                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
                   );
   if (EFI_ERROR (Status)) {
-    FreePool (Handle);
-    return Status;
+    goto ON_ERROR;
   }
 
   //
-  // Search for the proper REST EX instance.
+  // Get the information of REST service provided by this EFI REST EX driver
   //
-  while (NoBuffer != 0) {
-    ChildHandle = NULL;
-    Status      = NetLibCreateServiceChild (
-                    *(Handle + (NoBuffer - 1)),
-                    Image,
-                    &gEfiRestExServiceBindingProtocolGuid,
-                    &ChildHandle
-                    );
-    if (!EFI_ERROR (Status)) {
-      Status = gBS->OpenProtocol (
-                      ChildHandle,
-                      &gEfiRestExProtocolGuid,
-                      (VOID **)&RestEx,
-                      Image,
-                      NULL,
-                      EFI_OPEN_PROTOCOL_GET_PROTOCOL
-                      );
-      if (EFI_ERROR (Status)) {
-        goto ON_ERROR;
-      }
-
-      //
-      // Get the information of REST service provided by this EFI REST EX driver
-      //
-      Status = RestEx->GetService (
-                         RestEx,
-                         &RestExServiceInfo
-                         );
-      if (EFI_ERROR (Status)) {
-        goto ON_ERROR;
-      }
-
-      //
-      // Check REST EX property.
-      //
-      switch (ConfigType) {
-        case EfiRestExConfigHttp:
-          LenOfConfig = sizeof (EFI_REST_EX_HTTP_CONFIG_DATA);
-          break;
-
-        case EfiRestExConfigUnspecific:
-          LenOfConfig = REST_EX_CONFIG_DATA_LEN_UNKNOWN;
-          break;
+  Status = RestEx->GetService (
+                     RestEx,
+                     &RestExServiceInfo
+                     );
+  if (EFI_ERROR (Status)) {
+    goto ON_ERROR;
+  }
+  //
+  // Check REST EX property.
+  //
+  switch (ConfigType) {
+  case EfiRestExConfigHttp:
+    LenOfConfig = sizeof (EFI_REST_EX_HTTP_CONFIG_DATA);
+    break;
 
-        default:
-          goto ON_ERROR;
-      }
+  case EfiRestExConfigUnspecific:
+    LenOfConfig = REST_EX_CONFIG_DATA_LEN_UNKNOWN;
+    break;
 
-      if ((RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceAccessMode != AccessMode) ||
-          (RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceType != ServiceType) ||
-          (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigType != ConfigType) ||
-          ((LenOfConfig != REST_EX_CONFIG_DATA_LEN_UNKNOWN) && (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigDataLength != LenOfConfig)))
-      {
-        goto ON_ERROR;
-      }
-    }
+  default:
+    goto ON_ERROR;
+  }
+  if (RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceAccessMode != AccessMode ||
+      RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceType != ServiceType ||
+      RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigType != ConfigType ||
+      ((LenOfConfig != REST_EX_CONFIG_DATA_LEN_UNKNOWN) && (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigDataLength != LenOfConfig))) {
+    goto ON_ERROR;
+  }
 
-    //
-    // This is proper REST EX instance.
-    //
-    *ChildInstanceHandle = ChildHandle;
-    FreePool (Handle);
-    return EFI_SUCCESS;
+  //
+  // This is proper REST EX instance.
+  //
+  *ChildInstanceHandle = ChildHandle;
+  return EFI_SUCCESS;
 
 ON_ERROR:;
     NetLibDestroyServiceChild (
-      *(Handle + (NoBuffer - 1)),
+      Controller,
       Image,
       &gEfiRestExServiceBindingProtocolGuid,
       ChildHandle
       );
-    NoBuffer--;
-  }
-
-  FreePool (Handle);
   return EFI_NOT_FOUND;
 }
diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
index 29e7b0624ebf..86ec88ddafc4 100644
--- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
+++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
@@ -87,6 +87,7 @@ CreateRestExInstance (
   EFI_STATUS  Status;
 
   Status = RestExLibCreateChild (
+             Instance->NetworkInterface->OpenDriverControllerHandle,
              Instance->Owner,
              FixedPcdGetBool (PcdRedfishDiscoverAccessModeInBand) ? EfiRestExServiceInBandAccess : EfiRestExServiceOutOfBandAccess,
              EfiRestExConfigHttp,
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/5] RedfishPkg: Add missing newline character
  2023-04-14  8:18 [PATCH 0/5] Adding necessary changes for RedfishPkg Minh Nguyen
                   ` (2 preceding siblings ...)
  2023-04-14  8:19 ` [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface Minh Nguyen
@ 2023-04-14  8:19 ` Minh Nguyen
  2023-04-14  8:53   ` Nickle Wang
  2023-04-14  8:19 ` [PATCH 5/5] RedfishPkg: Fix compile issue on Linux Minh Nguyen
  4 siblings, 1 reply; 17+ messages in thread
From: Minh Nguyen @ 2023-04-14  8:19 UTC (permalink / raw)
  To: devel; +Cc: patches, abner.chang, nicklew, igork, nhi, tinhnguyen,
	Minh Nguyen

From: Nhi Pham <nhi@os.amperecomputing.com>

This adds a missing newline character to make the error log
readable in case the module is failed.

Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
---
 RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
index 45fc6e2182bc..c4fa59193a2f 100644
--- a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
+++ b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
@@ -8,6 +8,7 @@
   (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
   Copyright (C) 2022 Advanced Micro Devices, Inc. All rights reserved.<BR>
   Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -59,7 +60,7 @@ RedfishCreateSmbiosTable42 (
   Status = RedfishPlatformHostInterfaceDeviceDescriptor (&DeviceType, &DeviceDescriptor);
   if (EFI_ERROR (Status)) {
     if (Status == EFI_NOT_FOUND) {
-      DEBUG ((DEBUG_ERROR, "%a: No Redfish host interface descriptor is provided on this platform.", __func__));
+      DEBUG ((DEBUG_ERROR, "%a: No Redfish host interface descriptor is provided on this platform.\n", __func__));
       return EFI_NOT_FOUND;
     }
 
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/5] RedfishPkg: Fix compile issue on Linux
  2023-04-14  8:18 [PATCH 0/5] Adding necessary changes for RedfishPkg Minh Nguyen
                   ` (3 preceding siblings ...)
  2023-04-14  8:19 ` [PATCH 4/5] RedfishPkg: Add missing newline character Minh Nguyen
@ 2023-04-14  8:19 ` Minh Nguyen
  2023-04-14 14:13   ` Chang, Abner
  4 siblings, 1 reply; 17+ messages in thread
From: Minh Nguyen @ 2023-04-14  8:19 UTC (permalink / raw)
  To: devel
  Cc: patches, abner.chang, nicklew, igork, nhi, tinhnguyen, Vu Nguyen,
	Minh Nguyen

From: Vu Nguyen <vunguyen@os.amperecomputing.com>

It requires a fixed size array to store the content of device path PCD.
Add the array size to solve this issue.

Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
---
 RedfishPkg/Include/Pcd/RestExServiceDevicePath.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/RedfishPkg/Include/Pcd/RestExServiceDevicePath.h b/RedfishPkg/Include/Pcd/RestExServiceDevicePath.h
index 91b1198297c2..57fc199f61f2 100644
--- a/RedfishPkg/Include/Pcd/RestExServiceDevicePath.h
+++ b/RedfishPkg/Include/Pcd/RestExServiceDevicePath.h
@@ -4,6 +4,7 @@
 
   Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
   (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
+  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
 
     SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -14,6 +15,8 @@
 
 #include <Protocol/DevicePath.h>
 
+#define MAX_DEVICE_PATH_NODE      40
+
 typedef enum {
   DEVICE_PATH_MATCH_MAC_NODE = 1,
   DEVICE_PATH_MATCH_PCI_NODE = 2,
@@ -32,7 +35,7 @@ typedef struct {
   //    0x03,0x0b,0x25,0x00,0x00,0x50,0x56,0xc0,0x00,0x02,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x01,
   //    0x7f,0xff,0x04,0x00}
   //
-  EFI_DEVICE_PATH_PROTOCOL    DevicePath[];
+  EFI_DEVICE_PATH_PROTOCOL      DevicePath[MAX_DEVICE_PATH_NODE];
 } REST_EX_SERVICE_DEVICE_PATH_DATA;
 
 #endif
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/5] RedfishPkg: Correct variable type to prevent memory corruption
  2023-04-14  8:18 ` [PATCH 1/5] RedfishPkg: Correct variable type to prevent memory corruption Minh Nguyen
@ 2023-04-14  8:52   ` Nickle Wang
  0 siblings, 0 replies; 17+ messages in thread
From: Nickle Wang @ 2023-04-14  8:52 UTC (permalink / raw)
  To: Minh Nguyen, devel@edk2.groups.io
  Cc: patches@amperecomputing.com, abner.chang@amd.com, igork@ami.com,
	nhi@os.amperecomputing.com, tinhnguyen@os.amperecomputing.com,
	Vu Nguyen

Reviewed-by: Nickle Wang <nicklew@nvidia.com>

Regards,
Nickle

> -----Original Message-----
> From: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> Sent: Friday, April 14, 2023 4:19 PM
> To: devel@edk2.groups.io
> Cc: patches@amperecomputing.com; abner.chang@amd.com; Nickle Wang
> <nicklew@nvidia.com>; igork@ami.com; nhi@os.amperecomputing.com;
> tinhnguyen@os.amperecomputing.com; Vu Nguyen
> <vunguyen@os.amperecomputing.com>; Minh Nguyen
> <minhnguyen@os.amperecomputing.com>
> Subject: [PATCH 1/5] RedfishPkg: Correct variable type to prevent memory
> corruption
> 
> External email: Use caution opening links or attachments
> 
> 
> From: Vu Nguyen <vunguyen@os.amperecomputing.com>
> 
> Id will be casted by CoreOpenProtocol, declare this variable with a wrong type
> might result in the corruption of other local variables.
> 
> Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> ---
>  RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h
> b/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h
> index 4a9e9f7b81a4..6b94e5814c43 100644
> --- a/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h
> +++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h
> @@ -3,6 +3,7 @@
> 
>    Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
>    (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
> +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -84,7 +85,7 @@ struct _RESTEX_SERVICE {
>    //
>    EFI_HANDLE                      HttpChildHandle;
> 
> -  UINT32                          Id;
> +  UINT32                          *Id;
> 
>    EFI_REST_EX_SERVICE_INFO        RestExServiceInfo;
>  };
> --
> 2.39.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/5] RedfishPkg: Add missing newline character
  2023-04-14  8:19 ` [PATCH 4/5] RedfishPkg: Add missing newline character Minh Nguyen
@ 2023-04-14  8:53   ` Nickle Wang
  0 siblings, 0 replies; 17+ messages in thread
From: Nickle Wang @ 2023-04-14  8:53 UTC (permalink / raw)
  To: Minh Nguyen, devel@edk2.groups.io
  Cc: patches@amperecomputing.com, abner.chang@amd.com, igork@ami.com,
	nhi@os.amperecomputing.com, tinhnguyen@os.amperecomputing.com

Reviewed-by: Nickle Wang <nicklew@nvidia.com>

Regards,
Nickle

> -----Original Message-----
> From: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> Sent: Friday, April 14, 2023 4:19 PM
> To: devel@edk2.groups.io
> Cc: patches@amperecomputing.com; abner.chang@amd.com; Nickle Wang
> <nicklew@nvidia.com>; igork@ami.com; nhi@os.amperecomputing.com;
> tinhnguyen@os.amperecomputing.com; Minh Nguyen
> <minhnguyen@os.amperecomputing.com>
> Subject: [PATCH 4/5] RedfishPkg: Add missing newline character
> 
> External email: Use caution opening links or attachments
> 
> 
> From: Nhi Pham <nhi@os.amperecomputing.com>
> 
> This adds a missing newline character to make the error log readable in case the
> module is failed.
> 
> Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> ---
>  RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
> b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
> index 45fc6e2182bc..c4fa59193a2f 100644
> --- a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
> +++ b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
> @@ -8,6 +8,7 @@
>    (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
>    Copyright (C) 2022 Advanced Micro Devices, Inc. All rights reserved.<BR>
>    Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -59,7 +60,7 @@ RedfishCreateSmbiosTable42 (
>    Status = RedfishPlatformHostInterfaceDeviceDescriptor (&DeviceType,
> &DeviceDescriptor);
>    if (EFI_ERROR (Status)) {
>      if (Status == EFI_NOT_FOUND) {
> -      DEBUG ((DEBUG_ERROR, "%a: No Redfish host interface descriptor is
> provided on this platform.", __func__));
> +      DEBUG ((DEBUG_ERROR, "%a: No Redfish host interface descriptor is
> + provided on this platform.\n", __func__));
>        return EFI_NOT_FOUND;
>      }
> 
> --
> 2.39.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero length region
  2023-04-14  8:19 ` [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero length region Minh Nguyen
@ 2023-04-14  9:03   ` Nickle Wang
  2023-04-14  9:17     ` minhn
  2023-04-14  9:20     ` Minh Nguyen OS
  0 siblings, 2 replies; 17+ messages in thread
From: Nickle Wang @ 2023-04-14  9:03 UTC (permalink / raw)
  To: Minh Nguyen, devel@edk2.groups.io
  Cc: patches@amperecomputing.com, abner.chang@amd.com, igork@ami.com,
	nhi@os.amperecomputing.com, tinhnguyen@os.amperecomputing.com,
	Vu Nguyen

Hi Minh,

> Prevent assertion of allocate zero length region

May I know where does zero length allocation happen? According to my understanding, setting the "SubnetAddrInfoIPv6Number" to zero here and it will be handled by below condition check:

https://github.com/tianocore/edk2/blob/55b67b6950e648338adfe8ec54aeb26ed89d2c97/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c#L1005

Then the code leaves function with EFI_NOT_FOUND error. 

Thanks,
Nickle

> -----Original Message-----
> From: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> Sent: Friday, April 14, 2023 4:19 PM
> To: devel@edk2.groups.io
> Cc: patches@amperecomputing.com; abner.chang@amd.com; Nickle Wang
> <nicklew@nvidia.com>; igork@ami.com; nhi@os.amperecomputing.com;
> tinhnguyen@os.amperecomputing.com; Vu Nguyen
> <vunguyen@os.amperecomputing.com>; Minh Nguyen
> <minhnguyen@os.amperecomputing.com>
> Subject: [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero length
> region
> 
> External email: Use caution opening links or attachments
> 
> 
> From: Vu Nguyen <vunguyen@os.amperecomputing.com>
> 
> This change also fix miscellaneous errors on condition checking and debug
> argument missing.
> 
> Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> ---
>  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 583c6f78e19c..29e7b0624ebf 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -5,6 +5,7 @@
>    (C) Copyright 2021 Hewlett Packard Enterprise Development LP<BR>
>    Copyright (c) 2022, AMD Incorporated. All rights reserved.
>    Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -272,7 +273,7 @@ Tcp6GetSubnetInfo (
>    if (IpModedata.AddressCount == 0) {
>      DEBUG ((DEBUG_INFO, "%a: No IPv6 address configured.\n", __func__));
>      Instance->SubnetAddrInfoIPv6Number = 0;
> -    return EFI_SUCCESS;
> +    return EFI_NOT_FOUND;
>    }
> 
>    if (Instance->SubnetAddrInfoIPv6 != NULL) { @@ -926,7 +927,7 @@
> AddAndSignalNewRedfishService (
>      }
> 
>      Status = gBS->SignalEvent (Instance->DiscoverToken->Event);
> -    if (!EFI_ERROR (Status)) {
> +    if (EFI_ERROR (Status)) {
>        DEBUG ((DEBUG_ERROR, "%a:No event to signal!\n", __func__));
>      }
>    }
> --
> 2.39.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero length region
  2023-04-14  9:03   ` Nickle Wang
@ 2023-04-14  9:17     ` minhn
  2023-04-14  9:28       ` Nickle Wang
  2023-04-14  9:20     ` Minh Nguyen OS
  1 sibling, 1 reply; 17+ messages in thread
From: minhn @ 2023-04-14  9:17 UTC (permalink / raw)
  To: Nickle Wang, Minh Nguyen OS, devel@edk2.groups.io
  Cc: Open Source Submission, abner.chang@amd.com, igork@ami.com,
	Nhi Pham OS, Tinh Nguyen OS, Vu Nguyen OS

Hi Wang,

 Thanks for this point, you're right. It's a redundant change and I will update in v2 patches, but the second change is necessary. How do you think about that?

Thanks,
Minh Nguyen

-----Original Message-----
From: Nickle Wang <nicklew@nvidia.com> 
Sent: Friday, April 14, 2023 4:04 PM
To: Minh Nguyen OS <minhnguyen@os.amperecomputing.com>; devel@edk2.groups.io
Cc: Open Source Submission <patches@amperecomputing.com>; abner.chang@amd.com; igork@ami.com; Nhi Pham OS <nhi@os.amperecomputing.com>; Tinh Nguyen OS <tinhnguyen@os.amperecomputing.com>; Vu Nguyen OS <vunguyen@os.amperecomputing.com>
Subject: RE: [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero length region

[EXTERNAL EMAIL NOTICE: This email originated from an external sender. Please be mindful of safe email handling and proprietary information protection practices.]


Hi Minh,

> Prevent assertion of allocate zero length region

May I know where does zero length allocation happen? According to my understanding, setting the "SubnetAddrInfoIPv6Number" to zero here and it will be handled by below condition check:

https://github.com/tianocore/edk2/blob/55b67b6950e648338adfe8ec54aeb26ed89d2c97/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c#L1005

Then the code leaves function with EFI_NOT_FOUND error.

Thanks,
Nickle

> -----Original Message-----
> From: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> Sent: Friday, April 14, 2023 4:19 PM
> To: devel@edk2.groups.io
> Cc: patches@amperecomputing.com; abner.chang@amd.com; Nickle Wang 
> <nicklew@nvidia.com>; igork@ami.com; nhi@os.amperecomputing.com; 
> tinhnguyen@os.amperecomputing.com; Vu Nguyen 
> <vunguyen@os.amperecomputing.com>; Minh Nguyen 
> <minhnguyen@os.amperecomputing.com>
> Subject: [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero 
> length region
>
> External email: Use caution opening links or attachments
>
>
> From: Vu Nguyen <vunguyen@os.amperecomputing.com>
>
> This change also fix miscellaneous errors on condition checking and 
> debug argument missing.
>
> Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> ---
>  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 583c6f78e19c..29e7b0624ebf 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -5,6 +5,7 @@
>    (C) Copyright 2021 Hewlett Packard Enterprise Development LP<BR>
>    Copyright (c) 2022, AMD Incorporated. All rights reserved.
>    Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -272,7 +273,7 @@ Tcp6GetSubnetInfo (
>    if (IpModedata.AddressCount == 0) {
>      DEBUG ((DEBUG_INFO, "%a: No IPv6 address configured.\n", __func__));
>      Instance->SubnetAddrInfoIPv6Number = 0;
> -    return EFI_SUCCESS;
> +    return EFI_NOT_FOUND;
>    }
>
>    if (Instance->SubnetAddrInfoIPv6 != NULL) { @@ -926,7 +927,7 @@ 
> AddAndSignalNewRedfishService (
>      }
>
>      Status = gBS->SignalEvent (Instance->DiscoverToken->Event);
> -    if (!EFI_ERROR (Status)) {
> +    if (EFI_ERROR (Status)) {
>        DEBUG ((DEBUG_ERROR, "%a:No event to signal!\n", __func__));
>      }
>    }
> --
> 2.39.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero length region
  2023-04-14  9:03   ` Nickle Wang
  2023-04-14  9:17     ` minhn
@ 2023-04-14  9:20     ` Minh Nguyen OS
  1 sibling, 0 replies; 17+ messages in thread
From: Minh Nguyen OS @ 2023-04-14  9:20 UTC (permalink / raw)
  To: Nickle Wang, devel@edk2.groups.io
  Cc: Open Source Submission, abner.chang@amd.com, igork@ami.com,
	Nhi Pham OS, Tinh Nguyen OS, Vu Nguyen OS

Hi Wang,

 Thanks for this point, you're right. It's a redundant change and I will update in v2 patches, but the second change is necessary. How do you think about that?

Thanks,
Minh Nguyen

-----Original Message-----
From: Nickle Wang <nicklew@nvidia.com> 
Sent: Friday, April 14, 2023 4:04 PM
To: Minh Nguyen OS <minhnguyen@os.amperecomputing.com>; devel@edk2.groups.io
Cc: Open Source Submission <patches@amperecomputing.com>; abner.chang@amd.com; igork@ami.com; Nhi Pham OS <nhi@os.amperecomputing.com>; Tinh Nguyen OS <tinhnguyen@os.amperecomputing.com>; Vu Nguyen OS <vunguyen@os.amperecomputing.com>
Subject: RE: [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero length region

Hi Minh,

> Prevent assertion of allocate zero length region

May I know where does zero length allocation happen? According to my understanding, setting the "SubnetAddrInfoIPv6Number" to zero here and it will be handled by below condition check:

https://github.com/tianocore/edk2/blob/55b67b6950e648338adfe8ec54aeb26ed89d2c97/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c#L1005

Then the code leaves function with EFI_NOT_FOUND error. 

Thanks,
Nickle

> -----Original Message-----
> From: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> Sent: Friday, April 14, 2023 4:19 PM
> To: devel@edk2.groups.io
> Cc: patches@amperecomputing.com; abner.chang@amd.com; Nickle Wang 
> <nicklew@nvidia.com>; igork@ami.com; nhi@os.amperecomputing.com; 
> tinhnguyen@os.amperecomputing.com; Vu Nguyen 
> <vunguyen@os.amperecomputing.com>; Minh Nguyen 
> <minhnguyen@os.amperecomputing.com>
> Subject: [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero 
> length region
> 
> External email: Use caution opening links or attachments
> 
> 
> From: Vu Nguyen <vunguyen@os.amperecomputing.com>
> 
> This change also fix miscellaneous errors on condition checking and 
> debug argument missing.
> 
> Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> ---
>  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 583c6f78e19c..29e7b0624ebf 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -5,6 +5,7 @@
>    (C) Copyright 2021 Hewlett Packard Enterprise Development LP<BR>
>    Copyright (c) 2022, AMD Incorporated. All rights reserved.
>    Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -272,7 +273,7 @@ Tcp6GetSubnetInfo (
>    if (IpModedata.AddressCount == 0) {
>      DEBUG ((DEBUG_INFO, "%a: No IPv6 address configured.\n", __func__));
>      Instance->SubnetAddrInfoIPv6Number = 0;
> -    return EFI_SUCCESS;
> +    return EFI_NOT_FOUND;
>    }
> 
>    if (Instance->SubnetAddrInfoIPv6 != NULL) { @@ -926,7 +927,7 @@ 
> AddAndSignalNewRedfishService (
>      }
> 
>      Status = gBS->SignalEvent (Instance->DiscoverToken->Event);
> -    if (!EFI_ERROR (Status)) {
> +    if (EFI_ERROR (Status)) {
>        DEBUG ((DEBUG_ERROR, "%a:No event to signal!\n", __func__));
>      }
>    }
> --
> 2.39.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero length region
  2023-04-14  9:17     ` minhn
@ 2023-04-14  9:28       ` Nickle Wang
  0 siblings, 0 replies; 17+ messages in thread
From: Nickle Wang @ 2023-04-14  9:28 UTC (permalink / raw)
  To: Minh Nguyen, Minh Nguyen OS, devel@edk2.groups.io
  Cc: Open Source Submission, abner.chang@amd.com, igork@ami.com,
	Nhi Pham OS, Tinh Nguyen OS, Vu Nguyen OS

Thanks for your quick response.

> but the second change is necessary. How do you think about that?

Yes, second change is necessary. Please keep second change in v2 patches.

Thanks,
Nickle

> -----Original Message-----
> From: Minh Nguyen <minhn@amperecomputing.com>
> Sent: Friday, April 14, 2023 5:18 PM
> To: Nickle Wang <nicklew@nvidia.com>; Minh Nguyen OS
> <minhnguyen@os.amperecomputing.com>; devel@edk2.groups.io
> Cc: Open Source Submission <patches@amperecomputing.com>;
> abner.chang@amd.com; igork@ami.com; Nhi Pham OS
> <nhi@os.amperecomputing.com>; Tinh Nguyen OS
> <tinhnguyen@os.amperecomputing.com>; Vu Nguyen OS
> <vunguyen@os.amperecomputing.com>
> Subject: RE: [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero length
> region
>
> External email: Use caution opening links or attachments
>
>
> Hi Wang,
>
>  Thanks for this point, you're right. It's a redundant change and I will update in v2
> patches, but the second change is necessary. How do you think about that?
>
> Thanks,
> Minh Nguyen
>
> -----Original Message-----
> From: Nickle Wang <nicklew@nvidia.com>
> Sent: Friday, April 14, 2023 4:04 PM
> To: Minh Nguyen OS <minhnguyen@os.amperecomputing.com>;
> devel@edk2.groups.io
> Cc: Open Source Submission <patches@amperecomputing.com>;
> abner.chang@amd.com; igork@ami.com; Nhi Pham OS
> <nhi@os.amperecomputing.com>; Tinh Nguyen OS
> <tinhnguyen@os.amperecomputing.com>; Vu Nguyen OS
> <vunguyen@os.amperecomputing.com>
> Subject: RE: [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero length
> region
>
> [EXTERNAL EMAIL NOTICE: This email originated from an external sender.
> Please be mindful of safe email handling and proprietary information protection
> practices.]
>
>
> Hi Minh,
>
> > Prevent assertion of allocate zero length region
>
> May I know where does zero length allocation happen? According to my
> understanding, setting the "SubnetAddrInfoIPv6Number" to zero here and it will
> be handled by below condition check:
>
> https://github.c/
> om%2Ftianocore%2Fedk2%2Fblob%2F55b67b6950e648338adfe8ec54aeb26ed8
> 9d2c97%2FRedfishPkg%2FRedfishDiscoverDxe%2FRedfishDiscoverDxe.c%23L10
> 05&data=05%7C01%7Cnicklew%40nvidia.com%7Cca169d4eb55a4b11fb8a08db
> 3cc925aa%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C6381706068
> 44107782%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
> uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=4MAT%
> 2FDmrUBy%2BVF1%2F%2BNVTxg0iNOQtJJ%2BdlzXrNiluFA8%3D&reserved=0
>
> Then the code leaves function with EFI_NOT_FOUND error.
>
> Thanks,
> Nickle
>
> > -----Original Message-----
> > From: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> > Sent: Friday, April 14, 2023 4:19 PM
> > To: devel@edk2.groups.io
> > Cc: patches@amperecomputing.com; abner.chang@amd.com; Nickle Wang
> > <nicklew@nvidia.com>; igork@ami.com; nhi@os.amperecomputing.com;
> > tinhnguyen@os.amperecomputing.com; Vu Nguyen
> > <vunguyen@os.amperecomputing.com>; Minh Nguyen
> > <minhnguyen@os.amperecomputing.com>
> > Subject: [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero
> > length region
> >
> > External email: Use caution opening links or attachments
> >
> >
> > From: Vu Nguyen <vunguyen@os.amperecomputing.com>
> >
> > This change also fix miscellaneous errors on condition checking and
> > debug argument missing.
> >
> > Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> > ---
> >  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > index 583c6f78e19c..29e7b0624ebf 100644
> > --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > @@ -5,6 +5,7 @@
> >    (C) Copyright 2021 Hewlett Packard Enterprise Development LP<BR>
> >    Copyright (c) 2022, AMD Incorporated. All rights reserved.
> >    Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> > +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
> >
> >    SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -272,7 +273,7 @@ Tcp6GetSubnetInfo (
> >    if (IpModedata.AddressCount == 0) {
> >      DEBUG ((DEBUG_INFO, "%a: No IPv6 address configured.\n", __func__));
> >      Instance->SubnetAddrInfoIPv6Number = 0;
> > -    return EFI_SUCCESS;
> > +    return EFI_NOT_FOUND;
> >    }
> >
> >    if (Instance->SubnetAddrInfoIPv6 != NULL) { @@ -926,7 +927,7 @@
> > AddAndSignalNewRedfishService (
> >      }
> >
> >      Status = gBS->SignalEvent (Instance->DiscoverToken->Event);
> > -    if (!EFI_ERROR (Status)) {
> > +    if (EFI_ERROR (Status)) {
> >        DEBUG ((DEBUG_ERROR, "%a:No event to signal!\n", __func__));
> >      }
> >    }
> > --
> > 2.39.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/5] RedfishPkg: Fix compile issue on Linux
  2023-04-14  8:19 ` [PATCH 5/5] RedfishPkg: Fix compile issue on Linux Minh Nguyen
@ 2023-04-14 14:13   ` Chang, Abner
  0 siblings, 0 replies; 17+ messages in thread
From: Chang, Abner @ 2023-04-14 14:13 UTC (permalink / raw)
  To: Minh Nguyen, devel@edk2.groups.io
  Cc: patches@amperecomputing.com, nicklew@nvidia.com, igork@ami.com,
	nhi@os.amperecomputing.com, tinhnguyen@os.amperecomputing.com,
	Vu Nguyen

[AMD Official Use Only - General]

Reviewed-by: Abner Chang <Abner.Chang@amd.com>

> -----Original Message-----
> From: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> Sent: Friday, April 14, 2023 4:19 PM
> To: devel@edk2.groups.io
> Cc: patches@amperecomputing.com; Chang, Abner
> <Abner.Chang@amd.com>; nicklew@nvidia.com; igork@ami.com;
> nhi@os.amperecomputing.com; tinhnguyen@os.amperecomputing.com; Vu
> Nguyen <vunguyen@os.amperecomputing.com>; Minh Nguyen
> <minhnguyen@os.amperecomputing.com>
> Subject: [PATCH 5/5] RedfishPkg: Fix compile issue on Linux
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> From: Vu Nguyen <vunguyen@os.amperecomputing.com>
> 
> It requires a fixed size array to store the content of device path PCD.
> Add the array size to solve this issue.
> 
> Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> ---
>  RedfishPkg/Include/Pcd/RestExServiceDevicePath.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/RedfishPkg/Include/Pcd/RestExServiceDevicePath.h
> b/RedfishPkg/Include/Pcd/RestExServiceDevicePath.h
> index 91b1198297c2..57fc199f61f2 100644
> --- a/RedfishPkg/Include/Pcd/RestExServiceDevicePath.h
> +++ b/RedfishPkg/Include/Pcd/RestExServiceDevicePath.h
> @@ -4,6 +4,7 @@
> 
>    Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
>    (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
> +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
> 
>      SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -14,6 +15,8 @@
> 
>  #include <Protocol/DevicePath.h>
> 
> +#define MAX_DEVICE_PATH_NODE      40
> +
>  typedef enum {
>    DEVICE_PATH_MATCH_MAC_NODE = 1,
>    DEVICE_PATH_MATCH_PCI_NODE = 2,
> @@ -32,7 +35,7 @@ typedef struct {
>    //
> 0x03,0x0b,0x25,0x00,0x00,0x50,0x56,0xc0,0x00,0x02,0x00,0x00,0x00,0x00,0x0
> 0,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0
> x00,0x00,0x00,0x00,0x00,0x00,0x00,0x01,
>    //    0x7f,0xff,0x04,0x00}
>    //
> -  EFI_DEVICE_PATH_PROTOCOL    DevicePath[];
> +  EFI_DEVICE_PATH_PROTOCOL      DevicePath[MAX_DEVICE_PATH_NODE];
>  } REST_EX_SERVICE_DEVICE_PATH_DATA;
> 
>  #endif
> --
> 2.39.0

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface
  2023-04-14  8:19 ` [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface Minh Nguyen
@ 2023-04-16  4:36   ` Chang, Abner
  2023-04-20  1:23     ` Minh Nguyen OS
  0 siblings, 1 reply; 17+ messages in thread
From: Chang, Abner @ 2023-04-16  4:36 UTC (permalink / raw)
  To: Minh Nguyen, devel@edk2.groups.io
  Cc: patches@amperecomputing.com, nicklew@nvidia.com, igork@ami.com,
	nhi@os.amperecomputing.com, tinhnguyen@os.amperecomputing.com,
	Vu Nguyen

[AMD Official Use Only - General]

Hi Minh,
That is weird I can't find this patch set on group.io, and I can't apply this patch from email neither. Could you please check it?

Thanks
Abner

> -----Original Message-----
> From: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> Sent: Friday, April 14, 2023 4:19 PM
> To: devel@edk2.groups.io
> Cc: patches@amperecomputing.com; Chang, Abner
> <Abner.Chang@amd.com>; nicklew@nvidia.com; igork@ami.com;
> nhi@os.amperecomputing.com; tinhnguyen@os.amperecomputing.com; Vu
> Nguyen <vunguyen@os.amperecomputing.com>; Minh Nguyen
> <minhnguyen@os.amperecomputing.com>
> Subject: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> From: Vu Nguyen <vunguyen@os.amperecomputing.com>
> 
> When a MAC address matching interface is found, a RestEx child will be
> created to provide the Redfish communication on that interface.
> Currently, It will try to locate all RestEx binding services and choose the first
> satisfied instance without taking care about current selected interface. This
> might raise an issue on the system with multiple network devices that the
> RestEx child was installed on wrong interface.
> 
> Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> ---
>  RedfishPkg/Include/Library/RestExLib.h             |   3 +
>  RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c     | 153 ++++++++----------
> --
>  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c |   1 +
>  3 files changed, 63 insertions(+), 94 deletions(-)
> 
> diff --git a/RedfishPkg/Include/Library/RestExLib.h
> b/RedfishPkg/Include/Library/RestExLib.h
> index bc4e4ca6caa7..2c32c3684cf4 100644
> --- a/RedfishPkg/Include/Library/RestExLib.h
> +++ b/RedfishPkg/Include/Library/RestExLib.h
> @@ -2,6 +2,7 @@
>    This library provides help functions for REST EX Protocol.
> 
>    (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
> +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -20,6 +21,7 @@
>    This function allows the caller to create child handle for specific
>    REST server.
> 
> +  @param[in]  Controller           The controller handle used of selected
> interface.
>    @param[in]  Image                The image handle used to open service.
>    @param[in]  AccessMode           Access mode of REST server.
>    @param[in]  ConfigType           Underlying configuration to communicate
> with REST server.
> @@ -32,6 +34,7 @@
>  **/
>  EFI_STATUS
>  RestExLibCreateChild (
> +  IN EFI_HANDLE                       Controller,
>    IN EFI_HANDLE                       Image,
>    IN EFI_REST_EX_SERVICE_ACCESS_MODE  AccessMode,
>    IN EFI_REST_EX_CONFIG_TYPE          ConfigType,
> diff --git a/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> b/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> index d9acad24dec1..6002be93c54e 100644
> --- a/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> +++ b/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> @@ -2,6 +2,7 @@
>    This library provides help functions for REST EX Protocol.
> 
>    (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
> +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> @@ -9,6 +10,7 @@
> 
>  #include <Uefi.h>
>  #include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
>  #include <Library/MemoryAllocationLib.h>  #include <Library/NetLib.h>
> #include <Library/UefiBootServicesTableLib.h>
> @@ -21,6 +23,7 @@
>    This function allows the caller to create child handle for specific
>    REST server.
> 
> +  @param[in]  Controller           The controller handle used of selected
> interface.
>    @param[in]  Image                The image handle used to open service.
>    @param[in]  AccessMode           Access mode of REST server.
>    @param[in]  ConfigType           Underlying configuration to communicate
> with REST server.
> @@ -33,6 +36,7 @@
>  **/
>  EFI_STATUS
>  RestExLibCreateChild (
> +  IN EFI_HANDLE                       Controller,
>    IN EFI_HANDLE                       Image,
>    IN EFI_REST_EX_SERVICE_ACCESS_MODE  AccessMode,
>    IN EFI_REST_EX_CONFIG_TYPE          ConfigType,
> @@ -41,8 +45,6 @@ RestExLibCreateChild (
>    )
>  {
>    EFI_STATUS                Status;
> -  UINTN                     NoBuffer;
> -  EFI_HANDLE                *Handle;
>    EFI_HANDLE                ChildHandle;
>    EFI_REST_EX_PROTOCOL      *RestEx;
>    EFI_REST_EX_SERVICE_INFO  *RestExServiceInfo; @@ -59,116 +61,79 @@
> RestExLibCreateChild (
>    }
> 
>    *ChildInstanceHandle = NULL;
> -  //
> -  // Locate all REST EX binding service.
> -  //
> -  Handle   = NULL;
> -  NoBuffer = 0;
> -  Status   = gBS->LocateHandleBuffer (
> -                    ByProtocol,
> -                    &gEfiRestExServiceBindingProtocolGuid,
> -                    NULL,
> -                    &NoBuffer,
> -                    &Handle
> -                    );
> -  if (EFI_ERROR (Status) && (Status != EFI_BUFFER_TOO_SMALL)) {
> +
> +  ChildHandle = NULL;
> +  Status = NetLibCreateServiceChild (
> +             Controller,
> +             Image,
> +             &gEfiRestExServiceBindingProtocolGuid,
> +             &ChildHandle
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: Failed to create service child - %r \n",
> +      __FUNCTION__,
> +      Status
> +      ));
>      return Status;
>    }
> -
> -  Handle = (EFI_HANDLE *)AllocateZeroPool (sizeof (EFI_HANDLE) *
> NoBuffer);
> -  if (Handle == NULL) {
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> -
> -  Status = gBS->LocateHandleBuffer (
> -                  ByProtocol,
> -                  &gEfiRestExServiceBindingProtocolGuid,
> +  Status = gBS->OpenProtocol (
> +                  ChildHandle,
> +                  &gEfiRestExProtocolGuid,
> +                  (VOID **)&RestEx,
> +                  Image,
>                    NULL,
> -                  &NoBuffer,
> -                  &Handle
> +                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
>                    );
>    if (EFI_ERROR (Status)) {
> -    FreePool (Handle);
> -    return Status;
> +    goto ON_ERROR;
>    }
> 
>    //
> -  // Search for the proper REST EX instance.
> +  // Get the information of REST service provided by this EFI REST EX
> + driver
>    //
> -  while (NoBuffer != 0) {
> -    ChildHandle = NULL;
> -    Status      = NetLibCreateServiceChild (
> -                    *(Handle + (NoBuffer - 1)),
> -                    Image,
> -                    &gEfiRestExServiceBindingProtocolGuid,
> -                    &ChildHandle
> -                    );
> -    if (!EFI_ERROR (Status)) {
> -      Status = gBS->OpenProtocol (
> -                      ChildHandle,
> -                      &gEfiRestExProtocolGuid,
> -                      (VOID **)&RestEx,
> -                      Image,
> -                      NULL,
> -                      EFI_OPEN_PROTOCOL_GET_PROTOCOL
> -                      );
> -      if (EFI_ERROR (Status)) {
> -        goto ON_ERROR;
> -      }
> -
> -      //
> -      // Get the information of REST service provided by this EFI REST EX driver
> -      //
> -      Status = RestEx->GetService (
> -                         RestEx,
> -                         &RestExServiceInfo
> -                         );
> -      if (EFI_ERROR (Status)) {
> -        goto ON_ERROR;
> -      }
> -
> -      //
> -      // Check REST EX property.
> -      //
> -      switch (ConfigType) {
> -        case EfiRestExConfigHttp:
> -          LenOfConfig = sizeof (EFI_REST_EX_HTTP_CONFIG_DATA);
> -          break;
> -
> -        case EfiRestExConfigUnspecific:
> -          LenOfConfig = REST_EX_CONFIG_DATA_LEN_UNKNOWN;
> -          break;
> +  Status = RestEx->GetService (
> +                     RestEx,
> +                     &RestExServiceInfo
> +                     );
> +  if (EFI_ERROR (Status)) {
> +    goto ON_ERROR;
> +  }
> +  //
> +  // Check REST EX property.
> +  //
> +  switch (ConfigType) {
> +  case EfiRestExConfigHttp:
> +    LenOfConfig = sizeof (EFI_REST_EX_HTTP_CONFIG_DATA);
> +    break;
> 
> -        default:
> -          goto ON_ERROR;
> -      }
> +  case EfiRestExConfigUnspecific:
> +    LenOfConfig = REST_EX_CONFIG_DATA_LEN_UNKNOWN;
> +    break;
> 
> -      if ((RestExServiceInfo-
> >EfiRestExServiceInfoV10.RestServiceAccessMode != AccessMode) ||
> -          (RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceType !=
> ServiceType) ||
> -          (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigType !=
> ConfigType) ||
> -          ((LenOfConfig != REST_EX_CONFIG_DATA_LEN_UNKNOWN) &&
> (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigDataLength !=
> LenOfConfig)))
> -      {
> -        goto ON_ERROR;
> -      }
> -    }
> +  default:
> +    goto ON_ERROR;
> +  }
> +  if (RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceAccessMode !=
> AccessMode ||
> +      RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceType !=
> ServiceType ||
> +      RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigType !=
> ConfigType ||
> +      ((LenOfConfig != REST_EX_CONFIG_DATA_LEN_UNKNOWN) &&
> (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigDataLength !=
> LenOfConfig))) {
> +    goto ON_ERROR;
> +  }
> 
> -    //
> -    // This is proper REST EX instance.
> -    //
> -    *ChildInstanceHandle = ChildHandle;
> -    FreePool (Handle);
> -    return EFI_SUCCESS;
> +  //
> +  // This is proper REST EX instance.
> +  //
> +  *ChildInstanceHandle = ChildHandle;
> +  return EFI_SUCCESS;
> 
>  ON_ERROR:;
>      NetLibDestroyServiceChild (
> -      *(Handle + (NoBuffer - 1)),
> +      Controller,
>        Image,
>        &gEfiRestExServiceBindingProtocolGuid,
>        ChildHandle
>        );
> -    NoBuffer--;
> -  }
> -
> -  FreePool (Handle);
>    return EFI_NOT_FOUND;
>  }
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 29e7b0624ebf..86ec88ddafc4 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -87,6 +87,7 @@ CreateRestExInstance (
>    EFI_STATUS  Status;
> 
>    Status = RestExLibCreateChild (
> +             Instance->NetworkInterface->OpenDriverControllerHandle,
>               Instance->Owner,
>               FixedPcdGetBool (PcdRedfishDiscoverAccessModeInBand) ?
> EfiRestExServiceInBandAccess : EfiRestExServiceOutOfBandAccess,
>               EfiRestExConfigHttp,
> --
> 2.39.0

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface
  2023-04-16  4:36   ` Chang, Abner
@ 2023-04-20  1:23     ` Minh Nguyen OS
  2023-04-20  1:28       ` Chang, Abner
  0 siblings, 1 reply; 17+ messages in thread
From: Minh Nguyen OS @ 2023-04-20  1:23 UTC (permalink / raw)
  To: Chang, Abner, devel@edk2.groups.io
  Cc: Open Source Submission, nicklew@nvidia.com, igork@ami.com,
	Nhi Pham OS, Tinh Nguyen OS, Vu Nguyen OS

[-- Attachment #1: Type: text/plain, Size: 11799 bytes --]

Hi Chang,

  It seems like my email has the problem, please expect the delay for my v2 patches while my email's fixing. Thanks for your patience.

Thanks,
Minh Nguyen
________________________________
From: Chang, Abner <Abner.Chang@amd.com>
Sent: Sunday, April 16, 2023 11:36 AM
To: Minh Nguyen OS <minhnguyen@os.amperecomputing.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Open Source Submission <patches@amperecomputing.com>; nicklew@nvidia.com <nicklew@nvidia.com>; igork@ami.com <igork@ami.com>; Nhi Pham OS <nhi@os.amperecomputing.com>; Tinh Nguyen OS <tinhnguyen@os.amperecomputing.com>; Vu Nguyen OS <vunguyen@os.amperecomputing.com>
Subject: RE: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface

[AMD Official Use Only - General]

Hi Minh,
That is weird I can't find this patch set on group.io, and I can't apply this patch from email neither. Could you please check it?

Thanks
Abner

> -----Original Message-----
> From: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> Sent: Friday, April 14, 2023 4:19 PM
> To: devel@edk2.groups.io
> Cc: patches@amperecomputing.com; Chang, Abner
> <Abner.Chang@amd.com>; nicklew@nvidia.com; igork@ami.com;
> nhi@os.amperecomputing.com; tinhnguyen@os.amperecomputing.com; Vu
> Nguyen <vunguyen@os.amperecomputing.com>; Minh Nguyen
> <minhnguyen@os.amperecomputing.com>
> Subject: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> From: Vu Nguyen <vunguyen@os.amperecomputing.com>
>
> When a MAC address matching interface is found, a RestEx child will be
> created to provide the Redfish communication on that interface.
> Currently, It will try to locate all RestEx binding services and choose the first
> satisfied instance without taking care about current selected interface. This
> might raise an issue on the system with multiple network devices that the
> RestEx child was installed on wrong interface.
>
> Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> ---
>  RedfishPkg/Include/Library/RestExLib.h             |   3 +
>  RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c     | 153 ++++++++----------
> --
>  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c |   1 +
>  3 files changed, 63 insertions(+), 94 deletions(-)
>
> diff --git a/RedfishPkg/Include/Library/RestExLib.h
> b/RedfishPkg/Include/Library/RestExLib.h
> index bc4e4ca6caa7..2c32c3684cf4 100644
> --- a/RedfishPkg/Include/Library/RestExLib.h
> +++ b/RedfishPkg/Include/Library/RestExLib.h
> @@ -2,6 +2,7 @@
>    This library provides help functions for REST EX Protocol.
>
>    (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
> +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -20,6 +21,7 @@
>    This function allows the caller to create child handle for specific
>    REST server.
>
> +  @param[in]  Controller           The controller handle used of selected
> interface.
>    @param[in]  Image                The image handle used to open service.
>    @param[in]  AccessMode           Access mode of REST server.
>    @param[in]  ConfigType           Underlying configuration to communicate
> with REST server.
> @@ -32,6 +34,7 @@
>  **/
>  EFI_STATUS
>  RestExLibCreateChild (
> +  IN EFI_HANDLE                       Controller,
>    IN EFI_HANDLE                       Image,
>    IN EFI_REST_EX_SERVICE_ACCESS_MODE  AccessMode,
>    IN EFI_REST_EX_CONFIG_TYPE          ConfigType,
> diff --git a/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> b/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> index d9acad24dec1..6002be93c54e 100644
> --- a/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> +++ b/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> @@ -2,6 +2,7 @@
>    This library provides help functions for REST EX Protocol.
>
>    (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
> +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -9,6 +10,7 @@
>
>  #include <Uefi.h>
>  #include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
>  #include <Library/MemoryAllocationLib.h>  #include <Library/NetLib.h>
> #include <Library/UefiBootServicesTableLib.h>
> @@ -21,6 +23,7 @@
>    This function allows the caller to create child handle for specific
>    REST server.
>
> +  @param[in]  Controller           The controller handle used of selected
> interface.
>    @param[in]  Image                The image handle used to open service.
>    @param[in]  AccessMode           Access mode of REST server.
>    @param[in]  ConfigType           Underlying configuration to communicate
> with REST server.
> @@ -33,6 +36,7 @@
>  **/
>  EFI_STATUS
>  RestExLibCreateChild (
> +  IN EFI_HANDLE                       Controller,
>    IN EFI_HANDLE                       Image,
>    IN EFI_REST_EX_SERVICE_ACCESS_MODE  AccessMode,
>    IN EFI_REST_EX_CONFIG_TYPE          ConfigType,
> @@ -41,8 +45,6 @@ RestExLibCreateChild (
>    )
>  {
>    EFI_STATUS                Status;
> -  UINTN                     NoBuffer;
> -  EFI_HANDLE                *Handle;
>    EFI_HANDLE                ChildHandle;
>    EFI_REST_EX_PROTOCOL      *RestEx;
>    EFI_REST_EX_SERVICE_INFO  *RestExServiceInfo; @@ -59,116 +61,79 @@
> RestExLibCreateChild (
>    }
>
>    *ChildInstanceHandle = NULL;
> -  //
> -  // Locate all REST EX binding service.
> -  //
> -  Handle   = NULL;
> -  NoBuffer = 0;
> -  Status   = gBS->LocateHandleBuffer (
> -                    ByProtocol,
> -                    &gEfiRestExServiceBindingProtocolGuid,
> -                    NULL,
> -                    &NoBuffer,
> -                    &Handle
> -                    );
> -  if (EFI_ERROR (Status) && (Status != EFI_BUFFER_TOO_SMALL)) {
> +
> +  ChildHandle = NULL;
> +  Status = NetLibCreateServiceChild (
> +             Controller,
> +             Image,
> +             &gEfiRestExServiceBindingProtocolGuid,
> +             &ChildHandle
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: Failed to create service child - %r \n",
> +      __FUNCTION__,
> +      Status
> +      ));
>      return Status;
>    }
> -
> -  Handle = (EFI_HANDLE *)AllocateZeroPool (sizeof (EFI_HANDLE) *
> NoBuffer);
> -  if (Handle == NULL) {
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> -
> -  Status = gBS->LocateHandleBuffer (
> -                  ByProtocol,
> -                  &gEfiRestExServiceBindingProtocolGuid,
> +  Status = gBS->OpenProtocol (
> +                  ChildHandle,
> +                  &gEfiRestExProtocolGuid,
> +                  (VOID **)&RestEx,
> +                  Image,
>                    NULL,
> -                  &NoBuffer,
> -                  &Handle
> +                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
>                    );
>    if (EFI_ERROR (Status)) {
> -    FreePool (Handle);
> -    return Status;
> +    goto ON_ERROR;
>    }
>
>    //
> -  // Search for the proper REST EX instance.
> +  // Get the information of REST service provided by this EFI REST EX
> + driver
>    //
> -  while (NoBuffer != 0) {
> -    ChildHandle = NULL;
> -    Status      = NetLibCreateServiceChild (
> -                    *(Handle + (NoBuffer - 1)),
> -                    Image,
> -                    &gEfiRestExServiceBindingProtocolGuid,
> -                    &ChildHandle
> -                    );
> -    if (!EFI_ERROR (Status)) {
> -      Status = gBS->OpenProtocol (
> -                      ChildHandle,
> -                      &gEfiRestExProtocolGuid,
> -                      (VOID **)&RestEx,
> -                      Image,
> -                      NULL,
> -                      EFI_OPEN_PROTOCOL_GET_PROTOCOL
> -                      );
> -      if (EFI_ERROR (Status)) {
> -        goto ON_ERROR;
> -      }
> -
> -      //
> -      // Get the information of REST service provided by this EFI REST EX driver
> -      //
> -      Status = RestEx->GetService (
> -                         RestEx,
> -                         &RestExServiceInfo
> -                         );
> -      if (EFI_ERROR (Status)) {
> -        goto ON_ERROR;
> -      }
> -
> -      //
> -      // Check REST EX property.
> -      //
> -      switch (ConfigType) {
> -        case EfiRestExConfigHttp:
> -          LenOfConfig = sizeof (EFI_REST_EX_HTTP_CONFIG_DATA);
> -          break;
> -
> -        case EfiRestExConfigUnspecific:
> -          LenOfConfig = REST_EX_CONFIG_DATA_LEN_UNKNOWN;
> -          break;
> +  Status = RestEx->GetService (
> +                     RestEx,
> +                     &RestExServiceInfo
> +                     );
> +  if (EFI_ERROR (Status)) {
> +    goto ON_ERROR;
> +  }
> +  //
> +  // Check REST EX property.
> +  //
> +  switch (ConfigType) {
> +  case EfiRestExConfigHttp:
> +    LenOfConfig = sizeof (EFI_REST_EX_HTTP_CONFIG_DATA);
> +    break;
>
> -        default:
> -          goto ON_ERROR;
> -      }
> +  case EfiRestExConfigUnspecific:
> +    LenOfConfig = REST_EX_CONFIG_DATA_LEN_UNKNOWN;
> +    break;
>
> -      if ((RestExServiceInfo-
> >EfiRestExServiceInfoV10.RestServiceAccessMode != AccessMode) ||
> -          (RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceType !=
> ServiceType) ||
> -          (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigType !=
> ConfigType) ||
> -          ((LenOfConfig != REST_EX_CONFIG_DATA_LEN_UNKNOWN) &&
> (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigDataLength !=
> LenOfConfig)))
> -      {
> -        goto ON_ERROR;
> -      }
> -    }
> +  default:
> +    goto ON_ERROR;
> +  }
> +  if (RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceAccessMode !=
> AccessMode ||
> +      RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceType !=
> ServiceType ||
> +      RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigType !=
> ConfigType ||
> +      ((LenOfConfig != REST_EX_CONFIG_DATA_LEN_UNKNOWN) &&
> (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigDataLength !=
> LenOfConfig))) {
> +    goto ON_ERROR;
> +  }
>
> -    //
> -    // This is proper REST EX instance.
> -    //
> -    *ChildInstanceHandle = ChildHandle;
> -    FreePool (Handle);
> -    return EFI_SUCCESS;
> +  //
> +  // This is proper REST EX instance.
> +  //
> +  *ChildInstanceHandle = ChildHandle;
> +  return EFI_SUCCESS;
>
>  ON_ERROR:;
>      NetLibDestroyServiceChild (
> -      *(Handle + (NoBuffer - 1)),
> +      Controller,
>        Image,
>        &gEfiRestExServiceBindingProtocolGuid,
>        ChildHandle
>        );
> -    NoBuffer--;
> -  }
> -
> -  FreePool (Handle);
>    return EFI_NOT_FOUND;
>  }
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 29e7b0624ebf..86ec88ddafc4 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -87,6 +87,7 @@ CreateRestExInstance (
>    EFI_STATUS  Status;
>
>    Status = RestExLibCreateChild (
> +             Instance->NetworkInterface->OpenDriverControllerHandle,
>               Instance->Owner,
>               FixedPcdGetBool (PcdRedfishDiscoverAccessModeInBand) ?
> EfiRestExServiceInBandAccess : EfiRestExServiceOutOfBandAccess,
>               EfiRestExConfigHttp,
> --
> 2.39.0

[-- Attachment #2: Type: text/html, Size: 24119 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface
  2023-04-20  1:23     ` Minh Nguyen OS
@ 2023-04-20  1:28       ` Chang, Abner
  2023-04-20  1:33         ` Minh Nguyen OS
  0 siblings, 1 reply; 17+ messages in thread
From: Chang, Abner @ 2023-04-20  1:28 UTC (permalink / raw)
  To: Minh Nguyen OS, devel@edk2.groups.io
  Cc: Open Source Submission, nicklew@nvidia.com, igork@ami.com,
	Nhi Pham OS, Tinh Nguyen OS, Vu Nguyen OS

[-- Attachment #1: Type: text/plain, Size: 12801 bytes --]

[AMD Official Use Only - General]

Hi Minh,
Is this the first time you send email to groups.io, if yes please also check it with Miki
miki.demeter@intel.com to confirm is your mail passed through groups.io or not.

Thanks
Abner


Get Outlook for Android<https://aka.ms/AAb9ysg>

________________________________
From: Minh Nguyen OS <minhnguyen@os.amperecomputing.com>
Sent: Thursday, April 20, 2023, 9:23 AM
To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Open Source Submission <patches@amperecomputing.com>; nicklew@nvidia.com <nicklew@nvidia.com>; igork@ami.com <igork@ami.com>; Nhi Pham OS <nhi@os.amperecomputing.com>; Tinh Nguyen OS <tinhnguyen@os.amperecomputing.com>; Vu Nguyen OS <vunguyen@os.amperecomputing.com>
Subject: Re: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

Hi Chang,

  It seems like my email has the problem, please expect the delay for my v2 patches while my email's fixing. Thanks for your patience.

Thanks,
Minh Nguyen
________________________________
From: Chang, Abner <Abner.Chang@amd.com>
Sent: Sunday, April 16, 2023 11:36 AM
To: Minh Nguyen OS <minhnguyen@os.amperecomputing.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Open Source Submission <patches@amperecomputing.com>; nicklew@nvidia.com <nicklew@nvidia.com>; igork@ami.com <igork@ami.com>; Nhi Pham OS <nhi@os.amperecomputing.com>; Tinh Nguyen OS <tinhnguyen@os.amperecomputing.com>; Vu Nguyen OS <vunguyen@os.amperecomputing.com>
Subject: RE: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface

[AMD Official Use Only - General]

Hi Minh,
That is weird I can't find this patch set on group.io, and I can't apply this patch from email neither. Could you please check it?

Thanks
Abner

> -----Original Message-----
> From: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> Sent: Friday, April 14, 2023 4:19 PM
> To: devel@edk2.groups.io
> Cc: patches@amperecomputing.com; Chang, Abner
> <Abner.Chang@amd.com>; nicklew@nvidia.com; igork@ami.com;
> nhi@os.amperecomputing.com; tinhnguyen@os.amperecomputing.com; Vu
> Nguyen <vunguyen@os.amperecomputing.com>; Minh Nguyen
> <minhnguyen@os.amperecomputing.com>
> Subject: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> From: Vu Nguyen <vunguyen@os.amperecomputing.com>
>
> When a MAC address matching interface is found, a RestEx child will be
> created to provide the Redfish communication on that interface.
> Currently, It will try to locate all RestEx binding services and choose the first
> satisfied instance without taking care about current selected interface. This
> might raise an issue on the system with multiple network devices that the
> RestEx child was installed on wrong interface.
>
> Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> ---
>  RedfishPkg/Include/Library/RestExLib.h             |   3 +
>  RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c     | 153 ++++++++----------
> --
>  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c |   1 +
>  3 files changed, 63 insertions(+), 94 deletions(-)
>
> diff --git a/RedfishPkg/Include/Library/RestExLib.h
> b/RedfishPkg/Include/Library/RestExLib.h
> index bc4e4ca6caa7..2c32c3684cf4 100644
> --- a/RedfishPkg/Include/Library/RestExLib.h
> +++ b/RedfishPkg/Include/Library/RestExLib.h
> @@ -2,6 +2,7 @@
>    This library provides help functions for REST EX Protocol.
>
>    (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
> +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -20,6 +21,7 @@
>    This function allows the caller to create child handle for specific
>    REST server.
>
> +  @param[in]  Controller           The controller handle used of selected
> interface.
>    @param[in]  Image                The image handle used to open service.
>    @param[in]  AccessMode           Access mode of REST server.
>    @param[in]  ConfigType           Underlying configuration to communicate
> with REST server.
> @@ -32,6 +34,7 @@
>  **/
>  EFI_STATUS
>  RestExLibCreateChild (
> +  IN EFI_HANDLE                       Controller,
>    IN EFI_HANDLE                       Image,
>    IN EFI_REST_EX_SERVICE_ACCESS_MODE  AccessMode,
>    IN EFI_REST_EX_CONFIG_TYPE          ConfigType,
> diff --git a/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> b/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> index d9acad24dec1..6002be93c54e 100644
> --- a/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> +++ b/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> @@ -2,6 +2,7 @@
>    This library provides help functions for REST EX Protocol.
>
>    (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
> +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -9,6 +10,7 @@
>
>  #include <Uefi.h>
>  #include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
>  #include <Library/MemoryAllocationLib.h>  #include <Library/NetLib.h>
> #include <Library/UefiBootServicesTableLib.h>
> @@ -21,6 +23,7 @@
>    This function allows the caller to create child handle for specific
>    REST server.
>
> +  @param[in]  Controller           The controller handle used of selected
> interface.
>    @param[in]  Image                The image handle used to open service.
>    @param[in]  AccessMode           Access mode of REST server.
>    @param[in]  ConfigType           Underlying configuration to communicate
> with REST server.
> @@ -33,6 +36,7 @@
>  **/
>  EFI_STATUS
>  RestExLibCreateChild (
> +  IN EFI_HANDLE                       Controller,
>    IN EFI_HANDLE                       Image,
>    IN EFI_REST_EX_SERVICE_ACCESS_MODE  AccessMode,
>    IN EFI_REST_EX_CONFIG_TYPE          ConfigType,
> @@ -41,8 +45,6 @@ RestExLibCreateChild (
>    )
>  {
>    EFI_STATUS                Status;
> -  UINTN                     NoBuffer;
> -  EFI_HANDLE                *Handle;
>    EFI_HANDLE                ChildHandle;
>    EFI_REST_EX_PROTOCOL      *RestEx;
>    EFI_REST_EX_SERVICE_INFO  *RestExServiceInfo; @@ -59,116 +61,79 @@
> RestExLibCreateChild (
>    }
>
>    *ChildInstanceHandle = NULL;
> -  //
> -  // Locate all REST EX binding service.
> -  //
> -  Handle   = NULL;
> -  NoBuffer = 0;
> -  Status   = gBS->LocateHandleBuffer (
> -                    ByProtocol,
> -                    &gEfiRestExServiceBindingProtocolGuid,
> -                    NULL,
> -                    &NoBuffer,
> -                    &Handle
> -                    );
> -  if (EFI_ERROR (Status) && (Status != EFI_BUFFER_TOO_SMALL)) {
> +
> +  ChildHandle = NULL;
> +  Status = NetLibCreateServiceChild (
> +             Controller,
> +             Image,
> +             &gEfiRestExServiceBindingProtocolGuid,
> +             &ChildHandle
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: Failed to create service child - %r \n",
> +      __FUNCTION__,
> +      Status
> +      ));
>      return Status;
>    }
> -
> -  Handle = (EFI_HANDLE *)AllocateZeroPool (sizeof (EFI_HANDLE) *
> NoBuffer);
> -  if (Handle == NULL) {
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> -
> -  Status = gBS->LocateHandleBuffer (
> -                  ByProtocol,
> -                  &gEfiRestExServiceBindingProtocolGuid,
> +  Status = gBS->OpenProtocol (
> +                  ChildHandle,
> +                  &gEfiRestExProtocolGuid,
> +                  (VOID **)&RestEx,
> +                  Image,
>                    NULL,
> -                  &NoBuffer,
> -                  &Handle
> +                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
>                    );
>    if (EFI_ERROR (Status)) {
> -    FreePool (Handle);
> -    return Status;
> +    goto ON_ERROR;
>    }
>
>    //
> -  // Search for the proper REST EX instance.
> +  // Get the information of REST service provided by this EFI REST EX
> + driver
>    //
> -  while (NoBuffer != 0) {
> -    ChildHandle = NULL;
> -    Status      = NetLibCreateServiceChild (
> -                    *(Handle + (NoBuffer - 1)),
> -                    Image,
> -                    &gEfiRestExServiceBindingProtocolGuid,
> -                    &ChildHandle
> -                    );
> -    if (!EFI_ERROR (Status)) {
> -      Status = gBS->OpenProtocol (
> -                      ChildHandle,
> -                      &gEfiRestExProtocolGuid,
> -                      (VOID **)&RestEx,
> -                      Image,
> -                      NULL,
> -                      EFI_OPEN_PROTOCOL_GET_PROTOCOL
> -                      );
> -      if (EFI_ERROR (Status)) {
> -        goto ON_ERROR;
> -      }
> -
> -      //
> -      // Get the information of REST service provided by this EFI REST EX driver
> -      //
> -      Status = RestEx->GetService (
> -                         RestEx,
> -                         &RestExServiceInfo
> -                         );
> -      if (EFI_ERROR (Status)) {
> -        goto ON_ERROR;
> -      }
> -
> -      //
> -      // Check REST EX property.
> -      //
> -      switch (ConfigType) {
> -        case EfiRestExConfigHttp:
> -          LenOfConfig = sizeof (EFI_REST_EX_HTTP_CONFIG_DATA);
> -          break;
> -
> -        case EfiRestExConfigUnspecific:
> -          LenOfConfig = REST_EX_CONFIG_DATA_LEN_UNKNOWN;
> -          break;
> +  Status = RestEx->GetService (
> +                     RestEx,
> +                     &RestExServiceInfo
> +                     );
> +  if (EFI_ERROR (Status)) {
> +    goto ON_ERROR;
> +  }
> +  //
> +  // Check REST EX property.
> +  //
> +  switch (ConfigType) {
> +  case EfiRestExConfigHttp:
> +    LenOfConfig = sizeof (EFI_REST_EX_HTTP_CONFIG_DATA);
> +    break;
>
> -        default:
> -          goto ON_ERROR;
> -      }
> +  case EfiRestExConfigUnspecific:
> +    LenOfConfig = REST_EX_CONFIG_DATA_LEN_UNKNOWN;
> +    break;
>
> -      if ((RestExServiceInfo-
> >EfiRestExServiceInfoV10.RestServiceAccessMode != AccessMode) ||
> -          (RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceType !=
> ServiceType) ||
> -          (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigType !=
> ConfigType) ||
> -          ((LenOfConfig != REST_EX_CONFIG_DATA_LEN_UNKNOWN) &&
> (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigDataLength !=
> LenOfConfig)))
> -      {
> -        goto ON_ERROR;
> -      }
> -    }
> +  default:
> +    goto ON_ERROR;
> +  }
> +  if (RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceAccessMode !=
> AccessMode ||
> +      RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceType !=
> ServiceType ||
> +      RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigType !=
> ConfigType ||
> +      ((LenOfConfig != REST_EX_CONFIG_DATA_LEN_UNKNOWN) &&
> (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigDataLength !=
> LenOfConfig))) {
> +    goto ON_ERROR;
> +  }
>
> -    //
> -    // This is proper REST EX instance.
> -    //
> -    *ChildInstanceHandle = ChildHandle;
> -    FreePool (Handle);
> -    return EFI_SUCCESS;
> +  //
> +  // This is proper REST EX instance.
> +  //
> +  *ChildInstanceHandle = ChildHandle;
> +  return EFI_SUCCESS;
>
>  ON_ERROR:;
>      NetLibDestroyServiceChild (
> -      *(Handle + (NoBuffer - 1)),
> +      Controller,
>        Image,
>        &gEfiRestExServiceBindingProtocolGuid,
>        ChildHandle
>        );
> -    NoBuffer--;
> -  }
> -
> -  FreePool (Handle);
>    return EFI_NOT_FOUND;
>  }
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 29e7b0624ebf..86ec88ddafc4 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -87,6 +87,7 @@ CreateRestExInstance (
>    EFI_STATUS  Status;
>
>    Status = RestExLibCreateChild (
> +             Instance->NetworkInterface->OpenDriverControllerHandle,
>               Instance->Owner,
>               FixedPcdGetBool (PcdRedfishDiscoverAccessModeInBand) ?
> EfiRestExServiceInBandAccess : EfiRestExServiceOutOfBandAccess,
>               EfiRestExConfigHttp,
> --
> 2.39.0


[-- Attachment #2: Type: text/html, Size: 26215 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface
  2023-04-20  1:28       ` Chang, Abner
@ 2023-04-20  1:33         ` Minh Nguyen OS
  0 siblings, 0 replies; 17+ messages in thread
From: Minh Nguyen OS @ 2023-04-20  1:33 UTC (permalink / raw)
  To: Chang, Abner, devel@edk2.groups.io
  Cc: Open Source Submission, nicklew@nvidia.com, igork@ami.com,
	Nhi Pham OS, Tinh Nguyen OS, Vu Nguyen OS

[-- Attachment #1: Type: text/plain, Size: 13588 bytes --]

Hi Chang,

 Yes, this is the first time and the issue comes from my internal mail and waiting for fixing. Thanks for that information and will check with miki.demeter@intel.com as well.

Thanks,
Minh Nguyen.
________________________________
From: Chang, Abner <Abner.Chang@amd.com>
Sent: Thursday, April 20, 2023 8:28 AM
To: Minh Nguyen OS <minhnguyen@os.amperecomputing.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Open Source Submission <patches@amperecomputing.com>; nicklew@nvidia.com <nicklew@nvidia.com>; igork@ami.com <igork@ami.com>; Nhi Pham OS <nhi@os.amperecomputing.com>; Tinh Nguyen OS <tinhnguyen@os.amperecomputing.com>; Vu Nguyen OS <vunguyen@os.amperecomputing.com>
Subject: Re: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface


[AMD Official Use Only - General]

Hi Minh,
Is this the first time you send email to groups.io, if yes please also check it with Miki
miki.demeter@intel.com to confirm is your mail passed through groups.io or not.

Thanks
Abner


Get Outlook for Android<https://aka.ms/AAb9ysg>

________________________________
From: Minh Nguyen OS <minhnguyen@os.amperecomputing.com>
Sent: Thursday, April 20, 2023, 9:23 AM
To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Open Source Submission <patches@amperecomputing.com>; nicklew@nvidia.com <nicklew@nvidia.com>; igork@ami.com <igork@ami.com>; Nhi Pham OS <nhi@os.amperecomputing.com>; Tinh Nguyen OS <tinhnguyen@os.amperecomputing.com>; Vu Nguyen OS <vunguyen@os.amperecomputing.com>
Subject: Re: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

Hi Chang,

  It seems like my email has the problem, please expect the delay for my v2 patches while my email's fixing. Thanks for your patience.

Thanks,
Minh Nguyen
________________________________
From: Chang, Abner <Abner.Chang@amd.com>
Sent: Sunday, April 16, 2023 11:36 AM
To: Minh Nguyen OS <minhnguyen@os.amperecomputing.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Open Source Submission <patches@amperecomputing.com>; nicklew@nvidia.com <nicklew@nvidia.com>; igork@ami.com <igork@ami.com>; Nhi Pham OS <nhi@os.amperecomputing.com>; Tinh Nguyen OS <tinhnguyen@os.amperecomputing.com>; Vu Nguyen OS <vunguyen@os.amperecomputing.com>
Subject: RE: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface

[AMD Official Use Only - General]

Hi Minh,
That is weird I can't find this patch set on group.io, and I can't apply this patch from email neither. Could you please check it?

Thanks
Abner

> -----Original Message-----
> From: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> Sent: Friday, April 14, 2023 4:19 PM
> To: devel@edk2.groups.io
> Cc: patches@amperecomputing.com; Chang, Abner
> <Abner.Chang@amd.com>; nicklew@nvidia.com; igork@ami.com;
> nhi@os.amperecomputing.com; tinhnguyen@os.amperecomputing.com; Vu
> Nguyen <vunguyen@os.amperecomputing.com>; Minh Nguyen
> <minhnguyen@os.amperecomputing.com>
> Subject: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> From: Vu Nguyen <vunguyen@os.amperecomputing.com>
>
> When a MAC address matching interface is found, a RestEx child will be
> created to provide the Redfish communication on that interface.
> Currently, It will try to locate all RestEx binding services and choose the first
> satisfied instance without taking care about current selected interface. This
> might raise an issue on the system with multiple network devices that the
> RestEx child was installed on wrong interface.
>
> Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> ---
>  RedfishPkg/Include/Library/RestExLib.h             |   3 +
>  RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c     | 153 ++++++++----------
> --
>  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c |   1 +
>  3 files changed, 63 insertions(+), 94 deletions(-)
>
> diff --git a/RedfishPkg/Include/Library/RestExLib.h
> b/RedfishPkg/Include/Library/RestExLib.h
> index bc4e4ca6caa7..2c32c3684cf4 100644
> --- a/RedfishPkg/Include/Library/RestExLib.h
> +++ b/RedfishPkg/Include/Library/RestExLib.h
> @@ -2,6 +2,7 @@
>    This library provides help functions for REST EX Protocol.
>
>    (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
> +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -20,6 +21,7 @@
>    This function allows the caller to create child handle for specific
>    REST server.
>
> +  @param[in]  Controller           The controller handle used of selected
> interface.
>    @param[in]  Image                The image handle used to open service.
>    @param[in]  AccessMode           Access mode of REST server.
>    @param[in]  ConfigType           Underlying configuration to communicate
> with REST server.
> @@ -32,6 +34,7 @@
>  **/
>  EFI_STATUS
>  RestExLibCreateChild (
> +  IN EFI_HANDLE                       Controller,
>    IN EFI_HANDLE                       Image,
>    IN EFI_REST_EX_SERVICE_ACCESS_MODE  AccessMode,
>    IN EFI_REST_EX_CONFIG_TYPE          ConfigType,
> diff --git a/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> b/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> index d9acad24dec1..6002be93c54e 100644
> --- a/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> +++ b/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> @@ -2,6 +2,7 @@
>    This library provides help functions for REST EX Protocol.
>
>    (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
> +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -9,6 +10,7 @@
>
>  #include <Uefi.h>
>  #include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
>  #include <Library/MemoryAllocationLib.h>  #include <Library/NetLib.h>
> #include <Library/UefiBootServicesTableLib.h>
> @@ -21,6 +23,7 @@
>    This function allows the caller to create child handle for specific
>    REST server.
>
> +  @param[in]  Controller           The controller handle used of selected
> interface.
>    @param[in]  Image                The image handle used to open service.
>    @param[in]  AccessMode           Access mode of REST server.
>    @param[in]  ConfigType           Underlying configuration to communicate
> with REST server.
> @@ -33,6 +36,7 @@
>  **/
>  EFI_STATUS
>  RestExLibCreateChild (
> +  IN EFI_HANDLE                       Controller,
>    IN EFI_HANDLE                       Image,
>    IN EFI_REST_EX_SERVICE_ACCESS_MODE  AccessMode,
>    IN EFI_REST_EX_CONFIG_TYPE          ConfigType,
> @@ -41,8 +45,6 @@ RestExLibCreateChild (
>    )
>  {
>    EFI_STATUS                Status;
> -  UINTN                     NoBuffer;
> -  EFI_HANDLE                *Handle;
>    EFI_HANDLE                ChildHandle;
>    EFI_REST_EX_PROTOCOL      *RestEx;
>    EFI_REST_EX_SERVICE_INFO  *RestExServiceInfo; @@ -59,116 +61,79 @@
> RestExLibCreateChild (
>    }
>
>    *ChildInstanceHandle = NULL;
> -  //
> -  // Locate all REST EX binding service.
> -  //
> -  Handle   = NULL;
> -  NoBuffer = 0;
> -  Status   = gBS->LocateHandleBuffer (
> -                    ByProtocol,
> -                    &gEfiRestExServiceBindingProtocolGuid,
> -                    NULL,
> -                    &NoBuffer,
> -                    &Handle
> -                    );
> -  if (EFI_ERROR (Status) && (Status != EFI_BUFFER_TOO_SMALL)) {
> +
> +  ChildHandle = NULL;
> +  Status = NetLibCreateServiceChild (
> +             Controller,
> +             Image,
> +             &gEfiRestExServiceBindingProtocolGuid,
> +             &ChildHandle
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: Failed to create service child - %r \n",
> +      __FUNCTION__,
> +      Status
> +      ));
>      return Status;
>    }
> -
> -  Handle = (EFI_HANDLE *)AllocateZeroPool (sizeof (EFI_HANDLE) *
> NoBuffer);
> -  if (Handle == NULL) {
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> -
> -  Status = gBS->LocateHandleBuffer (
> -                  ByProtocol,
> -                  &gEfiRestExServiceBindingProtocolGuid,
> +  Status = gBS->OpenProtocol (
> +                  ChildHandle,
> +                  &gEfiRestExProtocolGuid,
> +                  (VOID **)&RestEx,
> +                  Image,
>                    NULL,
> -                  &NoBuffer,
> -                  &Handle
> +                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
>                    );
>    if (EFI_ERROR (Status)) {
> -    FreePool (Handle);
> -    return Status;
> +    goto ON_ERROR;
>    }
>
>    //
> -  // Search for the proper REST EX instance.
> +  // Get the information of REST service provided by this EFI REST EX
> + driver
>    //
> -  while (NoBuffer != 0) {
> -    ChildHandle = NULL;
> -    Status      = NetLibCreateServiceChild (
> -                    *(Handle + (NoBuffer - 1)),
> -                    Image,
> -                    &gEfiRestExServiceBindingProtocolGuid,
> -                    &ChildHandle
> -                    );
> -    if (!EFI_ERROR (Status)) {
> -      Status = gBS->OpenProtocol (
> -                      ChildHandle,
> -                      &gEfiRestExProtocolGuid,
> -                      (VOID **)&RestEx,
> -                      Image,
> -                      NULL,
> -                      EFI_OPEN_PROTOCOL_GET_PROTOCOL
> -                      );
> -      if (EFI_ERROR (Status)) {
> -        goto ON_ERROR;
> -      }
> -
> -      //
> -      // Get the information of REST service provided by this EFI REST EX driver
> -      //
> -      Status = RestEx->GetService (
> -                         RestEx,
> -                         &RestExServiceInfo
> -                         );
> -      if (EFI_ERROR (Status)) {
> -        goto ON_ERROR;
> -      }
> -
> -      //
> -      // Check REST EX property.
> -      //
> -      switch (ConfigType) {
> -        case EfiRestExConfigHttp:
> -          LenOfConfig = sizeof (EFI_REST_EX_HTTP_CONFIG_DATA);
> -          break;
> -
> -        case EfiRestExConfigUnspecific:
> -          LenOfConfig = REST_EX_CONFIG_DATA_LEN_UNKNOWN;
> -          break;
> +  Status = RestEx->GetService (
> +                     RestEx,
> +                     &RestExServiceInfo
> +                     );
> +  if (EFI_ERROR (Status)) {
> +    goto ON_ERROR;
> +  }
> +  //
> +  // Check REST EX property.
> +  //
> +  switch (ConfigType) {
> +  case EfiRestExConfigHttp:
> +    LenOfConfig = sizeof (EFI_REST_EX_HTTP_CONFIG_DATA);
> +    break;
>
> -        default:
> -          goto ON_ERROR;
> -      }
> +  case EfiRestExConfigUnspecific:
> +    LenOfConfig = REST_EX_CONFIG_DATA_LEN_UNKNOWN;
> +    break;
>
> -      if ((RestExServiceInfo-
> >EfiRestExServiceInfoV10.RestServiceAccessMode != AccessMode) ||
> -          (RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceType !=
> ServiceType) ||
> -          (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigType !=
> ConfigType) ||
> -          ((LenOfConfig != REST_EX_CONFIG_DATA_LEN_UNKNOWN) &&
> (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigDataLength !=
> LenOfConfig)))
> -      {
> -        goto ON_ERROR;
> -      }
> -    }
> +  default:
> +    goto ON_ERROR;
> +  }
> +  if (RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceAccessMode !=
> AccessMode ||
> +      RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceType !=
> ServiceType ||
> +      RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigType !=
> ConfigType ||
> +      ((LenOfConfig != REST_EX_CONFIG_DATA_LEN_UNKNOWN) &&
> (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigDataLength !=
> LenOfConfig))) {
> +    goto ON_ERROR;
> +  }
>
> -    //
> -    // This is proper REST EX instance.
> -    //
> -    *ChildInstanceHandle = ChildHandle;
> -    FreePool (Handle);
> -    return EFI_SUCCESS;
> +  //
> +  // This is proper REST EX instance.
> +  //
> +  *ChildInstanceHandle = ChildHandle;
> +  return EFI_SUCCESS;
>
>  ON_ERROR:;
>      NetLibDestroyServiceChild (
> -      *(Handle + (NoBuffer - 1)),
> +      Controller,
>        Image,
>        &gEfiRestExServiceBindingProtocolGuid,
>        ChildHandle
>        );
> -    NoBuffer--;
> -  }
> -
> -  FreePool (Handle);
>    return EFI_NOT_FOUND;
>  }
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 29e7b0624ebf..86ec88ddafc4 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -87,6 +87,7 @@ CreateRestExInstance (
>    EFI_STATUS  Status;
>
>    Status = RestExLibCreateChild (
> +             Instance->NetworkInterface->OpenDriverControllerHandle,
>               Instance->Owner,
>               FixedPcdGetBool (PcdRedfishDiscoverAccessModeInBand) ?
> EfiRestExServiceInBandAccess : EfiRestExServiceOutOfBandAccess,
>               EfiRestExConfigHttp,
> --
> 2.39.0


[-- Attachment #2: Type: text/html, Size: 29789 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2023-04-20  1:33 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-14  8:18 [PATCH 0/5] Adding necessary changes for RedfishPkg Minh Nguyen
2023-04-14  8:18 ` [PATCH 1/5] RedfishPkg: Correct variable type to prevent memory corruption Minh Nguyen
2023-04-14  8:52   ` Nickle Wang
2023-04-14  8:19 ` [PATCH 2/5] RedfishPkg: Prevent assertion of allocate zero length region Minh Nguyen
2023-04-14  9:03   ` Nickle Wang
2023-04-14  9:17     ` minhn
2023-04-14  9:28       ` Nickle Wang
2023-04-14  9:20     ` Minh Nguyen OS
2023-04-14  8:19 ` [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface Minh Nguyen
2023-04-16  4:36   ` Chang, Abner
2023-04-20  1:23     ` Minh Nguyen OS
2023-04-20  1:28       ` Chang, Abner
2023-04-20  1:33         ` Minh Nguyen OS
2023-04-14  8:19 ` [PATCH 4/5] RedfishPkg: Add missing newline character Minh Nguyen
2023-04-14  8:53   ` Nickle Wang
2023-04-14  8:19 ` [PATCH 5/5] RedfishPkg: Fix compile issue on Linux Minh Nguyen
2023-04-14 14:13   ` Chang, Abner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox