public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [edk2-redfish-client][PATCH V2] RedfishClientPkg: Add ETag PCD and revise Redfish ETag functions
@ 2024-01-04  8:56 Chang, Abner via groups.io
  2024-01-05  0:20 ` Mike Maslenkin
  0 siblings, 1 reply; 4+ messages in thread
From: Chang, Abner via groups.io @ 2024-01-04  8:56 UTC (permalink / raw)
  To: devel; +Cc: Nickle Wang, Igor Kulchytskyy, Mike Maslenkin

From: Abner Chang <abner.chang@amd.com>

Add PCD to disable ETag capability for the case Redfish
service doesn't support ETag.

Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 RedfishClientPkg/RedfishClientPkg.dec         |   2 +
 .../RedfishFeatureUtilityLib.inf              |   1 +
 .../Library/RedfishFeatureUtilityLib.h        |  46 +++-
 .../Features/Bios/v1_0_9/Dxe/BiosDxe.c        |  18 +-
 .../v1_0_4/Common/BootOptionCommon.c          |   4 +-
 .../BootOption/v1_0_4/Dxe/BootOptionDxe.c     |  16 +-
 .../v1_5_0/Dxe/ComputerSystemDxe.c            |  16 +-
 .../Features/Memory/V1_7_1/Dxe/MemoryDxe.c    |  16 +-
 .../RedfishFeatureUtilityLib.c                | 208 ++++++++++++------
 9 files changed, 197 insertions(+), 130 deletions(-)

diff --git a/RedfishClientPkg/RedfishClientPkg.dec b/RedfishClientPkg/RedfishClientPkg.dec
index b350facae0..8adef327fb 100644
--- a/RedfishClientPkg/RedfishClientPkg.dec
+++ b/RedfishClientPkg/RedfishClientPkg.dec
@@ -76,6 +76,8 @@
   gEfiRedfishClientPkgTokenSpaceGuid.PcdDefaultRedfishVersion|L"v1"|VOID*|0x10000004
   ## The number of seconds that the firmware will wait before system reboot
   gEfiRedfishClientPkgTokenSpaceGuid.PcdRedfishSystemRebootTimeout|5|UINT16|0x20000002
+  ## Default capability of Redfish service side ETAG support
+  gEfiRedfishClientPkgTokenSpaceGuid.PcdRedfishServiceEtagSupported|TRUE|BOOLEAN|0x10000005
 
 [PcdsDynamicEx]
   ## The flag used to indicate that system reboot is required due to system configuration change
diff --git a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.inf b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.inf
index 718273b248..fde6a176d0 100644
--- a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.inf
+++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.inf
@@ -54,6 +54,7 @@
 
 [Pcd]
   gEfiRedfishClientPkgTokenSpaceGuid.PcdRedfishSystemRebootRequired
+  gEfiRedfishClientPkgTokenSpaceGuid.PcdRedfishServiceEtagSupported
 
 [Guids]
 
diff --git a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
index 9513a65617..834ea0fcfe 100644
--- a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
+++ b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
@@ -81,7 +81,7 @@ CopyConfiglanguageList (
 
 /**
 
-  Get number of node from the string. Node is seperated by '/'.
+  Get number of node from the string. Node is separated by '/'.
 
   @param[in]  NodeString             The node string to parse.
 
@@ -559,6 +559,19 @@ GetEtagWithUri (
   IN  EFI_STRING  Uri
   );
 
+/**
+
+  This function returns a boolean of ETAG support on Redfish service side.
+
+  @retval     TRUE    ETAG is supported on Redfish service.
+  @retval     FALSE   ETAG is not supported on Redfish service.
+
+**/
+BOOLEAN
+CheckIsServerEtagSupported (
+  VOID
+  );
+
 /**
 
   Get @odata.id from give HTTP payload. It's call responsibility to release returned buffer.
@@ -931,22 +944,33 @@ CompareRedfishPropertyVagueValues (
   );
 
 /**
+  Find "ETag" from either HTTP header or Redfish response.
 
-  Find "ETag" and "Location" from either HTTP header or Redfish response.
+  @param[in]  Response        HTTP response
+  @param[out] Etag            String buffer to return ETag
 
-  @param[in]  Response    HTTP response
-  @param[out] Etag        String buffer to return ETag
-  @param[out] Location    String buffer to return Location
+  @retval  EFI_SUCCESS            ETag is returned in Etag
+  @retval  EFI_UNSUPPORTED        ETag is unsupported
+  @retval  EFI_INVALID_PARAMETER  Response, Etag or both are NULL.
 
-  @retval     EFI_SUCCESS     Data is found and returned.
-  @retval     Others          Errors occur.
+**/
+EFI_STATUS
+GetHttpResponseEtag (
+  IN  REDFISH_RESPONSE  *Response,
+  OUT CHAR8             **Etag
+  );
+
+/**
+  Find "Location" from either HTTP header or Redfish response.
+
+  @param[in]  Response        HTTP response
+  @param[out] Location        String buffer to return Location
 
 **/
 EFI_STATUS
-GetEtagAndLocation (
-  IN  REDFISH_RESPONSE *Response,
-  OUT CHAR8 **Etag, OPTIONAL
-  OUT EFI_STRING        *Location    OPTIONAL
+GetHttpResponseLocation (
+  IN  REDFISH_RESPONSE  *Response,
+  OUT EFI_STRING        *Location
   );
 
 /**
diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
index 2a49c5cd22..14e854c861 100644
--- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
+++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
@@ -153,14 +153,10 @@ RedfishResourceConsumeResource (
   ASSERT (Private->Json != NULL);
 
   //
-  // Find etag in HTTP response header
+  // Searching for etag in HTTP response header
   //
-  Etag   = NULL;
-  Status = GetEtagAndLocation (ExpectedResponse, &Etag, NULL);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a, failed to get ETag from HTTP header\n", __func__));
-  }
-
+  Etag = NULL;
+  GetHttpResponseEtag (ExpectedResponse, &Etag);
   Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
   if (EFI_ERROR (Status)) {
     if (Status != EFI_ALREADY_STARTED) {
@@ -365,12 +361,8 @@ RedfishResourceCheck (
   //
   // Find etag in HTTP response header
   //
-  Etag   = NULL;
-  Status = GetEtagAndLocation (&Response, &Etag, NULL);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n", __func__));
-  }
-
+  Etag = NULL;
+  GetHttpResponseEtag (&Response, &Etag);
   Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a, failed to check resource from: %s: %r\n", __func__, Uri, Status));
diff --git a/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCommon.c b/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCommon.c
index 0d4c2162c6..0b9a72abc3 100644
--- a/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCommon.c
+++ b/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCommon.c
@@ -455,9 +455,9 @@ RedfishProvisioningResourceCommon (
   }
 
   //
-  // per Redfish spec. the URL of new resource will be returned in "Location" header.
+  // Per Redfish spec. the URL of new resource will be returned in "Location" header.
   //
-  Status = GetEtagAndLocation (&Response, NULL, &NewResourceLocation);
+  Status = GetHttpResponseLocation (&Response, &NewResourceLocation);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a: cannot find new location: %r\n", __func__, Status));
     goto RELEASE_RESOURCE;
diff --git a/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c b/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
index ba090c51d3..204c6b0757 100644
--- a/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
+++ b/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
@@ -154,12 +154,8 @@ RedfishResourceConsumeResource (
   //
   // Find etag in HTTP response header
   //
-  Etag   = NULL;
-  Status = GetEtagAndLocation (ExpectedResponse, &Etag, NULL);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n", __func__));
-  }
-
+  Etag = NULL;
+  GetHttpResponseEtag (ExpectedResponse, &Etag);
   Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a: failed to consume resource from: %s: %r\n", __func__, Private->Uri, Status));
@@ -358,12 +354,8 @@ RedfishResourceCheck (
   //
   // Find etag in HTTP response header
   //
-  Etag   = NULL;
-  Status = GetEtagAndLocation (&Response, &Etag, NULL);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n", __func__));
-  }
-
+  Etag = NULL;
+  GetHttpResponseEtag (&Response, &Etag);
   Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
   if (EFI_ERROR (Status)) {
     DEBUG ((REDFISH_BOOT_OPTION_DEBUG_TRACE, "%a: failed to check resource from: %s: %r\n", __func__, Uri, Status));
diff --git a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Dxe/ComputerSystemDxe.c b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Dxe/ComputerSystemDxe.c
index 0bbaa92bf4..7805bcf36b 100644
--- a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Dxe/ComputerSystemDxe.c
+++ b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Dxe/ComputerSystemDxe.c
@@ -149,12 +149,8 @@ RedfishResourceConsumeResource (
   //
   // Find etag in HTTP response header
   //
-  Etag   = NULL;
-  Status = GetEtagAndLocation (ExpectedResponse, &Etag, NULL);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a, failed to get ETag from HTTP header\n", __func__));
-  }
-
+  Etag = NULL;
+  GetHttpResponseEtag (ExpectedResponse, &Etag);
   Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
   if (EFI_ERROR (Status)) {
     if (Status != EFI_ALREADY_STARTED) {
@@ -359,12 +355,8 @@ RedfishResourceCheck (
   //
   // Find etag in HTTP response header
   //
-  Etag   = NULL;
-  Status = GetEtagAndLocation (&Response, &Etag, NULL);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n", __func__));
-  }
-
+  Etag = NULL;
+  GetHttpResponseEtag (&Response, &Etag);
   Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a, failed to check resource from: %s: %r\n", __func__, Uri, Status));
diff --git a/RedfishClientPkg/Features/Memory/V1_7_1/Dxe/MemoryDxe.c b/RedfishClientPkg/Features/Memory/V1_7_1/Dxe/MemoryDxe.c
index 9230078051..b87da775b3 100644
--- a/RedfishClientPkg/Features/Memory/V1_7_1/Dxe/MemoryDxe.c
+++ b/RedfishClientPkg/Features/Memory/V1_7_1/Dxe/MemoryDxe.c
@@ -149,12 +149,8 @@ RedfishResourceConsumeResource (
   //
   // Find etag in HTTP response header
   //
-  Etag   = NULL;
-  Status = GetEtagAndLocation (ExpectedResponse, &Etag, NULL);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a, failed to get ETag from HTTP header\n", __func__));
-  }
-
+  Etag = NULL;
+  GetHttpResponseEtag (ExpectedResponse, &Etag);
   Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
   if (EFI_ERROR (Status)) {
     if (Status != EFI_ALREADY_STARTED) {
@@ -359,12 +355,8 @@ RedfishResourceCheck (
   //
   // Find etag in HTTP response header
   //
-  Etag   = NULL;
-  Status = GetEtagAndLocation (&Response, &Etag, NULL);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n", __func__));
-  }
-
+  Etag = NULL;
+  GetHttpResponseEtag (&Response, &Etag);
   Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a, failed to check resource from: %s: %r\n", __func__, Uri, Status));
diff --git a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
index 1c2d40f622..7709a9d1a2 100644
--- a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
+++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
@@ -133,6 +133,11 @@ SetEtagFromUri (
   REDFISH_RESPONSE  Response;
   EFI_STRING        PendingSettingUri;
 
+  if (!CheckIsServerEtagSupported ()) {
+    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported\n", __func__));
+    return EFI_SUCCESS;
+  }
+
   if ((RedfishService == NULL) || IS_EMPTY_STRING (Uri)) {
     return EFI_INVALID_PARAMETER;
   }
@@ -157,9 +162,9 @@ SetEtagFromUri (
   //
   // Find etag in HTTP response header
   //
-  Status = GetEtagAndLocation (&Response, &Etag, NULL);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n", __func__));
+  Status = GetHttpResponseEtag (&Response, &Etag);
+  if (EFI_ERROR (Status) && (Status != EFI_UNSUPPORTED)) {
+    DEBUG ((DEBUG_ERROR, "%a: Failed to get ETag from HTTP header\n", __func__));
     Status = EFI_NOT_FOUND;
     goto ON_RELEASE;
   }
@@ -241,6 +246,11 @@ SetEtagWithUri (
   EFI_STATUS  Status;
   CHAR8       *AsciiUri;
 
+  if (!CheckIsServerEtagSupported ()) {
+    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported\n", __func__));
+    return EFI_SUCCESS;
+  }
+
   if (IS_EMPTY_STRING (EtagStr) || IS_EMPTY_STRING (Uri)) {
     return EFI_INVALID_PARAMETER;
   }
@@ -286,6 +296,11 @@ GetEtagWithUri (
     return NULL;
   }
 
+  if (!CheckIsServerEtagSupported ()) {
+    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported\n", __func__));
+    return NULL;
+  }
+
   Status = RedfishLocateProtocol ((VOID **)&mEtagProtocol, &gEdkIIRedfishETagProtocolGuid);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a: fail to locate gEdkIIRedfishETagProtocolGuid: %r\n", __func__, Status));
@@ -1726,44 +1741,39 @@ RedfishFeatureGetUnifiedArrayTypeConfigureLang (
 }
 
 /**
+  Find "ETag" from either HTTP header or Redfish response.
 
-  Find "ETag" and "Location" from either HTTP header or Redfish response.
+  @param[in]  Response        HTTP response
+  @param[out] Etag            String buffer to return ETag
 
-  @param[in]  Response    HTTP response
-  @param[out] Etag        String buffer to return ETag
-  @param[out] Location    String buffer to return Location
-
-  @retval     EFI_SUCCESS     Data is found and returned.
-  @retval     Others          Errors occur.
+  @retval  EFI_SUCCESS            ETag is returned in Etag
+  @retval  EFI_UNSUPPORTED        ETag is unsupported
+  @retval  EFI_INVALID_PARAMETER  Response, Etag or both are NULL.
 
 **/
 EFI_STATUS
-GetEtagAndLocation (
-  IN  REDFISH_RESPONSE *Response,
-  OUT CHAR8 **Etag, OPTIONAL
-  OUT EFI_STRING        *Location    OPTIONAL
+GetHttpResponseEtag (
+  IN  REDFISH_RESPONSE  *Response,
+  OUT CHAR8             **Etag
   )
 {
+  EFI_STATUS        Status;
   EDKII_JSON_VALUE  JsonValue;
   EDKII_JSON_VALUE  OdataValue;
   CHAR8             *OdataString;
-  CHAR8             *AsciiLocation;
   EFI_HTTP_HEADER   *Header;
-  EFI_STATUS        Status;
 
-  if (Response == NULL) {
+  if ((Response == NULL) || (Etag == NULL)) {
     return EFI_INVALID_PARAMETER;
   }
 
-  if ((Etag == NULL) && (Location == NULL)) {
-    return EFI_SUCCESS;
-  }
-
   Status = EFI_SUCCESS;
-
-  if (Etag != NULL) {
-    *Etag = NULL;
-
+  *Etag  = NULL;
+  if (!CheckIsServerEtagSupported ()) {
+    // Don't look for ETAG header or property in this case.
+    DEBUG ((DEBUG_INFO, "%a: WARNING - No ETag support on Redfish service.\n", __func__));
+    return EFI_UNSUPPORTED;
+  } else {
     if (*(Response->StatusCode) == HTTP_STATUS_200_OK) {
       Header = HttpFindHeader (Response->HeaderCount, Response->Headers, HTTP_HEADER_ETAG);
       if (Header != NULL) {
@@ -1793,51 +1803,94 @@ GetEtagAndLocation (
 
     if (*Etag == NULL) {
       Status = EFI_NOT_FOUND;
+      DEBUG ((DEBUG_ERROR, "%a: Failed to retrieve ETag from HTTP response paylaod.\n", __func__));
     }
   }
 
-  if (Location != NULL) {
-    *Location     = NULL;
-    AsciiLocation = NULL;
+  return Status;
+}
 
-    if (*(Response->StatusCode) == HTTP_STATUS_200_OK) {
-      Header = HttpFindHeader (Response->HeaderCount, Response->Headers, HTTP_HEADER_LOCATION);
-      if (Header != NULL) {
-        AsciiLocation = AllocateCopyPool (AsciiStrSize (Header->FieldValue), Header->FieldValue);
-        ASSERT (AsciiLocation != NULL);
-      }
+/**
+  Find "Location" from either HTTP header or Redfish response.
+
+  @param[in]  Response        HTTP response
+  @param[out] Location        String buffer to return Location
+
+**/
+EFI_STATUS
+GetHttpResponseLocation (
+  IN  REDFISH_RESPONSE  *Response,
+  OUT EFI_STRING        *Location
+  )
+{
+  EFI_STATUS        Status;
+  EDKII_JSON_VALUE  JsonValue;
+  EDKII_JSON_VALUE  OdataValue;
+  CHAR8             *OdataString;
+  CHAR8             *AsciiLocation;
+  EFI_HTTP_HEADER   *Header;
+
+  if ((Response == NULL) || (Location == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Status        = EFI_SUCCESS;
+  *Location     = NULL;
+  AsciiLocation = NULL;
+  if (*(Response->StatusCode) == HTTP_STATUS_200_OK) {
+    Header = HttpFindHeader (Response->HeaderCount, Response->Headers, HTTP_HEADER_LOCATION);
+    if (Header != NULL) {
+      AsciiLocation = AllocateCopyPool (AsciiStrSize (Header->FieldValue), Header->FieldValue);
+      ASSERT (AsciiLocation != NULL);
     }
+  }
 
-    //
-    // No header is returned. Search payload for location.
-    //
-    if ((*Location == NULL) && (Response->Payload != NULL)) {
-      JsonValue = RedfishJsonInPayload (Response->Payload);
-      if (JsonValue != NULL) {
-        OdataValue = JsonObjectGetValue (JsonValueGetObject (JsonValue), "@odata.id");
-        if (OdataValue != NULL) {
-          OdataString = (CHAR8 *)JsonValueGetAsciiString (OdataValue);
-          if (OdataString != NULL) {
-            AsciiLocation = AllocateCopyPool (AsciiStrSize (OdataString), OdataString);
-            ASSERT (AsciiLocation != NULL);
-          }
+  //
+  // No header is returned. Search payload for location.
+  //
+  if ((*Location == NULL) && (Response->Payload != NULL)) {
+    JsonValue = RedfishJsonInPayload (Response->Payload);
+    if (JsonValue != NULL) {
+      OdataValue = JsonObjectGetValue (JsonValueGetObject (JsonValue), "@odata.id");
+      if (OdataValue != NULL) {
+        OdataString = (CHAR8 *)JsonValueGetAsciiString (OdataValue);
+        if (OdataString != NULL) {
+          AsciiLocation = AllocateCopyPool (AsciiStrSize (OdataString), OdataString);
+          ASSERT (AsciiLocation != NULL);
         }
-
-        JsonValueFree (JsonValue);
       }
-    }
 
-    if (AsciiLocation != NULL) {
-      *Location = StrAsciiToUnicode (AsciiLocation);
-      FreePool (AsciiLocation);
-    } else {
-      Status = EFI_NOT_FOUND;
+      JsonValueFree (JsonValue);
     }
   }
 
+  if (AsciiLocation != NULL) {
+    *Location = StrAsciiToUnicode (AsciiLocation);
+    FreePool (AsciiLocation);
+  } else {
+    Status = EFI_NOT_FOUND;
+    DEBUG ((DEBUG_ERROR, "%a: Failed to retrieve Location from HTTP response paylaod.\n", __func__));
+  }
+
   return Status;
 }
 
+/**
+
+  This function returns a boolean of ETAG support on Redfish service side.
+
+  @retval     TRUE    ETAG is supported on Redfish service.
+  @retval     FALSE   ETAG is not supported on Redfish service.
+
+**/
+BOOLEAN
+CheckIsServerEtagSupported (
+  VOID
+  )
+{
+  return FixedPcdGetBool (PcdRedfishServiceEtagSupported);
+}
+
 /**
 
   Create HTTP payload and send them to redfish service with PATCH method.
@@ -1861,7 +1914,7 @@ CreatePayloadToPatchResource (
 {
   REDFISH_PAYLOAD   Payload;
   EDKII_JSON_VALUE  ResourceJsonValue;
-  REDFISH_RESPONSE  PostResponse;
+  REDFISH_RESPONSE  PatchResponse;
   EFI_STATUS        Status;
 
   if ((Service == NULL) || (TargetPayload == NULL) || IS_EMPTY_STRING (Json)) {
@@ -1876,8 +1929,8 @@ CreatePayloadToPatchResource (
     goto EXIT_FREE_JSON_VALUE;
   }
 
-  ZeroMem (&PostResponse, sizeof (REDFISH_RESPONSE));
-  Status = RedfishPatchToPayload (TargetPayload, Payload, &PostResponse);
+  ZeroMem (&PatchResponse, sizeof (REDFISH_RESPONSE));
+  Status = RedfishPatchToPayload (TargetPayload, Payload, &PatchResponse);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a:%d Failed to PATCH payload to Redfish service.\n", __func__, __LINE__));
 
@@ -1885,7 +1938,7 @@ CreatePayloadToPatchResource (
     DEBUG ((DEBUG_ERROR, "%a: Request:\n", __func__));
     DumpRedfishPayload (DEBUG_ERROR, Payload);
     DEBUG ((DEBUG_ERROR, "%a: Response:\n", __func__));
-    DumpRedfishResponse (__func__, DEBUG_ERROR, &PostResponse);
+    DumpRedfishResponse (__func__, DEBUG_ERROR, &PatchResponse);
     DEBUG_CODE_END ();
     goto EXIT_FREE_JSON_VALUE;
   }
@@ -1893,16 +1946,20 @@ CreatePayloadToPatchResource (
   //
   // Find ETag
   //
-  Status = GetEtagAndLocation (&PostResponse, Etag, NULL);
-  if (EFI_ERROR (Status)) {
+  Status = GetHttpResponseEtag (&PatchResponse, Etag);
+  if (Status == EFI_UNSUPPORTED) {
+    Status = EFI_SUCCESS;
+    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported on Redfish service.\n", __func__));
+  } else {
     Status = EFI_DEVICE_ERROR;
+    DEBUG ((DEBUG_ERROR, "%a: Fail to get Location header nor proerty from HTTP response payload.\n", __func__));
   }
 
   RedfishFreeResponse (
-    PostResponse.StatusCode,
-    PostResponse.HeaderCount,
-    PostResponse.Headers,
-    PostResponse.Payload
+    PatchResponse.StatusCode,
+    PatchResponse.HeaderCount,
+    PatchResponse.Headers,
+    PatchResponse.Payload
     );
 
 EXIT_FREE_JSON_VALUE:
@@ -1935,7 +1992,7 @@ CreatePayloadToPostResource (
   IN  REDFISH_PAYLOAD  *TargetPayload,
   IN  CHAR8            *Json,
   OUT EFI_STRING       *Location,
-  OUT CHAR8            **Etag
+  OUT CHAR8            **Etag OPTIONAL
   )
 {
   REDFISH_PAYLOAD   Payload;
@@ -1943,7 +2000,7 @@ CreatePayloadToPostResource (
   REDFISH_RESPONSE  PostResponse;
   EFI_STATUS        Status;
 
-  if ((Service == NULL) || (TargetPayload == NULL) || IS_EMPTY_STRING (Json) || (Location == NULL) || (Etag == NULL)) {
+  if ((Service == NULL) || (TargetPayload == NULL) || IS_EMPTY_STRING (Json) || (Location == NULL)) {
     return EFI_INVALID_PARAMETER;
   }
 
@@ -1970,12 +2027,22 @@ CreatePayloadToPostResource (
     goto EXIT_FREE_JSON_VALUE;
   }
 
+  Status = GetHttpResponseEtag (&PostResponse, Etag);
+  if (Status == EFI_UNSUPPORTED) {
+    Status = EFI_SUCCESS;
+    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported on Redfish service.\n", __func__));
+  } else if (EFI_ERROR (Status)) {
+    Status = EFI_DEVICE_ERROR;
+    DEBUG ((DEBUG_ERROR, "%a: Fail to get ETAG header nor ETAG propertyfrom HTTP response payload.\n", __func__));
+  }
+
   //
   // per Redfish spec. the URL of new resource will be returned in "Location" header.
   //
-  Status = GetEtagAndLocation (&PostResponse, Etag, Location);
+  Status = GetHttpResponseLocation (&PostResponse, Location);
   if (EFI_ERROR (Status)) {
     Status = EFI_DEVICE_ERROR;
+    DEBUG ((DEBUG_ERROR, "%a: Fail to get Location header nor proerty from HTTP response payload.\n", __func__));
   }
 
   RedfishFreeResponse (
@@ -3117,6 +3184,11 @@ CheckEtag (
 {
   CHAR8  *EtagInDb;
 
+  if (!CheckIsServerEtagSupported ()) {
+    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported, always returns FALSE to consume resource (Performance would be reduced).\n", __func__));
+    return FALSE;
+  }
+
   if (IS_EMPTY_STRING (Uri)) {
     return FALSE;
   }
-- 
2.37.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113153): https://edk2.groups.io/g/devel/message/113153
Mute This Topic: https://groups.io/mt/103519287/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [edk2-redfish-client][PATCH V2] RedfishClientPkg: Add ETag PCD and revise Redfish ETag functions
  2024-01-04  8:56 [edk2-devel] [edk2-redfish-client][PATCH V2] RedfishClientPkg: Add ETag PCD and revise Redfish ETag functions Chang, Abner via groups.io
@ 2024-01-05  0:20 ` Mike Maslenkin
  2024-01-05  9:03   ` Chang, Abner via groups.io
  0 siblings, 1 reply; 4+ messages in thread
From: Mike Maslenkin @ 2024-01-05  0:20 UTC (permalink / raw)
  To: devel, abner.chang; +Cc: Nickle Wang, Igor Kulchytskyy

Looks good to me.

But could it be possible to rephrase "ETAG is not supported on Redfish
service." ?
May be I misunderstand, but I assume "Redfish service" is a service at
BMC side, while we are disabling ETAG functionality at Redfish client
side.
README.md says "Redfish service hosted by Board Management Controller
(BMC) in server".
Currently there is no method to get server features (AFAIR), so we
disable a work with those explicitly on the client side.

Regards,
Mike.

On Thu, Jan 4, 2024 at 11:57 AM Chang, Abner via groups.io
<abner.chang=amd.com@groups.io> wrote:
>
> From: Abner Chang <abner.chang@amd.com>
>
> Add PCD to disable ETag capability for the case Redfish
> service doesn't support ETag.
>
> Signed-off-by: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
>  RedfishClientPkg/RedfishClientPkg.dec         |   2 +
>  .../RedfishFeatureUtilityLib.inf              |   1 +
>  .../Library/RedfishFeatureUtilityLib.h        |  46 +++-
>  .../Features/Bios/v1_0_9/Dxe/BiosDxe.c        |  18 +-
>  .../v1_0_4/Common/BootOptionCommon.c          |   4 +-
>  .../BootOption/v1_0_4/Dxe/BootOptionDxe.c     |  16 +-
>  .../v1_5_0/Dxe/ComputerSystemDxe.c            |  16 +-
>  .../Features/Memory/V1_7_1/Dxe/MemoryDxe.c    |  16 +-
>  .../RedfishFeatureUtilityLib.c                | 208 ++++++++++++------
>  9 files changed, 197 insertions(+), 130 deletions(-)
>
> diff --git a/RedfishClientPkg/RedfishClientPkg.dec b/RedfishClientPkg/RedfishClientPkg.dec
> index b350facae0..8adef327fb 100644
> --- a/RedfishClientPkg/RedfishClientPkg.dec
> +++ b/RedfishClientPkg/RedfishClientPkg.dec
> @@ -76,6 +76,8 @@
>    gEfiRedfishClientPkgTokenSpaceGuid.PcdDefaultRedfishVersion|L"v1"|VOID*|0x10000004
>    ## The number of seconds that the firmware will wait before system reboot
>    gEfiRedfishClientPkgTokenSpaceGuid.PcdRedfishSystemRebootTimeout|5|UINT16|0x20000002
> +  ## Default capability of Redfish service side ETAG support
> +  gEfiRedfishClientPkgTokenSpaceGuid.PcdRedfishServiceEtagSupported|TRUE|BOOLEAN|0x10000005
>
>  [PcdsDynamicEx]
>    ## The flag used to indicate that system reboot is required due to system configuration change
> diff --git a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.inf b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.inf
> index 718273b248..fde6a176d0 100644
> --- a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.inf
> +++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.inf
> @@ -54,6 +54,7 @@
>
>  [Pcd]
>    gEfiRedfishClientPkgTokenSpaceGuid.PcdRedfishSystemRebootRequired
> +  gEfiRedfishClientPkgTokenSpaceGuid.PcdRedfishServiceEtagSupported
>
>  [Guids]
>
> diff --git a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> index 9513a65617..834ea0fcfe 100644
> --- a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> +++ b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> @@ -81,7 +81,7 @@ CopyConfiglanguageList (
>
>  /**
>
> -  Get number of node from the string. Node is seperated by '/'.
> +  Get number of node from the string. Node is separated by '/'.
>
>    @param[in]  NodeString             The node string to parse.
>
> @@ -559,6 +559,19 @@ GetEtagWithUri (
>    IN  EFI_STRING  Uri
>    );
>
> +/**
> +
> +  This function returns a boolean of ETAG support on Redfish service side.
> +
> +  @retval     TRUE    ETAG is supported on Redfish service.
> +  @retval     FALSE   ETAG is not supported on Redfish service.
> +
> +**/
> +BOOLEAN
> +CheckIsServerEtagSupported (
> +  VOID
> +  );
> +
>  /**
>
>    Get @odata.id from give HTTP payload. It's call responsibility to release returned buffer.
> @@ -931,22 +944,33 @@ CompareRedfishPropertyVagueValues (
>    );
>
>  /**
> +  Find "ETag" from either HTTP header or Redfish response.
>
> -  Find "ETag" and "Location" from either HTTP header or Redfish response.
> +  @param[in]  Response        HTTP response
> +  @param[out] Etag            String buffer to return ETag
>
> -  @param[in]  Response    HTTP response
> -  @param[out] Etag        String buffer to return ETag
> -  @param[out] Location    String buffer to return Location
> +  @retval  EFI_SUCCESS            ETag is returned in Etag
> +  @retval  EFI_UNSUPPORTED        ETag is unsupported
> +  @retval  EFI_INVALID_PARAMETER  Response, Etag or both are NULL.
>
> -  @retval     EFI_SUCCESS     Data is found and returned.
> -  @retval     Others          Errors occur.
> +**/
> +EFI_STATUS
> +GetHttpResponseEtag (
> +  IN  REDFISH_RESPONSE  *Response,
> +  OUT CHAR8             **Etag
> +  );
> +
> +/**
> +  Find "Location" from either HTTP header or Redfish response.
> +
> +  @param[in]  Response        HTTP response
> +  @param[out] Location        String buffer to return Location
>
>  **/
>  EFI_STATUS
> -GetEtagAndLocation (
> -  IN  REDFISH_RESPONSE *Response,
> -  OUT CHAR8 **Etag, OPTIONAL
> -  OUT EFI_STRING        *Location    OPTIONAL
> +GetHttpResponseLocation (
> +  IN  REDFISH_RESPONSE  *Response,
> +  OUT EFI_STRING        *Location
>    );
>
>  /**
> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> index 2a49c5cd22..14e854c861 100644
> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> @@ -153,14 +153,10 @@ RedfishResourceConsumeResource (
>    ASSERT (Private->Json != NULL);
>
>    //
> -  // Find etag in HTTP response header
> +  // Searching for etag in HTTP response header
>    //
> -  Etag   = NULL;
> -  Status = GetEtagAndLocation (ExpectedResponse, &Etag, NULL);
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "%a, failed to get ETag from HTTP header\n", __func__));
> -  }
> -
> +  Etag = NULL;
> +  GetHttpResponseEtag (ExpectedResponse, &Etag);
>    Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
>    if (EFI_ERROR (Status)) {
>      if (Status != EFI_ALREADY_STARTED) {
> @@ -365,12 +361,8 @@ RedfishResourceCheck (
>    //
>    // Find etag in HTTP response header
>    //
> -  Etag   = NULL;
> -  Status = GetEtagAndLocation (&Response, &Etag, NULL);
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n", __func__));
> -  }
> -
> +  Etag = NULL;
> +  GetHttpResponseEtag (&Response, &Etag);
>    Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "%a, failed to check resource from: %s: %r\n", __func__, Uri, Status));
> diff --git a/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCommon.c b/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCommon.c
> index 0d4c2162c6..0b9a72abc3 100644
> --- a/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCommon.c
> +++ b/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCommon.c
> @@ -455,9 +455,9 @@ RedfishProvisioningResourceCommon (
>    }
>
>    //
> -  // per Redfish spec. the URL of new resource will be returned in "Location" header.
> +  // Per Redfish spec. the URL of new resource will be returned in "Location" header.
>    //
> -  Status = GetEtagAndLocation (&Response, NULL, &NewResourceLocation);
> +  Status = GetHttpResponseLocation (&Response, &NewResourceLocation);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "%a: cannot find new location: %r\n", __func__, Status));
>      goto RELEASE_RESOURCE;
> diff --git a/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c b/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
> index ba090c51d3..204c6b0757 100644
> --- a/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
> +++ b/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
> @@ -154,12 +154,8 @@ RedfishResourceConsumeResource (
>    //
>    // Find etag in HTTP response header
>    //
> -  Etag   = NULL;
> -  Status = GetEtagAndLocation (ExpectedResponse, &Etag, NULL);
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n", __func__));
> -  }
> -
> +  Etag = NULL;
> +  GetHttpResponseEtag (ExpectedResponse, &Etag);
>    Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "%a: failed to consume resource from: %s: %r\n", __func__, Private->Uri, Status));
> @@ -358,12 +354,8 @@ RedfishResourceCheck (
>    //
>    // Find etag in HTTP response header
>    //
> -  Etag   = NULL;
> -  Status = GetEtagAndLocation (&Response, &Etag, NULL);
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n", __func__));
> -  }
> -
> +  Etag = NULL;
> +  GetHttpResponseEtag (&Response, &Etag);
>    Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((REDFISH_BOOT_OPTION_DEBUG_TRACE, "%a: failed to check resource from: %s: %r\n", __func__, Uri, Status));
> diff --git a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Dxe/ComputerSystemDxe.c b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Dxe/ComputerSystemDxe.c
> index 0bbaa92bf4..7805bcf36b 100644
> --- a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Dxe/ComputerSystemDxe.c
> +++ b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Dxe/ComputerSystemDxe.c
> @@ -149,12 +149,8 @@ RedfishResourceConsumeResource (
>    //
>    // Find etag in HTTP response header
>    //
> -  Etag   = NULL;
> -  Status = GetEtagAndLocation (ExpectedResponse, &Etag, NULL);
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "%a, failed to get ETag from HTTP header\n", __func__));
> -  }
> -
> +  Etag = NULL;
> +  GetHttpResponseEtag (ExpectedResponse, &Etag);
>    Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
>    if (EFI_ERROR (Status)) {
>      if (Status != EFI_ALREADY_STARTED) {
> @@ -359,12 +355,8 @@ RedfishResourceCheck (
>    //
>    // Find etag in HTTP response header
>    //
> -  Etag   = NULL;
> -  Status = GetEtagAndLocation (&Response, &Etag, NULL);
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n", __func__));
> -  }
> -
> +  Etag = NULL;
> +  GetHttpResponseEtag (&Response, &Etag);
>    Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "%a, failed to check resource from: %s: %r\n", __func__, Uri, Status));
> diff --git a/RedfishClientPkg/Features/Memory/V1_7_1/Dxe/MemoryDxe.c b/RedfishClientPkg/Features/Memory/V1_7_1/Dxe/MemoryDxe.c
> index 9230078051..b87da775b3 100644
> --- a/RedfishClientPkg/Features/Memory/V1_7_1/Dxe/MemoryDxe.c
> +++ b/RedfishClientPkg/Features/Memory/V1_7_1/Dxe/MemoryDxe.c
> @@ -149,12 +149,8 @@ RedfishResourceConsumeResource (
>    //
>    // Find etag in HTTP response header
>    //
> -  Etag   = NULL;
> -  Status = GetEtagAndLocation (ExpectedResponse, &Etag, NULL);
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "%a, failed to get ETag from HTTP header\n", __func__));
> -  }
> -
> +  Etag = NULL;
> +  GetHttpResponseEtag (ExpectedResponse, &Etag);
>    Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
>    if (EFI_ERROR (Status)) {
>      if (Status != EFI_ALREADY_STARTED) {
> @@ -359,12 +355,8 @@ RedfishResourceCheck (
>    //
>    // Find etag in HTTP response header
>    //
> -  Etag   = NULL;
> -  Status = GetEtagAndLocation (&Response, &Etag, NULL);
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n", __func__));
> -  }
> -
> +  Etag = NULL;
> +  GetHttpResponseEtag (&Response, &Etag);
>    Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "%a, failed to check resource from: %s: %r\n", __func__, Uri, Status));
> diff --git a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
> index 1c2d40f622..7709a9d1a2 100644
> --- a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
> +++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
> @@ -133,6 +133,11 @@ SetEtagFromUri (
>    REDFISH_RESPONSE  Response;
>    EFI_STRING        PendingSettingUri;
>
> +  if (!CheckIsServerEtagSupported ()) {
> +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported\n", __func__));
> +    return EFI_SUCCESS;
> +  }
> +
>    if ((RedfishService == NULL) || IS_EMPTY_STRING (Uri)) {
>      return EFI_INVALID_PARAMETER;
>    }
> @@ -157,9 +162,9 @@ SetEtagFromUri (
>    //
>    // Find etag in HTTP response header
>    //
> -  Status = GetEtagAndLocation (&Response, &Etag, NULL);
> -  if (EFI_ERROR (Status)) {
> -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n", __func__));
> +  Status = GetHttpResponseEtag (&Response, &Etag);
> +  if (EFI_ERROR (Status) && (Status != EFI_UNSUPPORTED)) {
> +    DEBUG ((DEBUG_ERROR, "%a: Failed to get ETag from HTTP header\n", __func__));
>      Status = EFI_NOT_FOUND;
>      goto ON_RELEASE;
>    }
> @@ -241,6 +246,11 @@ SetEtagWithUri (
>    EFI_STATUS  Status;
>    CHAR8       *AsciiUri;
>
> +  if (!CheckIsServerEtagSupported ()) {
> +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported\n", __func__));
> +    return EFI_SUCCESS;
> +  }
> +
>    if (IS_EMPTY_STRING (EtagStr) || IS_EMPTY_STRING (Uri)) {
>      return EFI_INVALID_PARAMETER;
>    }
> @@ -286,6 +296,11 @@ GetEtagWithUri (
>      return NULL;
>    }
>
> +  if (!CheckIsServerEtagSupported ()) {
> +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported\n", __func__));
> +    return NULL;
> +  }
> +
>    Status = RedfishLocateProtocol ((VOID **)&mEtagProtocol, &gEdkIIRedfishETagProtocolGuid);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "%a: fail to locate gEdkIIRedfishETagProtocolGuid: %r\n", __func__, Status));
> @@ -1726,44 +1741,39 @@ RedfishFeatureGetUnifiedArrayTypeConfigureLang (
>  }
>
>  /**
> +  Find "ETag" from either HTTP header or Redfish response.
>
> -  Find "ETag" and "Location" from either HTTP header or Redfish response.
> +  @param[in]  Response        HTTP response
> +  @param[out] Etag            String buffer to return ETag
>
> -  @param[in]  Response    HTTP response
> -  @param[out] Etag        String buffer to return ETag
> -  @param[out] Location    String buffer to return Location
> -
> -  @retval     EFI_SUCCESS     Data is found and returned.
> -  @retval     Others          Errors occur.
> +  @retval  EFI_SUCCESS            ETag is returned in Etag
> +  @retval  EFI_UNSUPPORTED        ETag is unsupported
> +  @retval  EFI_INVALID_PARAMETER  Response, Etag or both are NULL.
>
>  **/
>  EFI_STATUS
> -GetEtagAndLocation (
> -  IN  REDFISH_RESPONSE *Response,
> -  OUT CHAR8 **Etag, OPTIONAL
> -  OUT EFI_STRING        *Location    OPTIONAL
> +GetHttpResponseEtag (
> +  IN  REDFISH_RESPONSE  *Response,
> +  OUT CHAR8             **Etag
>    )
>  {
> +  EFI_STATUS        Status;
>    EDKII_JSON_VALUE  JsonValue;
>    EDKII_JSON_VALUE  OdataValue;
>    CHAR8             *OdataString;
> -  CHAR8             *AsciiLocation;
>    EFI_HTTP_HEADER   *Header;
> -  EFI_STATUS        Status;
>
> -  if (Response == NULL) {
> +  if ((Response == NULL) || (Etag == NULL)) {
>      return EFI_INVALID_PARAMETER;
>    }
>
> -  if ((Etag == NULL) && (Location == NULL)) {
> -    return EFI_SUCCESS;
> -  }
> -
>    Status = EFI_SUCCESS;
> -
> -  if (Etag != NULL) {
> -    *Etag = NULL;
> -
> +  *Etag  = NULL;
> +  if (!CheckIsServerEtagSupported ()) {
> +    // Don't look for ETAG header or property in this case.
> +    DEBUG ((DEBUG_INFO, "%a: WARNING - No ETag support on Redfish service.\n", __func__));
> +    return EFI_UNSUPPORTED;
> +  } else {
>      if (*(Response->StatusCode) == HTTP_STATUS_200_OK) {
>        Header = HttpFindHeader (Response->HeaderCount, Response->Headers, HTTP_HEADER_ETAG);
>        if (Header != NULL) {
> @@ -1793,51 +1803,94 @@ GetEtagAndLocation (
>
>      if (*Etag == NULL) {
>        Status = EFI_NOT_FOUND;
> +      DEBUG ((DEBUG_ERROR, "%a: Failed to retrieve ETag from HTTP response paylaod.\n", __func__));
>      }
>    }
>
> -  if (Location != NULL) {
> -    *Location     = NULL;
> -    AsciiLocation = NULL;
> +  return Status;
> +}
>
> -    if (*(Response->StatusCode) == HTTP_STATUS_200_OK) {
> -      Header = HttpFindHeader (Response->HeaderCount, Response->Headers, HTTP_HEADER_LOCATION);
> -      if (Header != NULL) {
> -        AsciiLocation = AllocateCopyPool (AsciiStrSize (Header->FieldValue), Header->FieldValue);
> -        ASSERT (AsciiLocation != NULL);
> -      }
> +/**
> +  Find "Location" from either HTTP header or Redfish response.
> +
> +  @param[in]  Response        HTTP response
> +  @param[out] Location        String buffer to return Location
> +
> +**/
> +EFI_STATUS
> +GetHttpResponseLocation (
> +  IN  REDFISH_RESPONSE  *Response,
> +  OUT EFI_STRING        *Location
> +  )
> +{
> +  EFI_STATUS        Status;
> +  EDKII_JSON_VALUE  JsonValue;
> +  EDKII_JSON_VALUE  OdataValue;
> +  CHAR8             *OdataString;
> +  CHAR8             *AsciiLocation;
> +  EFI_HTTP_HEADER   *Header;
> +
> +  if ((Response == NULL) || (Location == NULL)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Status        = EFI_SUCCESS;
> +  *Location     = NULL;
> +  AsciiLocation = NULL;
> +  if (*(Response->StatusCode) == HTTP_STATUS_200_OK) {
> +    Header = HttpFindHeader (Response->HeaderCount, Response->Headers, HTTP_HEADER_LOCATION);
> +    if (Header != NULL) {
> +      AsciiLocation = AllocateCopyPool (AsciiStrSize (Header->FieldValue), Header->FieldValue);
> +      ASSERT (AsciiLocation != NULL);
>      }
> +  }
>
> -    //
> -    // No header is returned. Search payload for location.
> -    //
> -    if ((*Location == NULL) && (Response->Payload != NULL)) {
> -      JsonValue = RedfishJsonInPayload (Response->Payload);
> -      if (JsonValue != NULL) {
> -        OdataValue = JsonObjectGetValue (JsonValueGetObject (JsonValue), "@odata.id");
> -        if (OdataValue != NULL) {
> -          OdataString = (CHAR8 *)JsonValueGetAsciiString (OdataValue);
> -          if (OdataString != NULL) {
> -            AsciiLocation = AllocateCopyPool (AsciiStrSize (OdataString), OdataString);
> -            ASSERT (AsciiLocation != NULL);
> -          }
> +  //
> +  // No header is returned. Search payload for location.
> +  //
> +  if ((*Location == NULL) && (Response->Payload != NULL)) {
> +    JsonValue = RedfishJsonInPayload (Response->Payload);
> +    if (JsonValue != NULL) {
> +      OdataValue = JsonObjectGetValue (JsonValueGetObject (JsonValue), "@odata.id");
> +      if (OdataValue != NULL) {
> +        OdataString = (CHAR8 *)JsonValueGetAsciiString (OdataValue);
> +        if (OdataString != NULL) {
> +          AsciiLocation = AllocateCopyPool (AsciiStrSize (OdataString), OdataString);
> +          ASSERT (AsciiLocation != NULL);
>          }
> -
> -        JsonValueFree (JsonValue);
>        }
> -    }
>
> -    if (AsciiLocation != NULL) {
> -      *Location = StrAsciiToUnicode (AsciiLocation);
> -      FreePool (AsciiLocation);
> -    } else {
> -      Status = EFI_NOT_FOUND;
> +      JsonValueFree (JsonValue);
>      }
>    }
>
> +  if (AsciiLocation != NULL) {
> +    *Location = StrAsciiToUnicode (AsciiLocation);
> +    FreePool (AsciiLocation);
> +  } else {
> +    Status = EFI_NOT_FOUND;
> +    DEBUG ((DEBUG_ERROR, "%a: Failed to retrieve Location from HTTP response paylaod.\n", __func__));
> +  }
> +
>    return Status;
>  }
>
> +/**
> +
> +  This function returns a boolean of ETAG support on Redfish service side.
> +
> +  @retval     TRUE    ETAG is supported on Redfish service.
> +  @retval     FALSE   ETAG is not supported on Redfish service.
> +
> +**/
> +BOOLEAN
> +CheckIsServerEtagSupported (
> +  VOID
> +  )
> +{
> +  return FixedPcdGetBool (PcdRedfishServiceEtagSupported);
> +}
> +
>  /**
>
>    Create HTTP payload and send them to redfish service with PATCH method.
> @@ -1861,7 +1914,7 @@ CreatePayloadToPatchResource (
>  {
>    REDFISH_PAYLOAD   Payload;
>    EDKII_JSON_VALUE  ResourceJsonValue;
> -  REDFISH_RESPONSE  PostResponse;
> +  REDFISH_RESPONSE  PatchResponse;
>    EFI_STATUS        Status;
>
>    if ((Service == NULL) || (TargetPayload == NULL) || IS_EMPTY_STRING (Json)) {
> @@ -1876,8 +1929,8 @@ CreatePayloadToPatchResource (
>      goto EXIT_FREE_JSON_VALUE;
>    }
>
> -  ZeroMem (&PostResponse, sizeof (REDFISH_RESPONSE));
> -  Status = RedfishPatchToPayload (TargetPayload, Payload, &PostResponse);
> +  ZeroMem (&PatchResponse, sizeof (REDFISH_RESPONSE));
> +  Status = RedfishPatchToPayload (TargetPayload, Payload, &PatchResponse);
>    if (EFI_ERROR (Status)) {
>      DEBUG ((DEBUG_ERROR, "%a:%d Failed to PATCH payload to Redfish service.\n", __func__, __LINE__));
>
> @@ -1885,7 +1938,7 @@ CreatePayloadToPatchResource (
>      DEBUG ((DEBUG_ERROR, "%a: Request:\n", __func__));
>      DumpRedfishPayload (DEBUG_ERROR, Payload);
>      DEBUG ((DEBUG_ERROR, "%a: Response:\n", __func__));
> -    DumpRedfishResponse (__func__, DEBUG_ERROR, &PostResponse);
> +    DumpRedfishResponse (__func__, DEBUG_ERROR, &PatchResponse);
>      DEBUG_CODE_END ();
>      goto EXIT_FREE_JSON_VALUE;
>    }
> @@ -1893,16 +1946,20 @@ CreatePayloadToPatchResource (
>    //
>    // Find ETag
>    //
> -  Status = GetEtagAndLocation (&PostResponse, Etag, NULL);
> -  if (EFI_ERROR (Status)) {
> +  Status = GetHttpResponseEtag (&PatchResponse, Etag);
> +  if (Status == EFI_UNSUPPORTED) {
> +    Status = EFI_SUCCESS;
> +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported on Redfish service.\n", __func__));
> +  } else {
>      Status = EFI_DEVICE_ERROR;
> +    DEBUG ((DEBUG_ERROR, "%a: Fail to get Location header nor proerty from HTTP response payload.\n", __func__));
>    }
>
>    RedfishFreeResponse (
> -    PostResponse.StatusCode,
> -    PostResponse.HeaderCount,
> -    PostResponse.Headers,
> -    PostResponse.Payload
> +    PatchResponse.StatusCode,
> +    PatchResponse.HeaderCount,
> +    PatchResponse.Headers,
> +    PatchResponse.Payload
>      );
>
>  EXIT_FREE_JSON_VALUE:
> @@ -1935,7 +1992,7 @@ CreatePayloadToPostResource (
>    IN  REDFISH_PAYLOAD  *TargetPayload,
>    IN  CHAR8            *Json,
>    OUT EFI_STRING       *Location,
> -  OUT CHAR8            **Etag
> +  OUT CHAR8            **Etag OPTIONAL
>    )
>  {
>    REDFISH_PAYLOAD   Payload;
> @@ -1943,7 +2000,7 @@ CreatePayloadToPostResource (
>    REDFISH_RESPONSE  PostResponse;
>    EFI_STATUS        Status;
>
> -  if ((Service == NULL) || (TargetPayload == NULL) || IS_EMPTY_STRING (Json) || (Location == NULL) || (Etag == NULL)) {
> +  if ((Service == NULL) || (TargetPayload == NULL) || IS_EMPTY_STRING (Json) || (Location == NULL)) {
>      return EFI_INVALID_PARAMETER;
>    }
>
> @@ -1970,12 +2027,22 @@ CreatePayloadToPostResource (
>      goto EXIT_FREE_JSON_VALUE;
>    }
>
> +  Status = GetHttpResponseEtag (&PostResponse, Etag);
> +  if (Status == EFI_UNSUPPORTED) {
> +    Status = EFI_SUCCESS;
> +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported on Redfish service.\n", __func__));
> +  } else if (EFI_ERROR (Status)) {
> +    Status = EFI_DEVICE_ERROR;
> +    DEBUG ((DEBUG_ERROR, "%a: Fail to get ETAG header nor ETAG propertyfrom HTTP response payload.\n", __func__));
> +  }
> +
>    //
>    // per Redfish spec. the URL of new resource will be returned in "Location" header.
>    //
> -  Status = GetEtagAndLocation (&PostResponse, Etag, Location);
> +  Status = GetHttpResponseLocation (&PostResponse, Location);
>    if (EFI_ERROR (Status)) {
>      Status = EFI_DEVICE_ERROR;
> +    DEBUG ((DEBUG_ERROR, "%a: Fail to get Location header nor proerty from HTTP response payload.\n", __func__));
>    }
>
>    RedfishFreeResponse (
> @@ -3117,6 +3184,11 @@ CheckEtag (
>  {
>    CHAR8  *EtagInDb;
>
> +  if (!CheckIsServerEtagSupported ()) {
> +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported, always returns FALSE to consume resource (Performance would be reduced).\n", __func__));
> +    return FALSE;
> +  }
> +
>    if (IS_EMPTY_STRING (Uri)) {
>      return FALSE;
>    }
> --
> 2.37.1.windows.1
>
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113202): https://edk2.groups.io/g/devel/message/113202
Mute This Topic: https://groups.io/mt/103519287/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [edk2-redfish-client][PATCH V2] RedfishClientPkg: Add ETag PCD and revise Redfish ETag functions
  2024-01-05  0:20 ` Mike Maslenkin
