public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [edk2-redfish-client][PATCH 3/6] RedfishClientPkg: feature driver enhancement.
@ 2023-11-23 14:34 Nickle Wang via groups.io
  2023-11-28  4:04 ` Chang, Abner via groups.io
  0 siblings, 1 reply; 4+ messages in thread
From: Nickle Wang via groups.io @ 2023-11-23 14:34 UTC (permalink / raw)
  To: devel; +Cc: Abner Chang, Igor Kulchytskyy, Nick Ramirez

-Fix typo (include function RedfisSetRedfishUri and parameter
MaxLengthConfigLang)
-Add more debug message.
-Do not save ETag to variable every time when new ETag is created
in SetEtagWithUri().
-Introduce SetEtagFromUri() to get ETag from BMC and save it
in variable.
-Introduce GetPendingSettings() to check and see if Redfish pending
settings is supported or not.

Signed-off-by: Nickle Wang <nicklew@nvidia.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Nick Ramirez <nramirez@nvidia.com>
---
 .../Library/RedfishFeatureUtilityLib.h        |  81 ++++++--
 .../Features/Bios/v1_0_9/Common/BiosCommon.c  |   4 +-
 .../v1_5_0/Common/ComputerSystemCommon.c      |   4 +-
 .../Memory/V1_7_1/Common/MemoryCommon.c       |   4 +-
 .../RedfishFeatureUtilityLib.c                | 192 +++++++++++++++++-
 5 files changed, 253 insertions(+), 32 deletions(-)

diff --git a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
index e2f728b2..6347585c 100644
--- a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
+++ b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
@@ -22,7 +22,7 @@
 
   Read redfish resource by given resource URI.
 
-  @param[in]  Service       Redfish srvice instacne to make query.
+  @param[in]  Service       Redfish service instance to make query.
   @param[in]  ResourceUri   Target resource URI.
   @param[out] Response      HTTP response from redfish service.
 
@@ -47,7 +47,7 @@ GetResourceByUri (
   @param[out] ArraySignatureClose       String to the close of array signature.
 
   @retval     EFI_SUCCESS            Index is found.
-  @retval     EFI_NOT_FOUND          The non-array configure language string is retured.
+  @retval     EFI_NOT_FOUND          The non-array configure language string is returned.
   @retval     EFI_INVALID_PARAMETER  The format of input ConfigureLang is wrong.
   @retval     Others                 Errors occur.
 
@@ -118,10 +118,10 @@ GetNumberOfRedpathNodes (
 
   @param[in]  NodeString             The node string to parse.
   @param[in]  Index                  Index of the node.
-  @param[out] EndOfNodePtr           Pointer to receive the poitner to
+  @param[out] EndOfNodePtr           Pointer to receive the pointer to
                                      the last character of node string.
 
-  @retval     EFI_STRING             the begining of the node string.
+  @retval     EFI_STRING             the beginning of the node string.
 
 **/
 EFI_STRING
@@ -140,7 +140,7 @@ GetRedpathNodeByIndex (
   @param[out] Index                 The array index number.
 
   @retval     EFI_SUCCESS            Index is found.
-  @retval     EFI_NOT_FOUND          The non-array configure language string is retured.
+  @retval     EFI_NOT_FOUND          The non-array configure language string is returned.
   @retval     EFI_INVALID_PARAMETER  The format of input ConfigureLang is wrong.
   @retval     Others                 Errors occur.
 
@@ -188,9 +188,9 @@ DestroyConfiglanguageList (
 
   @param[in]  DestConfigLang        Pointer to the node's configure language string.
                                     The memory pointed by ConfigLang must be allocated
-                                    through memory allocation interface. Becasue we will replace
+                                    through memory allocation interface. Because we will replace
                                     the pointer in this function.
-  @param[in]  MaxtLengthConfigLang  The maximum length of ConfigLang.
+  @param[in]  MaxLengthConfigLang   The maximum length of ConfigLang.
   @param[in]  ConfigLangInstance    Pointer to Collection member instance.
 
   @retval     EFI_SUCCESS     The instance is inserted to the configure language.
@@ -200,7 +200,7 @@ DestroyConfiglanguageList (
 EFI_STATUS
 SetResourceConfigLangMemberInstance (
   IN EFI_STRING                              *DestConfigLang,
-  IN UINTN                                   MaxtLengthConfigLang,
+  IN UINTN                                   MaxLengthConfigLang,
   IN REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG  *ConfigLangInstance
   );
 
@@ -244,7 +244,7 @@ ApplyFeatureSettingsStringType (
 
 /**
 
-  Apply property value to UEFI HII database in numric type.
+  Apply property value to UEFI HII database in numeric type.
 
   @param[in]  Schema        Property schema.
   @param[in]  Version       Property schema version.
@@ -356,7 +356,7 @@ ApplyFeatureSettingsNumericArrayType (
   @param[in]  Schema        Property schema.
   @param[in]  Version       Property schema version.
   @param[in]  ConfigureLang Configure language refers to this property.
-  @param[in]  ArrayHead     Head of Redfich CS boolean array value.
+  @param[in]  ArrayHead     Head of Redfish CS boolean array value.
 
   @retval     EFI_SUCCESS     New value is applied successfully.
   @retval     Others          Errors occur.
@@ -421,19 +421,19 @@ CreatePayloadToPatchResource (
   @param[in]    ConfigLang        ConfigLang to save
   @param[in]    Uri               Redfish Uri to save
 
-  @retval  EFI_INVALID_PARAMETR   SystemId is NULL or EMPTY
+  @retval  EFI_INVALID_PARAMETER  SystemId is NULL or EMPTY
   @retval  EFI_SUCCESS            Redfish uri is saved
 
 **/
 EFI_STATUS
-RedfisSetRedfishUri (
+RedfishSetRedfishUri (
   IN    EFI_STRING  ConfigLang,
   IN    EFI_STRING  Uri
   );
 
 /**
 
-  Get the property name by given Configure Langauge.
+  Get the property name by given Configure Language.
 
   @param[in]  ResourceUri              URI of root of resource.
   @param[in]  ConfigureLang            Configure Language string.
@@ -527,6 +527,25 @@ PropertyChecker (
   IN BOOLEAN  ProvisionMode
   );
 
+/**
+  This function query ETag from given URI string and keep it in database.
+
+  @param[in]  Service               Redfish service instance to make query.
+  @param[in]  Uri                   URI to query ETag.
+  @param[in]  CheckPendingSettings  Set this to true and @Redfish.Settings will
+                                    be handled together. FALSE otherwise.
+
+  @retval     EFI_SUCCESS     ETAG and URI are applied successfully.
+  @retval     Others          Errors occur.
+
+**/
+EFI_STATUS
+SetEtagFromUri (
+  IN  REDFISH_SERVICE  *RedfishService,
+  IN  EFI_STRING       Uri,
+  IN  BOOLEAN          CheckPendingSettings
+  );
+
 /**
 
   Keep ETAG string and URI string in database.
@@ -576,7 +595,7 @@ GetOdataId (
 
 /**
 
-  Return config language from given URI and prperty name. It's call responsibility to release returned buffer.
+  Return config language from given URI and property name. It's call responsibility to release returned buffer.
 
   @param[in] Uri            The URI to match
   @param[in] PropertyName   The property name of resource. This is optional.
@@ -790,7 +809,7 @@ MatchPropertyWithJsonContext (
 
 /**
 
-  Create string array and append to arry node in Redfish JSON convert format.
+  Create string array and append to array node in Redfish JSON convert format.
 
   @param[in,out]  Head          The head of string array.
   @param[in]      StringArray   Input string array.
@@ -809,7 +828,7 @@ AddRedfishCharArray (
 
 /**
 
-  Create numeric array and append to arry node in Redfish JSON convert format.
+  Create numeric array and append to array node in Redfish JSON convert format.
 
   @param[in,out]  Head           The head of string array.
   @param[in]      NumericArray   Input numeric array.
@@ -828,7 +847,7 @@ AddRedfishNumericArray (
 
 /**
 
-  Create boolean array and append to arry node in Redfish JSON convert format.
+  Create boolean array and append to array node in Redfish JSON convert format.
 
   @param[in,out]  Head           The head of string array.
   @param[in]      BooleanArray   Input boolean array.
@@ -871,7 +890,7 @@ CompareRedfishStringArrayValues (
   Check and see if value in Redfish numeric array are all the same as the one
   from HII configuration.
 
-  @param[in]  Head          The head of Redfish CS numeraic array.
+  @param[in]  Head          The head of Redfish CS numeric array.
   @param[in]  NumericArray  Input numeric array.
   @param[in]  ArraySize     The size of NumericArray.
 
@@ -914,9 +933,9 @@ CompareRedfishBooleanArrayValues (
   This is just a simple check.
 
   @param[in]  RedfishVagueKeyValuePtr     The vague key value sets on Redfish service.
-  @param[in]  RedfishVagueKeyValueNumber  The numebr of vague key value sets
+  @param[in]  RedfishVagueKeyValueNumber  The number of vague key value sets
   @param[in]  ConfigVagueKeyValuePtr      The vague configuration on platform.
-  @param[in]  ConfigVagueKeyValueNumber   The numebr of vague key value sets
+  @param[in]  ConfigVagueKeyValueNumber   The number of vague key value sets
 
   @retval     TRUE          All values are the same.
               FALSE         There is some difference.
@@ -949,4 +968,26 @@ GetEtagAndLocation (
   OUT EFI_STRING        *Location    OPTIONAL
   );
 
+/**
+
+  Check and see if "@Redfish.Settings" exist in given Payload. If found, return the
+  payload and URI to pending settings. Caller has to release "SettingPayload" and
+  "SettingUri".
+
+  @param[in]  Payload         Payload that may contain "@Redfish.Settings"
+  @param[out] SettingPayload  Payload keeps pending settings.
+  @param[out] SettingUri      URI to pending settings.
+
+  @retval     EFI_SUCCESS     Pending settings is found and returned.
+  @retval     Others          Error happens
+
+**/
+EFI_STATUS
+GetPendingSettings (
+  IN  REDFISH_SERVICE   RedfishService,
+  IN  REDFISH_PAYLOAD   Payload,
+  OUT REDFISH_RESPONSE  *SettingResponse,
+  OUT EFI_STRING        *SettingUri
+  );
+
 #endif
diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
index f96c90cc..98288d66 100644
--- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
+++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
@@ -338,7 +338,7 @@ ProvisioningBiosResource (
   // Keep location of new resource.
   //
   if (NewResourceLocation != NULL) {
-    RedfisSetRedfishUri (ConfigureLang, NewResourceLocation);
+    RedfishSetRedfishUri (ConfigureLang, NewResourceLocation);
   }
 
   //
@@ -745,7 +745,7 @@ RedfishIdentifyResourceCommon (
     //
     // Keep URI and ConfigLang mapping
     //
-    RedfisSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private->Uri);
+    RedfishSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private->Uri);
     //
     // Set the configuration language in the RESOURCE_INFORMATION_EXCHANGE.
     // This information is sent back to the parent resource (e.g. the collection driver).
diff --git a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/ComputerSystemCommon.c b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/ComputerSystemCommon.c
index 7ed1bd55..1ffb7d1d 100644
--- a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/ComputerSystemCommon.c
+++ b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/ComputerSystemCommon.c
@@ -1274,7 +1274,7 @@ ProvisioningComputerSystemResource (
   // Keep location of new resource.
   //
   if (NewResourceLocation != NULL) {
-    RedfisSetRedfishUri (ConfigureLang, NewResourceLocation);
+    RedfishSetRedfishUri (ConfigureLang, NewResourceLocation);
   }
 
   //
@@ -1621,7 +1621,7 @@ RedfishIdentifyResourceCommon (
     //
     // Keep URI and ConfigLang mapping
     //
-    RedfisSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private->Uri);
+    RedfishSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private->Uri);
     //
     // Set the configuration language in the RESOURCE_INFORMATION_EXCHANGE.
     // This information is sent back to the parent resource (e.g. the collection driver).
diff --git a/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c b/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
index b7434036..4c41f16b 100644
--- a/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
+++ b/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
@@ -2190,7 +2190,7 @@ ProvisioningMemoryResource (
   // Keep location of new resource.
   //
   if (NewResourceLocation != NULL) {
-    RedfisSetRedfishUri (ConfigureLang, NewResourceLocation);
+    RedfishSetRedfishUri (ConfigureLang, NewResourceLocation);
   }
 
   //
@@ -2537,7 +2537,7 @@ RedfishIdentifyResourceCommon (
     //
     // Keep URI and ConfigLang mapping
     //
-    RedfisSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private->Uri);
+    RedfishSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private->Uri);
     //
     // Set the configuration language in the RESOURCE_INFORMATION_EXCHANGE.
     // This information is sent back to the parent resource (e.g. the collection driver).
diff --git a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
index 13e29902..753cd7b2 100644
--- a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
+++ b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.c
@@ -107,6 +107,119 @@ GetArraykeyFromUri (
   return EFI_SUCCESS;
 }
 
+/**
+
+  This function query ETag from given URI string and keep it in database.
+
+  @param[in]  Service               Redfish service instance to make query.
+  @param[in]  Uri                   URI to query ETag.
+  @param[in]  CheckPendingSettings  Set this to true and @Redfish.Settings will
+                                    be handled together. FALSE otherwise.
+
+  @retval     EFI_SUCCESS     ETAG and URI are applied successfully.
+  @retval     Others          Errors occur.
+
+**/
+EFI_STATUS
+SetEtagFromUri (
+  IN  REDFISH_SERVICE  *RedfishService,
+  IN  EFI_STRING       Uri,
+  IN  BOOLEAN          CheckPendingSettings
+  )
+{
+  EFI_STATUS        Status;
+  CHAR8             *Etag;
+  CHAR8             *AsciiUri;
+  REDFISH_RESPONSE  Response;
+  EFI_STRING        PendingSettingUri;
+
+  if ((RedfishService == NULL) || IS_EMPTY_STRING (Uri)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  AsciiUri          = NULL;
+  Etag              = NULL;
+  PendingSettingUri = NULL;
+
+  Status = RedfishLocateProtocol ((VOID **)&mEtagProtocol, &gEdkIIRedfishETagProtocolGuid);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a: fail to locate gEdkIIRedfishETagProtocolGuid: %r\n", __func__, Status));
+    return Status;
+  }
+
+  Status = GetResourceByUri (RedfishService, Uri, &Response);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a: get resource from: %s failed\n", __func__, Uri));
+    return Status;
+  }
+
+  //
+  // 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 = EFI_NOT_FOUND;
+    goto ON_RELEASE;
+  }
+
+  AsciiUri = StrUnicodeToAscii (Uri);
+  if (AsciiUri == NULL) {
+    Status = EFI_OUT_OF_RESOURCES;
+    goto ON_RELEASE;
+  }
+
+  //
+  // Save ETag to variable.
+  //
+  Status = mEtagProtocol->Set (mEtagProtocol, AsciiUri, Etag);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a: failed to set ETag %a -> %a\n", __func__, Etag, AsciiUri));
+  }
+
+  if (CheckPendingSettings && (Response.Payload != NULL)) {
+    //
+    // Check to see if there is @Redfish.Settings
+    //
+    Status = GetPendingSettings (
+               RedfishService,
+               Response.Payload,
+               NULL,
+               &PendingSettingUri
+               );
+    if (!EFI_ERROR (Status)) {
+      DEBUG ((REDFISH_DEBUG_TRACE, "%a: @Redfish.Settings found: %s\n", __func__, PendingSettingUri));
+
+      Status = SetEtagFromUri (RedfishService, PendingSettingUri, FALSE);
+    }
+  }
+
+ON_RELEASE:
+
+  if (AsciiUri != NULL) {
+    FreePool (AsciiUri);
+  }
+
+  if (Etag != NULL) {
+    FreePool (Etag);
+  }
+
+  if (PendingSettingUri != NULL) {
+    FreePool (PendingSettingUri);
+  }
+
+  if (Response.Payload != NULL) {
+    RedfishFreeResponse (
+      Response.StatusCode,
+      Response.HeaderCount,
+      Response.Headers,
+      Response.Payload
+      );
+  }
+
+  return Status;
+}
+
 /**
 
   Keep ETAG string and URI string in database.
@@ -143,7 +256,6 @@ SetEtagWithUri (
   }
 
   mEtagProtocol->Set (mEtagProtocol, AsciiUri, EtagStr);
-  mEtagProtocol->Flush (mEtagProtocol);
 
   FreePool (AsciiUri);
 
@@ -505,6 +617,8 @@ ApplyFeatureSettingsVagueType (
     return EFI_INVALID_PARAMETER;
   }
 
+  DEBUG ((REDFISH_DEBUG_TRACE, "%a: schema: %a %a config lang: %s NumberOfVagueValues: %d\n", __func__, Schema, Version, ConfigureLang, NumberOfVagueValues));
+
   ConfigureLangAscii = AllocatePool (StrLen (ConfigureLang) + 1);
   if (ConfigureLangAscii == NULL) {
     Status = EFI_OUT_OF_RESOURCES;
@@ -1473,7 +1587,7 @@ DestroyConfiglanguageList (
                                     The memory pointed by ConfigLang must be allocated
                                     through memory allocation interface. Because we will replace
                                     the pointer in this function.
-  @param[in]  MaxtLengthConfigLang  The maximum length of ConfigLang.
+  @param[in]  MaxLengthConfigLang   The maximum length of ConfigLang.
   @param[in]  ConfigLangInstance    Pointer to Collection member instance.
 
   @retval     EFI_SUCCESS     The instance is inserted to the configure language.
@@ -1483,7 +1597,7 @@ DestroyConfiglanguageList (
 EFI_STATUS
 SetResourceConfigLangMemberInstance (
   IN EFI_STRING                              *DestConfigLang,
-  IN UINTN                                   MaxtLengthConfigLang,
+  IN UINTN                                   MaxLengthConfigLang,
   IN REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG  *ConfigLangInstance
   )
 {
@@ -1508,14 +1622,14 @@ SetResourceConfigLangMemberInstance (
     if (ConfigLangInstance->ConfigureLang == NULL) {
       return EFI_INVALID_PARAMETER;
     } else {
-      StrCatS (*DestConfigLang, MaxtLengthConfigLang, ConfigLangInstance->ConfigureLang);
+      StrCatS (*DestConfigLang, MaxLengthConfigLang, ConfigLangInstance->ConfigureLang);
       return EFI_SUCCESS;
     }
   }
 
   MaxStrLength  = StrSize (ThisConfigLang) + StrSize ((EFI_STRING)&InstanceStr);
   NewConfigLang = ThisConfigLang;
-  if (MaxtLengthConfigLang < MaxStrLength) {
+  if (MaxLengthConfigLang < MaxStrLength) {
     NewConfigLang = (EFI_STRING)AllocateZeroPool (MaxStrLength);
     if (NewConfigLang == NULL) {
       DEBUG ((DEBUG_ERROR, "%a: Fail to allocate memory for NewConfigLang.\n", __func__));
@@ -2191,7 +2305,7 @@ GetConfigureLang (
 
 **/
 EFI_STATUS
-RedfisSetRedfishUri (
+RedfishSetRedfishUri (
   IN    EFI_STRING  ConfigLang,
   IN    EFI_STRING  Uri
   )
@@ -3524,6 +3638,72 @@ CompareRedfishBooleanArrayValues (
   return TRUE;
 }
 
+/**
+
+  Check and see if "@Redfish.Settings" exist in given Payload. If found, return the
+  payload and URI to pending settings. Caller has to release "SettingPayload" and
+  "SettingUri".
+
+  @param[in]  Payload         Payload that may contain "@Redfish.Settings"
+  @param[out] SettingPayload  Payload keeps pending settings.
+  @param[out] SettingUri      URI to pending settings.
+
+  @retval     EFI_SUCCESS     Pending settings is found and returned.
+  @retval     Others          Error happens
+
+**/
+EFI_STATUS
+GetPendingSettings (
+  IN  REDFISH_SERVICE   RedfishService,
+  IN  REDFISH_PAYLOAD   Payload,
+  OUT REDFISH_RESPONSE  *SettingResponse,
+  OUT EFI_STRING        *SettingUri
+  )
+{
+  CONST CHAR8       *RedfishSettingsUriKeys[] = { "@Redfish.Settings", "SettingsObject", "@odata.id" };
+  EDKII_JSON_VALUE  JsonValue;
+  UINTN             Index;
+  EFI_STATUS        Status;
+
+  if ((RedfishService == NULL) || (Payload == NULL) || (SettingResponse == NULL) || (SettingUri == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  *SettingUri = NULL;
+  JsonValue   = RedfishJsonInPayload (Payload);
+
+  //
+  // Seeking RedfishSettings URI link.
+  //
+  for (Index = 0; Index < ARRAY_SIZE (RedfishSettingsUriKeys); Index++) {
+    if (JsonValue == NULL) {
+      break;
+    }
+
+    JsonValue = JsonObjectGetValue (JsonValueGetObject (JsonValue), RedfishSettingsUriKeys[Index]);
+  }
+
+  if (JsonValue != NULL) {
+    //
+    // Verify RedfishSettings URI link is valid to retrieve resource or not.
+    //
+    *SettingUri = JsonValueGetUnicodeString (JsonValue);
+    if (*SettingUri == NULL) {
+      return EFI_NOT_FOUND;
+    }
+
+    Status = GetResourceByUri (RedfishService, *SettingUri, SettingResponse);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "%a: @Redfish.Settings exists, get resource from: %s failed: %r\n", __func__, *SettingUri, Status));
+      return Status;
+    }
+
+    return EFI_SUCCESS;
+  }
+
+  return EFI_NOT_FOUND;
+}
+
 /**
 
   Check and see if any difference between two vague value set.
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111673): https://edk2.groups.io/g/devel/message/111673
Mute This Topic: https://groups.io/mt/102767544/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 3/6] RedfishClientPkg: feature driver enhancement.
  2023-11-23 14:34 [edk2-devel] [edk2-redfish-client][PATCH 3/6] RedfishClientPkg: feature driver enhancement Nickle Wang via groups.io
@ 2023-11-28  4:04 ` Chang, Abner via groups.io
  2023-11-28  6:44   ` Nickle Wang via groups.io
  0 siblings, 1 reply; 4+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-28  4:04 UTC (permalink / raw)
  To: Nickle Wang, devel@edk2.groups.io; +Cc: Igor Kulchytskyy, Nick Ramirez

[AMD Official Use Only - General]

> -----Original Message-----
> From: Nickle Wang <nicklew@nvidia.com>
> Sent: Thursday, November 23, 2023 10:34 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; Igor Kulchytskyy
> <igork@ami.com>; Nick Ramirez <nramirez@nvidia.com>
> Subject: [edk2-redfish-client][PATCH 3/6] RedfishClientPkg: feature driver
> enhancement.
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> -Fix typo (include function RedfisSetRedfishUri and parameter
> MaxLengthConfigLang)
> -Add more debug message.
> -Do not save ETag to variable every time when new ETag is created
> in SetEtagWithUri().
> -Introduce SetEtagFromUri() to get ETag from BMC and save it
> in variable.
> -Introduce GetPendingSettings() to check and see if Redfish pending
> settings is supported or not.
>
> Signed-off-by: Nickle Wang <nicklew@nvidia.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Nick Ramirez <nramirez@nvidia.com>
> ---
>  .../Library/RedfishFeatureUtilityLib.h        |  81 ++++++--
>  .../Features/Bios/v1_0_9/Common/BiosCommon.c  |   4 +-
>  .../v1_5_0/Common/ComputerSystemCommon.c      |   4 +-
>  .../Memory/V1_7_1/Common/MemoryCommon.c       |   4 +-
>  .../RedfishFeatureUtilityLib.c                | 192 +++++++++++++++++-
>  5 files changed, 253 insertions(+), 32 deletions(-)
>
> diff --git a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> index e2f728b2..6347585c 100644
> --- a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> +++ b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> @@ -22,7 +22,7 @@
>
>    Read redfish resource by given resource URI.
>
> -  @param[in]  Service       Redfish srvice instacne to make query.
> +  @param[in]  Service       Redfish service instance to make query.
>    @param[in]  ResourceUri   Target resource URI.
>    @param[out] Response      HTTP response from redfish service.
>
> @@ -47,7 +47,7 @@ GetResourceByUri (
>    @param[out] ArraySignatureClose       String to the close of array signature.
>
>    @retval     EFI_SUCCESS            Index is found.
> -  @retval     EFI_NOT_FOUND          The non-array configure language string is
> retured.
> +  @retval     EFI_NOT_FOUND          The non-array configure language string is
> returned.
>    @retval     EFI_INVALID_PARAMETER  The format of input ConfigureLang is
> wrong.
>    @retval     Others                 Errors occur.
>
> @@ -118,10 +118,10 @@ GetNumberOfRedpathNodes (
>
>    @param[in]  NodeString             The node string to parse.
>    @param[in]  Index                  Index of the node.
> -  @param[out] EndOfNodePtr           Pointer to receive the poitner to
> +  @param[out] EndOfNodePtr           Pointer to receive the pointer to
>                                       the last character of node string.
>
> -  @retval     EFI_STRING             the begining of the node string.
> +  @retval     EFI_STRING             the beginning of the node string.
>
>  **/
>  EFI_STRING
> @@ -140,7 +140,7 @@ GetRedpathNodeByIndex (
>    @param[out] Index                 The array index number.
>
>    @retval     EFI_SUCCESS            Index is found.
> -  @retval     EFI_NOT_FOUND          The non-array configure language string is
> retured.
> +  @retval     EFI_NOT_FOUND          The non-array configure language string is
> returned.
>    @retval     EFI_INVALID_PARAMETER  The format of input ConfigureLang is
> wrong.
>    @retval     Others                 Errors occur.
>
> @@ -188,9 +188,9 @@ DestroyConfiglanguageList (
>
>    @param[in]  DestConfigLang        Pointer to the node's configure language
> string.
>                                      The memory pointed by ConfigLang must be allocated
> -                                    through memory allocation interface. Becasue we will
> replace
> +                                    through memory allocation interface. Because we will
> replace
>                                      the pointer in this function.
> -  @param[in]  MaxtLengthConfigLang  The maximum length of ConfigLang.
> +  @param[in]  MaxLengthConfigLang   The maximum length of ConfigLang.
>    @param[in]  ConfigLangInstance    Pointer to Collection member instance.
>
>    @retval     EFI_SUCCESS     The instance is inserted to the configure language.
> @@ -200,7 +200,7 @@ DestroyConfiglanguageList (
>  EFI_STATUS
>  SetResourceConfigLangMemberInstance (
>    IN EFI_STRING                              *DestConfigLang,
> -  IN UINTN                                   MaxtLengthConfigLang,
> +  IN UINTN                                   MaxLengthConfigLang,
>    IN REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG  *ConfigLangInstance
>    );
>
> @@ -244,7 +244,7 @@ ApplyFeatureSettingsStringType (
>
>  /**
>
> -  Apply property value to UEFI HII database in numric type.
> +  Apply property value to UEFI HII database in numeric type.
>
>    @param[in]  Schema        Property schema.
>    @param[in]  Version       Property schema version.
> @@ -356,7 +356,7 @@ ApplyFeatureSettingsNumericArrayType (
>    @param[in]  Schema        Property schema.
>    @param[in]  Version       Property schema version.
>    @param[in]  ConfigureLang Configure language refers to this property.
> -  @param[in]  ArrayHead     Head of Redfich CS boolean array value.
> +  @param[in]  ArrayHead     Head of Redfish CS boolean array value.
>
>    @retval     EFI_SUCCESS     New value is applied successfully.
>    @retval     Others          Errors occur.
> @@ -421,19 +421,19 @@ CreatePayloadToPatchResource (
>    @param[in]    ConfigLang        ConfigLang to save
>    @param[in]    Uri               Redfish Uri to save
>
> -  @retval  EFI_INVALID_PARAMETR   SystemId is NULL or EMPTY
> +  @retval  EFI_INVALID_PARAMETER  SystemId is NULL or EMPTY
>    @retval  EFI_SUCCESS            Redfish uri is saved
>
>  **/
>  EFI_STATUS
> -RedfisSetRedfishUri (
> +RedfishSetRedfishUri (
>    IN    EFI_STRING  ConfigLang,
>    IN    EFI_STRING  Uri
>    );
>
>  /**
>
> -  Get the property name by given Configure Langauge.
> +  Get the property name by given Configure Language.
>
>    @param[in]  ResourceUri              URI of root of resource.
>    @param[in]  ConfigureLang            Configure Language string.
> @@ -527,6 +527,25 @@ PropertyChecker (
>    IN BOOLEAN  ProvisionMode
>    );
>
> +/**
> +  This function query ETag from given URI string and keep it in database.
> +
> +  @param[in]  Service               Redfish service instance to make query.
> +  @param[in]  Uri                   URI to query ETag.
> +  @param[in]  CheckPendingSettings  Set this to true and @Redfish.Settings
> will
> +                                    be handled together. FALSE otherwise.
> +
> +  @retval     EFI_SUCCESS     ETAG and URI are applied successfully.
> +  @retval     Others          Errors occur.
> +
> +**/
> +EFI_STATUS
> +SetEtagFromUri (
> +  IN  REDFISH_SERVICE  *RedfishService,
> +  IN  EFI_STRING       Uri,
> +  IN  BOOLEAN          CheckPendingSettings
> +  );
> +
>  /**
>
>    Keep ETAG string and URI string in database.
> @@ -576,7 +595,7 @@ GetOdataId (
>
>  /**
>
> -  Return config language from given URI and prperty name. It's call
> responsibility to release returned buffer.
> +  Return config language from given URI and property name. It's call
> responsibility to release returned buffer.
>
>    @param[in] Uri            The URI to match
>    @param[in] PropertyName   The property name of resource. This is optional.
> @@ -790,7 +809,7 @@ MatchPropertyWithJsonContext (
>
>  /**
>
> -  Create string array and append to arry node in Redfish JSON convert format.
> +  Create string array and append to array node in Redfish JSON convert
> format.
>
>    @param[in,out]  Head          The head of string array.
>    @param[in]      StringArray   Input string array.
> @@ -809,7 +828,7 @@ AddRedfishCharArray (
>
>  /**
>
> -  Create numeric array and append to arry node in Redfish JSON convert
> format.
> +  Create numeric array and append to array node in Redfish JSON convert
> format.
>
>    @param[in,out]  Head           The head of string array.
>    @param[in]      NumericArray   Input numeric array.
> @@ -828,7 +847,7 @@ AddRedfishNumericArray (
>
>  /**
>
> -  Create boolean array and append to arry node in Redfish JSON convert
> format.
> +  Create boolean array and append to array node in Redfish JSON convert
> format.
>
>    @param[in,out]  Head           The head of string array.
>    @param[in]      BooleanArray   Input boolean array.
> @@ -871,7 +890,7 @@ CompareRedfishStringArrayValues (
>    Check and see if value in Redfish numeric array are all the same as the one
>    from HII configuration.
>
> -  @param[in]  Head          The head of Redfish CS numeraic array.
> +  @param[in]  Head          The head of Redfish CS numeric array.
>    @param[in]  NumericArray  Input numeric array.
>    @param[in]  ArraySize     The size of NumericArray.
>
> @@ -914,9 +933,9 @@ CompareRedfishBooleanArrayValues (
>    This is just a simple check.
>
>    @param[in]  RedfishVagueKeyValuePtr     The vague key value sets on
> Redfish service.
> -  @param[in]  RedfishVagueKeyValueNumber  The numebr of vague key value
> sets
> +  @param[in]  RedfishVagueKeyValueNumber  The number of vague key
> value sets
>    @param[in]  ConfigVagueKeyValuePtr      The vague configuration on
> platform.
> -  @param[in]  ConfigVagueKeyValueNumber   The numebr of vague key value
> sets
> +  @param[in]  ConfigVagueKeyValueNumber   The number of vague key value
> sets
>
>    @retval     TRUE          All values are the same.
>                FALSE         There is some difference.
> @@ -949,4 +968,26 @@ GetEtagAndLocation (
>    OUT EFI_STRING        *Location    OPTIONAL
>    );
>
> +/**
> +
> +  Check and see if "@Redfish.Settings" exist in given Payload. If found, return
> the
> +  payload and URI to pending settings. Caller has to release "SettingPayload"
> and
> +  "SettingUri".
> +

> +  @param[in]  Payload         Payload that may contain "@Redfish.Settings"
> +  @param[out] SettingPayload  Payload keeps pending settings.
> +  @param[out] SettingUri      URI to pending settings.


Missed Doxgen tag for RedfishService.
Please also update the function header of GetPendingSettings in C source file.

> +
> +  @retval     EFI_SUCCESS     Pending settings is found and returned.
> +  @retval     Others          Error happens
> +
> +**/
> +EFI_STATUS
> +GetPendingSettings (
> +  IN  REDFISH_SERVICE   RedfishService,
> +  IN  REDFISH_PAYLOAD   Payload,
> +  OUT REDFISH_RESPONSE  *SettingResponse,
> +  OUT EFI_STRING        *SettingUri
> +  );
> +
>  #endif
> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> index f96c90cc..98288d66 100644
> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> @@ -338,7 +338,7 @@ ProvisioningBiosResource (
>    // Keep location of new resource.
>    //
>    if (NewResourceLocation != NULL) {
> -    RedfisSetRedfishUri (ConfigureLang, NewResourceLocation);
> +    RedfishSetRedfishUri (ConfigureLang, NewResourceLocation);
>    }
>
>    //
> @@ -745,7 +745,7 @@ RedfishIdentifyResourceCommon (
>      //
>      // Keep URI and ConfigLang mapping
>      //
> -    RedfisSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private->Uri);
> +    RedfishSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private->Uri);
>      //
>      // Set the configuration language in the
> RESOURCE_INFORMATION_EXCHANGE.
>      // This information is sent back to the parent resource (e.g. the collection
> driver).
> diff --git
> a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/Computer
> SystemCommon.c
> b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/Computer
> SystemCommon.c
> index 7ed1bd55..1ffb7d1d 100644
> ---
> a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/Computer
> SystemCommon.c
> +++
> b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/Computer
> SystemCommon.c
> @@ -1274,7 +1274,7 @@ ProvisioningComputerSystemResource (
>    // Keep location of new resource.
>    //
>    if (NewResourceLocation != NULL) {
> -    RedfisSetRedfishUri (ConfigureLang, NewResourceLocation);
> +    RedfishSetRedfishUri (ConfigureLang, NewResourceLocation);
>    }
>
>    //
> @@ -1621,7 +1621,7 @@ RedfishIdentifyResourceCommon (
>      //
>      // Keep URI and ConfigLang mapping
>      //
> -    RedfisSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private->Uri);
> +    RedfishSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private->Uri);
>      //
>      // Set the configuration language in the
> RESOURCE_INFORMATION_EXCHANGE.
>      // This information is sent back to the parent resource (e.g. the collection
> driver).
> diff --git
> a/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
> b/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.
> c
> index b7434036..4c41f16b 100644
> ---
> a/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
> +++
> b/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.
> c
> @@ -2190,7 +2190,7 @@ ProvisioningMemoryResource (
>    // Keep location of new resource.
>    //
>    if (NewResourceLocation != NULL) {
> -    RedfisSetRedfishUri (ConfigureLang, NewResourceLocation);
> +    RedfishSetRedfishUri (ConfigureLang, NewResourceLocation);
>    }
>
>    //
> @@ -2537,7 +2537,7 @@ RedfishIdentifyResourceCommon (
>      //
>      // Keep URI and ConfigLang mapping
>      //
> -    RedfisSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private->Uri);
> +    RedfishSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private->Uri);
>      //
>      // Set the configuration language in the
> RESOURCE_INFORMATION_EXCHANGE.
>      // This information is sent back to the parent resource (e.g. the collection
> driver).
> diff --git
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> index 13e29902..753cd7b2 100644
> ---
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> +++
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> c
> @@ -107,6 +107,119 @@ GetArraykeyFromUri (
>    return EFI_SUCCESS;
>  }
>
> +/**
> +
> +  This function query ETag from given URI string and keep it in database.
> +
> +  @param[in]  Service               Redfish service instance to make query.
> +  @param[in]  Uri                   URI to query ETag.
> +  @param[in]  CheckPendingSettings  Set this to true and @Redfish.Settings
> will
> +                                    be handled together. FALSE otherwise.
> +
> +  @retval     EFI_SUCCESS     ETAG and URI are applied successfully.
> +  @retval     Others          Errors occur.
> +
> +**/
> +EFI_STATUS
> +SetEtagFromUri (
> +  IN  REDFISH_SERVICE  *RedfishService,
> +  IN  EFI_STRING       Uri,
> +  IN  BOOLEAN          CheckPendingSettings
> +  )
> +{
> +  EFI_STATUS        Status;
> +  CHAR8             *Etag;
> +  CHAR8             *AsciiUri;
> +  REDFISH_RESPONSE  Response;
> +  EFI_STRING        PendingSettingUri;
> +
> +  if ((RedfishService == NULL) || IS_EMPTY_STRING (Uri)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  AsciiUri          = NULL;
> +  Etag              = NULL;
> +  PendingSettingUri = NULL;
> +
> +  Status = RedfishLocateProtocol ((VOID **)&mEtagProtocol,
> &gEdkIIRedfishETagProtocolGuid);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "%a: fail to locate
> gEdkIIRedfishETagProtocolGuid: %r\n", __func__, Status));
> +    return Status;
> +  }
> +
> +  Status = GetResourceByUri (RedfishService, Uri, &Response);
This may spend some time on network to get the whole resource from URI if the resource size is big, but we just need the information of ETAG header. We can think of using HEAD HTTP method.
However, this seems fine for now as the following code gets the pending settings from the same HTTP response body retrieved above.
The worst case would be there is no pending setting, however we waste network bandwidth to get the whole response body. Not sure if HTTP query $select can improve this or not for retrieving pending setting annotation.

Abner


> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "%a: get resource from: %s failed\n", __func__,
> Uri));
> +    return Status;
> +  }
> +
> +  //
> +  // 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 = EFI_NOT_FOUND;
> +    goto ON_RELEASE;
> +  }
> +
> +  AsciiUri = StrUnicodeToAscii (Uri);
> +  if (AsciiUri == NULL) {
> +    Status = EFI_OUT_OF_RESOURCES;
> +    goto ON_RELEASE;
> +  }
> +
> +  //
> +  // Save ETag to variable.
> +  //
> +  Status = mEtagProtocol->Set (mEtagProtocol, AsciiUri, Etag);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "%a: failed to set ETag %a -> %a\n", __func__,
> Etag, AsciiUri));
> +  }
> +
> +  if (CheckPendingSettings && (Response.Payload != NULL)) {
> +    //
> +    // Check to see if there is @Redfish.Settings
> +    //
> +    Status = GetPendingSettings (
> +               RedfishService,
> +               Response.Payload,
> +               NULL,
> +               &PendingSettingUri
> +               );
> +    if (!EFI_ERROR (Status)) {
> +      DEBUG ((REDFISH_DEBUG_TRACE, "%a: @Redfish.Settings found: %s\n",
> __func__, PendingSettingUri));
> +
> +      Status = SetEtagFromUri (RedfishService, PendingSettingUri, FALSE);
> +    }
> +  }
> +
> +ON_RELEASE:
> +
> +  if (AsciiUri != NULL) {
> +    FreePool (AsciiUri);
> +  }
> +
> +  if (Etag != NULL) {
> +    FreePool (Etag);
> +  }
> +
> +  if (PendingSettingUri != NULL) {
> +    FreePool (PendingSettingUri);
> +  }
> +
> +  if (Response.Payload != NULL) {
> +    RedfishFreeResponse (
> +      Response.StatusCode,
> +      Response.HeaderCount,
> +      Response.Headers,
> +      Response.Payload
> +      );
> +  }
> +
> +  return Status;
> +}
> +
>  /**
>
>    Keep ETAG string and URI string in database.
> @@ -143,7 +256,6 @@ SetEtagWithUri (
>    }
>
>    mEtagProtocol->Set (mEtagProtocol, AsciiUri, EtagStr);
> -  mEtagProtocol->Flush (mEtagProtocol);
>
>    FreePool (AsciiUri);
>
> @@ -505,6 +617,8 @@ ApplyFeatureSettingsVagueType (
>      return EFI_INVALID_PARAMETER;
>    }
>
> +  DEBUG ((REDFISH_DEBUG_TRACE, "%a: schema: %a %a config lang: %s
> NumberOfVagueValues: %d\n", __func__, Schema, Version, ConfigureLang,
> NumberOfVagueValues));
> +
>    ConfigureLangAscii = AllocatePool (StrLen (ConfigureLang) + 1);
>    if (ConfigureLangAscii == NULL) {
>      Status = EFI_OUT_OF_RESOURCES;
> @@ -1473,7 +1587,7 @@ DestroyConfiglanguageList (
>                                      The memory pointed by ConfigLang must be allocated
>                                      through memory allocation interface. Because we will
> replace
>                                      the pointer in this function.
> -  @param[in]  MaxtLengthConfigLang  The maximum length of ConfigLang.
> +  @param[in]  MaxLengthConfigLang   The maximum length of ConfigLang.
>    @param[in]  ConfigLangInstance    Pointer to Collection member instance.
>
>    @retval     EFI_SUCCESS     The instance is inserted to the configure language.
> @@ -1483,7 +1597,7 @@ DestroyConfiglanguageList (
>  EFI_STATUS
>  SetResourceConfigLangMemberInstance (
>    IN EFI_STRING                              *DestConfigLang,
> -  IN UINTN                                   MaxtLengthConfigLang,
> +  IN UINTN                                   MaxLengthConfigLang,
>    IN REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG  *ConfigLangInstance
>    )
>  {
> @@ -1508,14 +1622,14 @@ SetResourceConfigLangMemberInstance (
>      if (ConfigLangInstance->ConfigureLang == NULL) {
>        return EFI_INVALID_PARAMETER;
>      } else {
> -      StrCatS (*DestConfigLang, MaxtLengthConfigLang, ConfigLangInstance-
> >ConfigureLang);
> +      StrCatS (*DestConfigLang, MaxLengthConfigLang, ConfigLangInstance-
> >ConfigureLang);
>        return EFI_SUCCESS;
>      }
>    }
>
>    MaxStrLength  = StrSize (ThisConfigLang) + StrSize
> ((EFI_STRING)&InstanceStr);
>    NewConfigLang = ThisConfigLang;
> -  if (MaxtLengthConfigLang < MaxStrLength) {
> +  if (MaxLengthConfigLang < MaxStrLength) {
>      NewConfigLang = (EFI_STRING)AllocateZeroPool (MaxStrLength);
>      if (NewConfigLang == NULL) {
>        DEBUG ((DEBUG_ERROR, "%a: Fail to allocate memory for
> NewConfigLang.\n", __func__));
> @@ -2191,7 +2305,7 @@ GetConfigureLang (
>
>  **/
>  EFI_STATUS
> -RedfisSetRedfishUri (
> +RedfishSetRedfishUri (
>    IN    EFI_STRING  ConfigLang,
>    IN    EFI_STRING  Uri
>    )
> @@ -3524,6 +3638,72 @@ CompareRedfishBooleanArrayValues (
>    return TRUE;
>  }
>
> +/**
> +
> +  Check and see if "@Redfish.Settings" exist in given Payload. If found, return
> the
> +  payload and URI to pending settings. Caller has to release "SettingPayload"
> and
> +  "SettingUri".
> +
> +  @param[in]  Payload         Payload that may contain "@Redfish.Settings"
> +  @param[out] SettingPayload  Payload keeps pending settings.
> +  @param[out] SettingUri      URI to pending settings.
> +
> +  @retval     EFI_SUCCESS     Pending settings is found and returned.
> +  @retval     Others          Error happens
> +
> +**/
> +EFI_STATUS
> +GetPendingSettings (
> +  IN  REDFISH_SERVICE   RedfishService,
> +  IN  REDFISH_PAYLOAD   Payload,
> +  OUT REDFISH_RESPONSE  *SettingResponse,
> +  OUT EFI_STRING        *SettingUri
> +  )
> +{
> +  CONST CHAR8       *RedfishSettingsUriKeys[] = { "@Redfish.Settings",
> "SettingsObject", "@odata.id" };
> +  EDKII_JSON_VALUE  JsonValue;
> +  UINTN             Index;
> +  EFI_STATUS        Status;
> +
> +  if ((RedfishService == NULL) || (Payload == NULL) || (SettingResponse ==
> NULL) || (SettingUri == NULL)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  *SettingUri = NULL;
> +  JsonValue   = RedfishJsonInPayload (Payload);
> +
> +  //
> +  // Seeking RedfishSettings URI link.
> +  //
> +  for (Index = 0; Index < ARRAY_SIZE (RedfishSettingsUriKeys); Index++) {
> +    if (JsonValue == NULL) {
> +      break;
> +    }
> +
> +    JsonValue = JsonObjectGetValue (JsonValueGetObject (JsonValue),
> RedfishSettingsUriKeys[Index]);
> +  }
> +
> +  if (JsonValue != NULL) {
> +    //
> +    // Verify RedfishSettings URI link is valid to retrieve resource or not.
> +    //
> +    *SettingUri = JsonValueGetUnicodeString (JsonValue);
> +    if (*SettingUri == NULL) {
> +      return EFI_NOT_FOUND;
> +    }
> +
> +    Status = GetResourceByUri (RedfishService, *SettingUri, SettingResponse);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_ERROR, "%a: @Redfish.Settings exists, get resource
> from: %s failed: %r\n", __func__, *SettingUri, Status));
> +      return Status;
> +    }
> +
> +    return EFI_SUCCESS;
> +  }
> +
> +  return EFI_NOT_FOUND;
> +}
> +
>  /**
>
>    Check and see if any difference between two vague value set.
> --
> 2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111791): https://edk2.groups.io/g/devel/message/111791
Mute This Topic: https://groups.io/mt/102767544/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 3/6] RedfishClientPkg: feature driver enhancement.
  2023-11-28  4:04 ` Chang, Abner via groups.io
