Re: [edk2] [PATCH v2] NetworkPkg:Add scriptable configuration to iSCSI driver by

2017-02-20 Thread Ye, Ting 
Some minor comments for this patch:

1. IScsiConvertAttemptConfigDataToIfrNvDataByKeyword
Suggest to describe more details about this function, especially the 
difference with existing IScsiConvertAttemptConfigDataToIfrNvData.
Also suggest to add more description for other new added functions, for 
example, IScsiConfigAddAttemptsByKeywords. We might add the link to x-UEFI 
keywords to some functions.

2. typos in IScsiDriver.c: Maxmum->Maximum; Creat-> Create 
3. Suggest to update the name " InitiatorAttemptOrder", as "initiator" has 
special meaning to iSCSI. It might be confusing. 

Others are good to me.
Reviewed-by: Ye Ting  

Thanks,
Ting



-Original Message-
From: Zhang, Lubo 
Sent: Friday, February 17, 2017 2:56 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Fu, Siyuan ; Wu, Jiaxin 

Subject: [PATCH v2] NetworkPkg:Add scriptable configuration to iSCSI driver by

v2:
Add error handling if can not create Attempts in driver entry point.
Since we support to define a macro be a PCD value, we enhance our code
by modifying the structure in IFR_NVDATA. This effect code logic mainly
in Creating Keywords,Convert IFR NvData To AttemptConfigData ByKeyword and
reverse function.
Fix typo errors and sync based on the latest code.

Enable iSCSI keywords configuration based on x-UEFI
name space. we introduce new PCD to control the attempt
numbers which will be created in non activated state, besides
the Attempt name is changed to READ_ONLY attribute in UI.
We can invoke KEYWORD HANDLER Protocol to configure
the related keywords.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Wu Jiaxin 
---
 NetworkPkg/IScsiDxe/IScsiConfig.c| 1744 +-
 NetworkPkg/IScsiDxe/IScsiConfig.h|   78 +-
 NetworkPkg/IScsiDxe/IScsiConfigNVDataStruc.h |   67 +-
 NetworkPkg/IScsiDxe/IScsiConfigStrings.uni   |   28 +-
 NetworkPkg/IScsiDxe/IScsiConfigVfr.vfr   |   46 +-
 NetworkPkg/IScsiDxe/IScsiDriver.c|   28 +-
 NetworkPkg/IScsiDxe/IScsiDriver.h|3 +-
 NetworkPkg/IScsiDxe/IScsiDxe.inf |5 +-
 NetworkPkg/IScsiDxe/IScsiMisc.c  |  815 +++-
 NetworkPkg/IScsiDxe/IScsiMisc.h  |   38 +
 NetworkPkg/NetworkPkg.dec|6 +-
 11 files changed, 2506 insertions(+), 352 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiConfig.c 