@ 2024-01-05  9:03   ` Chang, Abner via groups.io
  2024-01-05 11:33     ` Mike Maslenkin
  0 siblings, 1 reply; 4+ messages in thread
From: Chang, Abner via groups.io @ 2024-01-05  9:03 UTC (permalink / raw)
  To: Mike Maslenkin, devel@edk2.groups.io; +Cc: Nickle Wang, Igor Kulchytskyy

[AMD Official Use Only - General]

Hi Mike,
This PCD is introduced for the platform that connects to the Redfish service which doesn't support ETag.
We disable the client code that handles ETag with setting this PCD to FALSE. So client will just consume any Redfish property from service even there is nothing changed. This knob doesn't control Redfish service behavior on BMC.
Yes, there is no method to detect if Redfish service supports ETag or not. So we introduce a client side knob to disable ETag checking although it mentions the service "should" support ETag in Redfish spec. However, a simple Redfish services may not implementing ETag HTTP header.
Does above clarify the question?

Thanks
Abner

> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Friday, January 5, 2024 8:20 AM
> To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>; Igor Kulchytskyy <igork@ami.com>
> Subject: Re: [edk2-devel] [edk2-redfish-client][PATCH V2] RedfishClientPkg:
> Add ETag PCD and revise Redfish ETag functions
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Looks good to me.
>
> But could it be possible to rephrase "ETAG is not supported on Redfish
> service." ?
> May be I misunderstand, but I assume "Redfish service" is a service at
> BMC side, while we are disabling ETAG functionality at Redfish client
> side.
> README.md says "Redfish service hosted by Board Management Controller
> (BMC) in server".
> Currently there is no method to get server features (AFAIR), so we
> disable a work with those explicitly on the client side.
>
> Regards,
> Mike.
>
> On Thu, Jan 4, 2024 at 11:57 AM Chang, Abner via groups.io
> <abner.chang=amd.com@groups.io> wrote:
> >
> > From: Abner Chang <abner.chang@amd.com>
> >
> > Add PCD to disable ETag capability for the case Redfish
> > service doesn't support ETag.
> >
> > Signed-off-by: Abner Chang <abner.chang@amd.com>
> > Cc: Nickle Wang <nicklew@nvidia.com>
> > Cc: Igor Kulchytskyy <igork@ami.com>
> > Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
> > ---
> >  RedfishClientPkg/RedfishClientPkg.dec         |   2 +
> >  .../RedfishFeatureUtilityLib.inf              |   1 +
> >  .../Library/RedfishFeatureUtilityLib.h        |  46 +++-
> >  .../Features/Bios/v1_0_9/Dxe/BiosDxe.c        |  18 +-
> >  .../v1_0_4/Common/BootOptionCommon.c          |   4 +-
> >  .../BootOption/v1_0_4/Dxe/BootOptionDxe.c     |  16 +-
> >  .../v1_5_0/Dxe/ComputerSystemDxe.c            |  16 +-
> >  .../Features/Memory/V1_7_1/Dxe/MemoryDxe.c    |  16 +-
> >  .../RedfishFeatureUtilityLib.c                | 208 ++++++++++++------
> >  9 files changed, 197 insertions(+), 130 deletions(-)
> >
> > diff --git a/RedfishClientPkg/RedfishClientPkg.dec
> b/RedfishClientPkg/RedfishClientPkg.dec
> > index b350facae0..8adef327fb 100644
> > --- a/RedfishClientPkg/RedfishClientPkg.dec
> > +++ b/RedfishClientPkg/RedfishClientPkg.dec
> > @@ -76,6 +76,8 @@
> >
> gEfiRedfishClientPkgTokenSpaceGuid.PcdDefaultRedfishVersion|L"v1"|VOID*
> |0x10000004
> >    ## The number of seconds that the firmware will wait before system reboot
> >
> gEfiRedfishClientPkgTokenSpaceGuid.PcdRedfishSystemRebootTimeout|5|UI
> NT16|0x20000002
> > +  ## Default capability of Redfish service side ETAG support
> > +
> gEfiRedfishClientPkgTokenSpaceGuid.PcdRedfishServiceEtagSupported|TRUE|
> BOOLEAN|0x10000005
> >
> >  [PcdsDynamicEx]
> >    ## The flag used to indicate that system reboot is required due to system
> configuration change
> > diff --git
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.i
> nf
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.i
> nf
> > index 718273b248..fde6a176d0 100644
> > ---
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.i
> nf
> > +++
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.i
> nf
> > @@ -54,6 +54,7 @@
> >
> >  [Pcd]
> >    gEfiRedfishClientPkgTokenSpaceGuid.PcdRedfishSystemRebootRequired
> > +  gEfiRedfishClientPkgTokenSpaceGuid.PcdRedfishServiceEtagSupported
> >
> >  [Guids]
> >
> > diff --git a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > index 9513a65617..834ea0fcfe 100644
> > --- a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > +++ b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > @@ -81,7 +81,7 @@ CopyConfiglanguageList (
> >
> >  /**
> >
> > -  Get number of node from the string. Node is seperated by '/'.
> > +  Get number of node from the string. Node is separated by '/'.
> >
> >    @param[in]  NodeString             The node string to parse.
> >
> > @@ -559,6 +559,19 @@ GetEtagWithUri (
> >    IN  EFI_STRING  Uri
> >    );
> >
> > +/**
> > +
> > +  This function returns a boolean of ETAG support on Redfish service side.
> > +
> > +  @retval     TRUE    ETAG is supported on Redfish service.
> > +  @retval     FALSE   ETAG is not supported on Redfish service.
> > +
> > +**/
> > +BOOLEAN
> > +CheckIsServerEtagSupported (
> > +  VOID
> > +  );
> > +
> >  /**
> >
> >    Get @odata.id from give HTTP payload. It's call responsibility to release
> returned buffer.
> > @@ -931,22 +944,33 @@ CompareRedfishPropertyVagueValues (
> >    );
> >
> >  /**
> > +  Find "ETag" from either HTTP header or Redfish response.
> >
> > -  Find "ETag" and "Location" from either HTTP header or Redfish response.
> > +  @param[in]  Response        HTTP response
> > +  @param[out] Etag            String buffer to return ETag
> >
> > -  @param[in]  Response    HTTP response
> > -  @param[out] Etag        String buffer to return ETag
> > -  @param[out] Location    String buffer to return Location
> > +  @retval  EFI_SUCCESS            ETag is returned in Etag
> > +  @retval  EFI_UNSUPPORTED        ETag is unsupported
> > +  @retval  EFI_INVALID_PARAMETER  Response, Etag or both are NULL.
> >
> > -  @retval     EFI_SUCCESS     Data is found and returned.
> > -  @retval     Others          Errors occur.
> > +**/
> > +EFI_STATUS
> > +GetHttpResponseEtag (
> > +  IN  REDFISH_RESPONSE  *Response,
> > +  OUT CHAR8             **Etag
> > +  );
> > +
> > +/**
> > +  Find "Location" from either HTTP header or Redfish response.
> > +
> > +  @param[in]  Response        HTTP response
> > +  @param[out] Location        String buffer to return Location
> >
> >  **/
> >  EFI_STATUS
> > -GetEtagAndLocation (
> > -  IN  REDFISH_RESPONSE *Response,
> > -  OUT CHAR8 **Etag, OPTIONAL
> > -  OUT EFI_STRING        *Location    OPTIONAL
> > +GetHttpResponseLocation (
> > +  IN  REDFISH_RESPONSE  *Response,
> > +  OUT EFI_STRING        *Location
> >    );
> >
> >  /**
> > diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> > index 2a49c5cd22..14e854c861 100644
> > --- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> > +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> > @@ -153,14 +153,10 @@ RedfishResourceConsumeResource (
> >    ASSERT (Private->Json != NULL);
> >
> >    //
> > -  // Find etag in HTTP response header
> > +  // Searching for etag in HTTP response header
> >    //
> > -  Etag   = NULL;
> > -  Status = GetEtagAndLocation (ExpectedResponse, &Etag, NULL);
> > -  if (EFI_ERROR (Status)) {
> > -    DEBUG ((DEBUG_ERROR, "%a, failed to get ETag from HTTP header\n",
> __func__));
> > -  }
> > -
> > +  Etag = NULL;
> > +  GetHttpResponseEtag (ExpectedResponse, &Etag);
> >    Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
> >    if (EFI_ERROR (Status)) {
> >      if (Status != EFI_ALREADY_STARTED) {
> > @@ -365,12 +361,8 @@ RedfishResourceCheck (
> >    //
> >    // Find etag in HTTP response header
> >    //
> > -  Etag   = NULL;
> > -  Status = GetEtagAndLocation (&Response, &Etag, NULL);
> > -  if (EFI_ERROR (Status)) {
> > -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n",
> __func__));
> > -  }
> > -
> > +  Etag = NULL;
> > +  GetHttpResponseEtag (&Response, &Etag);
> >    Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
> >    if (EFI_ERROR (Status)) {
> >      DEBUG ((DEBUG_ERROR, "%a, failed to check resource from: %s: %r\n",
> __func__, Uri, Status));
> > diff --git
> a/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCom
> mon.c
> b/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCom
> mon.c
> > index 0d4c2162c6..0b9a72abc3 100644
> > ---
> a/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCom
> mon.c
> > +++
> b/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCom
> mon.c
> > @@ -455,9 +455,9 @@ RedfishProvisioningResourceCommon (
> >    }
> >
> >    //
> > -  // per Redfish spec. the URL of new resource will be returned in "Location"
> header.
> > +  // Per Redfish spec. the URL of new resource will be returned in "Location"
> header.
> >    //
> > -  Status = GetEtagAndLocation (&Response, NULL, &NewResourceLocation);
> > +  Status = GetHttpResponseLocation (&Response, &NewResourceLocation);
> >    if (EFI_ERROR (Status)) {
> >      DEBUG ((DEBUG_ERROR, "%a: cannot find new location: %r\n", __func__,
> Status));
> >      goto RELEASE_RESOURCE;
> > diff --git
> a/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
> b/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
> > index ba090c51d3..204c6b0757 100644
> > --- a/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
> > +++
> b/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
> > @@ -154,12 +154,8 @@ RedfishResourceConsumeResource (
> >    //
> >    // Find etag in HTTP response header
> >    //
> > -  Etag   = NULL;
> > -  Status = GetEtagAndLocation (ExpectedResponse, &Etag, NULL);
> > -  if (EFI_ERROR (Status)) {
> > -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n",
> __func__));
> > -  }
> > -
> > +  Etag = NULL;
> > +  GetHttpResponseEtag (ExpectedResponse, &Etag);
> >    Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
> >    if (EFI_ERROR (Status)) {
> >      DEBUG ((DEBUG_ERROR, "%a: failed to consume resource from: %s:
> %r\n", __func__, Private->Uri, Status));
> > @@ -358,12 +354,8 @@ RedfishResourceCheck (
> >    //
> >    // Find etag in HTTP response header
> >    //
> > -  Etag   = NULL;
> > -  Status = GetEtagAndLocation (&Response, &Etag, NULL);
> > -  if (EFI_ERROR (Status)) {
> > -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n",
> __func__));
> > -  }
> > -
> > +  Etag = NULL;
> > +  GetHttpResponseEtag (&Response, &Etag);
> >    Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
> >    if (EFI_ERROR (Status)) {
> >      DEBUG ((REDFISH_BOOT_OPTION_DEBUG_TRACE, "%a: failed to check
> resource from: %s: %r\n", __func__, Uri, Status));
> > diff --git
> a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Dxe/ComputerSyste
> mDxe.c
> b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Dxe/ComputerSyste
> mDxe.c
> > index 0bbaa92bf4..7805bcf36b 100644
> > ---
> a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Dxe/ComputerSyste
> mDxe.c
> > +++
> b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Dxe/ComputerSyste
> mDxe.c
> > @@ -149,12 +149,8 @@ RedfishResourceConsumeResource (
> >    //
> >    // Find etag in HTTP response header
> >    //
> > -  Etag   = NULL;
> > -  Status = GetEtagAndLocation (ExpectedResponse, &Etag, NULL);
> > -  if (EFI_ERROR (Status)) {
> > -    DEBUG ((DEBUG_ERROR, "%a, failed to get ETag from HTTP header\n",
> __func__));
> > -  }
> > -
> > +  Etag = NULL;
> > +  GetHttpResponseEtag (ExpectedResponse, &Etag);
> >    Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
> >    if (EFI_ERROR (Status)) {
> >      if (Status != EFI_ALREADY_STARTED) {
> > @@ -359,12 +355,8 @@ RedfishResourceCheck (
> >    //
> >    // Find etag in HTTP response header
> >    //
> > -  Etag   = NULL;
> > -  Status = GetEtagAndLocation (&Response, &Etag, NULL);
> > -  if (EFI_ERROR (Status)) {
> > -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n",
> __func__));
> > -  }
> > -
> > +  Etag = NULL;
> > +  GetHttpResponseEtag (&Response, &Etag);
> >    Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
> >    if (EFI_ERROR (Status)) {
> >      DEBUG ((DEBUG_ERROR, "%a, failed to check resource from: %s: %r\n",
> __func__, Uri, Status));
> > diff --git a/RedfishClientPkg/Features/Memory/V1_7_1/Dxe/MemoryDxe.c
> b/RedfishClientPkg/Features/Memory/V1_7_1/Dxe/MemoryDxe.c
> > index 9230078051..b87da775b3 100644
> > --- a/RedfishClientPkg/Features/Memory/V1_7_1/Dxe/MemoryDxe.c
> > +++ b/RedfishClientPkg/Features/Memory/V1_7_1/Dxe/MemoryDxe.c
> > @@ -149,12 +149,8 @@ RedfishResourceConsumeResource (
> >    //
> >    // Find etag in HTTP response header
> >    //
> > -  Etag   = NULL;
> > -  Status = GetEtagAndLocation (ExpectedResponse, &Etag, NULL);
> > -  if (EFI_ERROR (Status)) {
> > -    DEBUG ((DEBUG_ERROR, "%a, failed to get ETag from HTTP header\n",
> __func__));
> > -  }
> > -
> > +  Etag = NULL;
> > +  GetHttpResponseEtag (ExpectedResponse, &Etag);
> >    Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
> >    if (EFI_ERROR (Status)) {
> >      if (Status != EFI_ALREADY_STARTED) {
> > @@ -359,12 +355,8 @@ RedfishResourceCheck (
> >    //
> >    // Find etag in HTTP response header
> >    //
> > -  Etag   = NULL;
> > -  Status = GetEtagAndLocation (&Response, &Etag, NULL);
> > -  if (EFI_ERROR (Status)) {
> > -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n",
> __func__));
> > -  }
> > -
> > +  Etag = NULL;
> > +  GetHttpResponseEtag (&Response, &Etag);
> >    Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
> >    if (EFI_ERROR (Status)) {
> >      DEBUG ((DEBUG_ERROR, "%a, failed to check resource from: %s: %r\n",
> __func__, Uri, Status));
> > diff --git
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> > index 1c2d40f622..7709a9d1a2 100644
> > ---
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> > +++
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> > @@ -133,6 +133,11 @@ SetEtagFromUri (
> >    REDFISH_RESPONSE  Response;
> >    EFI_STRING        PendingSettingUri;
> >
> > +  if (!CheckIsServerEtagSupported ()) {
> > +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported\n",
> __func__));
> > +    return EFI_SUCCESS;
> > +  }
> > +
> >    if ((RedfishService == NULL) || IS_EMPTY_STRING (Uri)) {
> >      return EFI_INVALID_PARAMETER;
> >    }
> > @@ -157,9 +162,9 @@ SetEtagFromUri (
> >    //
> >    // Find etag in HTTP response header
> >    //
> > -  Status = GetEtagAndLocation (&Response, &Etag, NULL);
> > -  if (EFI_ERROR (Status)) {
> > -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n",
> __func__));
> > +  Status = GetHttpResponseEtag (&Response, &Etag);
> > +  if (EFI_ERROR (Status) && (Status != EFI_UNSUPPORTED)) {
> > +    DEBUG ((DEBUG_ERROR, "%a: Failed to get ETag from HTTP header\n",
> __func__));
> >      Status = EFI_NOT_FOUND;
> >      goto ON_RELEASE;
> >    }
> > @@ -241,6 +246,11 @@ SetEtagWithUri (
> >    EFI_STATUS  Status;
> >    CHAR8       *AsciiUri;
> >
> > +  if (!CheckIsServerEtagSupported ()) {
> > +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported\n",
> __func__));
> > +    return EFI_SUCCESS;
> > +  }
> > +
> >    if (IS_EMPTY_STRING (EtagStr) || IS_EMPTY_STRING (Uri)) {
> >      return EFI_INVALID_PARAMETER;
> >    }
> > @@ -286,6 +296,11 @@ GetEtagWithUri (
> >      return NULL;
> >    }
> >
> > +  if (!CheckIsServerEtagSupported ()) {
> > +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported\n",
> __func__));
> > +    return NULL;
> > +  }
> > +
> >    Status = RedfishLocateProtocol ((VOID **)&mEtagProtocol,
> &gEdkIIRedfishETagProtocolGuid);
> >    if (EFI_ERROR (Status)) {
> >      DEBUG ((DEBUG_ERROR, "%a: fail to locate
> gEdkIIRedfishETagProtocolGuid: %r\n", __func__, Status));
> > @@ -1726,44 +1741,39 @@
> RedfishFeatureGetUnifiedArrayTypeConfigureLang (
> >  }
> >
> >  /**
> > +  Find "ETag" from either HTTP header or Redfish response.
> >
> > -  Find "ETag" and "Location" from either HTTP header or Redfish response.
> > +  @param[in]  Response        HTTP response
> > +  @param[out] Etag            String buffer to return ETag
> >
> > -  @param[in]  Response    HTTP response
> > -  @param[out] Etag        String buffer to return ETag
> > -  @param[out] Location    String buffer to return Location
> > -
> > -  @retval     EFI_SUCCESS     Data is found and returned.
> > -  @retval     Others          Errors occur.
> > +  @retval  EFI_SUCCESS            ETag is returned in Etag
> > +  @retval  EFI_UNSUPPORTED        ETag is unsupported
> > +  @retval  EFI_INVALID_PARAMETER  Response, Etag or both are NULL.
> >
> >  **/
> >  EFI_STATUS
> > -GetEtagAndLocation (
> > -  IN  REDFISH_RESPONSE *Response,
> > -  OUT CHAR8 **Etag, OPTIONAL
> > -  OUT EFI_STRING        *Location    OPTIONAL
> > +GetHttpResponseEtag (
> > +  IN  REDFISH_RESPONSE  *Response,
> > +  OUT CHAR8             **Etag
> >    )
> >  {
> > +  EFI_STATUS        Status;
> >    EDKII_JSON_VALUE  JsonValue;
> >    EDKII_JSON_VALUE  OdataValue;
> >    CHAR8             *OdataString;
> > -  CHAR8             *AsciiLocation;
> >    EFI_HTTP_HEADER   *Header;
> > -  EFI_STATUS        Status;
> >
> > -  if (Response == NULL) {
> > +  if ((Response == NULL) || (Etag == NULL)) {
> >      return EFI_INVALID_PARAMETER;
> >    }
> >
> > -  if ((Etag == NULL) && (Location == NULL)) {
> > -    return EFI_SUCCESS;
> > -  }
> > -
> >    Status = EFI_SUCCESS;
> > -
> > -  if (Etag != NULL) {
> > -    *Etag = NULL;
> > -
> > +  *Etag  = NULL;
> > +  if (!CheckIsServerEtagSupported ()) {
> > +    // Don't look for ETAG header or property in this case.
> > +    DEBUG ((DEBUG_INFO, "%a: WARNING - No ETag support on Redfish
> service.\n", __func__));
> > +    return EFI_UNSUPPORTED;
> > +  } else {
> >      if (*(Response->StatusCode) == HTTP_STATUS_200_OK) {
> >        Header = HttpFindHeader (Response->HeaderCount, Response-
> >Headers, HTTP_HEADER_ETAG);
> >        if (Header != NULL) {
> > @@ -1793,51 +1803,94 @@ GetEtagAndLocation (
> >
> >      if (*Etag == NULL) {
> >        Status = EFI_NOT_FOUND;
> > +      DEBUG ((DEBUG_ERROR, "%a: Failed to retrieve ETag from HTTP
> response paylaod.\n", __func__));
> >      }
> >    }
> >
> > -  if (Location != NULL) {
> > -    *Location     = NULL;
> > -    AsciiLocation = NULL;
> > +  return Status;
> > +}
> >
> > -    if (*(Response->StatusCode) == HTTP_STATUS_200_OK) {
> > -      Header = HttpFindHeader (Response->HeaderCount, Response-
> >Headers, HTTP_HEADER_LOCATION);
> > -      if (Header != NULL) {
> > -        AsciiLocation = AllocateCopyPool (AsciiStrSize (Header->FieldValue),
> Header->FieldValue);
> > -        ASSERT (AsciiLocation != NULL);
> > -      }
> > +/**
> > +  Find "Location" from either HTTP header or Redfish response.
> > +
> > +  @param[in]  Response        HTTP response
> > +  @param[out] Location        String buffer to return Location
> > +
> > +**/
> > +EFI_STATUS
> > +GetHttpResponseLocation (
> > +  IN  REDFISH_RESPONSE  *Response,
> > +  OUT EFI_STRING        *Location
> > +  )
> > +{
> > +  EFI_STATUS        Status;
> > +  EDKII_JSON_VALUE  JsonValue;
> > +  EDKII_JSON_VALUE  OdataValue;
> > +  CHAR8             *OdataString;
> > +  CHAR8             *AsciiLocation;
> > +  EFI_HTTP_HEADER   *Header;
> > +
> > +  if ((Response == NULL) || (Location == NULL)) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  Status        = EFI_SUCCESS;
> > +  *Location     = NULL;
> > +  AsciiLocation = NULL;
> > +  if (*(Response->StatusCode) == HTTP_STATUS_200_OK) {
> > +    Header = HttpFindHeader (Response->HeaderCount, Response->Headers,
> HTTP_HEADER_LOCATION);
> > +    if (Header != NULL) {
> > +      AsciiLocation = AllocateCopyPool (AsciiStrSize (Header->FieldValue),
> Header->FieldValue);
> > +      ASSERT (AsciiLocation != NULL);
> >      }
> > +  }
> >
> > -    //
> > -    // No header is returned. Search payload for location.
> > -    //
> > -    if ((*Location == NULL) && (Response->Payload != NULL)) {
> > -      JsonValue = RedfishJsonInPayload (Response->Payload);
> > -      if (JsonValue != NULL) {
> > -        OdataValue = JsonObjectGetValue (JsonValueGetObject (JsonValue),
> "@odata.id");
> > -        if (OdataValue != NULL) {
> > -          OdataString = (CHAR8 *)JsonValueGetAsciiString (OdataValue);
> > -          if (OdataString != NULL) {
> > -            AsciiLocation = AllocateCopyPool (AsciiStrSize (OdataString),
> OdataString);
> > -            ASSERT (AsciiLocation != NULL);
> > -          }
> > +  //
> > +  // No header is returned. Search payload for location.
> > +  //
> > +  if ((*Location == NULL) && (Response->Payload != NULL)) {
> > +    JsonValue = RedfishJsonInPayload (Response->Payload);
> > +    if (JsonValue != NULL) {
> > +      OdataValue = JsonObjectGetValue (JsonValueGetObject (JsonValue),
> "@odata.id");
> > +      if (OdataValue != NULL) {
> > +        OdataString = (CHAR8 *)JsonValueGetAsciiString (OdataValue);
> > +        if (OdataString != NULL) {
> > +          AsciiLocation = AllocateCopyPool (AsciiStrSize (OdataString),
> OdataString);
> > +          ASSERT (AsciiLocation != NULL);
> >          }
> > -
> > -        JsonValueFree (JsonValue);
> >        }
> > -    }
> >
> > -    if (AsciiLocation != NULL) {
> > -      *Location = StrAsciiToUnicode (AsciiLocation);
> > -      FreePool (AsciiLocation);
> > -    } else {
> > -      Status = EFI_NOT_FOUND;
> > +      JsonValueFree (JsonValue);
> >      }
> >    }
> >
> > +  if (AsciiLocation != NULL) {
> > +    *Location = StrAsciiToUnicode (AsciiLocation);
> > +    FreePool (AsciiLocation);
> > +  } else {
> > +    Status = EFI_NOT_FOUND;
> > +    DEBUG ((DEBUG_ERROR, "%a: Failed to retrieve Location from HTTP
> response paylaod.\n", __func__));
> > +  }
> > +
> >    return Status;
> >  }
> >
> > +/**
> > +
> > +  This function returns a boolean of ETAG support on Redfish service side.
> > +
> > +  @retval     TRUE    ETAG is supported on Redfish service.
> > +  @retval     FALSE   ETAG is not supported on Redfish service.
> > +
> > +**/
> > +BOOLEAN
> > +CheckIsServerEtagSupported (
> > +  VOID
> > +  )
> > +{
> > +  return FixedPcdGetBool (PcdRedfishServiceEtagSupported);
> > +}
> > +
> >  /**
> >
> >    Create HTTP payload and send them to redfish service with PATCH method.
> > @@ -1861,7 +1914,7 @@ CreatePayloadToPatchResource (
> >  {
> >    REDFISH_PAYLOAD   Payload;
> >    EDKII_JSON_VALUE  ResourceJsonValue;
> > -  REDFISH_RESPONSE  PostResponse;
> > +  REDFISH_RESPONSE  PatchResponse;
> >    EFI_STATUS        Status;
> >
> >    if ((Service == NULL) || (TargetPayload == NULL) || IS_EMPTY_STRING
> (Json)) {
> > @@ -1876,8 +1929,8 @@ CreatePayloadToPatchResource (
> >      goto EXIT_FREE_JSON_VALUE;
> >    }
> >
> > -  ZeroMem (&PostResponse, sizeof (REDFISH_RESPONSE));
> > -  Status = RedfishPatchToPayload (TargetPayload, Payload, &PostResponse);
> > +  ZeroMem (&PatchResponse, sizeof (REDFISH_RESPONSE));
> > +  Status = RedfishPatchToPayload (TargetPayload, Payload,
> &PatchResponse);
> >    if (EFI_ERROR (Status)) {
> >      DEBUG ((DEBUG_ERROR, "%a:%d Failed to PATCH payload to Redfish
> service.\n", __func__, __LINE__));
> >
> > @@ -1885,7 +1938,7 @@ CreatePayloadToPatchResource (
> >      DEBUG ((DEBUG_ERROR, "%a: Request:\n", __func__));
> >      DumpRedfishPayload (DEBUG_ERROR, Payload);
> >      DEBUG ((DEBUG_ERROR, "%a: Response:\n", __func__));
> > -    DumpRedfishResponse (__func__, DEBUG_ERROR, &PostResponse);
> > +    DumpRedfishResponse (__func__, DEBUG_ERROR, &PatchResponse);
> >      DEBUG_CODE_END ();
> >      goto EXIT_FREE_JSON_VALUE;
> >    }
> > @@ -1893,16 +1946,20 @@ CreatePayloadToPatchResource (
> >    //
> >    // Find ETag
> >    //
> > -  Status = GetEtagAndLocation (&PostResponse, Etag, NULL);
> > -  if (EFI_ERROR (Status)) {
> > +  Status = GetHttpResponseEtag (&PatchResponse, Etag);
> > +  if (Status == EFI_UNSUPPORTED) {
> > +    Status = EFI_SUCCESS;
> > +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported on
> Redfish service.\n", __func__));
> > +  } else {
> >      Status = EFI_DEVICE_ERROR;
> > +    DEBUG ((DEBUG_ERROR, "%a: Fail to get Location header nor proerty
> from HTTP response payload.\n", __func__));
> >    }
> >
> >    RedfishFreeResponse (
> > -    PostResponse.StatusCode,
> > -    PostResponse.HeaderCount,
> > -    PostResponse.Headers,
> > -    PostResponse.Payload
> > +    PatchResponse.StatusCode,
> > +    PatchResponse.HeaderCount,
> > +    PatchResponse.Headers,
> > +    PatchResponse.Payload
> >      );
> >
> >  EXIT_FREE_JSON_VALUE:
> > @@ -1935,7 +1992,7 @@ CreatePayloadToPostResource (
> >    IN  REDFISH_PAYLOAD  *TargetPayload,
> >    IN  CHAR8            *Json,
> >    OUT EFI_STRING       *Location,
> > -  OUT CHAR8            **Etag
> > +  OUT CHAR8            **Etag OPTIONAL
> >    )
> >  {
> >    REDFISH_PAYLOAD   Payload;
> > @@ -1943,7 +2000,7 @@ CreatePayloadToPostResource (
> >    REDFISH_RESPONSE  PostResponse;
> >    EFI_STATUS        Status;
> >
> > -  if ((Service == NULL) || (TargetPayload == NULL) || IS_EMPTY_STRING
> (Json) || (Location == NULL) || (Etag == NULL)) {
> > +  if ((Service == NULL) || (TargetPayload == NULL) || IS_EMPTY_STRING
> (Json) || (Location == NULL)) {
> >      return EFI_INVALID_PARAMETER;
> >    }
> >
> > @@ -1970,12 +2027,22 @@ CreatePayloadToPostResource (
> >      goto EXIT_FREE_JSON_VALUE;
> >    }
> >
> > +  Status = GetHttpResponseEtag (&PostResponse, Etag);
> > +  if (Status == EFI_UNSUPPORTED) {
> > +    Status = EFI_SUCCESS;
> > +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported on
> Redfish service.\n", __func__));
> > +  } else if (EFI_ERROR (Status)) {
> > +    Status = EFI_DEVICE_ERROR;
> > +    DEBUG ((DEBUG_ERROR, "%a: Fail to get ETAG header nor ETAG
> propertyfrom HTTP response payload.\n", __func__));
> > +  }
> > +
> >    //
> >    // per Redfish spec. the URL of new resource will be returned in "Location"
> header.
> >    //
> > -  Status = GetEtagAndLocation (&PostResponse, Etag, Location);
> > +  Status = GetHttpResponseLocation (&PostResponse, Location);
> >    if (EFI_ERROR (Status)) {
> >      Status = EFI_DEVICE_ERROR;
> > +    DEBUG ((DEBUG_ERROR, "%a: Fail to get Location header nor proerty
> from HTTP response payload.\n", __func__));
> >    }
> >
> >    RedfishFreeResponse (
> > @@ -3117,6 +3184,11 @@ CheckEtag (
> >  {
> >    CHAR8  *EtagInDb;
> >
> > +  if (!CheckIsServerEtagSupported ()) {
> > +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported, always
> returns FALSE to consume resource (Performance would be reduced).\n",
> __func__));
> > +    return FALSE;
> > +  }
> > +
> >    if (IS_EMPTY_STRING (Uri)) {
> >      return FALSE;
> >    }
> > --
> > 2.37.1.windows.1
> >
> >
> >
> > 
> >
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113232): https://edk2.groups.io/g/devel/message/113232
Mute This Topic: https://groups.io/mt/103519287/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

* Re: [edk2-devel] [edk2-redfish-client][PATCH V2] RedfishClientPkg: Add ETag PCD and revise Redfish ETag functions
  2024-01-05  9:03   ` Chang, Abner via groups.io
@ 2024-01-05 11:33     ` Mike Maslenkin
  0 siblings, 0 replies; 4+ messages in thread
From: Mike Maslenkin @ 2024-01-05 11:33 UTC (permalink / raw)
  To: Chang, Abner; +Cc: devel@edk2.groups.io, Nickle Wang, Igor Kulchytskyy

Hi Abner,

Thank you for the clarification.

Regards,
Mike.

On Fri, Jan 5, 2024 at 12:03 PM Chang, Abner <Abner.Chang@amd.com> wrote:
>
> [AMD Official Use Only - General]
>
> Hi Mike,
> This PCD is introduced for the platform that connects to the Redfish service which doesn't support ETag.
> We disable the client code that handles ETag with setting this PCD to FALSE. So client will just consume any Redfish property from service even there is nothing changed. This knob doesn't control Redfish service behavior on BMC.
> Yes, there is no method to detect if Redfish service supports ETag or not. So we introduce a client side knob to disable ETag checking although it mentions the service "should" support ETag in Redfish spec. However, a simple Redfish services may not implementing ETag HTTP header.
> Does above clarify the question?
>
> Thanks
> Abner
>
> > -----Original Message-----
> > From: Mike Maslenkin <mike.maslenkin@gmail.com>
> > Sent: Friday, January 5, 2024 8:20 AM
> > To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>
> > Cc: Nickle Wang <nicklew@nvidia.com>; Igor Kulchytskyy <igork@ami.com>
> > Subject: Re: [edk2-devel] [edk2-redfish-client][PATCH V2] RedfishClientPkg:
> > Add ETag PCD and revise Redfish ETag functions
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > Looks good to me.
> >
> > But could it be possible to rephrase "ETAG is not supported on Redfish
> > service." ?
> > May be I misunderstand, but I assume "Redfish service" is a service at
> > BMC side, while we are disabling ETAG functionality at Redfish client
> > side.
> > README.md says "Redfish service hosted by Board Management Controller
> > (BMC) in server".
> > Currently there is no method to get server features (AFAIR), so we
> > disable a work with those explicitly on the client side.
> >
> > Regards,
> > Mike.
> >
> > On Thu, Jan 4, 2024 at 11:57 AM Chang, Abner via groups.io
> > <abner.chang=amd.com@groups.io> wrote:
> > >
> > > From: Abner Chang <abner.chang@amd.com>
> > >
> > > Add PCD to disable ETag capability for the case Redfish
> > > service doesn't support ETag.
> > >
> > > Signed-off-by: Abner Chang <abner.chang@amd.com>
> > > Cc: Nickle Wang <nicklew@nvidia.com>
> > > Cc: Igor Kulchytskyy <igork@ami.com>
> > > Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
> > > ---
> > >  RedfishClientPkg/RedfishClientPkg.dec         |   2 +
> > >  .../RedfishFeatureUtilityLib.inf              |   1 +
> > >  .../Library/RedfishFeatureUtilityLib.h        |  46 +++-
> > >  .../Features/Bios/v1_0_9/Dxe/BiosDxe.c        |  18 +-
> > >  .../v1_0_4/Common/BootOptionCommon.c          |   4 +-
> > >  .../BootOption/v1_0_4/Dxe/BootOptionDxe.c     |  16 +-
> > >  .../v1_5_0/Dxe/ComputerSystemDxe.c            |  16 +-
> > >  .../Features/Memory/V1_7_1/Dxe/MemoryDxe.c    |  16 +-
> > >  .../RedfishFeatureUtilityLib.c                | 208 ++++++++++++------
> > >  9 files changed, 197 insertions(+), 130 deletions(-)
> > >
> > > diff --git a/RedfishClientPkg/RedfishClientPkg.dec
> > b/RedfishClientPkg/RedfishClientPkg.dec
> > > index b350facae0..8adef327fb 100644
> > > --- a/RedfishClientPkg/RedfishClientPkg.dec
> > > +++ b/RedfishClientPkg/RedfishClientPkg.dec
> > > @@ -76,6 +76,8 @@
> > >
> > gEfiRedfishClientPkgTokenSpaceGuid.PcdDefaultRedfishVersion|L"v1"|VOID*
> > |0x10000004
> > >    ## The number of seconds that the firmware will wait before system reboot
> > >
> > gEfiRedfishClientPkgTokenSpaceGuid.PcdRedfishSystemRebootTimeout|5|UI
> > NT16|0x20000002
> > > +  ## Default capability of Redfish service side ETAG support
> > > +
> > gEfiRedfishClientPkgTokenSpaceGuid.PcdRedfishServiceEtagSupported|TRUE|
> > BOOLEAN|0x10000005
> > >
> > >  [PcdsDynamicEx]
> > >    ## The flag used to indicate that system reboot is required due to system
> > configuration change
> > > diff --git
> > a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.i
> > nf
> > b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.i
> > nf
> > > index 718273b248..fde6a176d0 100644
> > > ---
> > a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.i
> > nf
> > > +++
> > b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.i
> > nf
> > > @@ -54,6 +54,7 @@
> > >
> > >  [Pcd]
> > >    gEfiRedfishClientPkgTokenSpaceGuid.PcdRedfishSystemRebootRequired
> > > +  gEfiRedfishClientPkgTokenSpaceGuid.PcdRedfishServiceEtagSupported
> > >
> > >  [Guids]
> > >
> > > diff --git a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > > index 9513a65617..834ea0fcfe 100644
> > > --- a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > > +++ b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > > @@ -81,7 +81,7 @@ CopyConfiglanguageList (
> > >
> > >  /**
> > >
> > > -  Get number of node from the string. Node is seperated by '/'.
> > > +  Get number of node from the string. Node is separated by '/'.
> > >
> > >    @param[in]  NodeString             The node string to parse.
> > >
> > > @@ -559,6 +559,19 @@ GetEtagWithUri (
> > >    IN  EFI_STRING  Uri
> > >    );
> > >
> > > +/**
> > > +
> > > +  This function returns a boolean of ETAG support on Redfish service side.
> > > +
> > > +  @retval     TRUE    ETAG is supported on Redfish service.
> > > +  @retval     FALSE   ETAG is not supported on Redfish service.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +CheckIsServerEtagSupported (
> > > +  VOID
> > > +  );
> > > +
> > >  /**
> > >
> > >    Get @odata.id from give HTTP payload. It's call responsibility to release
> > returned buffer.
> > > @@ -931,22 +944,33 @@ CompareRedfishPropertyVagueValues (
> > >    );
> > >
> > >  /**
> > > +  Find "ETag" from either HTTP header or Redfish response.
> > >
> > > -  Find "ETag" and "Location" from either HTTP header or Redfish response.
> > > +  @param[in]  Response        HTTP response
> > > +  @param[out] Etag            String buffer to return ETag
> > >
> > > -  @param[in]  Response    HTTP response
> > > -  @param[out] Etag        String buffer to return ETag
> > > -  @param[out] Location    String buffer to return Location
> > > +  @retval  EFI_SUCCESS            ETag is returned in Etag
> > > +  @retval  EFI_UNSUPPORTED        ETag is unsupported
> > > +  @retval  EFI_INVALID_PARAMETER  Response, Etag or both are NULL.
> > >
> > > -  @retval     EFI_SUCCESS     Data is found and returned.
> > > -  @retval     Others          Errors occur.
> > > +**/
> > > +EFI_STATUS
> > > +GetHttpResponseEtag (
> > > +  IN  REDFISH_RESPONSE  *Response,
> > > +  OUT CHAR8             **Etag
> > > +  );
> > > +
> > > +/**
> > > +  Find "Location" from either HTTP header or Redfish response.
> > > +
> > > +  @param[in]  Response        HTTP response
> > > +  @param[out] Location        String buffer to return Location
> > >
> > >  **/
> > >  EFI_STATUS
> > > -GetEtagAndLocation (
> > > -  IN  REDFISH_RESPONSE *Response,
> > > -  OUT CHAR8 **Etag, OPTIONAL
> > > -  OUT EFI_STRING        *Location    OPTIONAL
> > > +GetHttpResponseLocation (
> > > +  IN  REDFISH_RESPONSE  *Response,
> > > +  OUT EFI_STRING        *Location
> > >    );
> > >
> > >  /**
> > > diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> > b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> > > index 2a49c5cd22..14e854c861 100644
> > > --- a/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> > > +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> > > @@ -153,14 +153,10 @@ RedfishResourceConsumeResource (
> > >    ASSERT (Private->Json != NULL);
> > >
> > >    //
> > > -  // Find etag in HTTP response header
> > > +  // Searching for etag in HTTP response header
> > >    //
> > > -  Etag   = NULL;
> > > -  Status = GetEtagAndLocation (ExpectedResponse, &Etag, NULL);
> > > -  if (EFI_ERROR (Status)) {
> > > -    DEBUG ((DEBUG_ERROR, "%a, failed to get ETag from HTTP header\n",
> > __func__));
> > > -  }
> > > -
> > > +  Etag = NULL;
> > > +  GetHttpResponseEtag (ExpectedResponse, &Etag);
> > >    Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
> > >    if (EFI_ERROR (Status)) {
> > >      if (Status != EFI_ALREADY_STARTED) {
> > > @@ -365,12 +361,8 @@ RedfishResourceCheck (
> > >    //
> > >    // Find etag in HTTP response header
> > >    //
> > > -  Etag   = NULL;
> > > -  Status = GetEtagAndLocation (&Response, &Etag, NULL);
> > > -  if (EFI_ERROR (Status)) {
> > > -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n",
> > __func__));
> > > -  }
> > > -
> > > +  Etag = NULL;
> > > +  GetHttpResponseEtag (&Response, &Etag);
> > >    Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
> > >    if (EFI_ERROR (Status)) {
> > >      DEBUG ((DEBUG_ERROR, "%a, failed to check resource from: %s: %r\n",
> > __func__, Uri, Status));
> > > diff --git
> > a/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCom
> > mon.c
> > b/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCom
> > mon.c
> > > index 0d4c2162c6..0b9a72abc3 100644
> > > ---
> > a/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCom
> > mon.c
> > > +++
> > b/RedfishClientPkg/Features/BootOption/v1_0_4/Common/BootOptionCom
> > mon.c
> > > @@ -455,9 +455,9 @@ RedfishProvisioningResourceCommon (
> > >    }
> > >
> > >    //
> > > -  // per Redfish spec. the URL of new resource will be returned in "Location"
> > header.
> > > +  // Per Redfish spec. the URL of new resource will be returned in "Location"
> > header.
> > >    //
> > > -  Status = GetEtagAndLocation (&Response, NULL, &NewResourceLocation);
> > > +  Status = GetHttpResponseLocation (&Response, &NewResourceLocation);
> > >    if (EFI_ERROR (Status)) {
> > >      DEBUG ((DEBUG_ERROR, "%a: cannot find new location: %r\n", __func__,
> > Status));
> > >      goto RELEASE_RESOURCE;
> > > diff --git
> > a/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
> > b/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
> > > index ba090c51d3..204c6b0757 100644
> > > --- a/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
> > > +++
> > b/RedfishClientPkg/Features/BootOption/v1_0_4/Dxe/BootOptionDxe.c
> > > @@ -154,12 +154,8 @@ RedfishResourceConsumeResource (
> > >    //
> > >    // Find etag in HTTP response header
> > >    //
> > > -  Etag   = NULL;
> > > -  Status = GetEtagAndLocation (ExpectedResponse, &Etag, NULL);
> > > -  if (EFI_ERROR (Status)) {
> > > -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n",
> > __func__));
> > > -  }
> > > -
> > > +  Etag = NULL;
> > > +  GetHttpResponseEtag (ExpectedResponse, &Etag);
> > >    Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
> > >    if (EFI_ERROR (Status)) {
> > >      DEBUG ((DEBUG_ERROR, "%a: failed to consume resource from: %s:
> > %r\n", __func__, Private->Uri, Status));
> > > @@ -358,12 +354,8 @@ RedfishResourceCheck (
> > >    //
> > >    // Find etag in HTTP response header
> > >    //
> > > -  Etag   = NULL;
> > > -  Status = GetEtagAndLocation (&Response, &Etag, NULL);
> > > -  if (EFI_ERROR (Status)) {
> > > -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n",
> > __func__));
> > > -  }
> > > -
> > > +  Etag = NULL;
> > > +  GetHttpResponseEtag (&Response, &Etag);
> > >    Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
> > >    if (EFI_ERROR (Status)) {
> > >      DEBUG ((REDFISH_BOOT_OPTION_DEBUG_TRACE, "%a: failed to check
> > resource from: %s: %r\n", __func__, Uri, Status));
> > > diff --git
> > a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Dxe/ComputerSyste
> > mDxe.c
> > b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Dxe/ComputerSyste
> > mDxe.c
> > > index 0bbaa92bf4..7805bcf36b 100644
> > > ---
> > a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Dxe/ComputerSyste
> > mDxe.c
> > > +++
> > b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Dxe/ComputerSyste
> > mDxe.c
> > > @@ -149,12 +149,8 @@ RedfishResourceConsumeResource (
> > >    //
> > >    // Find etag in HTTP response header
> > >    //
> > > -  Etag   = NULL;
> > > -  Status = GetEtagAndLocation (ExpectedResponse, &Etag, NULL);
> > > -  if (EFI_ERROR (Status)) {
> > > -    DEBUG ((DEBUG_ERROR, "%a, failed to get ETag from HTTP header\n",
> > __func__));
> > > -  }
> > > -
> > > +  Etag = NULL;
> > > +  GetHttpResponseEtag (ExpectedResponse, &Etag);
> > >    Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
> > >    if (EFI_ERROR (Status)) {
> > >      if (Status != EFI_ALREADY_STARTED) {
> > > @@ -359,12 +355,8 @@ RedfishResourceCheck (
> > >    //
> > >    // Find etag in HTTP response header
> > >    //
> > > -  Etag   = NULL;
> > > -  Status = GetEtagAndLocation (&Response, &Etag, NULL);
> > > -  if (EFI_ERROR (Status)) {
> > > -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n",
> > __func__));
> > > -  }
> > > -
> > > +  Etag = NULL;
> > > +  GetHttpResponseEtag (&Response, &Etag);
> > >    Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
> > >    if (EFI_ERROR (Status)) {
> > >      DEBUG ((DEBUG_ERROR, "%a, failed to check resource from: %s: %r\n",
> > __func__, Uri, Status));
> > > diff --git a/RedfishClientPkg/Features/Memory/V1_7_1/Dxe/MemoryDxe.c
> > b/RedfishClientPkg/Features/Memory/V1_7_1/Dxe/MemoryDxe.c
> > > index 9230078051..b87da775b3 100644
> > > --- a/RedfishClientPkg/Features/Memory/V1_7_1/Dxe/MemoryDxe.c
> > > +++ b/RedfishClientPkg/Features/Memory/V1_7_1/Dxe/MemoryDxe.c
> > > @@ -149,12 +149,8 @@ RedfishResourceConsumeResource (
> > >    //
> > >    // Find etag in HTTP response header
> > >    //
> > > -  Etag   = NULL;
> > > -  Status = GetEtagAndLocation (ExpectedResponse, &Etag, NULL);
> > > -  if (EFI_ERROR (Status)) {
> > > -    DEBUG ((DEBUG_ERROR, "%a, failed to get ETag from HTTP header\n",
> > __func__));
> > > -  }
> > > -
> > > +  Etag = NULL;
> > > +  GetHttpResponseEtag (ExpectedResponse, &Etag);
> > >    Status = RedfishConsumeResourceCommon (Private, Private->Json, Etag);
> > >    if (EFI_ERROR (Status)) {
> > >      if (Status != EFI_ALREADY_STARTED) {
> > > @@ -359,12 +355,8 @@ RedfishResourceCheck (
> > >    //
> > >    // Find etag in HTTP response header
> > >    //
> > > -  Etag   = NULL;
> > > -  Status = GetEtagAndLocation (&Response, &Etag, NULL);
> > > -  if (EFI_ERROR (Status)) {
> > > -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n",
> > __func__));
> > > -  }
> > > -
> > > +  Etag = NULL;
> > > +  GetHttpResponseEtag (&Response, &Etag);
> > >    Status = RedfishCheckResourceCommon (Private, Private->Json, Etag);
> > >    if (EFI_ERROR (Status)) {
> > >      DEBUG ((DEBUG_ERROR, "%a, failed to check resource from: %s: %r\n",
> > __func__, Uri, Status));
> > > diff --git
> > a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > c
> > b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > c
> > > index 1c2d40f622..7709a9d1a2 100644
> > > ---
> > a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > c
> > > +++
> > b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > c
> > > @@ -133,6 +133,11 @@ SetEtagFromUri (
> > >    REDFISH_RESPONSE  Response;
> > >    EFI_STRING        PendingSettingUri;
> > >
> > > +  if (!CheckIsServerEtagSupported ()) {
> > > +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported\n",
> > __func__));
> > > +    return EFI_SUCCESS;
> > > +  }
> > > +
> > >    if ((RedfishService == NULL) || IS_EMPTY_STRING (Uri)) {
> > >      return EFI_INVALID_PARAMETER;
> > >    }
> > > @@ -157,9 +162,9 @@ SetEtagFromUri (
> > >    //
> > >    // Find etag in HTTP response header
> > >    //
> > > -  Status = GetEtagAndLocation (&Response, &Etag, NULL);
> > > -  if (EFI_ERROR (Status)) {
> > > -    DEBUG ((DEBUG_ERROR, "%a: failed to get ETag from HTTP header\n",
> > __func__));
> > > +  Status = GetHttpResponseEtag (&Response, &Etag);
> > > +  if (EFI_ERROR (Status) && (Status != EFI_UNSUPPORTED)) {
> > > +    DEBUG ((DEBUG_ERROR, "%a: Failed to get ETag from HTTP header\n",
> > __func__));
> > >      Status = EFI_NOT_FOUND;
> > >      goto ON_RELEASE;
> > >    }
> > > @@ -241,6 +246,11 @@ SetEtagWithUri (
> > >    EFI_STATUS  Status;
> > >    CHAR8       *AsciiUri;
> > >
> > > +  if (!CheckIsServerEtagSupported ()) {
> > > +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported\n",
> > __func__));
> > > +    return EFI_SUCCESS;
> > > +  }
> > > +
> > >    if (IS_EMPTY_STRING (EtagStr) || IS_EMPTY_STRING (Uri)) {
> > >      return EFI_INVALID_PARAMETER;
> > >    }
> > > @@ -286,6 +296,11 @@ GetEtagWithUri (
> > >      return NULL;
> > >    }
> > >
> > > +  if (!CheckIsServerEtagSupported ()) {
> > > +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported\n",
> > __func__));
> > > +    return NULL;
> > > +  }
> > > +
> > >    Status = RedfishLocateProtocol ((VOID **)&mEtagProtocol,
> > &gEdkIIRedfishETagProtocolGuid);
> > >    if (EFI_ERROR (Status)) {
> > >      DEBUG ((DEBUG_ERROR, "%a: fail to locate
> > gEdkIIRedfishETagProtocolGuid: %r\n", __func__, Status));
> > > @@ -1726,44 +1741,39 @@
> > RedfishFeatureGetUnifiedArrayTypeConfigureLang (
> > >  }
> > >
> > >  /**
> > > +  Find "ETag" from either HTTP header or Redfish response.
> > >
> > > -  Find "ETag" and "Location" from either HTTP header or Redfish response.
> > > +  @param[in]  Response        HTTP response
> > > +  @param[out] Etag            String buffer to return ETag
> > >
> > > -  @param[in]  Response    HTTP response
> > > -  @param[out] Etag        String buffer to return ETag
> > > -  @param[out] Location    String buffer to return Location
> > > -
> > > -  @retval     EFI_SUCCESS     Data is found and returned.
> > > -  @retval     Others          Errors occur.
> > > +  @retval  EFI_SUCCESS            ETag is returned in Etag
> > > +  @retval  EFI_UNSUPPORTED        ETag is unsupported
> > > +  @retval  EFI_INVALID_PARAMETER  Response, Etag or both are NULL.
> > >
> > >  **/
> > >  EFI_STATUS
> > > -GetEtagAndLocation (
> > > -  IN  REDFISH_RESPONSE *Response,
> > > -  OUT CHAR8 **Etag, OPTIONAL
> > > -  OUT EFI_STRING        *Location    OPTIONAL
> > > +GetHttpResponseEtag (
> > > +  IN  REDFISH_RESPONSE  *Response,
> > > +  OUT CHAR8             **Etag
> > >    )
> > >  {
> > > +  EFI_STATUS        Status;
> > >    EDKII_JSON_VALUE  JsonValue;
> > >    EDKII_JSON_VALUE  OdataValue;
> > >    CHAR8             *OdataString;
> > > -  CHAR8             *AsciiLocation;
> > >    EFI_HTTP_HEADER   *Header;
> > > -  EFI_STATUS        Status;
> > >
> > > -  if (Response == NULL) {
> > > +  if ((Response == NULL) || (Etag == NULL)) {
> > >      return EFI_INVALID_PARAMETER;
> > >    }
> > >
> > > -  if ((Etag == NULL) && (Location == NULL)) {
> > > -    return EFI_SUCCESS;
> > > -  }
> > > -
> > >    Status = EFI_SUCCESS;
> > > -
> > > -  if (Etag != NULL) {
> > > -    *Etag = NULL;
> > > -
> > > +  *Etag  = NULL;
> > > +  if (!CheckIsServerEtagSupported ()) {
> > > +    // Don't look for ETAG header or property in this case.
> > > +    DEBUG ((DEBUG_INFO, "%a: WARNING - No ETag support on Redfish
> > service.\n", __func__));
> > > +    return EFI_UNSUPPORTED;
> > > +  } else {
> > >      if (*(Response->StatusCode) == HTTP_STATUS_200_OK) {
> > >        Header = HttpFindHeader (Response->HeaderCount, Response-
> > >Headers, HTTP_HEADER_ETAG);
> > >        if (Header != NULL) {
> > > @@ -1793,51 +1803,94 @@ GetEtagAndLocation (
> > >
> > >      if (*Etag == NULL) {
> > >        Status = EFI_NOT_FOUND;
> > > +      DEBUG ((DEBUG_ERROR, "%a: Failed to retrieve ETag from HTTP
> > response paylaod.\n", __func__));
> > >      }
> > >    }
> > >
> > > -  if (Location != NULL) {
> > > -    *Location     = NULL;
> > > -    AsciiLocation = NULL;
> > > +  return Status;
> > > +}
> > >
> > > -    if (*(Response->StatusCode) == HTTP_STATUS_200_OK) {
> > > -      Header = HttpFindHeader (Response->HeaderCount, Response-
> > >Headers, HTTP_HEADER_LOCATION);
> > > -      if (Header != NULL) {
> > > -        AsciiLocation = AllocateCopyPool (AsciiStrSize (Header->FieldValue),
> > Header->FieldValue);
> > > -        ASSERT (AsciiLocation != NULL);
> > > -      }
> > > +/**
> > > +  Find "Location" from either HTTP header or Redfish response.
> > > +
> > > +  @param[in]  Response        HTTP response
> > > +  @param[out] Location        String buffer to return Location
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +GetHttpResponseLocation (
> > > +  IN  REDFISH_RESPONSE  *Response,
> > > +  OUT EFI_STRING        *Location
> > > +  )
> > > +{
> > > +  EFI_STATUS        Status;
> > > +  EDKII_JSON_VALUE  JsonValue;
> > > +  EDKII_JSON_VALUE  OdataValue;
> > > +  CHAR8             *OdataString;
> > > +  CHAR8             *AsciiLocation;
> > > +  EFI_HTTP_HEADER   *Header;
> > > +
> > > +  if ((Response == NULL) || (Location == NULL)) {
> > > +    return EFI_INVALID_PARAMETER;
> > > +  }
> > > +
> > > +  Status        = EFI_SUCCESS;
> > > +  *Location     = NULL;
> > > +  AsciiLocation = NULL;
> > > +  if (*(Response->StatusCode) == HTTP_STATUS_200_OK) {
> > > +    Header = HttpFindHeader (Response->HeaderCount, Response->Headers,
> > HTTP_HEADER_LOCATION);
> > > +    if (Header != NULL) {
> > > +      AsciiLocation = AllocateCopyPool (AsciiStrSize (Header->FieldValue),
> > Header->FieldValue);
> > > +      ASSERT (AsciiLocation != NULL);
> > >      }
> > > +  }
> > >
> > > -    //
> > > -    // No header is returned. Search payload for location.
> > > -    //
> > > -    if ((*Location == NULL) && (Response->Payload != NULL)) {
> > > -      JsonValue = RedfishJsonInPayload (Response->Payload);
> > > -      if (JsonValue != NULL) {
> > > -        OdataValue = JsonObjectGetValue (JsonValueGetObject (JsonValue),
> > "@odata.id");
> > > -        if (OdataValue != NULL) {
> > > -          OdataString = (CHAR8 *)JsonValueGetAsciiString (OdataValue);
> > > -          if (OdataString != NULL) {
> > > -            AsciiLocation = AllocateCopyPool (AsciiStrSize (OdataString),
> > OdataString);
> > > -            ASSERT (AsciiLocation != NULL);
> > > -          }
> > > +  //
> > > +  // No header is returned. Search payload for location.
> > > +  //
> > > +  if ((*Location == NULL) && (Response->Payload != NULL)) {
> > > +    JsonValue = RedfishJsonInPayload (Response->Payload);
> > > +    if (JsonValue != NULL) {
> > > +      OdataValue = JsonObjectGetValue (JsonValueGetObject (JsonValue),
> > "@odata.id");
> > > +      if (OdataValue != NULL) {
> > > +        OdataString = (CHAR8 *)JsonValueGetAsciiString (OdataValue);
> > > +        if (OdataString != NULL) {
> > > +          AsciiLocation = AllocateCopyPool (AsciiStrSize (OdataString),
> > OdataString);
> > > +          ASSERT (AsciiLocation != NULL);
> > >          }
> > > -
> > > -        JsonValueFree (JsonValue);
> > >        }
> > > -    }
> > >
> > > -    if (AsciiLocation != NULL) {
> > > -      *Location = StrAsciiToUnicode (AsciiLocation);
> > > -      FreePool (AsciiLocation);
> > > -    } else {
> > > -      Status = EFI_NOT_FOUND;
> > > +      JsonValueFree (JsonValue);
> > >      }
> > >    }
> > >
> > > +  if (AsciiLocation != NULL) {
> > > +    *Location = StrAsciiToUnicode (AsciiLocation);
> > > +    FreePool (AsciiLocation);
> > > +  } else {
> > > +    Status = EFI_NOT_FOUND;
> > > +    DEBUG ((DEBUG_ERROR, "%a: Failed to retrieve Location from HTTP
> > response paylaod.\n", __func__));
> > > +  }
> > > +
> > >    return Status;
> > >  }
> > >
> > > +/**
> > > +
> > > +  This function returns a boolean of ETAG support on Redfish service side.
> > > +
> > > +  @retval     TRUE    ETAG is supported on Redfish service.
> > > +  @retval     FALSE   ETAG is not supported on Redfish service.
> > > +
> > > +**/
> > > +BOOLEAN
> > > +CheckIsServerEtagSupported (
> > > +  VOID
> > > +  )
> > > +{
> > > +  return FixedPcdGetBool (PcdRedfishServiceEtagSupported);
> > > +}
> > > +
> > >  /**
> > >
> > >    Create HTTP payload and send them to redfish service with PATCH method.
> > > @@ -1861,7 +1914,7 @@ CreatePayloadToPatchResource (
> > >  {
> > >    REDFISH_PAYLOAD   Payload;
> > >    EDKII_JSON_VALUE  ResourceJsonValue;
> > > -  REDFISH_RESPONSE  PostResponse;
> > > +  REDFISH_RESPONSE  PatchResponse;
> > >    EFI_STATUS        Status;
> > >
> > >    if ((Service == NULL) || (TargetPayload == NULL) || IS_EMPTY_STRING
> > (Json)) {
> > > @@ -1876,8 +1929,8 @@ CreatePayloadToPatchResource (
> > >      goto EXIT_FREE_JSON_VALUE;
> > >    }
> > >
> > > -  ZeroMem (&PostResponse, sizeof (REDFISH_RESPONSE));
> > > -  Status = RedfishPatchToPayload (TargetPayload, Payload, &PostResponse);
> > > +  ZeroMem (&PatchResponse, sizeof (REDFISH_RESPONSE));
> > > +  Status = RedfishPatchToPayload (TargetPayload, Payload,
> > &PatchResponse);
> > >    if (EFI_ERROR (Status)) {
> > >      DEBUG ((DEBUG_ERROR, "%a:%d Failed to PATCH payload to Redfish
> > service.\n", __func__, __LINE__));
> > >
> > > @@ -1885,7 +1938,7 @@ CreatePayloadToPatchResource (
> > >      DEBUG ((DEBUG_ERROR, "%a: Request:\n", __func__));
> > >      DumpRedfishPayload (DEBUG_ERROR, Payload);
> > >      DEBUG ((DEBUG_ERROR, "%a: Response:\n", __func__));
> > > -    DumpRedfishResponse (__func__, DEBUG_ERROR, &PostResponse);
> > > +    DumpRedfishResponse (__func__, DEBUG_ERROR, &PatchResponse);
> > >      DEBUG_CODE_END ();
> > >      goto EXIT_FREE_JSON_VALUE;
> > >    }
> > > @@ -1893,16 +1946,20 @@ CreatePayloadToPatchResource (
> > >    //
> > >    // Find ETag
> > >    //
> > > -  Status = GetEtagAndLocation (&PostResponse, Etag, NULL);
> > > -  if (EFI_ERROR (Status)) {
> > > +  Status = GetHttpResponseEtag (&PatchResponse, Etag);
> > > +  if (Status == EFI_UNSUPPORTED) {
> > > +    Status = EFI_SUCCESS;
> > > +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported on
> > Redfish service.\n", __func__));
> > > +  } else {
> > >      Status = EFI_DEVICE_ERROR;
> > > +    DEBUG ((DEBUG_ERROR, "%a: Fail to get Location header nor proerty
> > from HTTP response payload.\n", __func__));
> > >    }
> > >
> > >    RedfishFreeResponse (
> > > -    PostResponse.StatusCode,
> > > -    PostResponse.HeaderCount,
> > > -    PostResponse.Headers,
> > > -    PostResponse.Payload
> > > +    PatchResponse.StatusCode,
> > > +    PatchResponse.HeaderCount,
> > > +    PatchResponse.Headers,
> > > +    PatchResponse.Payload
> > >      );
> > >
> > >  EXIT_FREE_JSON_VALUE:
> > > @@ -1935,7 +1992,7 @@ CreatePayloadToPostResource (
> > >    IN  REDFISH_PAYLOAD  *TargetPayload,
> > >    IN  CHAR8            *Json,
> > >    OUT EFI_STRING       *Location,
> > > -  OUT CHAR8            **Etag
> > > +  OUT CHAR8            **Etag OPTIONAL
> > >    )
> > >  {
> > >    REDFISH_PAYLOAD   Payload;
> > > @@ -1943,7 +2000,7 @@ CreatePayloadToPostResource (
> > >    REDFISH_RESPONSE  PostResponse;
> > >    EFI_STATUS        Status;
> > >
> > > -  if ((Service == NULL) || (TargetPayload == NULL) || IS_EMPTY_STRING
> > (Json) || (Location == NULL) || (Etag == NULL)) {
> > > +  if ((Service == NULL) || (TargetPayload == NULL) || IS_EMPTY_STRING
> > (Json) || (Location == NULL)) {
> > >      return EFI_INVALID_PARAMETER;
> > >    }
> > >
> > > @@ -1970,12 +2027,22 @@ CreatePayloadToPostResource (
> > >      goto EXIT_FREE_JSON_VALUE;
> > >    }
> > >
> > > +  Status = GetHttpResponseEtag (&PostResponse, Etag);
> > > +  if (Status == EFI_UNSUPPORTED) {
> > > +    Status = EFI_SUCCESS;
> > > +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported on
> > Redfish service.\n", __func__));
> > > +  } else if (EFI_ERROR (Status)) {
> > > +    Status = EFI_DEVICE_ERROR;
> > > +    DEBUG ((DEBUG_ERROR, "%a: Fail to get ETAG header nor ETAG
> > propertyfrom HTTP response payload.\n", __func__));
> > > +  }
> > > +
> > >    //
> > >    // per Redfish spec. the URL of new resource will be returned in "Location"
> > header.
> > >    //
> > > -  Status = GetEtagAndLocation (&PostResponse, Etag, Location);
> > > +  Status = GetHttpResponseLocation (&PostResponse, Location);
> > >    if (EFI_ERROR (Status)) {
> > >      Status = EFI_DEVICE_ERROR;
> > > +    DEBUG ((DEBUG_ERROR, "%a: Fail to get Location header nor proerty
> > from HTTP response payload.\n", __func__));
> > >    }
> > >
> > >    RedfishFreeResponse (
> > > @@ -3117,6 +3184,11 @@ CheckEtag (
> > >  {
> > >    CHAR8  *EtagInDb;
> > >
> > > +  if (!CheckIsServerEtagSupported ()) {
> > > +    DEBUG ((DEBUG_INFO, "%a: WARNING - ETAG is not supported, always
> > returns FALSE to consume resource (Performance would be reduced).\n",
> > __func__));
> > > +    return FALSE;
> > > +  }
> > > +
> > >    if (IS_EMPTY_STRING (Uri)) {
> > >      return FALSE;
> > >    }
> > > --
> > > 2.37.1.windows.1
> > >
> > >
> > >
> > > 
> > >
> > >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113275): https://edk2.groups.io/g/devel/message/113275
Mute This Topic: https://groups.io/mt/103519287/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

end of thread, other threads:[~2024-01-05 11:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-04  8:56 [edk2-devel] [edk2-redfish-client][PATCH V2] RedfishClientPkg: Add ETag PCD and revise Redfish ETag functions Chang, Abner via groups.io
2024-01-05  0:20 ` Mike Maslenkin
2024-01-05  9:03   ` Chang, Abner via groups.io
2024-01-05 11:33     ` Mike Maslenkin

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