[edk2-devel] [PATCH V2 RESEND] RedfishPkg: RedfishRestExDxe: Two PCDs for controlling the requests

Igor Kulchytskyy via groups.io posted 1 patch 1 year, 2 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
RedfishPkg/RedfishPkg.dec                           |  10 ++
RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf    |   2 +
RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c     |  23 ++--
RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c | 140 ++++++++++++--------
4 files changed, 113 insertions(+), 62 deletions(-)
[edk2-devel] [PATCH V2 RESEND] RedfishPkg: RedfishRestExDxe: Two PCDs for controlling the requests
Posted by Igor Kulchytskyy via groups.io 1 year, 2 months ago
Since BIOS should work with different BMC implementation
chunked requests as well as Expect header should be optional.
One PCD is used to enable/disable Expect header.
Another PCD is used to enable/disable chunked requests.

Cc: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Signed-off-by: Igor Kulchytskyy <igork@ami.com>
---
 RedfishPkg/RedfishPkg.dec                           |  10 ++
 RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf    |   2 +
 RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c     |  23 ++--
 RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c | 140 ++++++++++++--------
 4 files changed, 113 insertions(+), 62 deletions(-)

diff --git a/RedfishPkg/RedfishPkg.dec b/RedfishPkg/RedfishPkg.dec
index d2b189b..89ef32a 100644
--- a/RedfishPkg/RedfishPkg.dec
+++ b/RedfishPkg/RedfishPkg.dec
@@ -97,3 +97,13 @@
   # protocol instance.
   #
   gEfiRedfishPkgTokenSpaceGuid.PcdRedfishDiscoverAccessModeInBand|FALSE|BOOLEAN|0x00001002
+  #
+  # This PCD indicates if the EFI REST EX sends chunk request to Redfish service.
+  # Default is set to non chunk mode.
+  #
+  gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExChunkRequestMode|TRUE|BOOLEAN|0x00001003
+  #
+  # This PCD indicates if the EFI REST EX adds Expect header to the POST, PATCH, PUT requests to Redfish service.
+  # Default is set to not add.
+  #
+  gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExAddingExpect|FALSE|BOOLEAN|0x00001004
diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf b/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
index 75437b0..29003ae 100644
--- a/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
+++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
@@ -57,6 +57,8 @@

 [Pcd]
   gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExServiceAccessModeInBand   ## CONSUMES
+  gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExChunkRequestMode   ## CONSUMES
+  gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExAddingExpect   ## CONSUMES

 [UserExtensions.TianoCore."ExtraFiles"]
   RedfishRestExDxeExtra.uni
diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c b/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
index 952e9d1..0da0d10 100644
--- a/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
+++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
@@ -140,9 +140,6 @@ RedfishHttpAddExpectation (
   }

   *ItsWrite = FALSE;
-  if (PreservedRequestHeaders != NULL) {
-    *PreservedRequestHeaders = RequestMessage->Headers;
-  }

   if ((RequestMessage->Data.Request->Method != HttpMethodPut) && (RequestMessage->Data.Request->Method != HttpMethodPost) &&
       (RequestMessage->Data.Request->Method != HttpMethodPatch))
@@ -152,10 +149,20 @@ RedfishHttpAddExpectation (

   *ItsWrite = TRUE;

-  NewHeaders = AllocateZeroPool ((RequestMessage->HeaderCount + 1) * sizeof (EFI_HTTP_HEADER));
-  CopyMem ((VOID *)NewHeaders, (VOID *)RequestMessage->Headers, RequestMessage->HeaderCount * sizeof (EFI_HTTP_HEADER));
-  HttpSetFieldNameAndValue (NewHeaders + RequestMessage->HeaderCount, HTTP_HEADER_EXPECT, HTTP_EXPECT_100_CONTINUE);
-  RequestMessage->HeaderCount++;
-  RequestMessage->Headers = NewHeaders;
+  //
+  // Check PCD before adding Expect header
+  //
+  if(FixedPcdGetBool(PcdRedfishRestExAddingExpect)) {
+    if (PreservedRequestHeaders != NULL) {
+      *PreservedRequestHeaders = RequestMessage->Headers;
+    }
+
+    NewHeaders = AllocateZeroPool ((RequestMessage->HeaderCount + 1) * sizeof (EFI_HTTP_HEADER));
+    CopyMem ((VOID *)NewHeaders, (VOID *)RequestMessage->Headers, RequestMessage->HeaderCount * sizeof (EFI_HTTP_HEADER));
+    HttpSetFieldNameAndValue (NewHeaders + RequestMessage->HeaderCount, HTTP_HEADER_EXPECT, HTTP_EXPECT_100_CONTINUE);
+    RequestMessage->HeaderCount++;
+    RequestMessage->Headers = NewHeaders;
+  }
+
   return EFI_SUCCESS;
 }
diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
index 4b61fc0..fbd5e46 100644
--- a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
+++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
@@ -66,11 +66,13 @@ RedfishRestExSendReceive (
   HTTP_IO_SEND_NON_CHUNK_PROCESS  SendNonChunkProcess;
   EFI_HTTP_MESSAGE                ChunkTransferRequestMessage;

-  Status               = EFI_SUCCESS;
-  ResponseData         = NULL;
-  IsGetChunkedTransfer = FALSE;
-  SendChunkProcess     = HttpIoSendChunkNone;
-  SendNonChunkProcess  = HttpIoSendNonChunkNone;
+  Status                    = EFI_SUCCESS;
+  ResponseData              = NULL;
+  IsGetChunkedTransfer      = FALSE;
+  SendChunkProcess          = HttpIoSendChunkNone;
+  SendNonChunkProcess       = HttpIoSendNonChunkNone;
+  ItsWrite                  = FALSE;
+  PreservedRequestHeaders   = NULL;

   //
   // Validate the parameters
@@ -94,67 +96,85 @@ RedfishRestExSendReceive (
   DEBUG ((DEBUG_INFO, "\nRedfishRestExSendReceive():\n"));
   DEBUG ((DEBUG_INFO, "*** Perform HTTP Request Method - %d, URL: %s\n", RequestMessage->Data.Request->Method, RequestMessage->Data.Request->Url));

-  //
-  // Add header "Expect" to server, only for URL write.
-  //
-  Status = RedfishHttpAddExpectation (This, RequestMessage, &PreservedRequestHeaders, &ItsWrite);
-  if (EFI_ERROR (Status)) {
-    return Status;
-  }
+  if(FixedPcdGetBool(PcdRedfishRestExChunkRequestMode)){
+    //
+    // Add header "Expect" to server, only for URL write.
+    //
+    Status = RedfishHttpAddExpectation (This, RequestMessage, &PreservedRequestHeaders, &ItsWrite);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }

-  if (ItsWrite == TRUE) {
-    if (RequestMessage->BodyLength > HTTP_IO_MAX_SEND_PAYLOAD) {
-      //
-      // Send chunked transfer.
-      //
-      SendChunkProcess++;
-      CopyMem ((VOID *)&ChunkTransferRequestMessage, (VOID *)RequestMessage, sizeof (EFI_HTTP_MESSAGE));
-    } else {
-      SendNonChunkProcess++;
+    if (ItsWrite == TRUE) {
+      if (RequestMessage->BodyLength > HTTP_IO_MAX_SEND_PAYLOAD) {
+        //
+        // Send chunked transfer.
+        //
+        SendChunkProcess++;
+        CopyMem ((VOID *)&ChunkTransferRequestMessage, (VOID *)RequestMessage, sizeof (EFI_HTTP_MESSAGE));
+      } else {
+        SendNonChunkProcess++;
+      }
     }
   }

 ReSendRequest:;
-  //
-  // Send out the request to REST service.
-  //
-  if (ItsWrite == TRUE) {
+
+  if(FixedPcdGetBool(PcdRedfishRestExChunkRequestMode)){
     //
-    // This is write to URI
+    // Send the chunked request to REST service.
     //
-    if (SendChunkProcess > HttpIoSendChunkNone) {
+    if (ItsWrite == TRUE) {
       //
-      // This is chunk transfer for writing large payload.
-      // Send request header first and then handle the
-      // following request message body using chunk transfer.
+      // This is write to URI
       //
-      do {
-        Status = HttpIoSendChunkedTransfer (
+      if (SendChunkProcess > HttpIoSendChunkNone) {
+        //
+        // This is chunk transfer for writing large payload.
+        // Send request header first and then handle the
+        // following request message body using chunk transfer.
+        //
+        do {
+          Status = HttpIoSendChunkedTransfer (
+                     &(Instance->HttpIo),
+                     &SendChunkProcess,
+                     &ChunkTransferRequestMessage
+                     );
+          if (EFI_ERROR (Status)) {
+            goto ON_EXIT;
+          }
+        } while (SendChunkProcess == HttpIoSendChunkContent || SendChunkProcess == HttpIoSendChunkEndChunk);
+      } else {
+        //
+        // This is the non-chunk transfer, send request header first and then
+        // handle the following request message body using chunk transfer.
+        //
+        Status = HttpIoSendRequest (
                    &(Instance->HttpIo),
-                   &SendChunkProcess,
-                   &ChunkTransferRequestMessage
+                   (SendNonChunkProcess == HttpIoSendNonChunkContent) ? NULL : RequestMessage->Data.Request,
+                   (SendNonChunkProcess == HttpIoSendNonChunkContent) ? 0 : RequestMessage->HeaderCount,
+                   (SendNonChunkProcess == HttpIoSendNonChunkContent) ? NULL : RequestMessage->Headers,
+                   (SendNonChunkProcess == HttpIoSendNonChunkHeaderZeroContent) ? 0 : RequestMessage->BodyLength,
+                   (SendNonChunkProcess == HttpIoSendNonChunkHeaderZeroContent) ? NULL : RequestMessage->Body
                    );
-        if (EFI_ERROR (Status)) {
-          goto ON_EXIT;
-        }
-      } while (SendChunkProcess == HttpIoSendChunkContent || SendChunkProcess == HttpIoSendChunkEndChunk);
+      }
     } else {
       //
-      // This is the non-chunk transfer, send request header first and then
-      // handle the following request message body using chunk transfer.
+      // This is read from URI.
       //
       Status = HttpIoSendRequest (
                  &(Instance->HttpIo),
-                 (SendNonChunkProcess == HttpIoSendNonChunkContent) ? NULL : RequestMessage->Data.Request,
-                 (SendNonChunkProcess == HttpIoSendNonChunkContent) ? 0 : RequestMessage->HeaderCount,
-                 (SendNonChunkProcess == HttpIoSendNonChunkContent) ? NULL : RequestMessage->Headers,
-                 (SendNonChunkProcess == HttpIoSendNonChunkHeaderZeroContent) ? 0 : RequestMessage->BodyLength,
-                 (SendNonChunkProcess == HttpIoSendNonChunkHeaderZeroContent) ? NULL : RequestMessage->Body
+                 RequestMessage->Data.Request,
+                 RequestMessage->HeaderCount,
+                 RequestMessage->Headers,
+                 RequestMessage->BodyLength,
+                 RequestMessage->Body
                  );
     }
-  } else {
+  }
+  else{
     //
-    // This is read from URI.
+    // This is normal request to URI.
     //
     Status = HttpIoSendRequest (
                &(Instance->HttpIo),
@@ -213,7 +233,7 @@ ReSendRequest:;
   //
   // Restore the headers if it ever changed in RedfishHttpAddExpectation().
   //
-  if (RequestMessage->Headers != PreservedRequestHeaders) {
+  if (FixedPcdGetBool(PcdRedfishRestExAddingExpect) && RequestMessage->Headers != PreservedRequestHeaders) {
     FreePool (RequestMessage->Headers);
     RequestMessage->Headers = PreservedRequestHeaders; // Restore headers before we adding "Expect".
     RequestMessage->HeaderCount--;                     // Minus one header count for "Expect".
@@ -223,11 +243,23 @@ ReSendRequest:;
   if (ResponseData->Response.StatusCode == HTTP_STATUS_200_OK) {
     DEBUG ((DEBUG_INFO, "HTTP_STATUS_200_OK\n"));

-    if (SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
-      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all chunks."));
+    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) && SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
+      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all chunks.", ResponseData->Response.StatusCode));
       SendChunkProcess++;
       goto ReSendRequest;
     }
+  } else if (ResponseData->Response.StatusCode == HTTP_STATUS_204_NO_CONTENT) {
+    DEBUG ((DEBUG_INFO, "HTTP_STATUS_204_NO_CONTENT\n"));
+
+    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) && SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
+      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all chunks.", ResponseData->Response.StatusCode));
+      SendChunkProcess ++;
+      goto ReSendRequest;
+    }
+  } else if (ResponseData->Response.StatusCode == HTTP_STATUS_201_CREATED) {
+    DEBUG ((DEBUG_INFO, "HTTP_STATUS_201_CREATED\n"));
+  } else if (ResponseData->Response.StatusCode == HTTP_STATUS_202_ACCEPTED) {
+    DEBUG ((DEBUG_INFO, "HTTP_STATUS_202_ACCEPTED\n"));
   } else if (ResponseData->Response.StatusCode == HTTP_STATUS_413_REQUEST_ENTITY_TOO_LARGE) {
     DEBUG ((DEBUG_INFO, "HTTP_STATUS_413_REQUEST_ENTITY_TOO_LARGE\n"));

@@ -240,14 +272,14 @@ ReSendRequest:;
     goto ON_EXIT;
   } else if (ResponseData->Response.StatusCode == HTTP_STATUS_400_BAD_REQUEST) {
     DEBUG ((DEBUG_INFO, "HTTP_STATUS_400_BAD_REQUEST\n"));
-    if (SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
+    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) && SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
       DEBUG ((DEBUG_INFO, "Bad request may caused by zero length chunk. Try to send all chunks...\n"));
       SendChunkProcess++;
       goto ReSendRequest;
     }
   } else if (ResponseData->Response.StatusCode == HTTP_STATUS_100_CONTINUE) {
     DEBUG ((DEBUG_INFO, "HTTP_STATUS_100_CONTINUE\n"));
-    if (SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
+    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) && SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
       //
       // We get HTTP_STATUS_100_CONTINUE to send the body using chunk transfer.
       //
@@ -256,7 +288,7 @@ ReSendRequest:;
       goto ReSendRequest;
     }

-    if (SendNonChunkProcess == HttpIoSendNonChunkHeaderZeroContent) {
+    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) && SendNonChunkProcess == HttpIoSendNonChunkHeaderZeroContent) {
       DEBUG ((DEBUG_INFO, "HTTP_STATUS_100_CONTINUE for non chunk transfer...\n"));
       SendNonChunkProcess++;
       goto ReSendRequest;
@@ -394,7 +426,7 @@ ReSendRequest:;
       TotalReceivedSize += ResponseData->BodyLength;
     }

-    DEBUG ((DEBUG_INFO, "Total of lengh of Response :%d\n", TotalReceivedSize));
+    DEBUG ((DEBUG_INFO, "Total of length of Response :%d\n", TotalReceivedSize));
   }

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99799): https://edk2.groups.io/g/devel/message/99799
Mute This Topic: https://groups.io/mt/96835076/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V2 RESEND] RedfishPkg: RedfishRestExDxe: Two PCDs for controlling the requests
Posted by Chang, Abner via groups.io 1 year, 2 months ago
[AMD Official Use Only - General]

Hi Igor,
Few comments in below.

> -----Original Message-----
> From: Igor Kulchytskyy <igork@ami.com>
> Sent: Thursday, February 9, 2023 1:23 AM
> To: devel@edk2.groups.io
> Cc: Igor Kulchytskyy <igork@ami.com>; Chang, Abner
> <Abner.Chang@amd.com>; Nickle Wang <nicklew@nvidia.com>
> Subject: [PATCH V2 RESEND] RedfishPkg: RedfishRestExDxe: Two PCDs for
> controlling the requests
> 
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> 
> 
> Since BIOS should work with different BMC implementation chunked requests as
> well as Expect header should be optional.
> One PCD is used to enable/disable Expect header.
> Another PCD is used to enable/disable chunked requests.
> 
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Signed-off-by: Igor Kulchytskyy <igork@ami.com>
> ---
>  RedfishPkg/RedfishPkg.dec                           |  10 ++
>  RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf    |   2 +
>  RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c     |  23 ++--
>  RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c | 140 ++++++++++++----
> ----
>  4 files changed, 113 insertions(+), 62 deletions(-)
> 
> diff --git a/RedfishPkg/RedfishPkg.dec b/RedfishPkg/RedfishPkg.dec index
> d2b189b..89ef32a 100644
> --- a/RedfishPkg/RedfishPkg.dec
> +++ b/RedfishPkg/RedfishPkg.dec
> @@ -97,3 +97,13 @@
>    # protocol instance.
>    #
> 
> gEfiRedfishPkgTokenSpaceGuid.PcdRedfishDiscoverAccessModeInBand|FALSE|
> BOOLEAN|0x00001002
> +  #
> +  # This PCD indicates if the EFI REST EX sends chunk request to Redfish service.
> +  # Default is set to non chunk mode.
> +  #
> +
> +
> gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExChunkRequestMode|TRUE|BO
> O
> + LEAN|0x00001003
We can default set PcdRedfishRestExChunkRequestMode to FALSE.


> +  #
> +  # This PCD indicates if the EFI REST EX adds Expect header to the POST, PATCH,
> PUT requests to Redfish service.
> +  # Default is set to not add.
> +  #
> +
> +
> gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExAddingExpect|FALSE|BOOLEA
> + N|0x00001004
> diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
> b/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
> index 75437b0..29003ae 100644
> --- a/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
> +++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
> @@ -57,6 +57,8 @@
> 
>  [Pcd]
>    gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExServiceAccessModeInBand
> ## CONSUMES
> +  gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExChunkRequestMode   ##
> CONSUMES
> +  gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExAddingExpect   ##
> CONSUMES
> 
>  [UserExtensions.TianoCore."ExtraFiles"]
>    RedfishRestExDxeExtra.uni
> diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
> b/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
> index 952e9d1..0da0d10 100644
> --- a/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
> +++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
> @@ -140,9 +140,6 @@ RedfishHttpAddExpectation (
>    }
> 
>    *ItsWrite = FALSE;
> -  if (PreservedRequestHeaders != NULL) {
> -    *PreservedRequestHeaders = RequestMessage->Headers;
> -  }
> 
>    if ((RequestMessage->Data.Request->Method != HttpMethodPut) &&
> (RequestMessage->Data.Request->Method != HttpMethodPost) &&
>        (RequestMessage->Data.Request->Method != HttpMethodPatch)) @@ -
> 152,10 +149,20 @@ RedfishHttpAddExpectation (
> 
>    *ItsWrite = TRUE;
> 
> -  NewHeaders = AllocateZeroPool ((RequestMessage->HeaderCount + 1) *
> sizeof (EFI_HTTP_HEADER));
> -  CopyMem ((VOID *)NewHeaders, (VOID *)RequestMessage->Headers,
> RequestMessage->HeaderCount * sizeof (EFI_HTTP_HEADER));
> -  HttpSetFieldNameAndValue (NewHeaders + RequestMessage->HeaderCount,
> HTTP_HEADER_EXPECT, HTTP_EXPECT_100_CONTINUE);
> -  RequestMessage->HeaderCount++;
> -  RequestMessage->Headers = NewHeaders;
> +  //
> +  // Check PCD before adding Expect header  //
> +  if(FixedPcdGetBool(PcdRedfishRestExAddingExpect)) {
> +    if (PreservedRequestHeaders != NULL) {
> +      *PreservedRequestHeaders = RequestMessage->Headers;
> +    }
> +
> +    NewHeaders = AllocateZeroPool ((RequestMessage->HeaderCount + 1) *
> sizeof (EFI_HTTP_HEADER));
> +    CopyMem ((VOID *)NewHeaders, (VOID *)RequestMessage->Headers,
> RequestMessage->HeaderCount * sizeof (EFI_HTTP_HEADER));
> +    HttpSetFieldNameAndValue (NewHeaders + RequestMessage->HeaderCount,
> HTTP_HEADER_EXPECT, HTTP_EXPECT_100_CONTINUE);
> +    RequestMessage->HeaderCount++;
> +    RequestMessage->Headers = NewHeaders;  }
> +
>    return EFI_SUCCESS;
>  }
> diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
> b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
> index 4b61fc0..fbd5e46 100644
> --- a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
> +++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
> @@ -66,11 +66,13 @@ RedfishRestExSendReceive (
>    HTTP_IO_SEND_NON_CHUNK_PROCESS  SendNonChunkProcess;
>    EFI_HTTP_MESSAGE                ChunkTransferRequestMessage;
> 
> -  Status               = EFI_SUCCESS;
> -  ResponseData         = NULL;
> -  IsGetChunkedTransfer = FALSE;
> -  SendChunkProcess     = HttpIoSendChunkNone;
> -  SendNonChunkProcess  = HttpIoSendNonChunkNone;
> +  Status                    = EFI_SUCCESS;
> +  ResponseData              = NULL;
> +  IsGetChunkedTransfer      = FALSE;
> +  SendChunkProcess          = HttpIoSendChunkNone;
> +  SendNonChunkProcess       = HttpIoSendNonChunkNone;
> +  ItsWrite                  = FALSE;
> +  PreservedRequestHeaders   = NULL;
> 
>    //
>    // Validate the parameters
> @@ -94,67 +96,85 @@ RedfishRestExSendReceive (
>    DEBUG ((DEBUG_INFO, "\nRedfishRestExSendReceive():\n"));
>    DEBUG ((DEBUG_INFO, "*** Perform HTTP Request Method - %d, URL: %s\n",
> RequestMessage->Data.Request->Method, RequestMessage->Data.Request-
> >Url));
> 
> -  //
> -  // Add header "Expect" to server, only for URL write.
> -  //
> -  Status = RedfishHttpAddExpectation (This, RequestMessage,
> &PreservedRequestHeaders, &ItsWrite);
> -  if (EFI_ERROR (Status)) {
> -    return Status;
> -  }
> +  if(FixedPcdGetBool(PcdRedfishRestExChunkRequestMode)){
Due to whether to add "Expect" header is managed by PcdRedfishRestExAddingExpect, so we can relocate the condition check for PcdRedfishRestExChunkRequestMode to be above  "if (ItsWrite == TRUE) {" in 8 lines later.
With this, we can add code later to send the zero content payload with "Expect" header and check the HTTP Status for the HTTP Chunk transfer or other error cases.

> +    //
> +    // Add header "Expect" to server, only for URL write.
> +    //
> +    Status = RedfishHttpAddExpectation (This, RequestMessage,
> &PreservedRequestHeaders, &ItsWrite);
> +    if (EFI_ERROR (Status)) {
> +      return Status;
> +    }
> 
> -  if (ItsWrite == TRUE) {
> -    if (RequestMessage->BodyLength > HTTP_IO_MAX_SEND_PAYLOAD) {
> -      //
> -      // Send chunked transfer.
> -      //
> -      SendChunkProcess++;
> -      CopyMem ((VOID *)&ChunkTransferRequestMessage, (VOID
> *)RequestMessage, sizeof (EFI_HTTP_MESSAGE));
> -    } else {
> -      SendNonChunkProcess++;
> +    if (ItsWrite == TRUE) {
> +      if (RequestMessage->BodyLength > HTTP_IO_MAX_SEND_PAYLOAD) {
> +        //
> +        // Send chunked transfer.
> +        //
> +        SendChunkProcess++;
> +        CopyMem ((VOID *)&ChunkTransferRequestMessage, (VOID
> *)RequestMessage, sizeof (EFI_HTTP_MESSAGE));
> +      } else {
> +        SendNonChunkProcess++;
> +      }
>      }
>    }
> 
>  ReSendRequest:;
> -  //
> -  // Send out the request to REST service.
> -  //
> -  if (ItsWrite == TRUE) {
> +
> +  if(FixedPcdGetBool(PcdRedfishRestExChunkRequestMode)){
>      //
> -    // This is write to URI
> +    // Send the chunked request to REST service.
>      //
> -    if (SendChunkProcess > HttpIoSendChunkNone) {
> +    if (ItsWrite == TRUE) {
>        //
> -      // This is chunk transfer for writing large payload.
> -      // Send request header first and then handle the
> -      // following request message body using chunk transfer.
> +      // This is write to URI
>        //
> -      do {
> -        Status = HttpIoSendChunkedTransfer (
> +      if (SendChunkProcess > HttpIoSendChunkNone) {
> +        //
> +        // This is chunk transfer for writing large payload.
> +        // Send request header first and then handle the
> +        // following request message body using chunk transfer.
> +        //
> +        do {
> +          Status = HttpIoSendChunkedTransfer (
> +                     &(Instance->HttpIo),
> +                     &SendChunkProcess,
> +                     &ChunkTransferRequestMessage
> +                     );
> +          if (EFI_ERROR (Status)) {
> +            goto ON_EXIT;
> +          }
> +        } while (SendChunkProcess == HttpIoSendChunkContent ||
> SendChunkProcess == HttpIoSendChunkEndChunk);
> +      } else {
> +        //
> +        // This is the non-chunk transfer, send request header first and then
> +        // handle the following request message body using chunk transfer.
> +        //
> +        Status = HttpIoSendRequest (
>                     &(Instance->HttpIo),
> -                   &SendChunkProcess,
> -                   &ChunkTransferRequestMessage
> +                   (SendNonChunkProcess == HttpIoSendNonChunkContent) ? NULL :
> RequestMessage->Data.Request,
> +                   (SendNonChunkProcess == HttpIoSendNonChunkContent) ? 0 :
> RequestMessage->HeaderCount,
> +                   (SendNonChunkProcess == HttpIoSendNonChunkContent) ? NULL :
> RequestMessage->Headers,
> +                   (SendNonChunkProcess ==
> HttpIoSendNonChunkHeaderZeroContent) ? 0 : RequestMessage->BodyLength,
> +                   (SendNonChunkProcess ==
> + HttpIoSendNonChunkHeaderZeroContent) ? NULL : RequestMessage->Body
>                     );
> -        if (EFI_ERROR (Status)) {
> -          goto ON_EXIT;
> -        }
> -      } while (SendChunkProcess == HttpIoSendChunkContent ||
> SendChunkProcess == HttpIoSendChunkEndChunk);
> +      }
>      } else {
>        //
> -      // This is the non-chunk transfer, send request header first and then
> -      // handle the following request message body using chunk transfer.
> +      // This is read from URI.
>        //
>        Status = HttpIoSendRequest (
>                   &(Instance->HttpIo),
> -                 (SendNonChunkProcess == HttpIoSendNonChunkContent) ? NULL :
> RequestMessage->Data.Request,
> -                 (SendNonChunkProcess == HttpIoSendNonChunkContent) ? 0 :
> RequestMessage->HeaderCount,
> -                 (SendNonChunkProcess == HttpIoSendNonChunkContent) ? NULL :
> RequestMessage->Headers,
> -                 (SendNonChunkProcess == HttpIoSendNonChunkHeaderZeroContent) ?
> 0 : RequestMessage->BodyLength,
> -                 (SendNonChunkProcess == HttpIoSendNonChunkHeaderZeroContent) ?
> NULL : RequestMessage->Body
> +                 RequestMessage->Data.Request,
> +                 RequestMessage->HeaderCount,
> +                 RequestMessage->Headers,
> +                 RequestMessage->BodyLength,
> +                 RequestMessage->Body
>                   );
>      }
> -  } else {
> +  }
> +  else{
Please have "else" at the same line with above closed curly bracket and have a space between "else" and "{"
Thanks
Abner 

>      //
> -    // This is read from URI.
> +    // This is normal request to URI.
>      //
>      Status = HttpIoSendRequest (
>                 &(Instance->HttpIo),
> @@ -213,7 +233,7 @@ ReSendRequest:;
>    //
>    // Restore the headers if it ever changed in RedfishHttpAddExpectation().
>    //
> -  if (RequestMessage->Headers != PreservedRequestHeaders) {
> +  if (FixedPcdGetBool(PcdRedfishRestExAddingExpect) &&
> + RequestMessage->Headers != PreservedRequestHeaders) {
>      FreePool (RequestMessage->Headers);
>      RequestMessage->Headers = PreservedRequestHeaders; // Restore headers
> before we adding "Expect".
>      RequestMessage->HeaderCount--;                     // Minus one header count for
> "Expect".
> @@ -223,11 +243,23 @@ ReSendRequest:;
>    if (ResponseData->Response.StatusCode == HTTP_STATUS_200_OK) {
>      DEBUG ((DEBUG_INFO, "HTTP_STATUS_200_OK\n"));
> 
> -    if (SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> -      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all chunks."));
> +    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) &&
> SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> +      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all
> + chunks.", ResponseData->Response.StatusCode));
>        SendChunkProcess++;
>        goto ReSendRequest;
>      }
> +  } else if (ResponseData->Response.StatusCode ==
> HTTP_STATUS_204_NO_CONTENT) {
> +    DEBUG ((DEBUG_INFO, "HTTP_STATUS_204_NO_CONTENT\n"));
> +
> +    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) &&
> SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> +      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all chunks.",
> ResponseData->Response.StatusCode));
> +      SendChunkProcess ++;
> +      goto ReSendRequest;
> +    }
> +  } else if (ResponseData->Response.StatusCode ==
> HTTP_STATUS_201_CREATED) {
> +    DEBUG ((DEBUG_INFO, "HTTP_STATUS_201_CREATED\n"));  } else if
> + (ResponseData->Response.StatusCode == HTTP_STATUS_202_ACCEPTED) {
> +    DEBUG ((DEBUG_INFO, "HTTP_STATUS_202_ACCEPTED\n"));
>    } else if (ResponseData->Response.StatusCode ==
> HTTP_STATUS_413_REQUEST_ENTITY_TOO_LARGE) {
>      DEBUG ((DEBUG_INFO,
> "HTTP_STATUS_413_REQUEST_ENTITY_TOO_LARGE\n"));
> 
> @@ -240,14 +272,14 @@ ReSendRequest:;
>      goto ON_EXIT;
>    } else if (ResponseData->Response.StatusCode ==
> HTTP_STATUS_400_BAD_REQUEST) {
>      DEBUG ((DEBUG_INFO, "HTTP_STATUS_400_BAD_REQUEST\n"));
> -    if (SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> +    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) &&
> + SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
>        DEBUG ((DEBUG_INFO, "Bad request may caused by zero length chunk. Try
> to send all chunks...\n"));
>        SendChunkProcess++;
>        goto ReSendRequest;
>      }
>    } else if (ResponseData->Response.StatusCode ==
> HTTP_STATUS_100_CONTINUE) {
>      DEBUG ((DEBUG_INFO, "HTTP_STATUS_100_CONTINUE\n"));
> -    if (SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> +    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) &&
> + SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
>        //
>        // We get HTTP_STATUS_100_CONTINUE to send the body using chunk
> transfer.
>        //
> @@ -256,7 +288,7 @@ ReSendRequest:;
>        goto ReSendRequest;
>      }
> 
> -    if (SendNonChunkProcess == HttpIoSendNonChunkHeaderZeroContent) {
> +    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) &&
> + SendNonChunkProcess == HttpIoSendNonChunkHeaderZeroContent) {
>        DEBUG ((DEBUG_INFO, "HTTP_STATUS_100_CONTINUE for non chunk
> transfer...\n"));
>        SendNonChunkProcess++;
>        goto ReSendRequest;
> @@ -394,7 +426,7 @@ ReSendRequest:;
>        TotalReceivedSize += ResponseData->BodyLength;
>      }
> 
> -    DEBUG ((DEBUG_INFO, "Total of lengh of Response :%d\n",
> TotalReceivedSize));
> +    DEBUG ((DEBUG_INFO, "Total of length of Response :%d\n",
> + TotalReceivedSize));
>    }
> 
>    DEBUG ((DEBUG_INFO, "RedfishRestExSendReceive()- EFI_STATUS: %r\n",
> Status));
> --
> 2.6.1.windows.1
> -The information contained in this message may be confidential and proprietary
> to American Megatrends (AMI). This communication is intended to be read only
> by the individual or entity to whom it is addressed or by their designee. If the
> reader of this message is not the intended recipient, you are on notice that any
> distribution of this message, in any form, is strictly prohibited. Please promptly
> notify the sender by reply e-mail or by telephone at 770-246-8600, and then
> delete or destroy all copies of the transmission.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99816): https://edk2.groups.io/g/devel/message/99816
Mute This Topic: https://groups.io/mt/96835076/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V2 RESEND] RedfishPkg: RedfishRestExDxe: Two PCDs for controlling the requests
Posted by Igor Kulchytskyy via groups.io 1 year, 2 months ago
Hi Abner,
Thank you for your comments.
See my comments on your comments below.
Igor

-----Original Message-----
From: Chang, Abner <Abner.Chang@amd.com>
Sent: Wednesday, February 8, 2023 4:13 PM
To: Igor Kulchytskyy <igork@ami.com>; devel@edk2.groups.io
Cc: Nickle Wang <nicklew@nvidia.com>
Subject: [EXTERNAL] RE: [PATCH V2 RESEND] RedfishPkg: RedfishRestExDxe: Two PCDs for controlling the requests


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

[AMD Official Use Only - General]

Hi Igor,
Few comments in below.

> -----Original Message-----
> From: Igor Kulchytskyy <igork@ami.com>
> Sent: Thursday, February 9, 2023 1:23 AM
> To: devel@edk2.groups.io
> Cc: Igor Kulchytskyy <igork@ami.com>; Chang, Abner
> <Abner.Chang@amd.com>; Nickle Wang <nicklew@nvidia.com>
> Subject: [PATCH V2 RESEND] RedfishPkg: RedfishRestExDxe: Two PCDs for
> controlling the requests
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> Since BIOS should work with different BMC implementation chunked
> requests as well as Expect header should be optional.
> One PCD is used to enable/disable Expect header.
> Another PCD is used to enable/disable chunked requests.
>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Signed-off-by: Igor Kulchytskyy <igork@ami.com>
> ---
>  RedfishPkg/RedfishPkg.dec                           |  10 ++
>  RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf    |   2 +
>  RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c     |  23 ++--
>  RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c | 140
> ++++++++++++----
> ----
>  4 files changed, 113 insertions(+), 62 deletions(-)
>
> diff --git a/RedfishPkg/RedfishPkg.dec b/RedfishPkg/RedfishPkg.dec
> index d2b189b..89ef32a 100644
> --- a/RedfishPkg/RedfishPkg.dec
> +++ b/RedfishPkg/RedfishPkg.dec
> @@ -97,3 +97,13 @@
>    # protocol instance.
>    #
>
> gEfiRedfishPkgTokenSpaceGuid.PcdRedfishDiscoverAccessModeInBand|FALSE|
> BOOLEAN|0x00001002
> +  #
> +  # This PCD indicates if the EFI REST EX sends chunk request to Redfish service.
> +  # Default is set to non chunk mode.
> +  #
> +
> +
> gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExChunkRequestMode|TRUE|BO
> O
> + LEAN|0x00001003
We can default set PcdRedfishRestExChunkRequestMode to FALSE.
Igor: Agree. Just forgot to make it FALSE after I tested BIOS with Expect PCD disabled and Chunked PCD enabled.

> +  #
> +  # This PCD indicates if the EFI REST EX adds Expect header to the
> + POST, PATCH,
> PUT requests to Redfish service.
> +  # Default is set to not add.
> +  #
> +
> +
> gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExAddingExpect|FALSE|BOOLEA
> + N|0x00001004
> diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
> b/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
> index 75437b0..29003ae 100644
> --- a/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
> +++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
> @@ -57,6 +57,8 @@
>
>  [Pcd]
>
> gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExServiceAccessModeInBand
> ## CONSUMES
> +  gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExChunkRequestMode   ##
> CONSUMES
> +  gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExAddingExpect   ##
> CONSUMES
>
>  [UserExtensions.TianoCore."ExtraFiles"]
>    RedfishRestExDxeExtra.uni
> diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
> b/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
> index 952e9d1..0da0d10 100644
> --- a/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
> +++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
> @@ -140,9 +140,6 @@ RedfishHttpAddExpectation (
>    }
>
>    *ItsWrite = FALSE;
> -  if (PreservedRequestHeaders != NULL) {
> -    *PreservedRequestHeaders = RequestMessage->Headers;
> -  }
>
>    if ((RequestMessage->Data.Request->Method != HttpMethodPut) &&
> (RequestMessage->Data.Request->Method != HttpMethodPost) &&
>        (RequestMessage->Data.Request->Method != HttpMethodPatch)) @@ -
> 152,10 +149,20 @@ RedfishHttpAddExpectation (
>
>    *ItsWrite = TRUE;
>
> -  NewHeaders = AllocateZeroPool ((RequestMessage->HeaderCount + 1) *
> sizeof (EFI_HTTP_HEADER));
> -  CopyMem ((VOID *)NewHeaders, (VOID *)RequestMessage->Headers,
> RequestMessage->HeaderCount * sizeof (EFI_HTTP_HEADER));
> -  HttpSetFieldNameAndValue (NewHeaders + RequestMessage->HeaderCount,
> HTTP_HEADER_EXPECT, HTTP_EXPECT_100_CONTINUE);
> -  RequestMessage->HeaderCount++;
> -  RequestMessage->Headers = NewHeaders;
> +  //
> +  // Check PCD before adding Expect header  //
> +  if(FixedPcdGetBool(PcdRedfishRestExAddingExpect)) {
> +    if (PreservedRequestHeaders != NULL) {
> +      *PreservedRequestHeaders = RequestMessage->Headers;
> +    }
> +
> +    NewHeaders = AllocateZeroPool ((RequestMessage->HeaderCount + 1)
> + *
> sizeof (EFI_HTTP_HEADER));
> +    CopyMem ((VOID *)NewHeaders, (VOID *)RequestMessage->Headers,
> RequestMessage->HeaderCount * sizeof (EFI_HTTP_HEADER));
> +    HttpSetFieldNameAndValue (NewHeaders +
> + RequestMessage->HeaderCount,
> HTTP_HEADER_EXPECT, HTTP_EXPECT_100_CONTINUE);
> +    RequestMessage->HeaderCount++;
> +    RequestMessage->Headers = NewHeaders;  }
> +
>    return EFI_SUCCESS;
>  }
> diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
> b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
> index 4b61fc0..fbd5e46 100644
> --- a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
> +++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
> @@ -66,11 +66,13 @@ RedfishRestExSendReceive (
>    HTTP_IO_SEND_NON_CHUNK_PROCESS  SendNonChunkProcess;
>    EFI_HTTP_MESSAGE                ChunkTransferRequestMessage;
>
> -  Status               = EFI_SUCCESS;
> -  ResponseData         = NULL;
> -  IsGetChunkedTransfer = FALSE;
> -  SendChunkProcess     = HttpIoSendChunkNone;
> -  SendNonChunkProcess  = HttpIoSendNonChunkNone;
> +  Status                    = EFI_SUCCESS;
> +  ResponseData              = NULL;
> +  IsGetChunkedTransfer      = FALSE;
> +  SendChunkProcess          = HttpIoSendChunkNone;
> +  SendNonChunkProcess       = HttpIoSendNonChunkNone;
> +  ItsWrite                  = FALSE;
> +  PreservedRequestHeaders   = NULL;
>
>    //
>    // Validate the parameters
> @@ -94,67 +96,85 @@ RedfishRestExSendReceive (
>    DEBUG ((DEBUG_INFO, "\nRedfishRestExSendReceive():\n"));
>    DEBUG ((DEBUG_INFO, "*** Perform HTTP Request Method - %d, URL:
> %s\n",
> RequestMessage->Data.Request->Method, RequestMessage->Data.Request-
> >Url));
>
> -  //
> -  // Add header "Expect" to server, only for URL write.
> -  //
> -  Status = RedfishHttpAddExpectation (This, RequestMessage,
> &PreservedRequestHeaders, &ItsWrite);
> -  if (EFI_ERROR (Status)) {
> -    return Status;
> -  }
> +  if(FixedPcdGetBool(PcdRedfishRestExChunkRequestMode)){
Due to whether to add "Expect" header is managed by PcdRedfishRestExAddingExpect, so we can relocate the condition check for PcdRedfishRestExChunkRequestMode to be above  "if (ItsWrite == TRUE) {" in 8 lines later.
With this, we can add code later to send the zero content payload with "Expect" header and check the HTTP Status for the HTTP Chunk transfer or other error cases.
Igor: I'm not sure if it would help anyhow. We cannot use "Expect" without chunked request. If I understand it correctly, the main point of "Expect" header is to send just headers without body to see if the URI supported. But if chunked request is disabled then we would send the whole request with the body anyway. So, there is no difference if we send it with or without "Expect" header.
And another purpose of that RedfishHttpAddExpectation function, besides of adding "Expect" header, is to figure out if it is POST, PATCH or PUT request and set ItsWrite variable. But this variable also used only for the chunked requests. So, I think, it is better to keep it under if(FixedPcdGetBool(PcdRedfishRestExChunkRequestMode)) condition.
What do you think?

> +    //
> +    // Add header "Expect" to server, only for URL write.
> +    //
> +    Status = RedfishHttpAddExpectation (This, RequestMessage,
> &PreservedRequestHeaders, &ItsWrite);
> +    if (EFI_ERROR (Status)) {
> +      return Status;
> +    }
>
> -  if (ItsWrite == TRUE) {
> -    if (RequestMessage->BodyLength > HTTP_IO_MAX_SEND_PAYLOAD) {
> -      //
> -      // Send chunked transfer.
> -      //
> -      SendChunkProcess++;
> -      CopyMem ((VOID *)&ChunkTransferRequestMessage, (VOID
> *)RequestMessage, sizeof (EFI_HTTP_MESSAGE));
> -    } else {
> -      SendNonChunkProcess++;
> +    if (ItsWrite == TRUE) {
> +      if (RequestMessage->BodyLength > HTTP_IO_MAX_SEND_PAYLOAD) {
> +        //
> +        // Send chunked transfer.
> +        //
> +        SendChunkProcess++;
> +        CopyMem ((VOID *)&ChunkTransferRequestMessage, (VOID
> *)RequestMessage, sizeof (EFI_HTTP_MESSAGE));
> +      } else {
> +        SendNonChunkProcess++;
> +      }
>      }
>    }
>
>  ReSendRequest:;
> -  //
> -  // Send out the request to REST service.
> -  //
> -  if (ItsWrite == TRUE) {
> +
> +  if(FixedPcdGetBool(PcdRedfishRestExChunkRequestMode)){
>      //
> -    // This is write to URI
> +    // Send the chunked request to REST service.
>      //
> -    if (SendChunkProcess > HttpIoSendChunkNone) {
> +    if (ItsWrite == TRUE) {
>        //
> -      // This is chunk transfer for writing large payload.
> -      // Send request header first and then handle the
> -      // following request message body using chunk transfer.
> +      // This is write to URI
>        //
> -      do {
> -        Status = HttpIoSendChunkedTransfer (
> +      if (SendChunkProcess > HttpIoSendChunkNone) {
> +        //
> +        // This is chunk transfer for writing large payload.
> +        // Send request header first and then handle the
> +        // following request message body using chunk transfer.
> +        //
> +        do {
> +          Status = HttpIoSendChunkedTransfer (
> +                     &(Instance->HttpIo),
> +                     &SendChunkProcess,
> +                     &ChunkTransferRequestMessage
> +                     );
> +          if (EFI_ERROR (Status)) {
> +            goto ON_EXIT;
> +          }
> +        } while (SendChunkProcess == HttpIoSendChunkContent ||
> SendChunkProcess == HttpIoSendChunkEndChunk);
> +      } else {
> +        //
> +        // This is the non-chunk transfer, send request header first and then
> +        // handle the following request message body using chunk transfer.
> +        //
> +        Status = HttpIoSendRequest (
>                     &(Instance->HttpIo),
> -                   &SendChunkProcess,
> -                   &ChunkTransferRequestMessage
> +                   (SendNonChunkProcess == HttpIoSendNonChunkContent) ? NULL :
> RequestMessage->Data.Request,
> +                   (SendNonChunkProcess == HttpIoSendNonChunkContent) ? 0 :
> RequestMessage->HeaderCount,
> +                   (SendNonChunkProcess == HttpIoSendNonChunkContent) ? NULL :
> RequestMessage->Headers,
> +                   (SendNonChunkProcess ==
> HttpIoSendNonChunkHeaderZeroContent) ? 0 : RequestMessage->BodyLength,
> +                   (SendNonChunkProcess ==
> + HttpIoSendNonChunkHeaderZeroContent) ? NULL : RequestMessage->Body
>                     );
> -        if (EFI_ERROR (Status)) {
> -          goto ON_EXIT;
> -        }
> -      } while (SendChunkProcess == HttpIoSendChunkContent ||
> SendChunkProcess == HttpIoSendChunkEndChunk);
> +      }
>      } else {
>        //
> -      // This is the non-chunk transfer, send request header first and then
> -      // handle the following request message body using chunk transfer.
> +      // This is read from URI.
>        //
>        Status = HttpIoSendRequest (
>                   &(Instance->HttpIo),
> -                 (SendNonChunkProcess == HttpIoSendNonChunkContent) ? NULL :
> RequestMessage->Data.Request,
> -                 (SendNonChunkProcess == HttpIoSendNonChunkContent) ? 0 :
> RequestMessage->HeaderCount,
> -                 (SendNonChunkProcess == HttpIoSendNonChunkContent) ? NULL :
> RequestMessage->Headers,
> -                 (SendNonChunkProcess == HttpIoSendNonChunkHeaderZeroContent) ?
> 0 : RequestMessage->BodyLength,
> -                 (SendNonChunkProcess == HttpIoSendNonChunkHeaderZeroContent) ?
> NULL : RequestMessage->Body
> +                 RequestMessage->Data.Request,
> +                 RequestMessage->HeaderCount,
> +                 RequestMessage->Headers,
> +                 RequestMessage->BodyLength,
> +                 RequestMessage->Body
>                   );
>      }
> -  } else {
> +  }
> +  else{
Please have "else" at the same line with above closed curly bracket and have a space between "else" and "{"
Igor: My mistake. Fixed.
Thanks
Abner

>      //
> -    // This is read from URI.
> +    // This is normal request to URI.
>      //
>      Status = HttpIoSendRequest (
>                 &(Instance->HttpIo),
> @@ -213,7 +233,7 @@ ReSendRequest:;
>    //
>    // Restore the headers if it ever changed in RedfishHttpAddExpectation().
>    //
> -  if (RequestMessage->Headers != PreservedRequestHeaders) {
> +  if (FixedPcdGetBool(PcdRedfishRestExAddingExpect) &&
> + RequestMessage->Headers != PreservedRequestHeaders) {
>      FreePool (RequestMessage->Headers);
>      RequestMessage->Headers = PreservedRequestHeaders; // Restore
> headers before we adding "Expect".
>      RequestMessage->HeaderCount--;                     // Minus one header count for
> "Expect".
> @@ -223,11 +243,23 @@ ReSendRequest:;
>    if (ResponseData->Response.StatusCode == HTTP_STATUS_200_OK) {
>      DEBUG ((DEBUG_INFO, "HTTP_STATUS_200_OK\n"));
>
> -    if (SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> -      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all chunks."));
> +    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) &&
> SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> +      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all
> + chunks.", ResponseData->Response.StatusCode));
>        SendChunkProcess++;
>        goto ReSendRequest;
>      }
> +  } else if (ResponseData->Response.StatusCode ==
> HTTP_STATUS_204_NO_CONTENT) {
> +    DEBUG ((DEBUG_INFO, "HTTP_STATUS_204_NO_CONTENT\n"));
> +
> +    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) &&
> SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> +      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all
> + chunks.",
> ResponseData->Response.StatusCode));
> +      SendChunkProcess ++;
> +      goto ReSendRequest;
> +    }
> +  } else if (ResponseData->Response.StatusCode ==
> HTTP_STATUS_201_CREATED) {
> +    DEBUG ((DEBUG_INFO, "HTTP_STATUS_201_CREATED\n"));  } else if
> + (ResponseData->Response.StatusCode == HTTP_STATUS_202_ACCEPTED) {
> +    DEBUG ((DEBUG_INFO, "HTTP_STATUS_202_ACCEPTED\n"));
>    } else if (ResponseData->Response.StatusCode ==
> HTTP_STATUS_413_REQUEST_ENTITY_TOO_LARGE) {
>      DEBUG ((DEBUG_INFO,
> "HTTP_STATUS_413_REQUEST_ENTITY_TOO_LARGE\n"));
>
> @@ -240,14 +272,14 @@ ReSendRequest:;
>      goto ON_EXIT;
>    } else if (ResponseData->Response.StatusCode ==
> HTTP_STATUS_400_BAD_REQUEST) {
>      DEBUG ((DEBUG_INFO, "HTTP_STATUS_400_BAD_REQUEST\n"));
> -    if (SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> +    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) &&
> + SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
>        DEBUG ((DEBUG_INFO, "Bad request may caused by zero length
> chunk. Try to send all chunks...\n"));
>        SendChunkProcess++;
>        goto ReSendRequest;
>      }
>    } else if (ResponseData->Response.StatusCode ==
> HTTP_STATUS_100_CONTINUE) {
>      DEBUG ((DEBUG_INFO, "HTTP_STATUS_100_CONTINUE\n"));
> -    if (SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> +    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) &&
> + SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
>        //
>        // We get HTTP_STATUS_100_CONTINUE to send the body using chunk
> transfer.
>        //
> @@ -256,7 +288,7 @@ ReSendRequest:;
>        goto ReSendRequest;
>      }
>
> -    if (SendNonChunkProcess == HttpIoSendNonChunkHeaderZeroContent) {
> +    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) &&
> + SendNonChunkProcess == HttpIoSendNonChunkHeaderZeroContent) {
>        DEBUG ((DEBUG_INFO, "HTTP_STATUS_100_CONTINUE for non chunk
> transfer...\n"));
>        SendNonChunkProcess++;
>        goto ReSendRequest;
> @@ -394,7 +426,7 @@ ReSendRequest:;
>        TotalReceivedSize += ResponseData->BodyLength;
>      }
>
> -    DEBUG ((DEBUG_INFO, "Total of lengh of Response :%d\n",
> TotalReceivedSize));
> +    DEBUG ((DEBUG_INFO, "Total of length of Response :%d\n",
> + TotalReceivedSize));
>    }
>
>    DEBUG ((DEBUG_INFO, "RedfishRestExSendReceive()- EFI_STATUS: %r\n",
> Status));
> --
> 2.6.1.windows.1
> -The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI). This communication is
> intended to be read only by the individual or entity to whom it is
> addressed or by their designee. If the reader of this message is not
> the intended recipient, you are on notice that any distribution of
> this message, in any form, is strictly prohibited. Please promptly
> notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99826): https://edk2.groups.io/g/devel/message/99826
Mute This Topic: https://groups.io/mt/96835076/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V2 RESEND] RedfishPkg: RedfishRestExDxe: Two PCDs for controlling the requests
Posted by Chang, Abner via groups.io 1 year, 2 months ago
[AMD Official Use Only - General]



> -----Original Message-----
> From: Igor Kulchytskyy <igork@ami.com>
> Sent: Thursday, February 9, 2023 7:57 AM
> To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
> Cc: Nickle Wang <nicklew@nvidia.com>
> Subject: RE: [EXTERNAL] RE: [PATCH V2 RESEND] RedfishPkg: RedfishRestExDxe:
> Two PCDs for controlling the requests
> 
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> 
> 
> Hi Abner,
> Thank you for your comments.
> See my comments on your comments below.
> Igor
> 
> -----Original Message-----
> From: Chang, Abner <Abner.Chang@amd.com>
> Sent: Wednesday, February 8, 2023 4:13 PM
> To: Igor Kulchytskyy <igork@ami.com>; devel@edk2.groups.io
> Cc: Nickle Wang <nicklew@nvidia.com>
> Subject: [EXTERNAL] RE: [PATCH V2 RESEND] RedfishPkg: RedfishRestExDxe:
> Two PCDs for controlling the requests
> 
> 
> **CAUTION: The e-mail below is from an external source. Please exercise
> caution before opening attachments, clicking links, or following guidance.**
> 
> [AMD Official Use Only - General]
> 
> Hi Igor,
> Few comments in below.
> 
> > -----Original Message-----
> > From: Igor Kulchytskyy <igork@ami.com>
> > Sent: Thursday, February 9, 2023 1:23 AM
> > To: devel@edk2.groups.io
> > Cc: Igor Kulchytskyy <igork@ami.com>; Chang, Abner
> > <Abner.Chang@amd.com>; Nickle Wang <nicklew@nvidia.com>
> > Subject: [PATCH V2 RESEND] RedfishPkg: RedfishRestExDxe: Two PCDs for
> > controlling the requests
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > Since BIOS should work with different BMC implementation chunked
> > requests as well as Expect header should be optional.
> > One PCD is used to enable/disable Expect header.
> > Another PCD is used to enable/disable chunked requests.
> >
> > Cc: Abner Chang <abner.chang@amd.com>
> > Cc: Nickle Wang <nicklew@nvidia.com>
> > Signed-off-by: Igor Kulchytskyy <igork@ami.com>
> > ---
> >  RedfishPkg/RedfishPkg.dec                           |  10 ++
> >  RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf    |   2 +
> >  RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c     |  23 ++--
> >  RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c | 140
> > ++++++++++++----
> > ----
> >  4 files changed, 113 insertions(+), 62 deletions(-)
> >
> > diff --git a/RedfishPkg/RedfishPkg.dec b/RedfishPkg/RedfishPkg.dec
> > index d2b189b..89ef32a 100644
> > --- a/RedfishPkg/RedfishPkg.dec
> > +++ b/RedfishPkg/RedfishPkg.dec
> > @@ -97,3 +97,13 @@
> >    # protocol instance.
> >    #
> >
> >
> gEfiRedfishPkgTokenSpaceGuid.PcdRedfishDiscoverAccessModeInBand|FALSE|
> > BOOLEAN|0x00001002
> > +  #
> > +  # This PCD indicates if the EFI REST EX sends chunk request to Redfish
> service.
> > +  # Default is set to non chunk mode.
> > +  #
> > +
> > +
> >
> gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExChunkRequestMode|TRUE|BO
> > O
> > + LEAN|0x00001003
> We can default set PcdRedfishRestExChunkRequestMode to FALSE.
> Igor: Agree. Just forgot to make it FALSE after I tested BIOS with Expect PCD
> disabled and Chunked PCD enabled.
> 
> > +  #
> > +  # This PCD indicates if the EFI REST EX adds Expect header to the
> > + POST, PATCH,
> > PUT requests to Redfish service.
> > +  # Default is set to not add.
> > +  #
> > +
> > +
> >
> gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExAddingExpect|FALSE|BOOLEA
> > + N|0x00001004
> > diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
> > b/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
> > index 75437b0..29003ae 100644
> > --- a/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
> > +++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
> > @@ -57,6 +57,8 @@
> >
> >  [Pcd]
> >
> > gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExServiceAccessModeInBand
> > ## CONSUMES
> > +  gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExChunkRequestMode   ##
> > CONSUMES
> > +  gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExAddingExpect   ##
> > CONSUMES
> >
> >  [UserExtensions.TianoCore."ExtraFiles"]
> >    RedfishRestExDxeExtra.uni
> > diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
> > b/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
> > index 952e9d1..0da0d10 100644
> > --- a/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
> > +++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExImpl.c
> > @@ -140,9 +140,6 @@ RedfishHttpAddExpectation (
> >    }
> >
> >    *ItsWrite = FALSE;
> > -  if (PreservedRequestHeaders != NULL) {
> > -    *PreservedRequestHeaders = RequestMessage->Headers;
> > -  }
> >
> >    if ((RequestMessage->Data.Request->Method != HttpMethodPut) &&
> > (RequestMessage->Data.Request->Method != HttpMethodPost) &&
> >        (RequestMessage->Data.Request->Method != HttpMethodPatch)) @@ -
> > 152,10 +149,20 @@ RedfishHttpAddExpectation (
> >
> >    *ItsWrite = TRUE;
> >
> > -  NewHeaders = AllocateZeroPool ((RequestMessage->HeaderCount + 1) *
> > sizeof (EFI_HTTP_HEADER));
> > -  CopyMem ((VOID *)NewHeaders, (VOID *)RequestMessage->Headers,
> > RequestMessage->HeaderCount * sizeof (EFI_HTTP_HEADER));
> > -  HttpSetFieldNameAndValue (NewHeaders + RequestMessage->HeaderCount,
> > HTTP_HEADER_EXPECT, HTTP_EXPECT_100_CONTINUE);
> > -  RequestMessage->HeaderCount++;
> > -  RequestMessage->Headers = NewHeaders;
> > +  //
> > +  // Check PCD before adding Expect header  //
> > +  if(FixedPcdGetBool(PcdRedfishRestExAddingExpect)) {
> > +    if (PreservedRequestHeaders != NULL) {
> > +      *PreservedRequestHeaders = RequestMessage->Headers;
> > +    }
> > +
> > +    NewHeaders = AllocateZeroPool ((RequestMessage->HeaderCount + 1)
> > + *
> > sizeof (EFI_HTTP_HEADER));
> > +    CopyMem ((VOID *)NewHeaders, (VOID *)RequestMessage->Headers,
> > RequestMessage->HeaderCount * sizeof (EFI_HTTP_HEADER));
> > +    HttpSetFieldNameAndValue (NewHeaders +
> > + RequestMessage->HeaderCount,
> > HTTP_HEADER_EXPECT, HTTP_EXPECT_100_CONTINUE);
> > +    RequestMessage->HeaderCount++;
> > +    RequestMessage->Headers = NewHeaders;  }
> > +
> >    return EFI_SUCCESS;
> >  }
> > diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
> > b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
> > index 4b61fc0..fbd5e46 100644
> > --- a/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
> > +++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExProtocol.c
> > @@ -66,11 +66,13 @@ RedfishRestExSendReceive (
> >    HTTP_IO_SEND_NON_CHUNK_PROCESS  SendNonChunkProcess;
> >    EFI_HTTP_MESSAGE                ChunkTransferRequestMessage;
> >
> > -  Status               = EFI_SUCCESS;
> > -  ResponseData         = NULL;
> > -  IsGetChunkedTransfer = FALSE;
> > -  SendChunkProcess     = HttpIoSendChunkNone;
> > -  SendNonChunkProcess  = HttpIoSendNonChunkNone;
> > +  Status                    = EFI_SUCCESS;
> > +  ResponseData              = NULL;
> > +  IsGetChunkedTransfer      = FALSE;
> > +  SendChunkProcess          = HttpIoSendChunkNone;
> > +  SendNonChunkProcess       = HttpIoSendNonChunkNone;
> > +  ItsWrite                  = FALSE;
> > +  PreservedRequestHeaders   = NULL;
> >
> >    //
> >    // Validate the parameters
> > @@ -94,67 +96,85 @@ RedfishRestExSendReceive (
> >    DEBUG ((DEBUG_INFO, "\nRedfishRestExSendReceive():\n"));
> >    DEBUG ((DEBUG_INFO, "*** Perform HTTP Request Method - %d, URL:
> > %s\n",
> > RequestMessage->Data.Request->Method, RequestMessage->Data.Request-
> > >Url));
> >
> > -  //
> > -  // Add header "Expect" to server, only for URL write.
> > -  //
> > -  Status = RedfishHttpAddExpectation (This, RequestMessage,
> > &PreservedRequestHeaders, &ItsWrite);
> > -  if (EFI_ERROR (Status)) {
> > -    return Status;
> > -  }
> > +  if(FixedPcdGetBool(PcdRedfishRestExChunkRequestMode)){
> Due to whether to add "Expect" header is managed by
> PcdRedfishRestExAddingExpect, so we can relocate the condition check for
> PcdRedfishRestExChunkRequestMode to be above  "if (ItsWrite == TRUE) {" in 8
> lines later.
> With this, we can add code later to send the zero content payload with "Expect"
> header and check the HTTP Status for the HTTP Chunk transfer or other error
> cases.
> Igor: I'm not sure if it would help anyhow. We cannot use "Expect" without
> chunked request. If I understand it correctly, the main point of "Expect" header
> is to send just headers without body to see if the URI supported. But if chunked
> request is disabled then we would send the whole request with the body anyway.
> So, there is no difference if we send it with or without "Expect" header.
Actually you are right for the current implementation.  However, I would like to separate the Chunk PCD with Expect PCD because adding "expect" is not just for the chunk transfer. It can be also used to handle the "Method not Allowed" error if client patches a readable Redfish property. So use the Chunk PCD to determine whether adding  "Expect" or not is not quite correct. However...please see the next comment.

> And another purpose of that RedfishHttpAddExpectation function, besides of
> adding "Expect" header, is to figure out if it is POST, PATCH or PUT request and
> set ItsWrite variable. But this variable also used only for the chunked requests.
> So, I think, it is better to keep it under
> if(FixedPcdGetBool(PcdRedfishRestExChunkRequestMode)) condition.
> What do you think?
However, you are right at this point. I think we will have to separate the code that sets "IsWrite" flag from  RedfishHttpAddExpectation(), this makes this function clear and has no dependency with the chunk transfer.
Let's just leave it as it for now. We can enhance this later. 😊
Please just fix other two comments and send the V3 with my reviewed-by. Please also create a PR against to edk2 repo so I can merge it if V3 has no problems.

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

Thanks for this contribution.
Abner

> 
> > +    //
> > +    // Add header "Expect" to server, only for URL write.
> > +    //
> > +    Status = RedfishHttpAddExpectation (This, RequestMessage,
> > &PreservedRequestHeaders, &ItsWrite);
> > +    if (EFI_ERROR (Status)) {
> > +      return Status;
> > +    }
> >
> > -  if (ItsWrite == TRUE) {
> > -    if (RequestMessage->BodyLength > HTTP_IO_MAX_SEND_PAYLOAD) {
> > -      //
> > -      // Send chunked transfer.
> > -      //
> > -      SendChunkProcess++;
> > -      CopyMem ((VOID *)&ChunkTransferRequestMessage, (VOID
> > *)RequestMessage, sizeof (EFI_HTTP_MESSAGE));
> > -    } else {
> > -      SendNonChunkProcess++;
> > +    if (ItsWrite == TRUE) {
> > +      if (RequestMessage->BodyLength > HTTP_IO_MAX_SEND_PAYLOAD) {
> > +        //
> > +        // Send chunked transfer.
> > +        //
> > +        SendChunkProcess++;
> > +        CopyMem ((VOID *)&ChunkTransferRequestMessage, (VOID
> > *)RequestMessage, sizeof (EFI_HTTP_MESSAGE));
> > +      } else {
> > +        SendNonChunkProcess++;
> > +      }
> >      }
> >    }
> >
> >  ReSendRequest:;
> > -  //
> > -  // Send out the request to REST service.
> > -  //
> > -  if (ItsWrite == TRUE) {
> > +
> > +  if(FixedPcdGetBool(PcdRedfishRestExChunkRequestMode)){
> >      //
> > -    // This is write to URI
> > +    // Send the chunked request to REST service.
> >      //
> > -    if (SendChunkProcess > HttpIoSendChunkNone) {
> > +    if (ItsWrite == TRUE) {
> >        //
> > -      // This is chunk transfer for writing large payload.
> > -      // Send request header first and then handle the
> > -      // following request message body using chunk transfer.
> > +      // This is write to URI
> >        //
> > -      do {
> > -        Status = HttpIoSendChunkedTransfer (
> > +      if (SendChunkProcess > HttpIoSendChunkNone) {
> > +        //
> > +        // This is chunk transfer for writing large payload.
> > +        // Send request header first and then handle the
> > +        // following request message body using chunk transfer.
> > +        //
> > +        do {
> > +          Status = HttpIoSendChunkedTransfer (
> > +                     &(Instance->HttpIo),
> > +                     &SendChunkProcess,
> > +                     &ChunkTransferRequestMessage
> > +                     );
> > +          if (EFI_ERROR (Status)) {
> > +            goto ON_EXIT;
> > +          }
> > +        } while (SendChunkProcess == HttpIoSendChunkContent ||
> > SendChunkProcess == HttpIoSendChunkEndChunk);
> > +      } else {
> > +        //
> > +        // This is the non-chunk transfer, send request header first and then
> > +        // handle the following request message body using chunk transfer.
> > +        //
> > +        Status = HttpIoSendRequest (
> >                     &(Instance->HttpIo),
> > -                   &SendChunkProcess,
> > -                   &ChunkTransferRequestMessage
> > +                   (SendNonChunkProcess == HttpIoSendNonChunkContent) ? NULL :
> > RequestMessage->Data.Request,
> > +                   (SendNonChunkProcess == HttpIoSendNonChunkContent) ? 0 :
> > RequestMessage->HeaderCount,
> > +                   (SendNonChunkProcess == HttpIoSendNonChunkContent) ? NULL :
> > RequestMessage->Headers,
> > +                   (SendNonChunkProcess ==
> > HttpIoSendNonChunkHeaderZeroContent) ? 0 : RequestMessage->BodyLength,
> > +                   (SendNonChunkProcess ==
> > + HttpIoSendNonChunkHeaderZeroContent) ? NULL : RequestMessage->Body
> >                     );
> > -        if (EFI_ERROR (Status)) {
> > -          goto ON_EXIT;
> > -        }
> > -      } while (SendChunkProcess == HttpIoSendChunkContent ||
> > SendChunkProcess == HttpIoSendChunkEndChunk);
> > +      }
> >      } else {
> >        //
> > -      // This is the non-chunk transfer, send request header first and then
> > -      // handle the following request message body using chunk transfer.
> > +      // This is read from URI.
> >        //
> >        Status = HttpIoSendRequest (
> >                   &(Instance->HttpIo),
> > -                 (SendNonChunkProcess == HttpIoSendNonChunkContent) ? NULL :
> > RequestMessage->Data.Request,
> > -                 (SendNonChunkProcess == HttpIoSendNonChunkContent) ? 0 :
> > RequestMessage->HeaderCount,
> > -                 (SendNonChunkProcess == HttpIoSendNonChunkContent) ? NULL :
> > RequestMessage->Headers,
> > -                 (SendNonChunkProcess ==
> HttpIoSendNonChunkHeaderZeroContent) ?
> > 0 : RequestMessage->BodyLength,
> > -                 (SendNonChunkProcess ==
> HttpIoSendNonChunkHeaderZeroContent) ?
> > NULL : RequestMessage->Body
> > +                 RequestMessage->Data.Request,
> > +                 RequestMessage->HeaderCount,
> > +                 RequestMessage->Headers,
> > +                 RequestMessage->BodyLength,
> > +                 RequestMessage->Body
> >                   );
> >      }
> > -  } else {
> > +  }
> > +  else{
> Please have "else" at the same line with above closed curly bracket and have a
> space between "else" and "{"
> Igor: My mistake. Fixed.
> Thanks
> Abner
> 
> >      //
> > -    // This is read from URI.
> > +    // This is normal request to URI.
> >      //
> >      Status = HttpIoSendRequest (
> >                 &(Instance->HttpIo),
> > @@ -213,7 +233,7 @@ ReSendRequest:;
> >    //
> >    // Restore the headers if it ever changed in RedfishHttpAddExpectation().
> >    //
> > -  if (RequestMessage->Headers != PreservedRequestHeaders) {
> > +  if (FixedPcdGetBool(PcdRedfishRestExAddingExpect) &&
> > + RequestMessage->Headers != PreservedRequestHeaders) {
> >      FreePool (RequestMessage->Headers);
> >      RequestMessage->Headers = PreservedRequestHeaders; // Restore
> > headers before we adding "Expect".
> >      RequestMessage->HeaderCount--;                     // Minus one header count
> for
> > "Expect".
> > @@ -223,11 +243,23 @@ ReSendRequest:;
> >    if (ResponseData->Response.StatusCode == HTTP_STATUS_200_OK) {
> >      DEBUG ((DEBUG_INFO, "HTTP_STATUS_200_OK\n"));
> >
> > -    if (SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> > -      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all chunks."));
> > +    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) &&
> > SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> > +      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all
> > + chunks.", ResponseData->Response.StatusCode));
> >        SendChunkProcess++;
> >        goto ReSendRequest;
> >      }
> > +  } else if (ResponseData->Response.StatusCode ==
> > HTTP_STATUS_204_NO_CONTENT) {
> > +    DEBUG ((DEBUG_INFO, "HTTP_STATUS_204_NO_CONTENT\n"));
> > +
> > +    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) &&
> > SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> > +      DEBUG ((DEBUG_INFO, "This is chunk transfer, start to send all
> > + chunks.",
> > ResponseData->Response.StatusCode));
> > +      SendChunkProcess ++;
> > +      goto ReSendRequest;
> > +    }
> > +  } else if (ResponseData->Response.StatusCode ==
> > HTTP_STATUS_201_CREATED) {
> > +    DEBUG ((DEBUG_INFO, "HTTP_STATUS_201_CREATED\n"));  } else if
> > + (ResponseData->Response.StatusCode == HTTP_STATUS_202_ACCEPTED) {
> > +    DEBUG ((DEBUG_INFO, "HTTP_STATUS_202_ACCEPTED\n"));
> >    } else if (ResponseData->Response.StatusCode ==
> > HTTP_STATUS_413_REQUEST_ENTITY_TOO_LARGE) {
> >      DEBUG ((DEBUG_INFO,
> > "HTTP_STATUS_413_REQUEST_ENTITY_TOO_LARGE\n"));
> >
> > @@ -240,14 +272,14 @@ ReSendRequest:;
> >      goto ON_EXIT;
> >    } else if (ResponseData->Response.StatusCode ==
> > HTTP_STATUS_400_BAD_REQUEST) {
> >      DEBUG ((DEBUG_INFO, "HTTP_STATUS_400_BAD_REQUEST\n"));
> > -    if (SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> > +    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) &&
> > + SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> >        DEBUG ((DEBUG_INFO, "Bad request may caused by zero length
> > chunk. Try to send all chunks...\n"));
> >        SendChunkProcess++;
> >        goto ReSendRequest;
> >      }
> >    } else if (ResponseData->Response.StatusCode ==
> > HTTP_STATUS_100_CONTINUE) {
> >      DEBUG ((DEBUG_INFO, "HTTP_STATUS_100_CONTINUE\n"));
> > -    if (SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> > +    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) &&
> > + SendChunkProcess == HttpIoSendChunkHeaderZeroContent) {
> >        //
> >        // We get HTTP_STATUS_100_CONTINUE to send the body using chunk
> > transfer.
> >        //
> > @@ -256,7 +288,7 @@ ReSendRequest:;
> >        goto ReSendRequest;
> >      }
> >
> > -    if (SendNonChunkProcess == HttpIoSendNonChunkHeaderZeroContent) {
> > +    if (FixedPcdGetBool(PcdRedfishRestExChunkRequestMode) &&
> > + SendNonChunkProcess == HttpIoSendNonChunkHeaderZeroContent) {
> >        DEBUG ((DEBUG_INFO, "HTTP_STATUS_100_CONTINUE for non chunk
> > transfer...\n"));
> >        SendNonChunkProcess++;
> >        goto ReSendRequest;
> > @@ -394,7 +426,7 @@ ReSendRequest:;
> >        TotalReceivedSize += ResponseData->BodyLength;
> >      }
> >
> > -    DEBUG ((DEBUG_INFO, "Total of lengh of Response :%d\n",
> > TotalReceivedSize));
> > +    DEBUG ((DEBUG_INFO, "Total of length of Response :%d\n",
> > + TotalReceivedSize));
> >    }
> >
> >    DEBUG ((DEBUG_INFO, "RedfishRestExSendReceive()- EFI_STATUS: %r\n",
> > Status));
> > --
> > 2.6.1.windows.1
> > -The information contained in this message may be confidential and
> > proprietary to American Megatrends (AMI). This communication is
> > intended to be read only by the individual or entity to whom it is
> > addressed or by their designee. If the reader of this message is not
> > the intended recipient, you are on notice that any distribution of
> > this message, in any form, is strictly prohibited. Please promptly
> > notify the sender by reply e-mail or by telephone at 770-246-8600, and then
> delete or destroy all copies of the transmission.
> -The information contained in this message may be confidential and proprietary
> to American Megatrends (AMI). This communication is intended to be read only
> by the individual or entity to whom it is addressed or by their designee. If the
> reader of this message is not the intended recipient, you are on notice that any
> distribution of this message, in any form, is strictly prohibited. Please promptly
> notify the sender by reply e-mail or by telephone at 770-246-8600, and then
> delete or destroy all copies of the transmission.


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