b/NetworkPkg/IScsiDxe/IScsiConfig.c
index 40ea75a..d07bdcb 100644
--- a/NetworkPkg/IScsiDxe/IScsiConfig.c
+++ b/NetworkPkg/IScsiDxe/IScsiConfig.c
@@ -288,10 +288,102 @@ IScsiConvertIsIdToString (
 
   return EFI_SUCCESS;
 }
 
 /**
+  Get the Offset value specified by the input String.
+
+  @param[in]  Configuration  A null-terminated Unicode string in
+  format.
+  @param[in]  String The string is "=".
+  @param[out]  Value  The Offset value.
+
+  @retval EFI_OUT_OF_RESOURCES   Insufficient resources to store neccessary
+ structures.
+  @retval EFI_SUCCESSValue of  is outputted in Number
+ successfully.
+
+**/
+EFI_STATUS
+IScsiGetValue (
+  IN  CONST EFI_STRING Configuration,
+  IN  CHAR16   *String,
+  OUT UINTN*Value
+  )
+{
+  CHAR16   *StringPtr;
+  CHAR16   *TmpPtr;
+  CHAR16   *Str;
+  CHAR16   TmpStr[2];
+  UINTNLength;
+  UINTNLen;
+  UINTNIndex;
+  UINT8*Buf;
+  UINT8DigitUint8;
+  EFI_STATUS   Status;
+
+  //
+  // Get Value.
+  //
+  Buf = NULL;
+  StringPtr = StrStr (Configuration, String);
+  ASSERT(StringPtr != NULL);
+  StringPtr += StrLen (String);
+  TmpPtr = StringPtr;
+
+  while (*StringPtr != L'\0' && *StringPtr != L'&') {
+StringPtr ++;
+  }
+  Length = StringPtr - TmpPtr;
+  Len = Length + 1;
+
+  Str = AllocateZeroPool (Len * sizeof (CHAR16));
+  if (Str == NULL) {
+Status = EFI_OUT_OF_RESOURCES;
+goto Exit;
+  }
+
+  CopyMem (Str, TmpPtr, Len * sizeof (CHAR16));
+  *(Str + Length) = L'\0';
+
+  Len = (Len + 1) / 2;
+  Buf = (UINT8 *) AllocateZeroPool (Len);
+  if (Buf == NULL) {
+Status = EFI_OUT_OF_RESOURCES;
+goto Exit;
+  }
+
+  ZeroMem (TmpStr, sizeof (TmpStr));
+  for (Index = 0; Index < Length; Index ++) {
+TmpStr[0] = Str[Length - Index - 1];
+DigitUint8 = (UINT8) StrHexToUint64 (TmpStr);
+if ((Index & 1) == 0) {
+  Buf [Index/2] = DigitUint8;
+} else {
+  Buf [Index/2] = (UINT8) ((DigitUint8 << 4) + Buf [Index/2]);
+}
+

[edk2] [PATCH v2] NetworkPkg:Add scriptable configuration to iSCSI driver by

2017-02-16 Thread Zhang Lubo 
v2:
Add error handling if can not create Attempts in driver entry point.
Since we support to define a macro be a PCD value, we enhance our code
by modifying the structure in IFR_NVDATA. This effect code logic mainly
in Creating Keywords,Convert IFR NvData To AttemptConfigData ByKeyword and
reverse function.
Fix typo errors and sync based on the latest code.

Enable iSCSI keywords configuration based on x-UEFI
name space. we introduce new PCD to control the attempt
numbers which will be created in non activated state, besides
the Attempt name is changed to READ_ONLY attribute in UI.
We can invoke KEYWORD HANDLER Protocol to configure
the related keywords.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Wu Jiaxin 
---
 NetworkPkg/IScsiDxe/IScsiConfig.c| 1744 +-
 NetworkPkg/IScsiDxe/IScsiConfig.h|   78 +-
 NetworkPkg/IScsiDxe/IScsiConfigNVDataStruc.h |   67 +-
 NetworkPkg/IScsiDxe/IScsiConfigStrings.uni   |   28 +-
 NetworkPkg/IScsiDxe/IScsiConfigVfr.vfr   |   46 +-
 NetworkPkg/IScsiDxe/IScsiDriver.c|   28 +-
 NetworkPkg/IScsiDxe/IScsiDriver.h|3 +-
 NetworkPkg/IScsiDxe/IScsiDxe.inf |5 +-
 NetworkPkg/IScsiDxe/IScsiMisc.c  |  815 +++-
 NetworkPkg/IScsiDxe/IScsiMisc.h  |   38 +
 NetworkPkg/NetworkPkg.dec|6 +-
 11 files changed, 2506 insertions(+), 352 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiConfig.c 
b/NetworkPkg/IScsiDxe/IScsiConfig.c
index 40ea75a..d07bdcb 100644
--- a/NetworkPkg/IScsiDxe/IScsiConfig.c
+++ b/NetworkPkg/IScsiDxe/IScsiConfig.c
@@ -288,10 +288,102 @@ IScsiConvertIsIdToString (
 
   return EFI_SUCCESS;
 }
 
 /**
+  Get the Offset value specified by the input String.
+
+  @param[in]  Configuration  A null-terminated Unicode string in
+  format.
+  @param[in]  String The string is "=".
+  @param[out]  Value  The Offset value.
+
+  @retval EFI_OUT_OF_RESOURCES   Insufficient resources to store neccessary
+ structures.
+  @retval EFI_SUCCESSValue of  is outputted in Number
+ successfully.
+
+**/
+EFI_STATUS
+IScsiGetValue (
+  IN  CONST EFI_STRING Configuration,
+  IN  CHAR16   *String,
+  OUT UINTN*Value
+  )
+{
+  CHAR16   *StringPtr;
+  CHAR16   *TmpPtr;
+  CHAR16   *Str;
+  CHAR16   TmpStr[2];
+  UINTNLength;
+  UINTNLen;
+  UINTNIndex;
+  UINT8*Buf;
+  UINT8DigitUint8;
+  EFI_STATUS   Status;
+
+  //
+  // Get Value.
+  //
+  Buf = NULL;
+  StringPtr = StrStr (Configuration, String);
+  ASSERT(StringPtr != NULL);
+  StringPtr += StrLen (String);
+  TmpPtr = StringPtr;
+
+  while (*StringPtr != L'\0' && *StringPtr != L'&') {
+StringPtr ++;
+  }
+  Length = StringPtr - TmpPtr;
+  Len = Length + 1;
+
+  Str = AllocateZeroPool (Len * sizeof (CHAR16));
+  if (Str == NULL) {
+Status = EFI_OUT_OF_RESOURCES;
+goto Exit;
+  }
+
+  CopyMem (Str, TmpPtr, Len * sizeof (CHAR16));
+  *(Str + Length) = L'\0';
+
+  Len = (Len + 1) / 2;
+  Buf = (UINT8 *) AllocateZeroPool (Len);
+  if (Buf == NULL) {
+Status = EFI_OUT_OF_RESOURCES;
+goto Exit;
+  }
+
+  ZeroMem (TmpStr, sizeof (TmpStr));
+  for (Index = 0; Index < Length; Index ++) {
+TmpStr[0] = Str[Length - Index - 1];
+DigitUint8 = (UINT8) StrHexToUint64 (TmpStr);
+if ((Index & 1) == 0) {
+  Buf [Index/2] = DigitUint8;
+} else {
+  Buf [Index/2] = (UINT8) ((DigitUint8 << 4) + Buf [Index/2]);
+}
+  }
+
+  *Value = 0;
+  CopyMem (
+Value,
+Buf,
+(((Length + 1) / 2) < sizeof (UINTN)) ? ((Length + 1) / 2) : sizeof (UINTN)
+);
+
+  FreePool (Buf);
+  Status = EFI_SUCCESS;
+
+Exit:
+  if (Str != NULL) {
+FreePool (Str);
+  }
+
+  return Status;
+}
+
+/**
   Get the attempt config data from global structure by the ConfigIndex.
 
   @param[in]  AttemptConfigIndex The unique index indicates the attempt.
 
   @return   Pointer to the attempt config data.
@@ -347,10 +439,68 @@ IScsiConfigGetAttemptByNic (
   }
 
   return NULL;
 }
 
+/**
+  Extract the Index of the attempt list.
+
+  @param[in]   AttemptNameList The Name list of the Attempts.
+  @param[out]  AttemptIndexListThe Index list of the Attempts.
+  @param[in]   IsAddAttempts   If TRUE, Indicates add one or more attempts.
+   If FALSE, Indicates delete attempts or 
change attempt order.
+
+  @retval EFI_SUCCESS  The Attempt list is