@ 2023-11-28  6:44   ` Nickle Wang via groups.io
  2023-11-28  7:08     ` Chang, Abner via groups.io
  0 siblings, 1 reply; 4+ messages in thread
From: Nickle Wang via groups.io @ 2023-11-28  6:44 UTC (permalink / raw)
  To: Chang, Abner, devel@edk2.groups.io; +Cc: Igor Kulchytskyy, Nick Ramirez

Hi Abner,

Thanks for your review. Please find my comment below.

> This may spend some time on network to get the whole resource from URI if the
> resource size is big, but we just need the information of ETAG header. We can
> think of using HEAD HTTP method.
> However, this seems fine for now as the following code gets the pending settings
> from the same HTTP response body retrieved above.
> The worst case would be there is no pending setting, however we waste network
> bandwidth to get the whole response body. Not sure if HTTP query $select can
> improve this or not for retrieving pending setting annotation.

Yes, it is good idea to use HTTP HEAD method to get ETag and save the bandwidth. "$select" may reduce the data size but it may increase the loading of BMC while BMC is doing filtering.

HTTP HEAD relies on BMC support, and it seems to me that OpenBMC does not support HTTP HEAD yet. Let's put this to our to-do list and implement HTTP HEAD function while we have this support in BMC.

Thanks,
Nickle

> -----Original Message-----
> From: Chang, Abner <Abner.Chang@amd.com>
> Sent: Tuesday, November 28, 2023 12:05 PM
> To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io
> Cc: Igor Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@nvidia.com>
> Subject: RE: [edk2-redfish-client][PATCH 3/6] RedfishClientPkg: feature driver
> enhancement.
> 
> External email: Use caution opening links or attachments
> 
> 
> [AMD Official Use Only - General]
> 
> > -----Original Message-----
> > From: Nickle Wang <nicklew@nvidia.com>
> > Sent: Thursday, November 23, 2023 10:34 PM
> > To: devel@edk2.groups.io
> > Cc: Chang, Abner <Abner.Chang@amd.com>; Igor Kulchytskyy
> > <igork@ami.com>; Nick Ramirez <nramirez@nvidia.com>
> > Subject: [edk2-redfish-client][PATCH 3/6] RedfishClientPkg: feature
> > driver enhancement.
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > -Fix typo (include function RedfisSetRedfishUri and parameter
> > MaxLengthConfigLang)
> > -Add more debug message.
> > -Do not save ETag to variable every time when new ETag is created in
> > SetEtagWithUri().
> > -Introduce SetEtagFromUri() to get ETag from BMC and save it in
> > variable.
> > -Introduce GetPendingSettings() to check and see if Redfish pending
> > settings is supported or not.
> >
> > Signed-off-by: Nickle Wang <nicklew@nvidia.com>
> > Cc: Abner Chang <abner.chang@amd.com>
> > Cc: Igor Kulchytskyy <igork@ami.com>
> > Cc: Nick Ramirez <nramirez@nvidia.com>
> > ---
> >  .../Library/RedfishFeatureUtilityLib.h        |  81 ++++++--
> >  .../Features/Bios/v1_0_9/Common/BiosCommon.c  |   4 +-
> >  .../v1_5_0/Common/ComputerSystemCommon.c      |   4 +-
> >  .../Memory/V1_7_1/Common/MemoryCommon.c       |   4 +-
> >  .../RedfishFeatureUtilityLib.c                | 192 +++++++++++++++++-
> >  5 files changed, 253 insertions(+), 32 deletions(-)
> >
> > diff --git
> > a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > index e2f728b2..6347585c 100644
> > --- a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > +++ b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > @@ -22,7 +22,7 @@
> >
> >    Read redfish resource by given resource URI.
> >
> > -  @param[in]  Service       Redfish srvice instacne to make query.
> > +  @param[in]  Service       Redfish service instance to make query.
> >    @param[in]  ResourceUri   Target resource URI.
> >    @param[out] Response      HTTP response from redfish service.
> >
> > @@ -47,7 +47,7 @@ GetResourceByUri (
> >    @param[out] ArraySignatureClose       String to the close of array signature.
> >
> >    @retval     EFI_SUCCESS            Index is found.
> > -  @retval     EFI_NOT_FOUND          The non-array configure language string is
> > retured.
> > +  @retval     EFI_NOT_FOUND          The non-array configure language string is
> > returned.
> >    @retval     EFI_INVALID_PARAMETER  The format of input ConfigureLang is
> > wrong.
> >    @retval     Others                 Errors occur.
> >
> > @@ -118,10 +118,10 @@ GetNumberOfRedpathNodes (
> >
> >    @param[in]  NodeString             The node string to parse.
> >    @param[in]  Index                  Index of the node.
> > -  @param[out] EndOfNodePtr           Pointer to receive the poitner to
> > +  @param[out] EndOfNodePtr           Pointer to receive the pointer to
> >                                       the last character of node string.
> >
> > -  @retval     EFI_STRING             the begining of the node string.
> > +  @retval     EFI_STRING             the beginning of the node string.
> >
> >  **/
> >  EFI_STRING
> > @@ -140,7 +140,7 @@ GetRedpathNodeByIndex (
> >    @param[out] Index                 The array index number.
> >
> >    @retval     EFI_SUCCESS            Index is found.
> > -  @retval     EFI_NOT_FOUND          The non-array configure language string is
> > retured.
> > +  @retval     EFI_NOT_FOUND          The non-array configure language string is
> > returned.
> >    @retval     EFI_INVALID_PARAMETER  The format of input ConfigureLang is
> > wrong.
> >    @retval     Others                 Errors occur.
> >
> > @@ -188,9 +188,9 @@ DestroyConfiglanguageList (
> >
> >    @param[in]  DestConfigLang        Pointer to the node's configure language
> > string.
> >                                      The memory pointed by ConfigLang must be allocated
> > -                                    through memory allocation interface. Becasue we will
> > replace
> > +                                    through memory allocation
> > + interface. Because we will
> > replace
> >                                      the pointer in this function.
> > -  @param[in]  MaxtLengthConfigLang  The maximum length of ConfigLang.
> > +  @param[in]  MaxLengthConfigLang   The maximum length of ConfigLang.
> >    @param[in]  ConfigLangInstance    Pointer to Collection member instance.
> >
> >    @retval     EFI_SUCCESS     The instance is inserted to the configure language.
> > @@ -200,7 +200,7 @@ DestroyConfiglanguageList (  EFI_STATUS
> > SetResourceConfigLangMemberInstance (
> >    IN EFI_STRING                              *DestConfigLang,
> > -  IN UINTN                                   MaxtLengthConfigLang,
> > +  IN UINTN                                   MaxLengthConfigLang,
> >    IN REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG  *ConfigLangInstance
> >    );
> >
> > @@ -244,7 +244,7 @@ ApplyFeatureSettingsStringType (
> >
> >  /**
> >
> > -  Apply property value to UEFI HII database in numric type.
> > +  Apply property value to UEFI HII database in numeric type.
> >
> >    @param[in]  Schema        Property schema.
> >    @param[in]  Version       Property schema version.
> > @@ -356,7 +356,7 @@ ApplyFeatureSettingsNumericArrayType (
> >    @param[in]  Schema        Property schema.
> >    @param[in]  Version       Property schema version.
> >    @param[in]  ConfigureLang Configure language refers to this property.
> > -  @param[in]  ArrayHead     Head of Redfich CS boolean array value.
> > +  @param[in]  ArrayHead     Head of Redfish CS boolean array value.
> >
> >    @retval     EFI_SUCCESS     New value is applied successfully.
> >    @retval     Others          Errors occur.
> > @@ -421,19 +421,19 @@ CreatePayloadToPatchResource (
> >    @param[in]    ConfigLang        ConfigLang to save
> >    @param[in]    Uri               Redfish Uri to save
> >
> > -  @retval  EFI_INVALID_PARAMETR   SystemId is NULL or EMPTY
> > +  @retval  EFI_INVALID_PARAMETER  SystemId is NULL or EMPTY
> >    @retval  EFI_SUCCESS            Redfish uri is saved
> >
> >  **/
> >  EFI_STATUS
> > -RedfisSetRedfishUri (
> > +RedfishSetRedfishUri (
> >    IN    EFI_STRING  ConfigLang,
> >    IN    EFI_STRING  Uri
> >    );
> >
> >  /**
> >
> > -  Get the property name by given Configure Langauge.
> > +  Get the property name by given Configure Language.
> >
> >    @param[in]  ResourceUri              URI of root of resource.
> >    @param[in]  ConfigureLang            Configure Language string.
> > @@ -527,6 +527,25 @@ PropertyChecker (
> >    IN BOOLEAN  ProvisionMode
> >    );
> >
> > +/**
> > +  This function query ETag from given URI string and keep it in database.
> > +
> > +  @param[in]  Service               Redfish service instance to make query.
> > +  @param[in]  Uri                   URI to query ETag.
> > +  @param[in]  CheckPendingSettings  Set this to true and
> > + @Redfish.Settings
> > will
> > +                                    be handled together. FALSE otherwise.
> > +
> > +  @retval     EFI_SUCCESS     ETAG and URI are applied successfully.
> > +  @retval     Others          Errors occur.
> > +
> > +**/
> > +EFI_STATUS
> > +SetEtagFromUri (
> > +  IN  REDFISH_SERVICE  *RedfishService,
> > +  IN  EFI_STRING       Uri,
> > +  IN  BOOLEAN          CheckPendingSettings
> > +  );
> > +
> >  /**
> >
> >    Keep ETAG string and URI string in database.
> > @@ -576,7 +595,7 @@ GetOdataId (
> >
> >  /**
> >
> > -  Return config language from given URI and prperty name. It's call
> > responsibility to release returned buffer.
> > +  Return config language from given URI and property name. It's call
> > responsibility to release returned buffer.
> >
> >    @param[in] Uri            The URI to match
> >    @param[in] PropertyName   The property name of resource. This is optional.
> > @@ -790,7 +809,7 @@ MatchPropertyWithJsonContext (
> >
> >  /**
> >
> > -  Create string array and append to arry node in Redfish JSON convert format.
> > +  Create string array and append to array node in Redfish JSON
> > + convert
> > format.
> >
> >    @param[in,out]  Head          The head of string array.
> >    @param[in]      StringArray   Input string array.
> > @@ -809,7 +828,7 @@ AddRedfishCharArray (
> >
> >  /**
> >
> > -  Create numeric array and append to arry node in Redfish JSON
> > convert format.
> > +  Create numeric array and append to array node in Redfish JSON
> > + convert
> > format.
> >
> >    @param[in,out]  Head           The head of string array.
> >    @param[in]      NumericArray   Input numeric array.
> > @@ -828,7 +847,7 @@ AddRedfishNumericArray (
> >
> >  /**
> >
> > -  Create boolean array and append to arry node in Redfish JSON
> > convert format.
> > +  Create boolean array and append to array node in Redfish JSON
> > + convert
> > format.
> >
> >    @param[in,out]  Head           The head of string array.
> >    @param[in]      BooleanArray   Input boolean array.
> > @@ -871,7 +890,7 @@ CompareRedfishStringArrayValues (
> >    Check and see if value in Redfish numeric array are all the same as the one
> >    from HII configuration.
> >
> > -  @param[in]  Head          The head of Redfish CS numeraic array.
> > +  @param[in]  Head          The head of Redfish CS numeric array.
> >    @param[in]  NumericArray  Input numeric array.
> >    @param[in]  ArraySize     The size of NumericArray.
> >
> > @@ -914,9 +933,9 @@ CompareRedfishBooleanArrayValues (
> >    This is just a simple check.
> >
> >    @param[in]  RedfishVagueKeyValuePtr     The vague key value sets on
> > Redfish service.
> > -  @param[in]  RedfishVagueKeyValueNumber  The numebr of vague key
> > value sets
> > +  @param[in]  RedfishVagueKeyValueNumber  The number of vague key
> > value sets
> >    @param[in]  ConfigVagueKeyValuePtr      The vague configuration on
> > platform.
> > -  @param[in]  ConfigVagueKeyValueNumber   The numebr of vague key value
> > sets
> > +  @param[in]  ConfigVagueKeyValueNumber   The number of vague key value
> > sets
> >
> >    @retval     TRUE          All values are the same.
> >                FALSE         There is some difference.
> > @@ -949,4 +968,26 @@ GetEtagAndLocation (
> >    OUT EFI_STRING        *Location    OPTIONAL
> >    );
> >
> > +/**
> > +
> > +  Check and see if "@Redfish.Settings" exist in given Payload. If
> > + found, return
> > the
> > +  payload and URI to pending settings. Caller has to release "SettingPayload"
> > and
> > +  "SettingUri".
> > +
> 
> > +  @param[in]  Payload         Payload that may contain "@Redfish.Settings"
> > +  @param[out] SettingPayload  Payload keeps pending settings.
> > +  @param[out] SettingUri      URI to pending settings.
> 
> 
> Missed Doxgen tag for RedfishService.
> Please also update the function header of GetPendingSettings in C source file.
> 
> > +
> > +  @retval     EFI_SUCCESS     Pending settings is found and returned.
> > +  @retval     Others          Error happens
> > +
> > +**/
> > +EFI_STATUS
> > +GetPendingSettings (
> > +  IN  REDFISH_SERVICE   RedfishService,
> > +  IN  REDFISH_PAYLOAD   Payload,
> > +  OUT REDFISH_RESPONSE  *SettingResponse,
> > +  OUT EFI_STRING        *SettingUri
> > +  );
> > +
> >  #endif
> > diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> > b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> > index f96c90cc..98288d66 100644
> > --- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> > +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> > @@ -338,7 +338,7 @@ ProvisioningBiosResource (
> >    // Keep location of new resource.
> >    //
> >    if (NewResourceLocation != NULL) {
> > -    RedfisSetRedfishUri (ConfigureLang, NewResourceLocation);
> > +    RedfishSetRedfishUri (ConfigureLang, NewResourceLocation);
> >    }
> >
> >    //
> > @@ -745,7 +745,7 @@ RedfishIdentifyResourceCommon (
> >      //
> >      // Keep URI and ConfigLang mapping
> >      //
> > -    RedfisSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private->Uri);
> > +    RedfishSetRedfishUri (ConfigLangList.List[0].ConfigureLang,
> > + Private->Uri);
> >      //
> >      // Set the configuration language in the
> > RESOURCE_INFORMATION_EXCHANGE.
> >      // This information is sent back to the parent resource (e.g. the
> > collection driver).
> > diff --git
> > a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/Computer
> > SystemCommon.c
> > b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/Computer
> > SystemCommon.c
> > index 7ed1bd55..1ffb7d1d 100644
> > ---
> > a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/Computer
> > SystemCommon.c
> > +++
> > b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/Computer
> > SystemCommon.c
> > @@ -1274,7 +1274,7 @@ ProvisioningComputerSystemResource (
> >    // Keep location of new resource.
> >    //
> >    if (NewResourceLocation != NULL) {
> > -    RedfisSetRedfishUri (ConfigureLang, NewResourceLocation);
> > +    RedfishSetRedfishUri (ConfigureLang, NewResourceLocation);
> >    }
> >
> >    //
> > @@ -1621,7 +1621,7 @@ RedfishIdentifyResourceCommon (
> >      //
> >      // Keep URI and ConfigLang mapping
> >      //
> > -    RedfisSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private->Uri);
> > +    RedfishSetRedfishUri (ConfigLangList.List[0].ConfigureLang,
> > + Private->Uri);
> >      //
> >      // Set the configuration language in the
> > RESOURCE_INFORMATION_EXCHANGE.
> >      // This information is sent back to the parent resource (e.g. the
> > collection driver).
> > diff --git
> > a/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
> > b/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.
> > c
> > index b7434036..4c41f16b 100644
> > ---
> > a/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
> > +++
> > b/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.
> > c
> > @@ -2190,7 +2190,7 @@ ProvisioningMemoryResource (
> >    // Keep location of new resource.
> >    //
> >    if (NewResourceLocation != NULL) {
> > -    RedfisSetRedfishUri (ConfigureLang, NewResourceLocation);
> > +    RedfishSetRedfishUri (ConfigureLang, NewResourceLocation);
> >    }
> >
> >    //
> > @@ -2537,7 +2537,7 @@ RedfishIdentifyResourceCommon (
> >      //
> >      // Keep URI and ConfigLang mapping
> >      //
> > -    RedfisSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private->Uri);
> > +    RedfishSetRedfishUri (ConfigLangList.List[0].ConfigureLang,
> > + Private->Uri);
> >      //
> >      // Set the configuration language in the
> > RESOURCE_INFORMATION_EXCHANGE.
> >      // This information is sent back to the parent resource (e.g. the
> > collection driver).
> > diff --git
> > a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > c
> > b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > c
> > index 13e29902..753cd7b2 100644
> > ---
> > a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > c
> > +++
> > b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > c
> > @@ -107,6 +107,119 @@ GetArraykeyFromUri (
> >    return EFI_SUCCESS;
> >  }
> >
> > +/**
> > +
> > +  This function query ETag from given URI string and keep it in database.
> > +
> > +  @param[in]  Service               Redfish service instance to make query.
> > +  @param[in]  Uri                   URI to query ETag.
> > +  @param[in]  CheckPendingSettings  Set this to true and
> > + @Redfish.Settings
> > will
> > +                                    be handled together. FALSE otherwise.
> > +
> > +  @retval     EFI_SUCCESS     ETAG and URI are applied successfully.
> > +  @retval     Others          Errors occur.
> > +
> > +**/
> > +EFI_STATUS
> > +SetEtagFromUri (
> > +  IN  REDFISH_SERVICE  *RedfishService,
> > +  IN  EFI_STRING       Uri,
> > +  IN  BOOLEAN          CheckPendingSettings
> > +  )
> > +{
> > +  EFI_STATUS        Status;
> > +  CHAR8             *Etag;
> > +  CHAR8             *AsciiUri;
> > +  REDFISH_RESPONSE  Response;
> > +  EFI_STRING        PendingSettingUri;
> > +
> > +  if ((RedfishService == NULL) || IS_EMPTY_STRING (Uri)) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  AsciiUri          = NULL;
> > +  Etag              = NULL;
> > +  PendingSettingUri = NULL;
> > +
> > +  Status = RedfishLocateProtocol ((VOID **)&mEtagProtocol,
> > &gEdkIIRedfishETagProtocolGuid);
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "%a: fail to locate
> > gEdkIIRedfishETagProtocolGuid: %r\n", __func__, Status));
> > +    return Status;
> > +  }
> > +
> > +  Status = GetResourceByUri (RedfishService, Uri, &Response);
> This may spend some time on network to get the whole resource from URI if the
> resource size is big, but we just need the information of ETAG header. We can
> think of using HEAD HTTP method.
> However, this seems fine for now as the following code gets the pending settings
> from the same HTTP response body retrieved above.
> The worst case would be there is no pending setting, however we waste network
> bandwidth to get the whole response body. Not sure if HTTP query $select can
> improve this or not for retrieving pending setting annotation.
> 
> Abner
> 
> 
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "%a: get resource from: %s failed\n",
> > + __func__,
> > Uri));
> > +    return Status;
> > +  }
> > +
> > +  //
> > +  // 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 = EFI_NOT_FOUND;
> > +    goto ON_RELEASE;
> > +  }
> > +
> > +  AsciiUri = StrUnicodeToAscii (Uri);  if (AsciiUri == NULL) {
> > +    Status = EFI_OUT_OF_RESOURCES;
> > +    goto ON_RELEASE;
> > +  }
> > +
> > +  //
> > +  // Save ETag to variable.
> > +  //
> > +  Status = mEtagProtocol->Set (mEtagProtocol, AsciiUri, Etag);  if
> > + (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "%a: failed to set ETag %a -> %a\n",
> > + __func__,
> > Etag, AsciiUri));
> > +  }
> > +
> > +  if (CheckPendingSettings && (Response.Payload != NULL)) {
> > +    //
> > +    // Check to see if there is @Redfish.Settings
> > +    //
> > +    Status = GetPendingSettings (
> > +               RedfishService,
> > +               Response.Payload,
> > +               NULL,
> > +               &PendingSettingUri
> > +               );
> > +    if (!EFI_ERROR (Status)) {
> > +      DEBUG ((REDFISH_DEBUG_TRACE, "%a: @Redfish.Settings found:
> > + %s\n",
> > __func__, PendingSettingUri));
> > +
> > +      Status = SetEtagFromUri (RedfishService, PendingSettingUri, FALSE);
> > +    }
> > +  }
> > +
> > +ON_RELEASE:
> > +
> > +  if (AsciiUri != NULL) {
> > +    FreePool (AsciiUri);
> > +  }
> > +
> > +  if (Etag != NULL) {
> > +    FreePool (Etag);
> > +  }
> > +
> > +  if (PendingSettingUri != NULL) {
> > +    FreePool (PendingSettingUri);
> > +  }
> > +
> > +  if (Response.Payload != NULL) {
> > +    RedfishFreeResponse (
> > +      Response.StatusCode,
> > +      Response.HeaderCount,
> > +      Response.Headers,
> > +      Response.Payload
> > +      );
> > +  }
> > +
> > +  return Status;
> > +}
> > +
> >  /**
> >
> >    Keep ETAG string and URI string in database.
> > @@ -143,7 +256,6 @@ SetEtagWithUri (
> >    }
> >
> >    mEtagProtocol->Set (mEtagProtocol, AsciiUri, EtagStr);
> > -  mEtagProtocol->Flush (mEtagProtocol);
> >
> >    FreePool (AsciiUri);
> >
> > @@ -505,6 +617,8 @@ ApplyFeatureSettingsVagueType (
> >      return EFI_INVALID_PARAMETER;
> >    }
> >
> > +  DEBUG ((REDFISH_DEBUG_TRACE, "%a: schema: %a %a config lang: %s
> > NumberOfVagueValues: %d\n", __func__, Schema, Version, ConfigureLang,
> > NumberOfVagueValues));
> > +
> >    ConfigureLangAscii = AllocatePool (StrLen (ConfigureLang) + 1);
> >    if (ConfigureLangAscii == NULL) {
> >      Status = EFI_OUT_OF_RESOURCES;
> > @@ -1473,7 +1587,7 @@ DestroyConfiglanguageList (
> >                                      The memory pointed by ConfigLang must be allocated
> >                                      through memory allocation
> > interface. Because we will replace
> >                                      the pointer in this function.
> > -  @param[in]  MaxtLengthConfigLang  The maximum length of ConfigLang.
> > +  @param[in]  MaxLengthConfigLang   The maximum length of ConfigLang.
> >    @param[in]  ConfigLangInstance    Pointer to Collection member instance.
> >
> >    @retval     EFI_SUCCESS     The instance is inserted to the configure language.
> > @@ -1483,7 +1597,7 @@ DestroyConfiglanguageList (  EFI_STATUS
> > SetResourceConfigLangMemberInstance (
> >    IN EFI_STRING                              *DestConfigLang,
> > -  IN UINTN                                   MaxtLengthConfigLang,
> > +  IN UINTN                                   MaxLengthConfigLang,
> >    IN REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG  *ConfigLangInstance
> >    )
> >  {
> > @@ -1508,14 +1622,14 @@ SetResourceConfigLangMemberInstance (
> >      if (ConfigLangInstance->ConfigureLang == NULL) {
> >        return EFI_INVALID_PARAMETER;
> >      } else {
> > -      StrCatS (*DestConfigLang, MaxtLengthConfigLang, ConfigLangInstance-
> > >ConfigureLang);
> > +      StrCatS (*DestConfigLang, MaxLengthConfigLang,
> > + ConfigLangInstance-
> > >ConfigureLang);
> >        return EFI_SUCCESS;
> >      }
> >    }
> >
> >    MaxStrLength  = StrSize (ThisConfigLang) + StrSize
> > ((EFI_STRING)&InstanceStr);
> >    NewConfigLang = ThisConfigLang;
> > -  if (MaxtLengthConfigLang < MaxStrLength) {
> > +  if (MaxLengthConfigLang < MaxStrLength) {
> >      NewConfigLang = (EFI_STRING)AllocateZeroPool (MaxStrLength);
> >      if (NewConfigLang == NULL) {
> >        DEBUG ((DEBUG_ERROR, "%a: Fail to allocate memory for
> > NewConfigLang.\n", __func__)); @@ -2191,7 +2305,7 @@ GetConfigureLang
> > (
> >
> >  **/
> >  EFI_STATUS
> > -RedfisSetRedfishUri (
> > +RedfishSetRedfishUri (
> >    IN    EFI_STRING  ConfigLang,
> >    IN    EFI_STRING  Uri
> >    )
> > @@ -3524,6 +3638,72 @@ CompareRedfishBooleanArrayValues (
> >    return TRUE;
> >  }
> >
> > +/**
> > +
> > +  Check and see if "@Redfish.Settings" exist in given Payload. If
> > + found, return
> > the
> > +  payload and URI to pending settings. Caller has to release "SettingPayload"
> > and
> > +  "SettingUri".
> > +
> > +  @param[in]  Payload         Payload that may contain "@Redfish.Settings"
> > +  @param[out] SettingPayload  Payload keeps pending settings.
> > +  @param[out] SettingUri      URI to pending settings.
> > +
> > +  @retval     EFI_SUCCESS     Pending settings is found and returned.
> > +  @retval     Others          Error happens
> > +
> > +**/
> > +EFI_STATUS
> > +GetPendingSettings (
> > +  IN  REDFISH_SERVICE   RedfishService,
> > +  IN  REDFISH_PAYLOAD   Payload,
> > +  OUT REDFISH_RESPONSE  *SettingResponse,
> > +  OUT EFI_STRING        *SettingUri
> > +  )
> > +{
> > +  CONST CHAR8       *RedfishSettingsUriKeys[] = { "@Redfish.Settings",
> > "SettingsObject", "@odata.id" };
> > +  EDKII_JSON_VALUE  JsonValue;
> > +  UINTN             Index;
> > +  EFI_STATUS        Status;
> > +
> > +  if ((RedfishService == NULL) || (Payload == NULL) ||
> > + (SettingResponse ==
> > NULL) || (SettingUri == NULL)) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  *SettingUri = NULL;
> > +  JsonValue   = RedfishJsonInPayload (Payload);
> > +
> > +  //
> > +  // Seeking RedfishSettings URI link.
> > +  //
> > +  for (Index = 0; Index < ARRAY_SIZE (RedfishSettingsUriKeys); Index++) {
> > +    if (JsonValue == NULL) {
> > +      break;
> > +    }
> > +
> > +    JsonValue = JsonObjectGetValue (JsonValueGetObject (JsonValue),
> > RedfishSettingsUriKeys[Index]);
> > +  }
> > +
> > +  if (JsonValue != NULL) {
> > +    //
> > +    // Verify RedfishSettings URI link is valid to retrieve resource or not.
> > +    //
> > +    *SettingUri = JsonValueGetUnicodeString (JsonValue);
> > +    if (*SettingUri == NULL) {
> > +      return EFI_NOT_FOUND;
> > +    }
> > +
> > +    Status = GetResourceByUri (RedfishService, *SettingUri, SettingResponse);
> > +    if (EFI_ERROR (Status)) {
> > +      DEBUG ((DEBUG_ERROR, "%a: @Redfish.Settings exists, get
> > + resource
> > from: %s failed: %r\n", __func__, *SettingUri, Status));
> > +      return Status;
> > +    }
> > +
> > +    return EFI_SUCCESS;
> > +  }
> > +
> > +  return EFI_NOT_FOUND;
> > +}
> > +
> >  /**
> >
> >    Check and see if any difference between two vague value set.
> > --
> > 2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111797): https://edk2.groups.io/g/devel/message/111797
Mute This Topic: https://groups.io/mt/102767544/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 3/6] RedfishClientPkg: feature driver enhancement.
  2023-11-28  6:44   ` Nickle Wang via groups.io
@ 2023-11-28  7:08     ` Chang, Abner via groups.io
  0 siblings, 0 replies; 4+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-28  7:08 UTC (permalink / raw)
  To: Nickle Wang, devel@edk2.groups.io; +Cc: Igor Kulchytskyy, Nick Ramirez

[AMD Official Use Only - General]

Thanks Nickle, I just created BZ #4608 to track this TODO item.

For this patch,
Reviewed-by: Abner Chang <abner.chang@amd.com>

> -----Original Message-----
> From: Nickle Wang <nicklew@nvidia.com>
> Sent: Tuesday, November 28, 2023 2:44 PM
> To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
> Cc: Igor Kulchytskyy <igork@ami.com>; Nick Ramirez <nramirez@nvidia.com>
> Subject: RE: [edk2-redfish-client][PATCH 3/6] RedfishClientPkg: feature driver
> enhancement.
>
> [AMD Official Use Only - General]
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Hi Abner,
>
> Thanks for your review. Please find my comment below.
>
> > This may spend some time on network to get the whole resource from URI if
> the
> > resource size is big, but we just need the information of ETAG header. We can
> > think of using HEAD HTTP method.
> > However, this seems fine for now as the following code gets the pending
> settings
> > from the same HTTP response body retrieved above.
> > The worst case would be there is no pending setting, however we waste
> network
> > bandwidth to get the whole response body. Not sure if HTTP query $select
> can
> > improve this or not for retrieving pending setting annotation.
>
> Yes, it is good idea to use HTTP HEAD method to get ETag and save the
> bandwidth. "$select" may reduce the data size but it may increase the loading
> of BMC while BMC is doing filtering.
>
> HTTP HEAD relies on BMC support, and it seems to me that OpenBMC does
> not support HTTP HEAD yet. Let's put this to our to-do list and implement
> HTTP HEAD function while we have this support in BMC.
>
> Thanks,
> Nickle
>
> > -----Original Message-----
> > From: Chang, Abner <Abner.Chang@amd.com>
> > Sent: Tuesday, November 28, 2023 12:05 PM
> > To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io
> > Cc: Igor Kulchytskyy <igork@ami.com>; Nick Ramirez
> <nramirez@nvidia.com>
> > Subject: RE: [edk2-redfish-client][PATCH 3/6] RedfishClientPkg: feature
> driver
> > enhancement.
> >
> > External email: Use caution opening links or attachments
> >
> >
> > [AMD Official Use Only - General]
> >
> > > -----Original Message-----
> > > From: Nickle Wang <nicklew@nvidia.com>
> > > Sent: Thursday, November 23, 2023 10:34 PM
> > > To: devel@edk2.groups.io
> > > Cc: Chang, Abner <Abner.Chang@amd.com>; Igor Kulchytskyy
> > > <igork@ami.com>; Nick Ramirez <nramirez@nvidia.com>
> > > Subject: [edk2-redfish-client][PATCH 3/6] RedfishClientPkg: feature
> > > driver enhancement.
> > >
> > > Caution: This message originated from an External Source. Use proper
> > > caution when opening attachments, clicking links, or responding.
> > >
> > >
> > > -Fix typo (include function RedfisSetRedfishUri and parameter
> > > MaxLengthConfigLang)
> > > -Add more debug message.
> > > -Do not save ETag to variable every time when new ETag is created in
> > > SetEtagWithUri().
> > > -Introduce SetEtagFromUri() to get ETag from BMC and save it in
> > > variable.
> > > -Introduce GetPendingSettings() to check and see if Redfish pending
> > > settings is supported or not.
> > >
> > > Signed-off-by: Nickle Wang <nicklew@nvidia.com>
> > > Cc: Abner Chang <abner.chang@amd.com>
> > > Cc: Igor Kulchytskyy <igork@ami.com>
> > > Cc: Nick Ramirez <nramirez@nvidia.com>
> > > ---
> > >  .../Library/RedfishFeatureUtilityLib.h        |  81 ++++++--
> > >  .../Features/Bios/v1_0_9/Common/BiosCommon.c  |   4 +-
> > >  .../v1_5_0/Common/ComputerSystemCommon.c      |   4 +-
> > >  .../Memory/V1_7_1/Common/MemoryCommon.c       |   4 +-
> > >  .../RedfishFeatureUtilityLib.c                | 192 +++++++++++++++++-
> > >  5 files changed, 253 insertions(+), 32 deletions(-)
> > >
> > > diff --git
> > > a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > > b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > > index e2f728b2..6347585c 100644
> > > --- a/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > > +++ b/RedfishClientPkg/Include/Library/RedfishFeatureUtilityLib.h
> > > @@ -22,7 +22,7 @@
> > >
> > >    Read redfish resource by given resource URI.
> > >
> > > -  @param[in]  Service       Redfish srvice instacne to make query.
> > > +  @param[in]  Service       Redfish service instance to make query.
> > >    @param[in]  ResourceUri   Target resource URI.
> > >    @param[out] Response      HTTP response from redfish service.
> > >
> > > @@ -47,7 +47,7 @@ GetResourceByUri (
> > >    @param[out] ArraySignatureClose       String to the close of array
> signature.
> > >
> > >    @retval     EFI_SUCCESS            Index is found.
> > > -  @retval     EFI_NOT_FOUND          The non-array configure language string
> is
> > > retured.
> > > +  @retval     EFI_NOT_FOUND          The non-array configure language string
> is
> > > returned.
> > >    @retval     EFI_INVALID_PARAMETER  The format of input ConfigureLang
> is
> > > wrong.
> > >    @retval     Others                 Errors occur.
> > >
> > > @@ -118,10 +118,10 @@ GetNumberOfRedpathNodes (
> > >
> > >    @param[in]  NodeString             The node string to parse.
> > >    @param[in]  Index                  Index of the node.
> > > -  @param[out] EndOfNodePtr           Pointer to receive the poitner to
> > > +  @param[out] EndOfNodePtr           Pointer to receive the pointer to
> > >                                       the last character of node string.
> > >
> > > -  @retval     EFI_STRING             the begining of the node string.
> > > +  @retval     EFI_STRING             the beginning of the node string.
> > >
> > >  **/
> > >  EFI_STRING
> > > @@ -140,7 +140,7 @@ GetRedpathNodeByIndex (
> > >    @param[out] Index                 The array index number.
> > >
> > >    @retval     EFI_SUCCESS            Index is found.
> > > -  @retval     EFI_NOT_FOUND          The non-array configure language string
> is
> > > retured.
> > > +  @retval     EFI_NOT_FOUND          The non-array configure language string
> is
> > > returned.
> > >    @retval     EFI_INVALID_PARAMETER  The format of input ConfigureLang
> is
> > > wrong.
> > >    @retval     Others                 Errors occur.
> > >
> > > @@ -188,9 +188,9 @@ DestroyConfiglanguageList (
> > >
> > >    @param[in]  DestConfigLang        Pointer to the node's configure language
> > > string.
> > >                                      The memory pointed by ConfigLang must be allocated
> > > -                                    through memory allocation interface. Becasue we will
> > > replace
> > > +                                    through memory allocation
> > > + interface. Because we will
> > > replace
> > >                                      the pointer in this function.
> > > -  @param[in]  MaxtLengthConfigLang  The maximum length of
> ConfigLang.
> > > +  @param[in]  MaxLengthConfigLang   The maximum length of
> ConfigLang.
> > >    @param[in]  ConfigLangInstance    Pointer to Collection member instance.
> > >
> > >    @retval     EFI_SUCCESS     The instance is inserted to the configure
> language.
> > > @@ -200,7 +200,7 @@ DestroyConfiglanguageList (  EFI_STATUS
> > > SetResourceConfigLangMemberInstance (
> > >    IN EFI_STRING                              *DestConfigLang,
> > > -  IN UINTN                                   MaxtLengthConfigLang,
> > > +  IN UINTN                                   MaxLengthConfigLang,
> > >    IN REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG  *ConfigLangInstance
> > >    );
> > >
> > > @@ -244,7 +244,7 @@ ApplyFeatureSettingsStringType (
> > >
> > >  /**
> > >
> > > -  Apply property value to UEFI HII database in numric type.
> > > +  Apply property value to UEFI HII database in numeric type.
> > >
> > >    @param[in]  Schema        Property schema.
> > >    @param[in]  Version       Property schema version.
> > > @@ -356,7 +356,7 @@ ApplyFeatureSettingsNumericArrayType (
> > >    @param[in]  Schema        Property schema.
> > >    @param[in]  Version       Property schema version.
> > >    @param[in]  ConfigureLang Configure language refers to this property.
> > > -  @param[in]  ArrayHead     Head of Redfich CS boolean array value.
> > > +  @param[in]  ArrayHead     Head of Redfish CS boolean array value.
> > >
> > >    @retval     EFI_SUCCESS     New value is applied successfully.
> > >    @retval     Others          Errors occur.
> > > @@ -421,19 +421,19 @@ CreatePayloadToPatchResource (
> > >    @param[in]    ConfigLang        ConfigLang to save
> > >    @param[in]    Uri               Redfish Uri to save
> > >
> > > -  @retval  EFI_INVALID_PARAMETR   SystemId is NULL or EMPTY
> > > +  @retval  EFI_INVALID_PARAMETER  SystemId is NULL or EMPTY
> > >    @retval  EFI_SUCCESS            Redfish uri is saved
> > >
> > >  **/
> > >  EFI_STATUS
> > > -RedfisSetRedfishUri (
> > > +RedfishSetRedfishUri (
> > >    IN    EFI_STRING  ConfigLang,
> > >    IN    EFI_STRING  Uri
> > >    );
> > >
> > >  /**
> > >
> > > -  Get the property name by given Configure Langauge.
> > > +  Get the property name by given Configure Language.
> > >
> > >    @param[in]  ResourceUri              URI of root of resource.
> > >    @param[in]  ConfigureLang            Configure Language string.
> > > @@ -527,6 +527,25 @@ PropertyChecker (
> > >    IN BOOLEAN  ProvisionMode
> > >    );
> > >
> > > +/**
> > > +  This function query ETag from given URI string and keep it in database.
> > > +
> > > +  @param[in]  Service               Redfish service instance to make query.
> > > +  @param[in]  Uri                   URI to query ETag.
> > > +  @param[in]  CheckPendingSettings  Set this to true and
> > > + @Redfish.Settings
> > > will
> > > +                                    be handled together. FALSE otherwise.
> > > +
> > > +  @retval     EFI_SUCCESS     ETAG and URI are applied successfully.
> > > +  @retval     Others          Errors occur.
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +SetEtagFromUri (
> > > +  IN  REDFISH_SERVICE  *RedfishService,
> > > +  IN  EFI_STRING       Uri,
> > > +  IN  BOOLEAN          CheckPendingSettings
> > > +  );
> > > +
> > >  /**
> > >
> > >    Keep ETAG string and URI string in database.
> > > @@ -576,7 +595,7 @@ GetOdataId (
> > >
> > >  /**
> > >
> > > -  Return config language from given URI and prperty name. It's call
> > > responsibility to release returned buffer.
> > > +  Return config language from given URI and property name. It's call
> > > responsibility to release returned buffer.
> > >
> > >    @param[in] Uri            The URI to match
> > >    @param[in] PropertyName   The property name of resource. This is
> optional.
> > > @@ -790,7 +809,7 @@ MatchPropertyWithJsonContext (
> > >
> > >  /**
> > >
> > > -  Create string array and append to arry node in Redfish JSON convert
> format.
> > > +  Create string array and append to array node in Redfish JSON
> > > + convert
> > > format.
> > >
> > >    @param[in,out]  Head          The head of string array.
> > >    @param[in]      StringArray   Input string array.
> > > @@ -809,7 +828,7 @@ AddRedfishCharArray (
> > >
> > >  /**
> > >
> > > -  Create numeric array and append to arry node in Redfish JSON
> > > convert format.
> > > +  Create numeric array and append to array node in Redfish JSON
> > > + convert
> > > format.
> > >
> > >    @param[in,out]  Head           The head of string array.
> > >    @param[in]      NumericArray   Input numeric array.
> > > @@ -828,7 +847,7 @@ AddRedfishNumericArray (
> > >
> > >  /**
> > >
> > > -  Create boolean array and append to arry node in Redfish JSON
> > > convert format.
> > > +  Create boolean array and append to array node in Redfish JSON
> > > + convert
> > > format.
> > >
> > >    @param[in,out]  Head           The head of string array.
> > >    @param[in]      BooleanArray   Input boolean array.
> > > @@ -871,7 +890,7 @@ CompareRedfishStringArrayValues (
> > >    Check and see if value in Redfish numeric array are all the same as the one
> > >    from HII configuration.
> > >
> > > -  @param[in]  Head          The head of Redfish CS numeraic array.
> > > +  @param[in]  Head          The head of Redfish CS numeric array.
> > >    @param[in]  NumericArray  Input numeric array.
> > >    @param[in]  ArraySize     The size of NumericArray.
> > >
> > > @@ -914,9 +933,9 @@ CompareRedfishBooleanArrayValues (
> > >    This is just a simple check.
> > >
> > >    @param[in]  RedfishVagueKeyValuePtr     The vague key value sets on
> > > Redfish service.
> > > -  @param[in]  RedfishVagueKeyValueNumber  The numebr of vague key
> > > value sets
> > > +  @param[in]  RedfishVagueKeyValueNumber  The number of vague key
> > > value sets
> > >    @param[in]  ConfigVagueKeyValuePtr      The vague configuration on
> > > platform.
> > > -  @param[in]  ConfigVagueKeyValueNumber   The numebr of vague key
> value
> > > sets
> > > +  @param[in]  ConfigVagueKeyValueNumber   The number of vague key
> value
> > > sets
> > >
> > >    @retval     TRUE          All values are the same.
> > >                FALSE         There is some difference.
> > > @@ -949,4 +968,26 @@ GetEtagAndLocation (
> > >    OUT EFI_STRING        *Location    OPTIONAL
> > >    );
> > >
> > > +/**
> > > +
> > > +  Check and see if "@Redfish.Settings" exist in given Payload. If
> > > + found, return
> > > the
> > > +  payload and URI to pending settings. Caller has to release
> "SettingPayload"
> > > and
> > > +  "SettingUri".
> > > +
> >
> > > +  @param[in]  Payload         Payload that may contain "@Redfish.Settings"
> > > +  @param[out] SettingPayload  Payload keeps pending settings.
> > > +  @param[out] SettingUri      URI to pending settings.
> >
> >
> > Missed Doxgen tag for RedfishService.
> > Please also update the function header of GetPendingSettings in C source
> file.
> >
> > > +
> > > +  @retval     EFI_SUCCESS     Pending settings is found and returned.
> > > +  @retval     Others          Error happens
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +GetPendingSettings (
> > > +  IN  REDFISH_SERVICE   RedfishService,
> > > +  IN  REDFISH_PAYLOAD   Payload,
> > > +  OUT REDFISH_RESPONSE  *SettingResponse,
> > > +  OUT EFI_STRING        *SettingUri
> > > +  );
> > > +
> > >  #endif
> > > diff --git
> a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> > > b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> > > index f96c90cc..98288d66 100644
> > > --- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> > > +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> > > @@ -338,7 +338,7 @@ ProvisioningBiosResource (
> > >    // Keep location of new resource.
> > >    //
> > >    if (NewResourceLocation != NULL) {
> > > -    RedfisSetRedfishUri (ConfigureLang, NewResourceLocation);
> > > +    RedfishSetRedfishUri (ConfigureLang, NewResourceLocation);
> > >    }
> > >
> > >    //
> > > @@ -745,7 +745,7 @@ RedfishIdentifyResourceCommon (
> > >      //
> > >      // Keep URI and ConfigLang mapping
> > >      //
> > > -    RedfisSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private-
> >Uri);
> > > +    RedfishSetRedfishUri (ConfigLangList.List[0].ConfigureLang,
> > > + Private->Uri);
> > >      //
> > >      // Set the configuration language in the
> > > RESOURCE_INFORMATION_EXCHANGE.
> > >      // This information is sent back to the parent resource (e.g. the
> > > collection driver).
> > > diff --git
> > >
> a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/Computer
> > > SystemCommon.c
> > >
> b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/Computer
> > > SystemCommon.c
> > > index 7ed1bd55..1ffb7d1d 100644
> > > ---
> > >
> a/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/Computer
> > > SystemCommon.c
> > > +++
> > >
> b/RedfishClientPkg/Features/ComputerSystem/v1_5_0/Common/Computer
> > > SystemCommon.c
> > > @@ -1274,7 +1274,7 @@ ProvisioningComputerSystemResource (
> > >    // Keep location of new resource.
> > >    //
> > >    if (NewResourceLocation != NULL) {
> > > -    RedfisSetRedfishUri (ConfigureLang, NewResourceLocation);
> > > +    RedfishSetRedfishUri (ConfigureLang, NewResourceLocation);
> > >    }
> > >
> > >    //
> > > @@ -1621,7 +1621,7 @@ RedfishIdentifyResourceCommon (
> > >      //
> > >      // Keep URI and ConfigLang mapping
> > >      //
> > > -    RedfisSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private-
> >Uri);
> > > +    RedfishSetRedfishUri (ConfigLangList.List[0].ConfigureLang,
> > > + Private->Uri);
> > >      //
> > >      // Set the configuration language in the
> > > RESOURCE_INFORMATION_EXCHANGE.
> > >      // This information is sent back to the parent resource (e.g. the
> > > collection driver).
> > > diff --git
> > >
> a/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
> > >
> b/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.
> > > c
> > > index b7434036..4c41f16b 100644
> > > ---
> > >
> a/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.c
> > > +++
> > >
> b/RedfishClientPkg/Features/Memory/V1_7_1/Common/MemoryCommon.
> > > c
> > > @@ -2190,7 +2190,7 @@ ProvisioningMemoryResource (
> > >    // Keep location of new resource.
> > >    //
> > >    if (NewResourceLocation != NULL) {
> > > -    RedfisSetRedfishUri (ConfigureLang, NewResourceLocation);
> > > +    RedfishSetRedfishUri (ConfigureLang, NewResourceLocation);
> > >    }
> > >
> > >    //
> > > @@ -2537,7 +2537,7 @@ RedfishIdentifyResourceCommon (
> > >      //
> > >      // Keep URI and ConfigLang mapping
> > >      //
> > > -    RedfisSetRedfishUri (ConfigLangList.List[0].ConfigureLang, Private-
> >Uri);
> > > +    RedfishSetRedfishUri (ConfigLangList.List[0].ConfigureLang,
> > > + Private->Uri);
> > >      //
> > >      // Set the configuration language in the
> > > RESOURCE_INFORMATION_EXCHANGE.
> > >      // This information is sent back to the parent resource (e.g. the
> > > collection driver).
> > > diff --git
> > >
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > > c
> > >
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > > c
> > > index 13e29902..753cd7b2 100644
> > > ---
> > >
> a/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > > c
> > > +++
> > >
> b/RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilityLib.
> > > c
> > > @@ -107,6 +107,119 @@ GetArraykeyFromUri (
> > >    return EFI_SUCCESS;
> > >  }
> > >
> > > +/**
> > > +
> > > +  This function query ETag from given URI string and keep it in database.
> > > +
> > > +  @param[in]  Service               Redfish service instance to make query.
> > > +  @param[in]  Uri                   URI to query ETag.
> > > +  @param[in]  CheckPendingSettings  Set this to true and
> > > + @Redfish.Settings
> > > will
> > > +                                    be handled together. FALSE otherwise.
> > > +
> > > +  @retval     EFI_SUCCESS     ETAG and URI are applied successfully.
> > > +  @retval     Others          Errors occur.
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +SetEtagFromUri (
> > > +  IN  REDFISH_SERVICE  *RedfishService,
> > > +  IN  EFI_STRING       Uri,
> > > +  IN  BOOLEAN          CheckPendingSettings
> > > +  )
> > > +{
> > > +  EFI_STATUS        Status;
> > > +  CHAR8             *Etag;
> > > +  CHAR8             *AsciiUri;
> > > +  REDFISH_RESPONSE  Response;
> > > +  EFI_STRING        PendingSettingUri;
> > > +
> > > +  if ((RedfishService == NULL) || IS_EMPTY_STRING (Uri)) {
> > > +    return EFI_INVALID_PARAMETER;
> > > +  }
> > > +
> > > +  AsciiUri          = NULL;
> > > +  Etag              = NULL;
> > > +  PendingSettingUri = NULL;
> > > +
> > > +  Status = RedfishLocateProtocol ((VOID **)&mEtagProtocol,
> > > &gEdkIIRedfishETagProtocolGuid);
> > > +  if (EFI_ERROR (Status)) {
> > > +    DEBUG ((DEBUG_ERROR, "%a: fail to locate
> > > gEdkIIRedfishETagProtocolGuid: %r\n", __func__, Status));
> > > +    return Status;
> > > +  }
> > > +
> > > +  Status = GetResourceByUri (RedfishService, Uri, &Response);
> > This may spend some time on network to get the whole resource from URI if
> the
> > resource size is big, but we just need the information of ETAG header. We can
> > think of using HEAD HTTP method.
> > However, this seems fine for now as the following code gets the pending
> settings
> > from the same HTTP response body retrieved above.
> > The worst case would be there is no pending setting, however we waste
> network
> > bandwidth to get the whole response body. Not sure if HTTP query $select
> can
> > improve this or not for retrieving pending setting annotation.
> >
> > Abner
> >
> >
> > > +  if (EFI_ERROR (Status)) {
> > > +    DEBUG ((DEBUG_ERROR, "%a: get resource from: %s failed\n",
> > > + __func__,
> > > Uri));
> > > +    return Status;
> > > +  }
> > > +
> > > +  //
> > > +  // 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 = EFI_NOT_FOUND;
> > > +    goto ON_RELEASE;
> > > +  }
> > > +
> > > +  AsciiUri = StrUnicodeToAscii (Uri);  if (AsciiUri == NULL) {
> > > +    Status = EFI_OUT_OF_RESOURCES;
> > > +    goto ON_RELEASE;
> > > +  }
> > > +
> > > +  //
> > > +  // Save ETag to variable.
> > > +  //
> > > +  Status = mEtagProtocol->Set (mEtagProtocol, AsciiUri, Etag);  if
> > > + (EFI_ERROR (Status)) {
> > > +    DEBUG ((DEBUG_ERROR, "%a: failed to set ETag %a -> %a\n",
> > > + __func__,
> > > Etag, AsciiUri));
> > > +  }
> > > +
> > > +  if (CheckPendingSettings && (Response.Payload != NULL)) {
> > > +    //
> > > +    // Check to see if there is @Redfish.Settings
> > > +    //
> > > +    Status = GetPendingSettings (
> > > +               RedfishService,
> > > +               Response.Payload,
> > > +               NULL,
> > > +               &PendingSettingUri
> > > +               );
> > > +    if (!EFI_ERROR (Status)) {
> > > +      DEBUG ((REDFISH_DEBUG_TRACE, "%a: @Redfish.Settings found:
> > > + %s\n",
> > > __func__, PendingSettingUri));
> > > +
> > > +      Status = SetEtagFromUri (RedfishService, PendingSettingUri, FALSE);
> > > +    }
> > > +  }
> > > +
> > > +ON_RELEASE:
> > > +
> > > +  if (AsciiUri != NULL) {
> > > +    FreePool (AsciiUri);
> > > +  }
> > > +
> > > +  if (Etag != NULL) {
> > > +    FreePool (Etag);
> > > +  }
> > > +
> > > +  if (PendingSettingUri != NULL) {
> > > +    FreePool (PendingSettingUri);
> > > +  }
> > > +
> > > +  if (Response.Payload != NULL) {
> > > +    RedfishFreeResponse (
> > > +      Response.StatusCode,
> > > +      Response.HeaderCount,
> > > +      Response.Headers,
> > > +      Response.Payload
> > > +      );
> > > +  }
> > > +
> > > +  return Status;
> > > +}
> > > +
> > >  /**
> > >
> > >    Keep ETAG string and URI string in database.
> > > @@ -143,7 +256,6 @@ SetEtagWithUri (
> > >    }
> > >
> > >    mEtagProtocol->Set (mEtagProtocol, AsciiUri, EtagStr);
> > > -  mEtagProtocol->Flush (mEtagProtocol);
> > >
> > >    FreePool (AsciiUri);
> > >
> > > @@ -505,6 +617,8 @@ ApplyFeatureSettingsVagueType (
> > >      return EFI_INVALID_PARAMETER;
> > >    }
> > >
> > > +  DEBUG ((REDFISH_DEBUG_TRACE, "%a: schema: %a %a config lang: %s
> > > NumberOfVagueValues: %d\n", __func__, Schema, Version,
> ConfigureLang,
> > > NumberOfVagueValues));
> > > +
> > >    ConfigureLangAscii = AllocatePool (StrLen (ConfigureLang) + 1);
> > >    if (ConfigureLangAscii == NULL) {
> > >      Status = EFI_OUT_OF_RESOURCES;
> > > @@ -1473,7 +1587,7 @@ DestroyConfiglanguageList (
> > >                                      The memory pointed by ConfigLang must be allocated
> > >                                      through memory allocation
> > > interface. Because we will replace
> > >                                      the pointer in this function.
> > > -  @param[in]  MaxtLengthConfigLang  The maximum length of
> ConfigLang.
> > > +  @param[in]  MaxLengthConfigLang   The maximum length of
> ConfigLang.
> > >    @param[in]  ConfigLangInstance    Pointer to Collection member instance.
> > >
> > >    @retval     EFI_SUCCESS     The instance is inserted to the configure
> language.
> > > @@ -1483,7 +1597,7 @@ DestroyConfiglanguageList (  EFI_STATUS
> > > SetResourceConfigLangMemberInstance (
> > >    IN EFI_STRING                              *DestConfigLang,
> > > -  IN UINTN                                   MaxtLengthConfigLang,
> > > +  IN UINTN                                   MaxLengthConfigLang,
> > >    IN REDFISH_FEATURE_ARRAY_TYPE_CONFIG_LANG  *ConfigLangInstance
> > >    )
> > >  {
> > > @@ -1508,14 +1622,14 @@ SetResourceConfigLangMemberInstance (
> > >      if (ConfigLangInstance->ConfigureLang == NULL) {
> > >        return EFI_INVALID_PARAMETER;
> > >      } else {
> > > -      StrCatS (*DestConfigLang, MaxtLengthConfigLang,
> ConfigLangInstance-
> > > >ConfigureLang);
> > > +      StrCatS (*DestConfigLang, MaxLengthConfigLang,
> > > + ConfigLangInstance-
> > > >ConfigureLang);
> > >        return EFI_SUCCESS;
> > >      }
> > >    }
> > >
> > >    MaxStrLength  = StrSize (ThisConfigLang) + StrSize
> > > ((EFI_STRING)&InstanceStr);
> > >    NewConfigLang = ThisConfigLang;
> > > -  if (MaxtLengthConfigLang < MaxStrLength) {
> > > +  if (MaxLengthConfigLang < MaxStrLength) {
> > >      NewConfigLang = (EFI_STRING)AllocateZeroPool (MaxStrLength);
> > >      if (NewConfigLang == NULL) {
> > >        DEBUG ((DEBUG_ERROR, "%a: Fail to allocate memory for
> > > NewConfigLang.\n", __func__)); @@ -2191,7 +2305,7 @@
> GetConfigureLang
> > > (
> > >
> > >  **/
> > >  EFI_STATUS
> > > -RedfisSetRedfishUri (
> > > +RedfishSetRedfishUri (
> > >    IN    EFI_STRING  ConfigLang,
> > >    IN    EFI_STRING  Uri
> > >    )
> > > @@ -3524,6 +3638,72 @@ CompareRedfishBooleanArrayValues (
> > >    return TRUE;
> > >  }
> > >
> > > +/**
> > > +
> > > +  Check and see if "@Redfish.Settings" exist in given Payload. If
> > > + found, return
> > > the
> > > +  payload and URI to pending settings. Caller has to release
> "SettingPayload"
> > > and
> > > +  "SettingUri".
> > > +
> > > +  @param[in]  Payload         Payload that may contain "@Redfish.Settings"
> > > +  @param[out] SettingPayload  Payload keeps pending settings.
> > > +  @param[out] SettingUri      URI to pending settings.
> > > +
> > > +  @retval     EFI_SUCCESS     Pending settings is found and returned.
> > > +  @retval     Others          Error happens
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +GetPendingSettings (
> > > +  IN  REDFISH_SERVICE   RedfishService,
> > > +  IN  REDFISH_PAYLOAD   Payload,
> > > +  OUT REDFISH_RESPONSE  *SettingResponse,
> > > +  OUT EFI_STRING        *SettingUri
> > > +  )
> > > +{
> > > +  CONST CHAR8       *RedfishSettingsUriKeys[] = { "@Redfish.Settings",
> > > "SettingsObject", "@odata.id" };
> > > +  EDKII_JSON_VALUE  JsonValue;
> > > +  UINTN             Index;
> > > +  EFI_STATUS        Status;
> > > +
> > > +  if ((RedfishService == NULL) || (Payload == NULL) ||
> > > + (SettingResponse ==
> > > NULL) || (SettingUri == NULL)) {
> > > +    return EFI_INVALID_PARAMETER;
> > > +  }
> > > +
> > > +  *SettingUri = NULL;
> > > +  JsonValue   = RedfishJsonInPayload (Payload);
> > > +
> > > +  //
> > > +  // Seeking RedfishSettings URI link.
> > > +  //
> > > +  for (Index = 0; Index < ARRAY_SIZE (RedfishSettingsUriKeys); Index++) {
> > > +    if (JsonValue == NULL) {
> > > +      break;
> > > +    }
> > > +
> > > +    JsonValue = JsonObjectGetValue (JsonValueGetObject (JsonValue),
> > > RedfishSettingsUriKeys[Index]);
> > > +  }
> > > +
> > > +  if (JsonValue != NULL) {
> > > +    //
> > > +    // Verify RedfishSettings URI link is valid to retrieve resource or not.
> > > +    //
> > > +    *SettingUri = JsonValueGetUnicodeString (JsonValue);
> > > +    if (*SettingUri == NULL) {
> > > +      return EFI_NOT_FOUND;
> > > +    }
> > > +
> > > +    Status = GetResourceByUri (RedfishService, *SettingUri,
> SettingResponse);
> > > +    if (EFI_ERROR (Status)) {
> > > +      DEBUG ((DEBUG_ERROR, "%a: @Redfish.Settings exists, get
> > > + resource
> > > from: %s failed: %r\n", __func__, *SettingUri, Status));
> > > +      return Status;
> > > +    }
> > > +
> > > +    return EFI_SUCCESS;
> > > +  }
> > > +
> > > +  return EFI_NOT_FOUND;
> > > +}
> > > +
> > >  /**
> > >
> > >    Check and see if any difference between two vague value set.
> > > --
> > > 2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111798): https://edk2.groups.io/g/devel/message/111798
Mute This Topic: https://groups.io/mt/102767544/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:[~2023-11-28  7:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-23 14:34 [edk2-devel] [edk2-redfish-client][PATCH 3/6] RedfishClientPkg: feature driver enhancement Nickle Wang via groups.io
2023-11-28  4:04 ` Chang, Abner via groups.io
2023-11-28  6:44   ` Nickle Wang via groups.io
2023-11-28  7:08     ` Chang, Abner via groups.io

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