* [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
@ 2017-10-17 1:58 Jiaxin Wu
2017-10-17 1:58 ` [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support condition check Jiaxin Wu
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Jiaxin Wu @ 2017-10-17 1:58 UTC (permalink / raw)
To: edk2-devel; +Cc: Karunakar P, Ye Ting, Fu Siyuan, Wu Jiaxin
Base on the request of https://bugzilla.tianocore.org/show_bug.cgi?id=710,
we provide this patch to IPv6 condition check by leveraging AIP Protocol.
Cc: Karunakar P <karunakarp@amiindia.co.in>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Karunakar P <karunakarp@amiindia.co.in>
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
Jiaxin Wu (2):
NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
NetworkPkg/IScsiDxe: Add IPv6 support condition check.
NetworkPkg/HttpBootDxe/HttpBootDxe.c | 115 +++++++++++++++++++++++++++-
NetworkPkg/HttpBootDxe/HttpBootDxe.h | 2 +
NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 4 +-
NetworkPkg/IScsiDxe/IScsiConfig.c | 18 +++++
NetworkPkg/IScsiDxe/IScsiDriver.c | 2 +-
NetworkPkg/IScsiDxe/IScsiDriver.h | 1 +
NetworkPkg/IScsiDxe/IScsiDxe.inf | 2 +
NetworkPkg/IScsiDxe/IScsiImpl.h | 1 +
NetworkPkg/IScsiDxe/IScsiMisc.c | 135 ++++++++++++++++++++++++++++++++-
NetworkPkg/IScsiDxe/IScsiMisc.h | 4 +-
10 files changed, 278 insertions(+), 6 deletions(-)
--
1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
2017-10-17 1:58 [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI Jiaxin Wu
@ 2017-10-17 1:58 ` Jiaxin Wu
2017-10-17 1:58 ` [Patch 2/2] NetworkPkg/IScsiDxe: " Jiaxin Wu
2017-10-17 2:01 ` [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI Wu, Jiaxin
2 siblings, 0 replies; 15+ messages in thread
From: Jiaxin Wu @ 2017-10-17 1:58 UTC (permalink / raw)
To: edk2-devel; +Cc: Karunakar P, Ye Ting, Fu Siyuan, Wu Jiaxin
Base on the request of https://bugzilla.tianocore.org/show_bug.cgi?id=710,
we provide this patch to IPv6 condition check by leveraging AIP Protocol.
Cc: Karunakar P <karunakarp@amiindia.co.in>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Karunakar P <karunakarp@amiindia.co.in>
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
NetworkPkg/HttpBootDxe/HttpBootDxe.c | 115 ++++++++++++++++++++++++++++++++-
NetworkPkg/HttpBootDxe/HttpBootDxe.h | 2 +
NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 4 +-
3 files changed, 119 insertions(+), 2 deletions(-)
diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.c b/NetworkPkg/HttpBootDxe/HttpBootDxe.c
index 642e0fe..1a67a87 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDxe.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.c
@@ -1,9 +1,9 @@
/** @file
Driver Binding functions implementation for UEFI HTTP boot.
-Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials are licensed and made available under
the terms and conditions of the BSD License that accompanies this distribution.
The full text of the license may be found at
http://opensource.org/licenses/bsd-license.php.
@@ -33,10 +33,106 @@ EFI_DRIVER_BINDING_PROTOCOL gHttpBootIp6DxeDriverBinding = {
HTTP_BOOT_DXE_VERSION,
NULL,
NULL
};
+
+
+/**
+ Check whether UNDI protocol supports IPv6.
+
+ @param[in] ControllerHandle Controller handle.
+ @param[in] Private Pointer to HTTP_BOOT_PRIVATE_DATA.
+ @param[out] Ipv6Support TRUE if UNDI supports IPv6.
+
+ @retval EFI_SUCCESS Get the result whether UNDI supports IPv6 by NII or AIP protocol successfully.
+ @retval EFI_NOT_FOUND Don't know whether UNDI supports IPv6 since NII or AIP is not available.
+
+**/
+EFI_STATUS
+HttpBootCheckIpv6Support (
+ IN EFI_HANDLE ControllerHandle,
+ IN HTTP_BOOT_PRIVATE_DATA *Private,
+ OUT BOOLEAN *Ipv6Support
+ )
+{
+ EFI_HANDLE Handle;
+ EFI_ADAPTER_INFORMATION_PROTOCOL *Aip;
+ EFI_STATUS Status;
+ EFI_GUID *InfoTypesBuffer;
+ UINTN InfoTypeBufferCount;
+ UINTN TypeIndex;
+ BOOLEAN Supported;
+ VOID *InfoBlock;
+ UINTN InfoBlockSize;
+
+ ASSERT (Private != NULL && Ipv6Support != NULL);
+
+ //
+ // Check whether the UNDI supports IPv6 by NII protocol.
+ //
+ if (Private->Nii != NULL) {
+ *Ipv6Support = Private->Nii->Ipv6Supported;
+ return EFI_SUCCESS;
+ }
+
+ //
+ // Get the NIC handle by SNP protocol.
+ //
+ Handle = NetLibGetSnpHandle (ControllerHandle, NULL);
+ if (Handle == NULL) {
+ return EFI_NOT_FOUND;
+ }
+
+ Aip = NULL;
+ Status = gBS->HandleProtocol (
+ Handle,
+ &gEfiAdapterInformationProtocolGuid,
+ (VOID *) &Aip
+ );
+ if (EFI_ERROR (Status) || Aip == NULL) {
+ return EFI_NOT_FOUND;
+ }
+
+ InfoTypesBuffer = NULL;
+ InfoTypeBufferCount = 0;
+ Status = Aip->GetSupportedTypes (Aip, &InfoTypesBuffer, &InfoTypeBufferCount);
+ if (EFI_ERROR (Status) || InfoTypesBuffer == NULL) {
+ FreePool (InfoTypesBuffer);
+ return EFI_NOT_FOUND;
+ }
+
+ Supported = FALSE;
+ for (TypeIndex = 0; TypeIndex < InfoTypeBufferCount; TypeIndex++) {
+ if (CompareGuid (&InfoTypesBuffer[TypeIndex], &gEfiAdapterInfoUndiIpv6SupportGuid)) {
+ Supported = TRUE;
+ break;
+ }
+ }
+
+ FreePool (InfoTypesBuffer);
+ if (!Supported) {
+ return EFI_NOT_FOUND;
+ }
+
+ //
+ // We now have adapter information block.
+ //
+ InfoBlock = NULL;
+ InfoBlockSize = 0;
+ Status = Aip->GetInformation (Aip, &gEfiAdapterInfoUndiIpv6SupportGuid, &InfoBlock, &InfoBlockSize);
+ if (EFI_ERROR (Status) || InfoBlock == NULL) {
+ FreePool (InfoBlock);
+ return EFI_NOT_FOUND;
+ }
+
+ *Ipv6Support = ((EFI_ADAPTER_INFO_UNDI_IPV6_SUPPORT *) InfoBlock)->Ipv6Support;
+ FreePool (InfoBlock);
+
+ return EFI_SUCCESS;
+}
+
/**
Destroy the HTTP child based on IPv4 stack.
@param[in] This Pointer to the EFI_DRIVER_BINDING_PROTOCOL.
@param[in] Private Pointer to HTTP_BOOT_PRIVATE_DATA.
@@ -780,10 +876,11 @@ HttpBootIp6DxeDriverBindingStart (
EFI_STATUS Status;
HTTP_BOOT_PRIVATE_DATA *Private;
EFI_DEV_PATH *Node;
EFI_DEVICE_PATH_PROTOCOL *DevicePath;
UINT32 *Id;
+ BOOLEAN Ipv6Available;
Status = gBS->OpenProtocol (
ControllerHandle,
&gEfiCallerIdGuid,
(VOID **) &Id,
@@ -856,10 +953,26 @@ HttpBootIp6DxeDriverBindingStart (
if (EFI_ERROR (Status)) {
goto ON_ERROR;
}
}
+
+ //
+ // Set IPv6 available flag.
+ //
+ Status = HttpBootCheckIpv6Support (ControllerHandle, This->DriverBindingHandle, &Ipv6Available);
+ if (EFI_ERROR (Status)) {
+ //
+ // Fail to get the data whether UNDI supports IPv6.
+ // Set default value to TRUE.
+ //
+ Ipv6Available = TRUE;
+ }
+
+ if (!Ipv6Available) {
+ return EFI_UNSUPPORTED;
+ }
if (Private->Ip6Nic != NULL) {
//
// Already created before
//
diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.h b/NetworkPkg/HttpBootDxe/HttpBootDxe.h
index 2c07cf7..166bc45 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDxe.h
+++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.h
@@ -56,10 +56,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Protocol/Dns6.h>
#include <Protocol/Http.h>
#include <Protocol/Ip4Config2.h>
#include <Protocol/Ip6Config.h>
#include <Protocol/RamDisk.h>
+#include <Protocol/AdapterInformation.h>
+
//
// Produced Protocols
//
#include <Protocol/LoadFile.h>
#include <Protocol/HttpBootCallback.h>
diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.inf b/NetworkPkg/HttpBootDxe/HttpBootDxe.inf
index 4d6c5e5..6d2a772 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDxe.inf
+++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.inf
@@ -84,19 +84,21 @@
gEfiIp6ConfigProtocolGuid ## TO_START
gEfiNetworkInterfaceIdentifierProtocolGuid_31 ## SOMETIMES_CONSUMES
gEfiRamDiskProtocolGuid ## SOMETIMES_CONSUMES
gEfiHiiConfigAccessProtocolGuid ## BY_START
gEfiHttpBootCallbackProtocolGuid ## SOMETIMES_PRODUCES
-
+ gEfiAdapterInformationProtocolGuid ## SOMETIMES_CONSUMES
+
[Guids]
## SOMETIMES_CONSUMES ## GUID # HiiIsConfigHdrMatch mHttpBootConfigStorageName
## SOMETIMES_PRODUCES ## GUID # HiiConstructConfigHdr mHttpBootConfigStorageName
## SOMETIMES_PRODUCES ## GUID # HiiGetBrowserData mHttpBootConfigStorageName
## SOMETIMES_CONSUMES ## HII
gHttpBootConfigGuid
gEfiVirtualCdGuid ## SOMETIMES_CONSUMES ## GUID
gEfiVirtualDiskGuid ## SOMETIMES_CONSUMES ## GUID
+ gEfiAdapterInfoUndiIpv6SupportGuid ## SOMETIMES_CONSUMES ## GUID
[Pcd]
gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections ## CONSUMES
[UserExtensions.TianoCore."ExtraFiles"]
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Patch 2/2] NetworkPkg/IScsiDxe: Add IPv6 support condition check.
2017-10-17 1:58 [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI Jiaxin Wu
2017-10-17 1:58 ` [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support condition check Jiaxin Wu
@ 2017-10-17 1:58 ` Jiaxin Wu
2017-10-17 2:01 ` [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI Wu, Jiaxin
2 siblings, 0 replies; 15+ messages in thread
From: Jiaxin Wu @ 2017-10-17 1:58 UTC (permalink / raw)
To: edk2-devel; +Cc: Karunakar P, Ye Ting, Fu Siyuan, Wu Jiaxin
Base on the request of https://bugzilla.tianocore.org/show_bug.cgi?id=710,
we provide this patch to IPv6 condition check by leveraging AIP Protocol.
Cc: Karunakar P <karunakarp@amiindia.co.in>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Karunakar P <karunakarp@amiindia.co.in>
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
NetworkPkg/IScsiDxe/IScsiConfig.c | 18 +++++
NetworkPkg/IScsiDxe/IScsiDriver.c | 2 +-
NetworkPkg/IScsiDxe/IScsiDriver.h | 1 +
NetworkPkg/IScsiDxe/IScsiDxe.inf | 2 +
NetworkPkg/IScsiDxe/IScsiImpl.h | 1 +
NetworkPkg/IScsiDxe/IScsiMisc.c | 135 +++++++++++++++++++++++++++++++++++++-
NetworkPkg/IScsiDxe/IScsiMisc.h | 4 +-
7 files changed, 159 insertions(+), 4 deletions(-)
diff --git a/NetworkPkg/IScsiDxe/IScsiConfig.c b/NetworkPkg/IScsiDxe/IScsiConfig.c
index 52e51d6..082020d 100644
--- a/NetworkPkg/IScsiDxe/IScsiConfig.c
+++ b/NetworkPkg/IScsiDxe/IScsiConfig.c
@@ -3419,10 +3419,13 @@ IScsiFormCallback (
EFI_IP_ADDRESS Gateway;
ISCSI_CONFIG_IFR_NVDATA *IfrNvData;
ISCSI_CONFIG_IFR_NVDATA OldIfrNvData;
EFI_STATUS Status;
EFI_INPUT_KEY Key;
+ ISCSI_NIC_INFO *NicInfo;
+
+ NicInfo = NULL;
if ((Action == EFI_BROWSER_ACTION_FORM_OPEN) || (Action == EFI_BROWSER_ACTION_FORM_CLOSE)) {
//
// Do nothing for UEFI OPEN/CLOSE Action
//
@@ -3589,10 +3592,25 @@ IScsiFormCallback (
break;
case KEY_IP_MODE:
switch (Value->u8) {
case IP_MODE_IP6:
+ NicInfo = IScsiGetNicInfoByIndex (Private->Current->NicIndex);
+ if(!NicInfo->Ipv6Available) {
+ //
+ // Current NIC doesn't Support IPv6, hence use IPv4.
+ //
+ IfrNvData->IpMode = IP_MODE_IP4;
+
+ CreatePopUp (
+ EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
+ &Key,
+ L"Current NIC doesn't Support IPv6!",
+ NULL
+ );
+ }
+
case IP_MODE_IP4:
ZeroMem (IfrNvData->TargetIp, sizeof (IfrNvData->TargetIp));
Private->Current->AutoConfigureMode = 0;
break;
diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c b/NetworkPkg/IScsiDxe/IScsiDriver.c
index 2249919..fbeef97 100644
--- a/NetworkPkg/IScsiDxe/IScsiDriver.c
+++ b/NetworkPkg/IScsiDxe/IScsiDriver.c
@@ -438,11 +438,11 @@ IScsiStart (
}
//
// Record the incoming NIC info.
//
- Status = IScsiAddNic (ControllerHandle);
+ Status = IScsiAddNic (ControllerHandle, Image);
if (EFI_ERROR (Status)) {
return Status;
}
//
diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.h b/NetworkPkg/IScsiDxe/IScsiDriver.h
index 6c6e11b..2db93c5 100644
--- a/NetworkPkg/IScsiDxe/IScsiDriver.h
+++ b/NetworkPkg/IScsiDxe/IScsiDriver.h
@@ -79,10 +79,11 @@ typedef struct {
UINT8 NicIndex;
UINT16 VlanId;
UINTN BusNumber;
UINTN DeviceNumber;
UINTN FunctionNumber;
+ BOOLEAN Ipv6Available;
} ISCSI_NIC_INFO;
typedef struct _ISCSI_PRIVATE_PROTOCOL {
UINT32 Reserved;
} ISCSI_PRIVATE_PROTOCOL;
diff --git a/NetworkPkg/IScsiDxe/IScsiDxe.inf b/NetworkPkg/IScsiDxe/IScsiDxe.inf
index 01998a0..319c7fe 100644
--- a/NetworkPkg/IScsiDxe/IScsiDxe.inf
+++ b/NetworkPkg/IScsiDxe/IScsiDxe.inf
@@ -115,18 +115,20 @@
gEfiIScsiInitiatorNameProtocolGuid
## PRODUCES
gEfiAuthenticationInfoProtocolGuid
## SOMETIMES_CONSUMES
gEfiAdapterInformationProtocolGuid
+ gEfiNetworkInterfaceIdentifierProtocolGuid_31 ## SOMETIMES_CONSUMES
[Guids]
gEfiEventExitBootServicesGuid ## SOMETIMES_CONSUMES ## Event
gEfiIfrTianoGuid ## SOMETIMES_PRODUCES ## UNDEFINED
gEfiAcpiTableGuid ## SOMETIMES_CONSUMES ## SystemTable
gEfiAcpi10TableGuid ## SOMETIMES_CONSUMES ## SystemTable
gEfiAcpi20TableGuid ## SOMETIMES_CONSUMES ## SystemTable
gEfiAdapterInfoNetworkBootGuid ## SOMETIMES_CONSUMES ## UNDEFINED
+ gEfiAdapterInfoUndiIpv6SupportGuid ## SOMETIMES_CONSUMES ## GUID
## SOMETIMES_PRODUCES ## Variable:L"AttemptOrder"
## SOMETIMES_CONSUMES ## Variable:L"AttemptOrder"
## SOMETIMES_PRODUCES ## Variable:L"InitialAttemptOrder"
## SOMETIMES_CONSUMES ## Variable:L"InitialAttemptOrder"
diff --git a/NetworkPkg/IScsiDxe/IScsiImpl.h b/NetworkPkg/IScsiDxe/IScsiImpl.h
index 741c497..9e36da0 100644
--- a/NetworkPkg/IScsiDxe/IScsiImpl.h
+++ b/NetworkPkg/IScsiDxe/IScsiImpl.h
@@ -37,10 +37,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Protocol/AuthenticationInfo.h>
#include <Protocol/IScsiInitiatorName.h>
#include <Protocol/ScsiPassThruExt.h>
#include <Protocol/AdapterInformation.h>
+#include <Protocol/NetworkInterfaceIdentifier.h>
#include <Library/HiiLib.h>
#include <Library/UefiHiiServicesLib.h>
#include <Library/DevicePathLib.h>
#include <Library/DebugLib.h>
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c
index 0a0a3f5..9e4164c 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.c
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.c
@@ -464,22 +464,128 @@ IScsiGenRandom (
}
}
/**
+ Check whether UNDI protocol supports IPv6.
+
+ @param[in] ControllerHandle Controller handle.
+ @param[in] Image Handle of the image.
+ @param[out] Ipv6Support TRUE if UNDI supports IPv6.
+
+ @retval EFI_SUCCESS Get the result whether UNDI supports IPv6 by NII or AIP protocol successfully.
+ @retval EFI_NOT_FOUND Don't know whether UNDI supports IPv6 since NII or AIP is not available.
+
+**/
+EFI_STATUS
+IScsiCheckIpv6Support (
+ IN EFI_HANDLE ControllerHandle,
+ IN EFI_HANDLE Image,
+ OUT BOOLEAN *Ipv6Support
+ )
+{
+ EFI_HANDLE Handle;
+ EFI_ADAPTER_INFORMATION_PROTOCOL *Aip;
+ EFI_STATUS Status;
+ EFI_GUID *InfoTypesBuffer;
+ UINTN InfoTypeBufferCount;
+ UINTN TypeIndex;
+ BOOLEAN Supported;
+ VOID *InfoBlock;
+ UINTN InfoBlockSize;
+
+ EFI_NETWORK_INTERFACE_IDENTIFIER_PROTOCOL *Nii;
+
+ ASSERT (Ipv6Support != NULL);
+
+ //
+ // Check whether the UNDI supports IPv6 by NII protocol.
+ //
+ Status = gBS->OpenProtocol (
+ ControllerHandle,
+ &gEfiNetworkInterfaceIdentifierProtocolGuid_31,
+ (VOID **) &Nii,
+ Image,
+ ControllerHandle,
+ EFI_OPEN_PROTOCOL_GET_PROTOCOL
+ );
+ if (Status == EFI_SUCCESS) {
+ *Ipv6Support = Nii->Ipv6Supported;
+ return EFI_SUCCESS;
+ }
+
+ //
+ // Get the NIC handle by SNP protocol.
+ //
+ Handle = NetLibGetSnpHandle (ControllerHandle, NULL);
+ if (Handle == NULL) {
+ return EFI_NOT_FOUND;
+ }
+
+ Aip = NULL;
+ Status = gBS->HandleProtocol (
+ Handle,
+ &gEfiAdapterInformationProtocolGuid,
+ (VOID *) &Aip
+ );
+ if (EFI_ERROR (Status) || Aip == NULL) {
+ return EFI_NOT_FOUND;
+ }
+
+ InfoTypesBuffer = NULL;
+ InfoTypeBufferCount = 0;
+ Status = Aip->GetSupportedTypes (Aip, &InfoTypesBuffer, &InfoTypeBufferCount);
+ if (EFI_ERROR (Status) || InfoTypesBuffer == NULL) {
+ FreePool (InfoTypesBuffer);
+ return EFI_NOT_FOUND;
+ }
+
+ Supported = FALSE;
+ for (TypeIndex = 0; TypeIndex < InfoTypeBufferCount; TypeIndex++) {
+ if (CompareGuid (&InfoTypesBuffer[TypeIndex], &gEfiAdapterInfoUndiIpv6SupportGuid)) {
+ Supported = TRUE;
+ break;
+ }
+ }
+
+ FreePool (InfoTypesBuffer);
+ if (!Supported) {
+ return EFI_NOT_FOUND;
+ }
+
+ //
+ // We now have adapter information block.
+ //
+ InfoBlock = NULL;
+ InfoBlockSize = 0;
+ Status = Aip->GetInformation (Aip, &gEfiAdapterInfoUndiIpv6SupportGuid, &InfoBlock, &InfoBlockSize);
+ if (EFI_ERROR (Status) || InfoBlock == NULL) {
+ FreePool (InfoBlock);
+ return EFI_NOT_FOUND;
+ }
+
+ *Ipv6Support = ((EFI_ADAPTER_INFO_UNDI_IPV6_SUPPORT *) InfoBlock)->Ipv6Support;
+ FreePool (InfoBlock);
+
+ return EFI_SUCCESS;
+}
+
+/**
Record the NIC info in global structure.
@param[in] Controller The handle of the controller.
+ @param[in] Image Handle of the image.
@retval EFI_SUCCESS The operation is completed.
@retval EFI_OUT_OF_RESOURCES Do not have sufficient resources to finish this
operation.
**/
EFI_STATUS
IScsiAddNic (
- IN EFI_HANDLE Controller
+ IN EFI_HANDLE Controller,
+ IN EFI_HANDLE Image
)
{
EFI_STATUS Status;
ISCSI_NIC_INFO *NicInfo;
LIST_ENTRY *Entry;
@@ -507,10 +613,23 @@ IScsiAddNic (
NicInfo = NET_LIST_USER_STRUCT (Entry, ISCSI_NIC_INFO, Link);
if (NicInfo->HwAddressSize == HwAddressSize &&
CompareMem (&NicInfo->PermanentAddress, MacAddr.Addr, HwAddressSize) == 0 &&
NicInfo->VlanId == VlanId) {
mPrivate->CurrentNic = NicInfo->NicIndex;
+
+ //
+ // Set IPv6 available flag.
+ //
+ Status = IScsiCheckIpv6Support (Controller, Image, &NicInfo->Ipv6Available);
+ if (EFI_ERROR (Status)) {
+ //
+ // Fail to get the data whether UNDI supports IPv6.
+ // Set default value to TRUE.
+ //
+ NicInfo->Ipv6Available = TRUE;
+ }
+
return EFI_SUCCESS;
}
if (mPrivate->MaxNic < NicInfo->NicIndex) {
mPrivate->MaxNic = NicInfo->NicIndex;
@@ -528,11 +647,23 @@ IScsiAddNic (
CopyMem (&NicInfo->PermanentAddress, MacAddr.Addr, HwAddressSize);
NicInfo->HwAddressSize = (UINT32) HwAddressSize;
NicInfo->VlanId = VlanId;
NicInfo->NicIndex = (UINT8) (mPrivate->MaxNic + 1);
mPrivate->MaxNic = NicInfo->NicIndex;
-
+
+ //
+ // Set IPv6 available flag.
+ //
+ Status = IScsiCheckIpv6Support (Controller, Image, &NicInfo->Ipv6Available);
+ if (EFI_ERROR (Status)) {
+ //
+ // Fail to get the data whether UNDI supports IPv6.
+ // Set default value to TRUE.
+ //
+ NicInfo->Ipv6Available = TRUE;
+ }
+
//
// Get the PCI location.
//
IScsiGetNICPciLocation (
Controller,
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.h b/NetworkPkg/IScsiDxe/IScsiMisc.h
index caa2f94..659c026 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.h
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.h
@@ -215,19 +215,21 @@ IScsiGenRandom (
/**
Record the NIC information in a global structure.
@param[in] Controller The handle of the controller.
+ @param[in] Image Handle of the image.
@retval EFI_SUCCESS The operation is completed.
@retval EFI_OUT_OF_RESOURCES Do not have sufficient resource to finish this
operation.
**/
EFI_STATUS
IScsiAddNic (
- IN EFI_HANDLE Controller
+ IN EFI_HANDLE Controller,
+ IN EFI_HANDLE Image
);
/**
Delete the recorded NIC information from a global structure. Also delete corresponding
attempts.
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
2017-10-17 1:58 [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI Jiaxin Wu
2017-10-17 1:58 ` [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support condition check Jiaxin Wu
2017-10-17 1:58 ` [Patch 2/2] NetworkPkg/IScsiDxe: " Jiaxin Wu
@ 2017-10-17 2:01 ` Wu, Jiaxin
2017-10-17 10:12 ` Karunakar P
2 siblings, 1 reply; 15+ messages in thread
From: Wu, Jiaxin @ 2017-10-17 2:01 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Karunakar P, Ye, Ting, Fu, Siyuan
Hello Karunakar,
Base on your original changes attached in Bugzilla 701 (https://bugzilla.tianocore.org/show_bug.cgi?id=710), I created the formal series patches to support the IPv6 condition check for HTTP/ISCSI.
Please help to review/verify it.
BTW, To review the ISCSI part, please apply the "[Patch v2 0/2] NetworkPkg/IScsiDxe: Display InitiatorInfo correctly" first to avoid any patch conflict.
Thanks,
Jiaxin
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jiaxin Wu
> Sent: Tuesday, October 17, 2017 9:58 AM
> To: edk2-devel@lists.01.org
> Cc: Karunakar P <karunakarp@amiindia.co.in>; Ye, Ting <ting.ye@intel.com>;
> Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [edk2] [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
>
> Base on the request of https://bugzilla.tianocore.org/show_bug.cgi?id=710,
> we provide this patch to IPv6 condition check by leveraging AIP Protocol.
>
> Cc: Karunakar P <karunakarp@amiindia.co.in>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Karunakar P <karunakarp@amiindia.co.in>
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
>
> Jiaxin Wu (2):
> NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> NetworkPkg/IScsiDxe: Add IPv6 support condition check.
>
> NetworkPkg/HttpBootDxe/HttpBootDxe.c | 115
> +++++++++++++++++++++++++++-
> NetworkPkg/HttpBootDxe/HttpBootDxe.h | 2 +
> NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 4 +-
> NetworkPkg/IScsiDxe/IScsiConfig.c | 18 +++++
> NetworkPkg/IScsiDxe/IScsiDriver.c | 2 +-
> NetworkPkg/IScsiDxe/IScsiDriver.h | 1 +
> NetworkPkg/IScsiDxe/IScsiDxe.inf | 2 +
> NetworkPkg/IScsiDxe/IScsiImpl.h | 1 +
> NetworkPkg/IScsiDxe/IScsiMisc.c | 135
> ++++++++++++++++++++++++++++++++-
> NetworkPkg/IScsiDxe/IScsiMisc.h | 4 +-
> 10 files changed, 278 insertions(+), 6 deletions(-)
>
> --
> 1.9.5.msysgit.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
2017-10-17 2:01 ` [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI Wu, Jiaxin
@ 2017-10-17 10:12 ` Karunakar P
2017-10-17 11:35 ` Karunakar P
2017-10-18 7:05 ` Wu, Jiaxin
0 siblings, 2 replies; 15+ messages in thread
From: Karunakar P @ 2017-10-17 10:12 UTC (permalink / raw)
To: 'Wu, Jiaxin', edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan
Hi Jiaxin,
I Reviewed the changes for 3 features/Bugs and verified the same, Please find my below comments and issues faced
A. Display InitiatorInfo in attempt page even DHCP enabled
------------------------------------------------------------------------------------------------------------------------------------------
1. I applied IScsiConfigVfr.vfr changes and as well IScsiMisc.c changes
2. It displays initiator info properly when it's Enabled for DHCP
3. But, I found some different behavior in below case
a. Add an Attempt (Attempt1 -> Initiator Info Enabled for DHCP)
b. On reboot iSCSI attempt was success and Initiator Details shown properly ==> This is as expected
c. Edit the same Attempt1 details to IP6 and save changes and reset
d. Now Iscsi connection with IP6 ==> This is as Expected
e. Now if we again Change the Attempt1 to IP4, It is Displaying Subnet Mask ==> I guess we are not clearing It
I guess we need to do ZeroMem for initiator details before.
B. [Patch 2/2] NetworkPkg/IScsiDxe: Add IPv6 support condition check.
-----------------------------------------------------------------------------------------------------------------------------------------
-> This changes looks similar whatever I attached in Bugzilla, and verified the same with off board card witch doesn't support IP6
-> It works fine, I didn't find any issues on it.
C. [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
----------------------------------------------------------------------------------------------------------------------------------------------------------------------
I found some issues in this changes, please find my below comments
1. HttpBootCheckIpv6Support() function definition and function call parameter differs , To correct this I've done 1 insertion(+), 1 deletion(-) like below
...
+HttpBootCheckIpv6Support (
+ IN EFI_HANDLE ControllerHandle,
+ IN HTTP_BOOT_PRIVATE_DATA *Private,
+ OUT BOOLEAN *Ipv6Support
+ )
...
+ // Set IPv6 available flag.
+ //
+ Status = HttpBootCheckIpv6Support (ControllerHandle,
-This->DriverBindingHandle, &Ipv6Available);
+Private, &Ipv6Available);
...
2. With the above changes I've verified with Off board card which doesn't support IP6, But I'm facing below ASSERT
(324): CR has Bad Signature
EFI_STATUS
EFIAPI
HttpBootIp4DxeDriverBindingStart (
IN EFI_DRIVER_BINDING_PROTOCOL *This,
IN EFI_HANDLE ControllerHandle,
IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
)
{
...
if (!EFI_ERROR (Status)) {
Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); // ASSERTs here
} else {
.....
3. I would like add some points and info about the this ASSERT, which I've found
The ASSERT is happening because of FreePool (Private), mentioned exact line no below
EFI_STATUS
EFIAPI
HttpBootIp6DxeDriverBindingStart (
IN EFI_DRIVER_BINDING_PROTOCOL *This,
IN EFI_HANDLE ControllerHandle,
IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
)
{
...
Status = gBS->InstallProtocolInterface (
&ControllerHandle,
&gEfiCallerIdGuid,
EFI_NATIVE_INTERFACE,
&Private->Id
);
if (EFI_ERROR (Status)) {
goto ON_ERROR;
}
}
+
+ //
+ // Set IPv6 available flag.
+ //
+ Status = HttpBootCheckIpv6Support (ControllerHandle,
-This->DriverBindingHandle, &Ipv6Available);
+Private, &Ipv6Available);
+ if (EFI_ERROR (Status)) {
+ //
+ // Fail to get the data whether UNDI supports IPv6.
+ // Set default value to TRUE.
+ //
+ Ipv6Available = TRUE;
+ }
+
+ if (!Ipv6Available) {
+ return EFI_UNSUPPORTED;
+ }
if (Private->Ip6Nic != NULL) {
//
...
ON_ERROR:
HttpBootDestroyIp6Children(This, Private);
HttpBootConfigFormUnload (Private);
FreePool (Private); // If I comment this line ASSERT is not happening
return Status;
}
4. At your end could you please verify this IP6 Condition check for HTTP
Please correct if anything is wrong, Thanks for your support
Thank You,
Karunakar
-----Original Message-----
From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
Sent: Tuesday, October 17, 2017 7:32 AM
To: Wu, Jiaxin; edk2-devel@lists.01.org
Cc: Karunakar P; Ye, Ting; Fu, Siyuan
Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
Hello Karunakar,
Base on your original changes attached in Bugzilla 701 (https://bugzilla.tianocore.org/show_bug.cgi?id=710), I created the formal series patches to support the IPv6 condition check for HTTP/ISCSI.
Please help to review/verify it.
BTW, To review the ISCSI part, please apply the "[Patch v2 0/2] NetworkPkg/IScsiDxe: Display InitiatorInfo correctly" first to avoid any patch conflict.
Thanks,
Jiaxin
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jiaxin Wu
> Sent: Tuesday, October 17, 2017 9:58 AM
> To: edk2-devel@lists.01.org
> Cc: Karunakar P <karunakarp@amiindia.co.in>; Ye, Ting
> <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin
> <jiaxin.wu@intel.com>
> Subject: [edk2] [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
>
> Base on the request of
> https://bugzilla.tianocore.org/show_bug.cgi?id=710,
> we provide this patch to IPv6 condition check by leveraging AIP Protocol.
>
> Cc: Karunakar P <karunakarp@amiindia.co.in>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Karunakar P <karunakarp@amiindia.co.in>
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
>
> Jiaxin Wu (2):
> NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> NetworkPkg/IScsiDxe: Add IPv6 support condition check.
>
> NetworkPkg/HttpBootDxe/HttpBootDxe.c | 115
> +++++++++++++++++++++++++++-
> NetworkPkg/HttpBootDxe/HttpBootDxe.h | 2 +
> NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 4 +-
> NetworkPkg/IScsiDxe/IScsiConfig.c | 18 +++++
> NetworkPkg/IScsiDxe/IScsiDriver.c | 2 +-
> NetworkPkg/IScsiDxe/IScsiDriver.h | 1 +
> NetworkPkg/IScsiDxe/IScsiDxe.inf | 2 +
> NetworkPkg/IScsiDxe/IScsiImpl.h | 1 +
> NetworkPkg/IScsiDxe/IScsiMisc.c | 135
> ++++++++++++++++++++++++++++++++-
> NetworkPkg/IScsiDxe/IScsiMisc.h | 4 +-
> 10 files changed, 278 insertions(+), 6 deletions(-)
>
> --
> 1.9.5.msysgit.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
2017-10-17 10:12 ` Karunakar P
@ 2017-10-17 11:35 ` Karunakar P
2017-10-18 7:05 ` Wu, Jiaxin
1 sibling, 0 replies; 15+ messages in thread
From: Karunakar P @ 2017-10-17 11:35 UTC (permalink / raw)
To: 'Wu, Jiaxin', 'edk2-devel@lists.01.org'
Cc: 'Ye, Ting', 'Fu, Siyuan'
Not sure that previous mail was delivered, Hence resending the same Mail.
Thanks,
Karunakar
-----Original Message-----
From: Karunakar P
Sent: Tuesday, October 17, 2017 3:43 PM
To: 'Wu, Jiaxin'; edk2-devel@lists.01.org
Cc: Ye, Ting; Fu, Siyuan
Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
Hi Jiaxin,
I Reviewed the changes for 3 features/Bugs and verified the same, Please find my below comments and issues faced
A. Display InitiatorInfo in attempt page even DHCP enabled
------------------------------------------------------------------------------------------------------------------------------------------
1. I applied IScsiConfigVfr.vfr changes and as well IScsiMisc.c changes 2. It displays initiator info properly when it's Enabled for DHCP 3. But, I found some different behavior in below case
a. Add an Attempt (Attempt1 -> Initiator Info Enabled for DHCP)
b. On reboot iSCSI attempt was success and Initiator Details shown properly ==> This is as expected
c. Edit the same Attempt1 details to IP6 and save changes and reset
d. Now Iscsi connection with IP6 ==> This is as Expected
e. Now if we again Change the Attempt1 to IP4, It is Displaying Subnet Mask ==> I guess we are not clearing It
I guess we need to do ZeroMem for initiator details before.
B. [Patch 2/2] NetworkPkg/IScsiDxe: Add IPv6 support condition check.
-----------------------------------------------------------------------------------------------------------------------------------------
-> This changes looks similar whatever I attached in Bugzilla, and
-> verified the same with off board card witch doesn't support IP6 It works fine, I didn't find any issues on it.
C. [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
----------------------------------------------------------------------------------------------------------------------------------------------------------------------
I found some issues in this changes, please find my below comments 1. HttpBootCheckIpv6Support() function definition and function call parameter differs , To correct this I've done 1 insertion(+), 1 deletion(-) like below ...
+HttpBootCheckIpv6Support (
+ IN EFI_HANDLE ControllerHandle,
+ IN HTTP_BOOT_PRIVATE_DATA *Private,
+ OUT BOOLEAN *Ipv6Support
+ )
...
+ // Set IPv6 available flag.
+ //
+ Status = HttpBootCheckIpv6Support (ControllerHandle,
-This->DriverBindingHandle, &Ipv6Available);
+Private, &Ipv6Available);
...
2. With the above changes I've verified with Off board card which doesn't support IP6, But I'm facing below ASSERT
(324): CR has Bad Signature
EFI_STATUS
EFIAPI
HttpBootIp4DxeDriverBindingStart (
IN EFI_DRIVER_BINDING_PROTOCOL *This,
IN EFI_HANDLE ControllerHandle,
IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
)
{
...
if (!EFI_ERROR (Status)) {
Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); // ASSERTs here
} else {
.....
3. I would like add some points and info about the this ASSERT, which I've found The ASSERT is happening because of FreePool (Private), mentioned exact line no below
EFI_STATUS
EFIAPI
HttpBootIp6DxeDriverBindingStart (
IN EFI_DRIVER_BINDING_PROTOCOL *This,
IN EFI_HANDLE ControllerHandle,
IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
)
{
...
Status = gBS->InstallProtocolInterface (
&ControllerHandle,
&gEfiCallerIdGuid,
EFI_NATIVE_INTERFACE,
&Private->Id
);
if (EFI_ERROR (Status)) {
goto ON_ERROR;
}
}
+
+ //
+ // Set IPv6 available flag.
+ //
+ Status = HttpBootCheckIpv6Support (ControllerHandle,
-This->DriverBindingHandle, &Ipv6Available);
+Private, &Ipv6Available);
+ if (EFI_ERROR (Status)) {
+ //
+ // Fail to get the data whether UNDI supports IPv6.
+ // Set default value to TRUE.
+ //
+ Ipv6Available = TRUE;
+ }
+
+ if (!Ipv6Available) {
+ return EFI_UNSUPPORTED;
+ }
if (Private->Ip6Nic != NULL) {
//
...
ON_ERROR:
HttpBootDestroyIp6Children(This, Private);
HttpBootConfigFormUnload (Private);
FreePool (Private); // If I comment this line ASSERT is not happening
return Status;
}
4. At your end could you please verify this IP6 Condition check for HTTP
Please correct if anything is wrong, Thanks for your support
Thank You,
Karunakar
-----Original Message-----
From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
Sent: Tuesday, October 17, 2017 7:32 AM
To: Wu, Jiaxin; edk2-devel@lists.01.org
Cc: Karunakar P; Ye, Ting; Fu, Siyuan
Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
Hello Karunakar,
Base on your original changes attached in Bugzilla 701 (https://bugzilla.tianocore.org/show_bug.cgi?id=710), I created the formal series patches to support the IPv6 condition check for HTTP/ISCSI.
Please help to review/verify it.
BTW, To review the ISCSI part, please apply the "[Patch v2 0/2] NetworkPkg/IScsiDxe: Display InitiatorInfo correctly" first to avoid any patch conflict.
Thanks,
Jiaxin
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jiaxin Wu
> Sent: Tuesday, October 17, 2017 9:58 AM
> To: edk2-devel@lists.01.org
> Cc: Karunakar P <karunakarp@amiindia.co.in>; Ye, Ting
> <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin
> <jiaxin.wu@intel.com>
> Subject: [edk2] [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
>
> Base on the request of
> https://bugzilla.tianocore.org/show_bug.cgi?id=710,
> we provide this patch to IPv6 condition check by leveraging AIP Protocol.
>
> Cc: Karunakar P <karunakarp@amiindia.co.in>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Karunakar P <karunakarp@amiindia.co.in>
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
>
> Jiaxin Wu (2):
> NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> NetworkPkg/IScsiDxe: Add IPv6 support condition check.
>
> NetworkPkg/HttpBootDxe/HttpBootDxe.c | 115
> +++++++++++++++++++++++++++-
> NetworkPkg/HttpBootDxe/HttpBootDxe.h | 2 +
> NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 4 +-
> NetworkPkg/IScsiDxe/IScsiConfig.c | 18 +++++
> NetworkPkg/IScsiDxe/IScsiDriver.c | 2 +-
> NetworkPkg/IScsiDxe/IScsiDriver.h | 1 +
> NetworkPkg/IScsiDxe/IScsiDxe.inf | 2 +
> NetworkPkg/IScsiDxe/IScsiImpl.h | 1 +
> NetworkPkg/IScsiDxe/IScsiMisc.c | 135
> ++++++++++++++++++++++++++++++++-
> NetworkPkg/IScsiDxe/IScsiMisc.h | 4 +-
> 10 files changed, 278 insertions(+), 6 deletions(-)
>
> --
> 1.9.5.msysgit.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
2017-10-17 10:12 ` Karunakar P
2017-10-17 11:35 ` Karunakar P
@ 2017-10-18 7:05 ` Wu, Jiaxin
2017-10-18 8:05 ` Karunakar P
1 sibling, 1 reply; 15+ messages in thread
From: Wu, Jiaxin @ 2017-10-18 7:05 UTC (permalink / raw)
To: Karunakar P, edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan
Hi Karunakar,
Thanks your verification. Base on your comments, I refined the series patches as below to fix the issues:
[Patch v3 0/3] NetworkPkg/IScsiDxe: Display InitiatorInfo correctly.
NetworkPkg/IScsiDxe: Fix the incorrect/needless DHCP process.
NetworkPkg/IScsiDxe: Clean the previous ConfigData when switching the IP mode. /// This one is to fix the issue A.
NetworkPkg/IScsiDxe: Display InitiatorInfo in attempt page even DHCP enabled.
[Patch v2 0/2] Add IPv6 support condition check.
NetworkPkg/HttpBootDxe: Add IPv6 support condition check. /// B && C have been fixed in version 2.
NetworkPkg/IScsiDxe: Add IPv6 support condition check.
Please help to verify them.
Best Regards,
Jiaxin
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Karunakar P
> Sent: Tuesday, October 17, 2017 6:13 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: Re: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hi Jiaxin,
>
> I Reviewed the changes for 3 features/Bugs and verified the same, Please
> find my below comments and issues faced
>
> A. Display InitiatorInfo in attempt page even DHCP enabled
> ----------------------------------------------------------------------------------------------
> --------------------------------------------
> 1. I applied IScsiConfigVfr.vfr changes and as well IScsiMisc.c changes
> 2. It displays initiator info properly when it's Enabled for DHCP
> 3. But, I found some different behavior in below case
> a. Add an Attempt (Attempt1 -> Initiator Info Enabled for DHCP)
> b. On reboot iSCSI attempt was success and Initiator Details shown
> properly ==> This is as expected
> c. Edit the same Attempt1 details to IP6 and save changes and reset
> d. Now Iscsi connection with IP6 ==> This is as Expected
> e. Now if we again Change the Attempt1 to IP4, It is Displaying Subnet
> Mask ==> I guess we are not clearing It
>
> I guess we need to do ZeroMem for initiator details before.
>
>
> B. [Patch 2/2] NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> ----------------------------------------------------------------------------------------------
> -------------------------------------------
> -> This changes looks similar whatever I attached in Bugzilla, and verified the
> same with off board card witch doesn't support IP6
> -> It works fine, I didn't find any issues on it.
>
>
> C. [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> ----------------------------------------------------------------------------------------------
> ------------------------------------------------------------------------
> I found some issues in this changes, please find my below comments
> 1. HttpBootCheckIpv6Support() function definition and function call
> parameter differs , To correct this I've done 1 insertion(+), 1 deletion(-) like
> below
> ...
> +HttpBootCheckIpv6Support (
> + IN EFI_HANDLE ControllerHandle,
> + IN HTTP_BOOT_PRIVATE_DATA *Private,
> + OUT BOOLEAN *Ipv6Support
> + )
> ...
> + // Set IPv6 available flag.
> + //
> + Status = HttpBootCheckIpv6Support (ControllerHandle,
> -This->DriverBindingHandle, &Ipv6Available);
> +Private, &Ipv6Available);
> ...
>
> 2. With the above changes I've verified with Off board card which doesn't
> support IP6, But I'm facing below ASSERT
> (324): CR has Bad Signature
>
> EFI_STATUS
> EFIAPI
> HttpBootIp4DxeDriverBindingStart (
> IN EFI_DRIVER_BINDING_PROTOCOL *This,
> IN EFI_HANDLE ControllerHandle,
> IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> )
> {
> ...
> if (!EFI_ERROR (Status)) {
> Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); // ASSERTs
> here
> } else {
> .....
>
> 3. I would like add some points and info about the this ASSERT, which I've
> found
> The ASSERT is happening because of FreePool (Private), mentioned exact
> line no below
>
> EFI_STATUS
> EFIAPI
> HttpBootIp6DxeDriverBindingStart (
> IN EFI_DRIVER_BINDING_PROTOCOL *This,
> IN EFI_HANDLE ControllerHandle,
> IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> )
> {
> ...
> Status = gBS->InstallProtocolInterface (
> &ControllerHandle,
> &gEfiCallerIdGuid,
> EFI_NATIVE_INTERFACE,
> &Private->Id
> );
> if (EFI_ERROR (Status)) {
> goto ON_ERROR;
> }
>
> }
> +
> + //
> + // Set IPv6 available flag.
> + //
> + Status = HttpBootCheckIpv6Support (ControllerHandle,
> -This->DriverBindingHandle, &Ipv6Available);
> +Private, &Ipv6Available);
> + if (EFI_ERROR (Status)) {
> + //
> + // Fail to get the data whether UNDI supports IPv6.
> + // Set default value to TRUE.
> + //
> + Ipv6Available = TRUE;
> + }
> +
> + if (!Ipv6Available) {
> + return EFI_UNSUPPORTED;
> + }
>
> if (Private->Ip6Nic != NULL) {
> //
> ...
>
> ON_ERROR:
>
> HttpBootDestroyIp6Children(This, Private);
> HttpBootConfigFormUnload (Private);
> FreePool (Private); // If I comment this
> line ASSERT is not happening
>
> return Status;
> }
>
> 4. At your end could you please verify this IP6 Condition check for HTTP
>
> Please correct if anything is wrong, Thanks for your support
>
>
> Thank You,
> Karunakar
>
> -----Original Message-----
> From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> Sent: Tuesday, October 17, 2017 7:32 AM
> To: Wu, Jiaxin; edk2-devel@lists.01.org
> Cc: Karunakar P; Ye, Ting; Fu, Siyuan
> Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hello Karunakar,
>
> Base on your original changes attached in Bugzilla 701
> (https://bugzilla.tianocore.org/show_bug.cgi?id=710), I created the formal
> series patches to support the IPv6 condition check for HTTP/ISCSI.
>
> Please help to review/verify it.
>
> BTW, To review the ISCSI part, please apply the "[Patch v2 0/2]
> NetworkPkg/IScsiDxe: Display InitiatorInfo correctly" first to avoid any patch
> conflict.
>
> Thanks,
> Jiaxin
>
>
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Jiaxin Wu
> > Sent: Tuesday, October 17, 2017 9:58 AM
> > To: edk2-devel@lists.01.org
> > Cc: Karunakar P <karunakarp@amiindia.co.in>; Ye, Ting
> > <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin
> > <jiaxin.wu@intel.com>
> > Subject: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
> >
> > Base on the request of
> > https://bugzilla.tianocore.org/show_bug.cgi?id=710,
> > we provide this patch to IPv6 condition check by leveraging AIP Protocol.
> >
> > Cc: Karunakar P <karunakarp@amiindia.co.in>
> > Cc: Ye Ting <ting.ye@intel.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Karunakar P <karunakarp@amiindia.co.in>
> > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> >
> > Jiaxin Wu (2):
> > NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> > NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> >
> > NetworkPkg/HttpBootDxe/HttpBootDxe.c | 115
> > +++++++++++++++++++++++++++-
> > NetworkPkg/HttpBootDxe/HttpBootDxe.h | 2 +
> > NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 4 +-
> > NetworkPkg/IScsiDxe/IScsiConfig.c | 18 +++++
> > NetworkPkg/IScsiDxe/IScsiDriver.c | 2 +-
> > NetworkPkg/IScsiDxe/IScsiDriver.h | 1 +
> > NetworkPkg/IScsiDxe/IScsiDxe.inf | 2 +
> > NetworkPkg/IScsiDxe/IScsiImpl.h | 1 +
> > NetworkPkg/IScsiDxe/IScsiMisc.c | 135
> > ++++++++++++++++++++++++++++++++-
> > NetworkPkg/IScsiDxe/IScsiMisc.h | 4 +-
> > 10 files changed, 278 insertions(+), 6 deletions(-)
> >
> > --
> > 1.9.5.msysgit.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
2017-10-18 7:05 ` Wu, Jiaxin
@ 2017-10-18 8:05 ` Karunakar P
2017-10-18 9:00 ` Wu, Jiaxin
0 siblings, 1 reply; 15+ messages in thread
From: Karunakar P @ 2017-10-18 8:05 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan
Hi Jiaxin,
1. Cleaning the ConfigData when switching the mode will resolve the Issue A.
2. Will also verify the ASSERT issue and update you.
Could you please help in clarifying the below items,
1. Could you please let us know why the ASSERT happens?
2. I've not faced any ASSERT with the changes attached in Bugzilla, Did you find any issues/drawbacks with that?
-> In HttpBootCheckIpv6Support () definition Instead of getting Ipv6Support from Private->Nii->Ipv6Supported if we open the protocol there itself, Then there will not be issues in Destroying Children or FreePool(Private) right. Because we're going to check HttpBootCheckIpv6Support() before opening any instances in HttpBootIp6DxeDriverBindingStart().
Thanks for your great support.
Thank You,
Karunakar
________________________________________
From: Wu, Jiaxin [jiaxin.wu@intel.com]
Sent: 18 October 2017 12:35
To: Karunakar P; edk2-devel@lists.01.org
Cc: Ye, Ting; Fu, Siyuan
Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
Hi Karunakar,
Thanks your verification. Base on your comments, I refined the series patches as below to fix the issues:
[Patch v3 0/3] NetworkPkg/IScsiDxe: Display InitiatorInfo correctly.
NetworkPkg/IScsiDxe: Fix the incorrect/needless DHCP process.
NetworkPkg/IScsiDxe: Clean the previous ConfigData when switching the IP mode. /// This one is to fix the issue A.
NetworkPkg/IScsiDxe: Display InitiatorInfo in attempt page even DHCP enabled.
[Patch v2 0/2] Add IPv6 support condition check.
NetworkPkg/HttpBootDxe: Add IPv6 support condition check. /// B && C have been fixed in version 2.
NetworkPkg/IScsiDxe: Add IPv6 support condition check.
Please help to verify them.
Best Regards,
Jiaxin
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Karunakar P
> Sent: Tuesday, October 17, 2017 6:13 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: Re: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hi Jiaxin,
>
> I Reviewed the changes for 3 features/Bugs and verified the same, Please
> find my below comments and issues faced
>
> A. Display InitiatorInfo in attempt page even DHCP enabled
> ----------------------------------------------------------------------------------------------
> --------------------------------------------
> 1. I applied IScsiConfigVfr.vfr changes and as well IScsiMisc.c changes
> 2. It displays initiator info properly when it's Enabled for DHCP
> 3. But, I found some different behavior in below case
> a. Add an Attempt (Attempt1 -> Initiator Info Enabled for DHCP)
> b. On reboot iSCSI attempt was success and Initiator Details shown
> properly ==> This is as expected
> c. Edit the same Attempt1 details to IP6 and save changes and reset
> d. Now Iscsi connection with IP6 ==> This is as Expected
> e. Now if we again Change the Attempt1 to IP4, It is Displaying Subnet
> Mask ==> I guess we are not clearing It
>
> I guess we need to do ZeroMem for initiator details before.
>
>
> B. [Patch 2/2] NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> ----------------------------------------------------------------------------------------------
> -------------------------------------------
> -> This changes looks similar whatever I attached in Bugzilla, and verified the
> same with off board card witch doesn't support IP6
> -> It works fine, I didn't find any issues on it.
>
>
> C. [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> ----------------------------------------------------------------------------------------------
> ------------------------------------------------------------------------
> I found some issues in this changes, please find my below comments
> 1. HttpBootCheckIpv6Support() function definition and function call
> parameter differs , To correct this I've done 1 insertion(+), 1 deletion(-) like
> below
> ...
> +HttpBootCheckIpv6Support (
> + IN EFI_HANDLE ControllerHandle,
> + IN HTTP_BOOT_PRIVATE_DATA *Private,
> + OUT BOOLEAN *Ipv6Support
> + )
> ...
> + // Set IPv6 available flag.
> + //
> + Status = HttpBootCheckIpv6Support (ControllerHandle,
> -This->DriverBindingHandle, &Ipv6Available);
> +Private, &Ipv6Available);
> ...
>
> 2. With the above changes I've verified with Off board card which doesn't
> support IP6, But I'm facing below ASSERT
> (324): CR has Bad Signature
>
> EFI_STATUS
> EFIAPI
> HttpBootIp4DxeDriverBindingStart (
> IN EFI_DRIVER_BINDING_PROTOCOL *This,
> IN EFI_HANDLE ControllerHandle,
> IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> )
> {
> ...
> if (!EFI_ERROR (Status)) {
> Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); // ASSERTs
> here
> } else {
> .....
>
> 3. I would like add some points and info about the this ASSERT, which I've
> found
> The ASSERT is happening because of FreePool (Private), mentioned exact
> line no below
>
> EFI_STATUS
> EFIAPI
> HttpBootIp6DxeDriverBindingStart (
> IN EFI_DRIVER_BINDING_PROTOCOL *This,
> IN EFI_HANDLE ControllerHandle,
> IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> )
> {
> ...
> Status = gBS->InstallProtocolInterface (
> &ControllerHandle,
> &gEfiCallerIdGuid,
> EFI_NATIVE_INTERFACE,
> &Private->Id
> );
> if (EFI_ERROR (Status)) {
> goto ON_ERROR;
> }
>
> }
> +
> + //
> + // Set IPv6 available flag.
> + //
> + Status = HttpBootCheckIpv6Support (ControllerHandle,
> -This->DriverBindingHandle, &Ipv6Available);
> +Private, &Ipv6Available);
> + if (EFI_ERROR (Status)) {
> + //
> + // Fail to get the data whether UNDI supports IPv6.
> + // Set default value to TRUE.
> + //
> + Ipv6Available = TRUE;
> + }
> +
> + if (!Ipv6Available) {
> + return EFI_UNSUPPORTED;
> + }
>
> if (Private->Ip6Nic != NULL) {
> //
> ...
>
> ON_ERROR:
>
> HttpBootDestroyIp6Children(This, Private);
> HttpBootConfigFormUnload (Private);
> FreePool (Private); // If I comment this
> line ASSERT is not happening
>
> return Status;
> }
>
> 4. At your end could you please verify this IP6 Condition check for HTTP
>
> Please correct if anything is wrong, Thanks for your support
>
>
> Thank You,
> Karunakar
>
> -----Original Message-----
> From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> Sent: Tuesday, October 17, 2017 7:32 AM
> To: Wu, Jiaxin; edk2-devel@lists.01.org
> Cc: Karunakar P; Ye, Ting; Fu, Siyuan
> Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hello Karunakar,
>
> Base on your original changes attached in Bugzilla 701
> (https://bugzilla.tianocore.org/show_bug.cgi?id=710), I created the formal
> series patches to support the IPv6 condition check for HTTP/ISCSI.
>
> Please help to review/verify it.
>
> BTW, To review the ISCSI part, please apply the "[Patch v2 0/2]
> NetworkPkg/IScsiDxe: Display InitiatorInfo correctly" first to avoid any patch
> conflict.
>
> Thanks,
> Jiaxin
>
>
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Jiaxin Wu
> > Sent: Tuesday, October 17, 2017 9:58 AM
> > To: edk2-devel@lists.01.org
> > Cc: Karunakar P <karunakarp@amiindia.co.in>; Ye, Ting
> > <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin
> > <jiaxin.wu@intel.com>
> > Subject: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
> >
> > Base on the request of
> > https://bugzilla.tianocore.org/show_bug.cgi?id=710,
> > we provide this patch to IPv6 condition check by leveraging AIP Protocol.
> >
> > Cc: Karunakar P <karunakarp@amiindia.co.in>
> > Cc: Ye Ting <ting.ye@intel.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Karunakar P <karunakarp@amiindia.co.in>
> > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> >
> > Jiaxin Wu (2):
> > NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> > NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> >
> > NetworkPkg/HttpBootDxe/HttpBootDxe.c | 115
> > +++++++++++++++++++++++++++-
> > NetworkPkg/HttpBootDxe/HttpBootDxe.h | 2 +
> > NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 4 +-
> > NetworkPkg/IScsiDxe/IScsiConfig.c | 18 +++++
> > NetworkPkg/IScsiDxe/IScsiDriver.c | 2 +-
> > NetworkPkg/IScsiDxe/IScsiDriver.h | 1 +
> > NetworkPkg/IScsiDxe/IScsiDxe.inf | 2 +
> > NetworkPkg/IScsiDxe/IScsiImpl.h | 1 +
> > NetworkPkg/IScsiDxe/IScsiMisc.c | 135
> > ++++++++++++++++++++++++++++++++-
> > NetworkPkg/IScsiDxe/IScsiMisc.h | 4 +-
> > 10 files changed, 278 insertions(+), 6 deletions(-)
> >
> > --
> > 1.9.5.msysgit.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
2017-10-18 8:05 ` Karunakar P
@ 2017-10-18 9:00 ` Wu, Jiaxin
2017-10-18 11:43 ` Karunakar P
2017-10-23 16:41 ` Karunakar P
0 siblings, 2 replies; 15+ messages in thread
From: Wu, Jiaxin @ 2017-10-18 9:00 UTC (permalink / raw)
To: Karunakar P, edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan
Hi Karunakar,
Actually, in my part, I didn't meet the ASSERT no matter the "Ipv6Available" returned from HttpBootCheckIpv6Support is true or false. According your ASSERT case that happened in Ip4DxeDriverBindingStart, which is caused by the FreePool of Private in Ip6DxeDriverBindingStart, so I guess the Ip6DxeDriverBindingStart may be involved ahead of Ip4DxeDriverBindingStart, then something wrong happened in Ip6DxeDriverBindingStart and goto the ON_ERROR (Private is freed here!). so, you can add the breakpoint within Ip6DxeDriverBindingStart/Ip4DxeDriverBindingStart to check it.
Per my analysis above, it does the issue that may trigger the potential ASSERT. So, I refined the patch as version2. The principle of patch v2 is that IPv6 and IPv4 should not affect each other even any failure happen, but the original code doesn't follow that:).
Thanks,
Jiaxin
> -----Original Message-----
> From: Karunakar P [mailto:karunakarp@amiindia.co.in]
> Sent: Wednesday, October 18, 2017 4:06 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hi Jiaxin,
>
> 1. Cleaning the ConfigData when switching the mode will resolve the Issue A.
> 2. Will also verify the ASSERT issue and update you.
>
> Could you please help in clarifying the below items,
> 1. Could you please let us know why the ASSERT happens?
> 2. I've not faced any ASSERT with the changes attached in Bugzilla, Did you
> find any issues/drawbacks with that?
> -> In HttpBootCheckIpv6Support () definition Instead of getting Ipv6Support
> from Private->Nii->Ipv6Supported if we open the protocol there itself, Then
> there will not be issues in Destroying Children or FreePool(Private) right.
> Because we're going to check HttpBootCheckIpv6Support() before opening
> any instances in HttpBootIp6DxeDriverBindingStart().
>
> Thanks for your great support.
>
> Thank You,
> Karunakar
> ________________________________________
> From: Wu, Jiaxin [jiaxin.wu@intel.com]
> Sent: 18 October 2017 12:35
> To: Karunakar P; edk2-devel@lists.01.org
> Cc: Ye, Ting; Fu, Siyuan
> Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hi Karunakar,
>
> Thanks your verification. Base on your comments, I refined the series
> patches as below to fix the issues:
>
> [Patch v3 0/3] NetworkPkg/IScsiDxe: Display InitiatorInfo correctly.
> NetworkPkg/IScsiDxe: Fix the incorrect/needless DHCP process.
> NetworkPkg/IScsiDxe: Clean the previous ConfigData when switching
> the IP mode. /// This one is to fix the issue A.
> NetworkPkg/IScsiDxe: Display InitiatorInfo in attempt page even DHCP
> enabled.
>
> [Patch v2 0/2] Add IPv6 support condition check.
> NetworkPkg/HttpBootDxe: Add IPv6 support condition check. /// B
> && C have been fixed in version 2.
> NetworkPkg/IScsiDxe: Add IPv6 support condition check.
>
> Please help to verify them.
>
> Best Regards,
> Jiaxin
>
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Karunakar P
> > Sent: Tuesday, October 17, 2017 6:13 PM
> > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> > Subject: Re: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> >
> > Hi Jiaxin,
> >
> > I Reviewed the changes for 3 features/Bugs and verified the same, Please
> > find my below comments and issues faced
> >
> > A. Display InitiatorInfo in attempt page even DHCP enabled
> > -------------------------------------------------------------------------------------------
> ---
> > --------------------------------------------
> > 1. I applied IScsiConfigVfr.vfr changes and as well IScsiMisc.c changes
> > 2. It displays initiator info properly when it's Enabled for DHCP
> > 3. But, I found some different behavior in below case
> > a. Add an Attempt (Attempt1 -> Initiator Info Enabled for DHCP)
> > b. On reboot iSCSI attempt was success and Initiator Details shown
> > properly ==> This is as expected
> > c. Edit the same Attempt1 details to IP6 and save changes and reset
> > d. Now Iscsi connection with IP6 ==> This is as Expected
> > e. Now if we again Change the Attempt1 to IP4, It is Displaying Subnet
> > Mask ==> I guess we are not clearing It
> >
> > I guess we need to do ZeroMem for initiator details before.
> >
> >
> > B. [Patch 2/2] NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> > -------------------------------------------------------------------------------------------
> ---
> > -------------------------------------------
> > -> This changes looks similar whatever I attached in Bugzilla, and verified
> the
> > same with off board card witch doesn't support IP6
> > -> It works fine, I didn't find any issues on it.
> >
> >
> > C. [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> > -------------------------------------------------------------------------------------------
> ---
> > ------------------------------------------------------------------------
> > I found some issues in this changes, please find my below comments
> > 1. HttpBootCheckIpv6Support() function definition and function call
> > parameter differs , To correct this I've done 1 insertion(+), 1 deletion(-) like
> > below
> > ...
> > +HttpBootCheckIpv6Support (
> > + IN EFI_HANDLE ControllerHandle,
> > + IN HTTP_BOOT_PRIVATE_DATA *Private,
> > + OUT BOOLEAN *Ipv6Support
> > + )
> > ...
> > + // Set IPv6 available flag.
> > + //
> > + Status = HttpBootCheckIpv6Support (ControllerHandle,
> > -This->DriverBindingHandle, &Ipv6Available);
> > +Private, &Ipv6Available);
> > ...
> >
> > 2. With the above changes I've verified with Off board card which doesn't
> > support IP6, But I'm facing below ASSERT
> > (324): CR has Bad Signature
> >
> > EFI_STATUS
> > EFIAPI
> > HttpBootIp4DxeDriverBindingStart (
> > IN EFI_DRIVER_BINDING_PROTOCOL *This,
> > IN EFI_HANDLE ControllerHandle,
> > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> > )
> > {
> > ...
> > if (!EFI_ERROR (Status)) {
> > Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); // ASSERTs
> > here
> > } else {
> > .....
> >
> > 3. I would like add some points and info about the this ASSERT, which I've
> > found
> > The ASSERT is happening because of FreePool (Private), mentioned exact
> > line no below
> >
> > EFI_STATUS
> > EFIAPI
> > HttpBootIp6DxeDriverBindingStart (
> > IN EFI_DRIVER_BINDING_PROTOCOL *This,
> > IN EFI_HANDLE ControllerHandle,
> > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> > )
> > {
> > ...
> > Status = gBS->InstallProtocolInterface (
> > &ControllerHandle,
> > &gEfiCallerIdGuid,
> > EFI_NATIVE_INTERFACE,
> > &Private->Id
> > );
> > if (EFI_ERROR (Status)) {
> > goto ON_ERROR;
> > }
> >
> > }
> > +
> > + //
> > + // Set IPv6 available flag.
> > + //
> > + Status = HttpBootCheckIpv6Support (ControllerHandle,
> > -This->DriverBindingHandle, &Ipv6Available);
> > +Private, &Ipv6Available);
> > + if (EFI_ERROR (Status)) {
> > + //
> > + // Fail to get the data whether UNDI supports IPv6.
> > + // Set default value to TRUE.
> > + //
> > + Ipv6Available = TRUE;
> > + }
> > +
> > + if (!Ipv6Available) {
> > + return EFI_UNSUPPORTED;
> > + }
> >
> > if (Private->Ip6Nic != NULL) {
> > //
> > ...
> >
> > ON_ERROR:
> >
> > HttpBootDestroyIp6Children(This, Private);
> > HttpBootConfigFormUnload (Private);
> > FreePool (Private); // If I comment this
> > line ASSERT is not happening
> >
> > return Status;
> > }
> >
> > 4. At your end could you please verify this IP6 Condition check for HTTP
> >
> > Please correct if anything is wrong, Thanks for your support
> >
> >
> > Thank You,
> > Karunakar
> >
> > -----Original Message-----
> > From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> > Sent: Tuesday, October 17, 2017 7:32 AM
> > To: Wu, Jiaxin; edk2-devel@lists.01.org
> > Cc: Karunakar P; Ye, Ting; Fu, Siyuan
> > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> >
> > Hello Karunakar,
> >
> > Base on your original changes attached in Bugzilla 701
> > (https://bugzilla.tianocore.org/show_bug.cgi?id=710), I created the formal
> > series patches to support the IPv6 condition check for HTTP/ISCSI.
> >
> > Please help to review/verify it.
> >
> > BTW, To review the ISCSI part, please apply the "[Patch v2 0/2]
> > NetworkPkg/IScsiDxe: Display InitiatorInfo correctly" first to avoid any
> patch
> > conflict.
> >
> > Thanks,
> > Jiaxin
> >
> >
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > > Jiaxin Wu
> > > Sent: Tuesday, October 17, 2017 9:58 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Karunakar P <karunakarp@amiindia.co.in>; Ye, Ting
> > > <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin
> > > <jiaxin.wu@intel.com>
> > > Subject: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> > >
> > > Base on the request of
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=710,
> > > we provide this patch to IPv6 condition check by leveraging AIP Protocol.
> > >
> > > Cc: Karunakar P <karunakarp@amiindia.co.in>
> > > Cc: Ye Ting <ting.ye@intel.com>
> > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Karunakar P <karunakarp@amiindia.co.in>
> > > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > >
> > > Jiaxin Wu (2):
> > > NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> > > NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> > >
> > > NetworkPkg/HttpBootDxe/HttpBootDxe.c | 115
> > > +++++++++++++++++++++++++++-
> > > NetworkPkg/HttpBootDxe/HttpBootDxe.h | 2 +
> > > NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 4 +-
> > > NetworkPkg/IScsiDxe/IScsiConfig.c | 18 +++++
> > > NetworkPkg/IScsiDxe/IScsiDriver.c | 2 +-
> > > NetworkPkg/IScsiDxe/IScsiDriver.h | 1 +
> > > NetworkPkg/IScsiDxe/IScsiDxe.inf | 2 +
> > > NetworkPkg/IScsiDxe/IScsiImpl.h | 1 +
> > > NetworkPkg/IScsiDxe/IScsiMisc.c | 135
> > > ++++++++++++++++++++++++++++++++-
> > > NetworkPkg/IScsiDxe/IScsiMisc.h | 4 +-
> > > 10 files changed, 278 insertions(+), 6 deletions(-)
> > >
> > > --
> > > 1.9.5.msysgit.1
> > >
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
2017-10-18 9:00 ` Wu, Jiaxin
@ 2017-10-18 11:43 ` Karunakar P
2017-10-23 16:41 ` Karunakar P
1 sibling, 0 replies; 15+ messages in thread
From: Karunakar P @ 2017-10-18 11:43 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan
Hi Jixian,
Thanks for your Detailed explanation.
Will review/Verify the changes and update you on Friday.
Thank You,
Karunakar
________________________________________
From: Wu, Jiaxin [jiaxin.wu@intel.com]
Sent: 18 October 2017 14:30
To: Karunakar P; edk2-devel@lists.01.org
Cc: Ye, Ting; Fu, Siyuan
Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
Hi Karunakar,
Actually, in my part, I didn't meet the ASSERT no matter the "Ipv6Available" returned from HttpBootCheckIpv6Support is true or false. According your ASSERT case that happened in Ip4DxeDriverBindingStart, which is caused by the FreePool of Private in Ip6DxeDriverBindingStart, so I guess the Ip6DxeDriverBindingStart may be involved ahead of Ip4DxeDriverBindingStart, then something wrong happened in Ip6DxeDriverBindingStart and goto the ON_ERROR (Private is freed here!). so, you can add the breakpoint within Ip6DxeDriverBindingStart/Ip4DxeDriverBindingStart to check it.
Per my analysis above, it does the issue that may trigger the potential ASSERT. So, I refined the patch as version2. The principle of patch v2 is that IPv6 and IPv4 should not affect each other even any failure happen, but the original code doesn't follow that:).
Thanks,
Jiaxin
> -----Original Message-----
> From: Karunakar P [mailto:karunakarp@amiindia.co.in]
> Sent: Wednesday, October 18, 2017 4:06 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hi Jiaxin,
>
> 1. Cleaning the ConfigData when switching the mode will resolve the Issue A.
> 2. Will also verify the ASSERT issue and update you.
>
> Could you please help in clarifying the below items,
> 1. Could you please let us know why the ASSERT happens?
> 2. I've not faced any ASSERT with the changes attached in Bugzilla, Did you
> find any issues/drawbacks with that?
> -> In HttpBootCheckIpv6Support () definition Instead of getting Ipv6Support
> from Private->Nii->Ipv6Supported if we open the protocol there itself, Then
> there will not be issues in Destroying Children or FreePool(Private) right.
> Because we're going to check HttpBootCheckIpv6Support() before opening
> any instances in HttpBootIp6DxeDriverBindingStart().
>
> Thanks for your great support.
>
> Thank You,
> Karunakar
> ________________________________________
> From: Wu, Jiaxin [jiaxin.wu@intel.com]
> Sent: 18 October 2017 12:35
> To: Karunakar P; edk2-devel@lists.01.org
> Cc: Ye, Ting; Fu, Siyuan
> Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hi Karunakar,
>
> Thanks your verification. Base on your comments, I refined the series
> patches as below to fix the issues:
>
> [Patch v3 0/3] NetworkPkg/IScsiDxe: Display InitiatorInfo correctly.
> NetworkPkg/IScsiDxe: Fix the incorrect/needless DHCP process.
> NetworkPkg/IScsiDxe: Clean the previous ConfigData when switching
> the IP mode. /// This one is to fix the issue A.
> NetworkPkg/IScsiDxe: Display InitiatorInfo in attempt page even DHCP
> enabled.
>
> [Patch v2 0/2] Add IPv6 support condition check.
> NetworkPkg/HttpBootDxe: Add IPv6 support condition check. /// B
> && C have been fixed in version 2.
> NetworkPkg/IScsiDxe: Add IPv6 support condition check.
>
> Please help to verify them.
>
> Best Regards,
> Jiaxin
>
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Karunakar P
> > Sent: Tuesday, October 17, 2017 6:13 PM
> > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> > Subject: Re: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> >
> > Hi Jiaxin,
> >
> > I Reviewed the changes for 3 features/Bugs and verified the same, Please
> > find my below comments and issues faced
> >
> > A. Display InitiatorInfo in attempt page even DHCP enabled
> > -------------------------------------------------------------------------------------------
> ---
> > --------------------------------------------
> > 1. I applied IScsiConfigVfr.vfr changes and as well IScsiMisc.c changes
> > 2. It displays initiator info properly when it's Enabled for DHCP
> > 3. But, I found some different behavior in below case
> > a. Add an Attempt (Attempt1 -> Initiator Info Enabled for DHCP)
> > b. On reboot iSCSI attempt was success and Initiator Details shown
> > properly ==> This is as expected
> > c. Edit the same Attempt1 details to IP6 and save changes and reset
> > d. Now Iscsi connection with IP6 ==> This is as Expected
> > e. Now if we again Change the Attempt1 to IP4, It is Displaying Subnet
> > Mask ==> I guess we are not clearing It
> >
> > I guess we need to do ZeroMem for initiator details before.
> >
> >
> > B. [Patch 2/2] NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> > -------------------------------------------------------------------------------------------
> ---
> > -------------------------------------------
> > -> This changes looks similar whatever I attached in Bugzilla, and verified
> the
> > same with off board card witch doesn't support IP6
> > -> It works fine, I didn't find any issues on it.
> >
> >
> > C. [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> > -------------------------------------------------------------------------------------------
> ---
> > ------------------------------------------------------------------------
> > I found some issues in this changes, please find my below comments
> > 1. HttpBootCheckIpv6Support() function definition and function call
> > parameter differs , To correct this I've done 1 insertion(+), 1 deletion(-) like
> > below
> > ...
> > +HttpBootCheckIpv6Support (
> > + IN EFI_HANDLE ControllerHandle,
> > + IN HTTP_BOOT_PRIVATE_DATA *Private,
> > + OUT BOOLEAN *Ipv6Support
> > + )
> > ...
> > + // Set IPv6 available flag.
> > + //
> > + Status = HttpBootCheckIpv6Support (ControllerHandle,
> > -This->DriverBindingHandle, &Ipv6Available);
> > +Private, &Ipv6Available);
> > ...
> >
> > 2. With the above changes I've verified with Off board card which doesn't
> > support IP6, But I'm facing below ASSERT
> > (324): CR has Bad Signature
> >
> > EFI_STATUS
> > EFIAPI
> > HttpBootIp4DxeDriverBindingStart (
> > IN EFI_DRIVER_BINDING_PROTOCOL *This,
> > IN EFI_HANDLE ControllerHandle,
> > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> > )
> > {
> > ...
> > if (!EFI_ERROR (Status)) {
> > Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); // ASSERTs
> > here
> > } else {
> > .....
> >
> > 3. I would like add some points and info about the this ASSERT, which I've
> > found
> > The ASSERT is happening because of FreePool (Private), mentioned exact
> > line no below
> >
> > EFI_STATUS
> > EFIAPI
> > HttpBootIp6DxeDriverBindingStart (
> > IN EFI_DRIVER_BINDING_PROTOCOL *This,
> > IN EFI_HANDLE ControllerHandle,
> > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> > )
> > {
> > ...
> > Status = gBS->InstallProtocolInterface (
> > &ControllerHandle,
> > &gEfiCallerIdGuid,
> > EFI_NATIVE_INTERFACE,
> > &Private->Id
> > );
> > if (EFI_ERROR (Status)) {
> > goto ON_ERROR;
> > }
> >
> > }
> > +
> > + //
> > + // Set IPv6 available flag.
> > + //
> > + Status = HttpBootCheckIpv6Support (ControllerHandle,
> > -This->DriverBindingHandle, &Ipv6Available);
> > +Private, &Ipv6Available);
> > + if (EFI_ERROR (Status)) {
> > + //
> > + // Fail to get the data whether UNDI supports IPv6.
> > + // Set default value to TRUE.
> > + //
> > + Ipv6Available = TRUE;
> > + }
> > +
> > + if (!Ipv6Available) {
> > + return EFI_UNSUPPORTED;
> > + }
> >
> > if (Private->Ip6Nic != NULL) {
> > //
> > ...
> >
> > ON_ERROR:
> >
> > HttpBootDestroyIp6Children(This, Private);
> > HttpBootConfigFormUnload (Private);
> > FreePool (Private); // If I comment this
> > line ASSERT is not happening
> >
> > return Status;
> > }
> >
> > 4. At your end could you please verify this IP6 Condition check for HTTP
> >
> > Please correct if anything is wrong, Thanks for your support
> >
> >
> > Thank You,
> > Karunakar
> >
> > -----Original Message-----
> > From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> > Sent: Tuesday, October 17, 2017 7:32 AM
> > To: Wu, Jiaxin; edk2-devel@lists.01.org
> > Cc: Karunakar P; Ye, Ting; Fu, Siyuan
> > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> >
> > Hello Karunakar,
> >
> > Base on your original changes attached in Bugzilla 701
> > (https://bugzilla.tianocore.org/show_bug.cgi?id=710), I created the formal
> > series patches to support the IPv6 condition check for HTTP/ISCSI.
> >
> > Please help to review/verify it.
> >
> > BTW, To review the ISCSI part, please apply the "[Patch v2 0/2]
> > NetworkPkg/IScsiDxe: Display InitiatorInfo correctly" first to avoid any
> patch
> > conflict.
> >
> > Thanks,
> > Jiaxin
> >
> >
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > > Jiaxin Wu
> > > Sent: Tuesday, October 17, 2017 9:58 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Karunakar P <karunakarp@amiindia.co.in>; Ye, Ting
> > > <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin
> > > <jiaxin.wu@intel.com>
> > > Subject: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> > >
> > > Base on the request of
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=710,
> > > we provide this patch to IPv6 condition check by leveraging AIP Protocol.
> > >
> > > Cc: Karunakar P <karunakarp@amiindia.co.in>
> > > Cc: Ye Ting <ting.ye@intel.com>
> > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Karunakar P <karunakarp@amiindia.co.in>
> > > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > >
> > > Jiaxin Wu (2):
> > > NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> > > NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> > >
> > > NetworkPkg/HttpBootDxe/HttpBootDxe.c | 115
> > > +++++++++++++++++++++++++++-
> > > NetworkPkg/HttpBootDxe/HttpBootDxe.h | 2 +
> > > NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 4 +-
> > > NetworkPkg/IScsiDxe/IScsiConfig.c | 18 +++++
> > > NetworkPkg/IScsiDxe/IScsiDriver.c | 2 +-
> > > NetworkPkg/IScsiDxe/IScsiDriver.h | 1 +
> > > NetworkPkg/IScsiDxe/IScsiDxe.inf | 2 +
> > > NetworkPkg/IScsiDxe/IScsiImpl.h | 1 +
> > > NetworkPkg/IScsiDxe/IScsiMisc.c | 135
> > > ++++++++++++++++++++++++++++++++-
> > > NetworkPkg/IScsiDxe/IScsiMisc.h | 4 +-
> > > 10 files changed, 278 insertions(+), 6 deletions(-)
> > >
> > > --
> > > 1.9.5.msysgit.1
> > >
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
2017-10-18 9:00 ` Wu, Jiaxin
2017-10-18 11:43 ` Karunakar P
@ 2017-10-23 16:41 ` Karunakar P
2017-10-24 3:14 ` Wu, Jiaxin
1 sibling, 1 reply; 15+ messages in thread
From: Karunakar P @ 2017-10-23 16:41 UTC (permalink / raw)
To: 'Wu, Jiaxin', edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan
Hi Jiaxin,
1. I've verified, IPv6 support condition check for HTTP/ISCSI, It works fine and the ASSERT issue also resolved.
2. Regards Display Initiator IP, It resolves the issues the issues which I've mentioned previously.
3. I found some other issue in ISCSI, Below are the details
A. Add Attempt1 with Target Info via Static and provide Target Name & Target IP, Save changes.
B. If we are trying to add another attempt , It is taking Target IP as default IP which is same Target IP given in Attempt1.
Could you please check at your end provide your comments on 3rd one.
Thank You,
Karunakar
-----Original Message-----
From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
Sent: Wednesday, October 18, 2017 2:30 PM
To: Karunakar P; edk2-devel@lists.01.org
Cc: Ye, Ting; Fu, Siyuan
Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
Hi Karunakar,
Actually, in my part, I didn't meet the ASSERT no matter the "Ipv6Available" returned from HttpBootCheckIpv6Support is true or false. According your ASSERT case that happened in Ip4DxeDriverBindingStart, which is caused by the FreePool of Private in Ip6DxeDriverBindingStart, so I guess the Ip6DxeDriverBindingStart may be involved ahead of Ip4DxeDriverBindingStart, then something wrong happened in Ip6DxeDriverBindingStart and goto the ON_ERROR (Private is freed here!). so, you can add the breakpoint within Ip6DxeDriverBindingStart/Ip4DxeDriverBindingStart to check it.
Per my analysis above, it does the issue that may trigger the potential ASSERT. So, I refined the patch as version2. The principle of patch v2 is that IPv6 and IPv4 should not affect each other even any failure happen, but the original code doesn't follow that:).
Thanks,
Jiaxin
> -----Original Message-----
> From: Karunakar P [mailto:karunakarp@amiindia.co.in]
> Sent: Wednesday, October 18, 2017 4:06 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hi Jiaxin,
>
> 1. Cleaning the ConfigData when switching the mode will resolve the Issue A.
> 2. Will also verify the ASSERT issue and update you.
>
> Could you please help in clarifying the below items, 1. Could you
> please let us know why the ASSERT happens?
> 2. I've not faced any ASSERT with the changes attached in Bugzilla,
> Did you find any issues/drawbacks with that?
> -> In HttpBootCheckIpv6Support () definition Instead of getting
> -> Ipv6Support
> from Private->Nii->Ipv6Supported if we open the protocol there itself,
> Then there will not be issues in Destroying Children or FreePool(Private) right.
> Because we're going to check HttpBootCheckIpv6Support() before
> opening any instances in HttpBootIp6DxeDriverBindingStart().
>
> Thanks for your great support.
>
> Thank You,
> Karunakar
> ________________________________________
> From: Wu, Jiaxin [jiaxin.wu@intel.com]
> Sent: 18 October 2017 12:35
> To: Karunakar P; edk2-devel@lists.01.org
> Cc: Ye, Ting; Fu, Siyuan
> Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hi Karunakar,
>
> Thanks your verification. Base on your comments, I refined the series
> patches as below to fix the issues:
>
> [Patch v3 0/3] NetworkPkg/IScsiDxe: Display InitiatorInfo correctly.
> NetworkPkg/IScsiDxe: Fix the incorrect/needless DHCP process.
> NetworkPkg/IScsiDxe: Clean the previous ConfigData when
> switching the IP mode. /// This one is to fix the issue A.
> NetworkPkg/IScsiDxe: Display InitiatorInfo in attempt page
> even DHCP enabled.
>
> [Patch v2 0/2] Add IPv6 support condition check.
> NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> /// B && C have been fixed in version 2.
> NetworkPkg/IScsiDxe: Add IPv6 support condition check.
>
> Please help to verify them.
>
> Best Regards,
> Jiaxin
>
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> > Of Karunakar P
> > Sent: Tuesday, October 17, 2017 6:13 PM
> > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> > Subject: Re: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> >
> > Hi Jiaxin,
> >
> > I Reviewed the changes for 3 features/Bugs and verified the same,
> > Please find my below comments and issues faced
> >
> > A. Display InitiatorInfo in attempt page even DHCP enabled
> > --------------------------------------------------------------------
> > -----------------------
> ---
> > --------------------------------------------
> > 1. I applied IScsiConfigVfr.vfr changes and as well IScsiMisc.c
> > changes 2. It displays initiator info properly when it's Enabled for
> > DHCP 3. But, I found some different behavior in below case
> > a. Add an Attempt (Attempt1 -> Initiator Info Enabled for DHCP)
> > b. On reboot iSCSI attempt was success and Initiator Details
> > shown properly ==> This is as expected
> > c. Edit the same Attempt1 details to IP6 and save changes and reset
> > d. Now Iscsi connection with IP6 ==> This is as Expected
> > e. Now if we again Change the Attempt1 to IP4, It is Displaying
> > Subnet Mask ==> I guess we are not clearing It
> >
> > I guess we need to do ZeroMem for initiator details before.
> >
> >
> > B. [Patch 2/2] NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> > --------------------------------------------------------------------
> > -----------------------
> ---
> > -------------------------------------------
> > -> This changes looks similar whatever I attached in Bugzilla, and
> > -> verified
> the
> > same with off board card witch doesn't support IP6
> > -> It works fine, I didn't find any issues on it.
> >
> >
> > C. [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> > --------------------------------------------------------------------
> > -----------------------
> ---
> > --------------------------------------------------------------------
> > ---- I found some issues in this changes, please find my below
> > comments 1. HttpBootCheckIpv6Support() function definition and
> > function call parameter differs , To correct this I've done 1
> > insertion(+), 1 deletion(-) like below ...
> > +HttpBootCheckIpv6Support (
> > + IN EFI_HANDLE ControllerHandle,
> > + IN HTTP_BOOT_PRIVATE_DATA *Private,
> > + OUT BOOLEAN *Ipv6Support
> > + )
> > ...
> > + // Set IPv6 available flag.
> > + //
> > + Status = HttpBootCheckIpv6Support (ControllerHandle,
> > -This->DriverBindingHandle, &Ipv6Available);
> > +Private, &Ipv6Available);
> > ...
> >
> > 2. With the above changes I've verified with Off board card which
> > doesn't support IP6, But I'm facing below ASSERT
> > (324): CR has Bad Signature
> >
> > EFI_STATUS
> > EFIAPI
> > HttpBootIp4DxeDriverBindingStart (
> > IN EFI_DRIVER_BINDING_PROTOCOL *This,
> > IN EFI_HANDLE ControllerHandle,
> > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> > )
> > {
> > ...
> > if (!EFI_ERROR (Status)) {
> > Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); // ASSERTs
> > here
> > } else {
> > .....
> >
> > 3. I would like add some points and info about the this ASSERT,
> > which I've found The ASSERT is happening because of FreePool
> > (Private), mentioned exact line no below
> >
> > EFI_STATUS
> > EFIAPI
> > HttpBootIp6DxeDriverBindingStart (
> > IN EFI_DRIVER_BINDING_PROTOCOL *This,
> > IN EFI_HANDLE ControllerHandle,
> > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> > )
> > {
> > ...
> > Status = gBS->InstallProtocolInterface (
> > &ControllerHandle,
> > &gEfiCallerIdGuid,
> > EFI_NATIVE_INTERFACE,
> > &Private->Id
> > );
> > if (EFI_ERROR (Status)) {
> > goto ON_ERROR;
> > }
> >
> > }
> > +
> > + //
> > + // Set IPv6 available flag.
> > + //
> > + Status = HttpBootCheckIpv6Support (ControllerHandle,
> > -This->DriverBindingHandle, &Ipv6Available);
> > +Private, &Ipv6Available);
> > + if (EFI_ERROR (Status)) {
> > + //
> > + // Fail to get the data whether UNDI supports IPv6.
> > + // Set default value to TRUE.
> > + //
> > + Ipv6Available = TRUE;
> > + }
> > +
> > + if (!Ipv6Available) {
> > + return EFI_UNSUPPORTED;
> > + }
> >
> > if (Private->Ip6Nic != NULL) {
> > //
> > ...
> >
> > ON_ERROR:
> >
> > HttpBootDestroyIp6Children(This, Private);
> > HttpBootConfigFormUnload (Private);
> > FreePool (Private); // If I comment this
> > line ASSERT is not happening
> >
> > return Status;
> > }
> >
> > 4. At your end could you please verify this IP6 Condition check for
> > HTTP
> >
> > Please correct if anything is wrong, Thanks for your support
> >
> >
> > Thank You,
> > Karunakar
> >
> > -----Original Message-----
> > From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> > Sent: Tuesday, October 17, 2017 7:32 AM
> > To: Wu, Jiaxin; edk2-devel@lists.01.org
> > Cc: Karunakar P; Ye, Ting; Fu, Siyuan
> > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> >
> > Hello Karunakar,
> >
> > Base on your original changes attached in Bugzilla 701
> > (https://bugzilla.tianocore.org/show_bug.cgi?id=710), I created the
> > formal series patches to support the IPv6 condition check for HTTP/ISCSI.
> >
> > Please help to review/verify it.
> >
> > BTW, To review the ISCSI part, please apply the "[Patch v2 0/2]
> > NetworkPkg/IScsiDxe: Display InitiatorInfo correctly" first to avoid
> > any
> patch
> > conflict.
> >
> > Thanks,
> > Jiaxin
> >
> >
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> > > Behalf Of Jiaxin Wu
> > > Sent: Tuesday, October 17, 2017 9:58 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Karunakar P <karunakarp@amiindia.co.in>; Ye, Ting
> > > <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin
> > > <jiaxin.wu@intel.com>
> > > Subject: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> > >
> > > Base on the request of
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=710,
> > > we provide this patch to IPv6 condition check by leveraging AIP Protocol.
> > >
> > > Cc: Karunakar P <karunakarp@amiindia.co.in>
> > > Cc: Ye Ting <ting.ye@intel.com>
> > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Karunakar P <karunakarp@amiindia.co.in>
> > > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > >
> > > Jiaxin Wu (2):
> > > NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> > > NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> > >
> > > NetworkPkg/HttpBootDxe/HttpBootDxe.c | 115
> > > +++++++++++++++++++++++++++-
> > > NetworkPkg/HttpBootDxe/HttpBootDxe.h | 2 +
> > > NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 4 +-
> > > NetworkPkg/IScsiDxe/IScsiConfig.c | 18 +++++
> > > NetworkPkg/IScsiDxe/IScsiDriver.c | 2 +-
> > > NetworkPkg/IScsiDxe/IScsiDriver.h | 1 +
> > > NetworkPkg/IScsiDxe/IScsiDxe.inf | 2 +
> > > NetworkPkg/IScsiDxe/IScsiImpl.h | 1 +
> > > NetworkPkg/IScsiDxe/IScsiMisc.c | 135
> > > ++++++++++++++++++++++++++++++++-
> > > NetworkPkg/IScsiDxe/IScsiMisc.h | 4 +-
> > > 10 files changed, 278 insertions(+), 6 deletions(-)
> > >
> > > --
> > > 1.9.5.msysgit.1
> > >
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
2017-10-23 16:41 ` Karunakar P
@ 2017-10-24 3:14 ` Wu, Jiaxin
2017-10-24 7:13 ` Karunakar P
0 siblings, 1 reply; 15+ messages in thread
From: Wu, Jiaxin @ 2017-10-24 3:14 UTC (permalink / raw)
To: Karunakar P, edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan
Hi Karunakar,
For #3, it's the non-regression issue that will cause the target IP is invalid if it's the same with previous one. And I'd like to fix it by another patch since it's not related to the InitiatorInfo display.
Can you please report another Bugzilla for the ISCSI target address issue?
Thanks,
Jiaxin
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Karunakar P
> Sent: Tuesday, October 24, 2017 12:41 AM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: Re: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hi Jiaxin,
>
> 1. I've verified, IPv6 support condition check for HTTP/ISCSI, It works fine and
> the ASSERT issue also resolved.
> 2. Regards Display Initiator IP, It resolves the issues the issues which I've
> mentioned previously.
> 3. I found some other issue in ISCSI, Below are the details
> A. Add Attempt1 with Target Info via Static and provide Target Name &
> Target IP, Save changes.
> B. If we are trying to add another attempt , It is taking Target IP as default IP
> which is same Target IP given in Attempt1.
>
> Could you please check at your end provide your comments on 3rd one.
>
> Thank You,
> Karunakar
>
> -----Original Message-----
> From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> Sent: Wednesday, October 18, 2017 2:30 PM
> To: Karunakar P; edk2-devel@lists.01.org
> Cc: Ye, Ting; Fu, Siyuan
> Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hi Karunakar,
>
> Actually, in my part, I didn't meet the ASSERT no matter the "Ipv6Available"
> returned from HttpBootCheckIpv6Support is true or false. According your
> ASSERT case that happened in Ip4DxeDriverBindingStart, which is caused by
> the FreePool of Private in Ip6DxeDriverBindingStart, so I guess the
> Ip6DxeDriverBindingStart may be involved ahead of
> Ip4DxeDriverBindingStart, then something wrong happened in
> Ip6DxeDriverBindingStart and goto the ON_ERROR (Private is freed here!). so,
> you can add the breakpoint within
> Ip6DxeDriverBindingStart/Ip4DxeDriverBindingStart to check it.
>
> Per my analysis above, it does the issue that may trigger the potential ASSERT.
> So, I refined the patch as version2. The principle of patch v2 is that IPv6 and
> IPv4 should not affect each other even any failure happen, but the original
> code doesn't follow that:).
>
>
> Thanks,
> Jiaxin
>
>
> > -----Original Message-----
> > From: Karunakar P [mailto:karunakarp@amiindia.co.in]
> > Sent: Wednesday, October 18, 2017 4:06 PM
> > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> >
> > Hi Jiaxin,
> >
> > 1. Cleaning the ConfigData when switching the mode will resolve the Issue
> A.
> > 2. Will also verify the ASSERT issue and update you.
> >
> > Could you please help in clarifying the below items, 1. Could you
> > please let us know why the ASSERT happens?
> > 2. I've not faced any ASSERT with the changes attached in Bugzilla,
> > Did you find any issues/drawbacks with that?
> > -> In HttpBootCheckIpv6Support () definition Instead of getting
> > -> Ipv6Support
> > from Private->Nii->Ipv6Supported if we open the protocol there itself,
> > Then there will not be issues in Destroying Children or FreePool(Private)
> right.
> > Because we're going to check HttpBootCheckIpv6Support() before
> > opening any instances in HttpBootIp6DxeDriverBindingStart().
> >
> > Thanks for your great support.
> >
> > Thank You,
> > Karunakar
> > ________________________________________
> > From: Wu, Jiaxin [jiaxin.wu@intel.com]
> > Sent: 18 October 2017 12:35
> > To: Karunakar P; edk2-devel@lists.01.org
> > Cc: Ye, Ting; Fu, Siyuan
> > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> >
> > Hi Karunakar,
> >
> > Thanks your verification. Base on your comments, I refined the series
> > patches as below to fix the issues:
> >
> > [Patch v3 0/3] NetworkPkg/IScsiDxe: Display InitiatorInfo correctly.
> > NetworkPkg/IScsiDxe: Fix the incorrect/needless DHCP process.
> > NetworkPkg/IScsiDxe: Clean the previous ConfigData when
> > switching the IP mode. /// This one is to fix the issue A.
> > NetworkPkg/IScsiDxe: Display InitiatorInfo in attempt page
> > even DHCP enabled.
> >
> > [Patch v2 0/2] Add IPv6 support condition check.
> > NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> > /// B && C have been fixed in version 2.
> > NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> >
> > Please help to verify them.
> >
> > Best Regards,
> > Jiaxin
> >
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> > > Of Karunakar P
> > > Sent: Tuesday, October 17, 2017 6:13 PM
> > > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> > > Subject: Re: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > > HTTP/ISCSI.
> > >
> > > Hi Jiaxin,
> > >
> > > I Reviewed the changes for 3 features/Bugs and verified the same,
> > > Please find my below comments and issues faced
> > >
> > > A. Display InitiatorInfo in attempt page even DHCP enabled
> > > --------------------------------------------------------------------
> > > -----------------------
> > ---
> > > --------------------------------------------
> > > 1. I applied IScsiConfigVfr.vfr changes and as well IScsiMisc.c
> > > changes 2. It displays initiator info properly when it's Enabled for
> > > DHCP 3. But, I found some different behavior in below case
> > > a. Add an Attempt (Attempt1 -> Initiator Info Enabled for DHCP)
> > > b. On reboot iSCSI attempt was success and Initiator Details
> > > shown properly ==> This is as expected
> > > c. Edit the same Attempt1 details to IP6 and save changes and reset
> > > d. Now Iscsi connection with IP6 ==> This is as Expected
> > > e. Now if we again Change the Attempt1 to IP4, It is Displaying
> > > Subnet Mask ==> I guess we are not clearing It
> > >
> > > I guess we need to do ZeroMem for initiator details before.
> > >
> > >
> > > B. [Patch 2/2] NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> > > --------------------------------------------------------------------
> > > -----------------------
> > ---
> > > -------------------------------------------
> > > -> This changes looks similar whatever I attached in Bugzilla, and
> > > -> verified
> > the
> > > same with off board card witch doesn't support IP6
> > > -> It works fine, I didn't find any issues on it.
> > >
> > >
> > > C. [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support condition
> check.
> > > --------------------------------------------------------------------
> > > -----------------------
> > ---
> > > --------------------------------------------------------------------
> > > ---- I found some issues in this changes, please find my below
> > > comments 1. HttpBootCheckIpv6Support() function definition and
> > > function call parameter differs , To correct this I've done 1
> > > insertion(+), 1 deletion(-) like below ...
> > > +HttpBootCheckIpv6Support (
> > > + IN EFI_HANDLE ControllerHandle,
> > > + IN HTTP_BOOT_PRIVATE_DATA *Private,
> > > + OUT BOOLEAN *Ipv6Support
> > > + )
> > > ...
> > > + // Set IPv6 available flag.
> > > + //
> > > + Status = HttpBootCheckIpv6Support (ControllerHandle,
> > > -This->DriverBindingHandle, &Ipv6Available);
> > > +Private, &Ipv6Available);
> > > ...
> > >
> > > 2. With the above changes I've verified with Off board card which
> > > doesn't support IP6, But I'm facing below ASSERT
> > > (324): CR has Bad Signature
> > >
> > > EFI_STATUS
> > > EFIAPI
> > > HttpBootIp4DxeDriverBindingStart (
> > > IN EFI_DRIVER_BINDING_PROTOCOL *This,
> > > IN EFI_HANDLE ControllerHandle,
> > > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> > > )
> > > {
> > > ...
> > > if (!EFI_ERROR (Status)) {
> > > Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); //
> ASSERTs
> > > here
> > > } else {
> > > .....
> > >
> > > 3. I would like add some points and info about the this ASSERT,
> > > which I've found The ASSERT is happening because of FreePool
> > > (Private), mentioned exact line no below
> > >
> > > EFI_STATUS
> > > EFIAPI
> > > HttpBootIp6DxeDriverBindingStart (
> > > IN EFI_DRIVER_BINDING_PROTOCOL *This,
> > > IN EFI_HANDLE ControllerHandle,
> > > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> > > )
> > > {
> > > ...
> > > Status = gBS->InstallProtocolInterface (
> > > &ControllerHandle,
> > > &gEfiCallerIdGuid,
> > > EFI_NATIVE_INTERFACE,
> > > &Private->Id
> > > );
> > > if (EFI_ERROR (Status)) {
> > > goto ON_ERROR;
> > > }
> > >
> > > }
> > > +
> > > + //
> > > + // Set IPv6 available flag.
> > > + //
> > > + Status = HttpBootCheckIpv6Support (ControllerHandle,
> > > -This->DriverBindingHandle, &Ipv6Available);
> > > +Private, &Ipv6Available);
> > > + if (EFI_ERROR (Status)) {
> > > + //
> > > + // Fail to get the data whether UNDI supports IPv6.
> > > + // Set default value to TRUE.
> > > + //
> > > + Ipv6Available = TRUE;
> > > + }
> > > +
> > > + if (!Ipv6Available) {
> > > + return EFI_UNSUPPORTED;
> > > + }
> > >
> > > if (Private->Ip6Nic != NULL) {
> > > //
> > > ...
> > >
> > > ON_ERROR:
> > >
> > > HttpBootDestroyIp6Children(This, Private);
> > > HttpBootConfigFormUnload (Private);
> > > FreePool (Private); // If I comment
> this
> > > line ASSERT is not happening
> > >
> > > return Status;
> > > }
> > >
> > > 4. At your end could you please verify this IP6 Condition check for
> > > HTTP
> > >
> > > Please correct if anything is wrong, Thanks for your support
> > >
> > >
> > > Thank You,
> > > Karunakar
> > >
> > > -----Original Message-----
> > > From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> > > Sent: Tuesday, October 17, 2017 7:32 AM
> > > To: Wu, Jiaxin; edk2-devel@lists.01.org
> > > Cc: Karunakar P; Ye, Ting; Fu, Siyuan
> > > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > > HTTP/ISCSI.
> > >
> > > Hello Karunakar,
> > >
> > > Base on your original changes attached in Bugzilla 701
> > > (https://bugzilla.tianocore.org/show_bug.cgi?id=710), I created the
> > > formal series patches to support the IPv6 condition check for HTTP/ISCSI.
> > >
> > > Please help to review/verify it.
> > >
> > > BTW, To review the ISCSI part, please apply the "[Patch v2 0/2]
> > > NetworkPkg/IScsiDxe: Display InitiatorInfo correctly" first to avoid
> > > any
> > patch
> > > conflict.
> > >
> > > Thanks,
> > > Jiaxin
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> > > > Behalf Of Jiaxin Wu
> > > > Sent: Tuesday, October 17, 2017 9:58 AM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Karunakar P <karunakarp@amiindia.co.in>; Ye, Ting
> > > > <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin
> > > > <jiaxin.wu@intel.com>
> > > > Subject: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > > HTTP/ISCSI.
> > > >
> > > > Base on the request of
> > > > https://bugzilla.tianocore.org/show_bug.cgi?id=710,
> > > > we provide this patch to IPv6 condition check by leveraging AIP Protocol.
> > > >
> > > > Cc: Karunakar P <karunakarp@amiindia.co.in>
> > > > Cc: Ye Ting <ting.ye@intel.com>
> > > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > > Signed-off-by: Karunakar P <karunakarp@amiindia.co.in>
> > > > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > > >
> > > > Jiaxin Wu (2):
> > > > NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> > > > NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> > > >
> > > > NetworkPkg/HttpBootDxe/HttpBootDxe.c | 115
> > > > +++++++++++++++++++++++++++-
> > > > NetworkPkg/HttpBootDxe/HttpBootDxe.h | 2 +
> > > > NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 4 +-
> > > > NetworkPkg/IScsiDxe/IScsiConfig.c | 18 +++++
> > > > NetworkPkg/IScsiDxe/IScsiDriver.c | 2 +-
> > > > NetworkPkg/IScsiDxe/IScsiDriver.h | 1 +
> > > > NetworkPkg/IScsiDxe/IScsiDxe.inf | 2 +
> > > > NetworkPkg/IScsiDxe/IScsiImpl.h | 1 +
> > > > NetworkPkg/IScsiDxe/IScsiMisc.c | 135
> > > > ++++++++++++++++++++++++++++++++-
> > > > NetworkPkg/IScsiDxe/IScsiMisc.h | 4 +-
> > > > 10 files changed, 278 insertions(+), 6 deletions(-)
> > > >
> > > > --
> > > > 1.9.5.msysgit.1
> > > >
> > > > _______________________________________________
> > > > edk2-devel mailing list
> > > > edk2-devel@lists.01.org
> > > > https://lists.01.org/mailman/listinfo/edk2-devel
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
2017-10-24 3:14 ` Wu, Jiaxin
@ 2017-10-24 7:13 ` Karunakar P
2017-10-24 7:20 ` Wu, Jiaxin
0 siblings, 1 reply; 15+ messages in thread
From: Karunakar P @ 2017-10-24 7:13 UTC (permalink / raw)
To: 'Wu, Jiaxin', edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan
Hi Jiaxin,
Yes, I believe this issue(#) is not the regression for previous issues we have fixed.
I created a new bug For #3 and below are the details
https://bugzilla.tianocore.org/show_bug.cgi?id=743
Could you please let me know the scheduled check in date for previously reported issue/Features
Thank You,
Karunakar
-----Original Message-----
From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
Sent: Tuesday, October 24, 2017 8:45 AM
To: Karunakar P; edk2-devel@lists.01.org
Cc: Ye, Ting; Fu, Siyuan
Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
Hi Karunakar,
For #3, it's the non-regression issue that will cause the target IP is invalid if it's the same with previous one. And I'd like to fix it by another patch since it's not related to the InitiatorInfo display.
Can you please report another Bugzilla for the ISCSI target address issue?
Thanks,
Jiaxin
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Karunakar P
> Sent: Tuesday, October 24, 2017 12:41 AM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: Re: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hi Jiaxin,
>
> 1. I've verified, IPv6 support condition check for HTTP/ISCSI, It
> works fine and the ASSERT issue also resolved.
> 2. Regards Display Initiator IP, It resolves the issues the issues
> which I've mentioned previously.
> 3. I found some other issue in ISCSI, Below are the details A. Add
> Attempt1 with Target Info via Static and provide Target Name & Target
> IP, Save changes.
> B. If we are trying to add another attempt , It is taking Target IP as
> default IP which is same Target IP given in Attempt1.
>
> Could you please check at your end provide your comments on 3rd one.
>
> Thank You,
> Karunakar
>
> -----Original Message-----
> From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> Sent: Wednesday, October 18, 2017 2:30 PM
> To: Karunakar P; edk2-devel@lists.01.org
> Cc: Ye, Ting; Fu, Siyuan
> Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hi Karunakar,
>
> Actually, in my part, I didn't meet the ASSERT no matter the "Ipv6Available"
> returned from HttpBootCheckIpv6Support is true or false. According
> your ASSERT case that happened in Ip4DxeDriverBindingStart, which is
> caused by the FreePool of Private in Ip6DxeDriverBindingStart, so I
> guess the Ip6DxeDriverBindingStart may be involved ahead of
> Ip4DxeDriverBindingStart, then something wrong happened in
> Ip6DxeDriverBindingStart and goto the ON_ERROR (Private is freed
> here!). so, you can add the breakpoint within
> Ip6DxeDriverBindingStart/Ip4DxeDriverBindingStart to check it.
>
> Per my analysis above, it does the issue that may trigger the potential ASSERT.
> So, I refined the patch as version2. The principle of patch v2 is that
> IPv6 and
> IPv4 should not affect each other even any failure happen, but the
> original code doesn't follow that:).
>
>
> Thanks,
> Jiaxin
>
>
> > -----Original Message-----
> > From: Karunakar P [mailto:karunakarp@amiindia.co.in]
> > Sent: Wednesday, October 18, 2017 4:06 PM
> > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> >
> > Hi Jiaxin,
> >
> > 1. Cleaning the ConfigData when switching the mode will resolve the
> > Issue
> A.
> > 2. Will also verify the ASSERT issue and update you.
> >
> > Could you please help in clarifying the below items, 1. Could you
> > please let us know why the ASSERT happens?
> > 2. I've not faced any ASSERT with the changes attached in Bugzilla,
> > Did you find any issues/drawbacks with that?
> > -> In HttpBootCheckIpv6Support () definition Instead of getting
> > -> Ipv6Support
> > from Private->Nii->Ipv6Supported if we open the protocol there
> > itself, Then there will not be issues in Destroying Children or
> > FreePool(Private)
> right.
> > Because we're going to check HttpBootCheckIpv6Support() before
> > opening any instances in HttpBootIp6DxeDriverBindingStart().
> >
> > Thanks for your great support.
> >
> > Thank You,
> > Karunakar
> > ________________________________________
> > From: Wu, Jiaxin [jiaxin.wu@intel.com]
> > Sent: 18 October 2017 12:35
> > To: Karunakar P; edk2-devel@lists.01.org
> > Cc: Ye, Ting; Fu, Siyuan
> > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> >
> > Hi Karunakar,
> >
> > Thanks your verification. Base on your comments, I refined the
> > series patches as below to fix the issues:
> >
> > [Patch v3 0/3] NetworkPkg/IScsiDxe: Display InitiatorInfo correctly.
> > NetworkPkg/IScsiDxe: Fix the incorrect/needless DHCP process.
> > NetworkPkg/IScsiDxe: Clean the previous ConfigData when
> > switching the IP mode. /// This one is to fix the issue A.
> > NetworkPkg/IScsiDxe: Display InitiatorInfo in attempt
> > page even DHCP enabled.
> >
> > [Patch v2 0/2] Add IPv6 support condition check.
> > NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> > /// B && C have been fixed in version 2.
> > NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> >
> > Please help to verify them.
> >
> > Best Regards,
> > Jiaxin
> >
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> > > Behalf Of Karunakar P
> > > Sent: Tuesday, October 17, 2017 6:13 PM
> > > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> > > Subject: Re: [edk2] [Patch 0/2] Add IPv6 support condition check
> > > for HTTP/ISCSI.
> > >
> > > Hi Jiaxin,
> > >
> > > I Reviewed the changes for 3 features/Bugs and verified the same,
> > > Please find my below comments and issues faced
> > >
> > > A. Display InitiatorInfo in attempt page even DHCP enabled
> > > ------------------------------------------------------------------
> > > --
> > > -----------------------
> > ---
> > > --------------------------------------------
> > > 1. I applied IScsiConfigVfr.vfr changes and as well IScsiMisc.c
> > > changes 2. It displays initiator info properly when it's Enabled
> > > for DHCP 3. But, I found some different behavior in below case
> > > a. Add an Attempt (Attempt1 -> Initiator Info Enabled for DHCP)
> > > b. On reboot iSCSI attempt was success and Initiator Details
> > > shown properly ==> This is as expected
> > > c. Edit the same Attempt1 details to IP6 and save changes and reset
> > > d. Now Iscsi connection with IP6 ==> This is as Expected
> > > e. Now if we again Change the Attempt1 to IP4, It is
> > > Displaying Subnet Mask ==> I guess we are not clearing It
> > >
> > > I guess we need to do ZeroMem for initiator details before.
> > >
> > >
> > > B. [Patch 2/2] NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> > > ------------------------------------------------------------------
> > > --
> > > -----------------------
> > ---
> > > -------------------------------------------
> > > -> This changes looks similar whatever I attached in Bugzilla, and
> > > -> verified
> > the
> > > same with off board card witch doesn't support IP6
> > > -> It works fine, I didn't find any issues on it.
> > >
> > >
> > > C. [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support condition
> check.
> > > ------------------------------------------------------------------
> > > --
> > > -----------------------
> > ---
> > > ------------------------------------------------------------------
> > > --
> > > ---- I found some issues in this changes, please find my below
> > > comments 1. HttpBootCheckIpv6Support() function definition and
> > > function call parameter differs , To correct this I've done 1
> > > insertion(+), 1 deletion(-) like below ...
> > > +HttpBootCheckIpv6Support (
> > > + IN EFI_HANDLE ControllerHandle,
> > > + IN HTTP_BOOT_PRIVATE_DATA *Private,
> > > + OUT BOOLEAN *Ipv6Support
> > > + )
> > > ...
> > > + // Set IPv6 available flag.
> > > + //
> > > + Status = HttpBootCheckIpv6Support (ControllerHandle,
> > > -This->DriverBindingHandle, &Ipv6Available);
> > > +Private, &Ipv6Available);
> > > ...
> > >
> > > 2. With the above changes I've verified with Off board card which
> > > doesn't support IP6, But I'm facing below ASSERT
> > > (324): CR has Bad Signature
> > >
> > > EFI_STATUS
> > > EFIAPI
> > > HttpBootIp4DxeDriverBindingStart (
> > > IN EFI_DRIVER_BINDING_PROTOCOL *This,
> > > IN EFI_HANDLE ControllerHandle,
> > > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> > > )
> > > {
> > > ...
> > > if (!EFI_ERROR (Status)) {
> > > Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); //
> ASSERTs
> > > here
> > > } else {
> > > .....
> > >
> > > 3. I would like add some points and info about the this ASSERT,
> > > which I've found The ASSERT is happening because of FreePool
> > > (Private), mentioned exact line no below
> > >
> > > EFI_STATUS
> > > EFIAPI
> > > HttpBootIp6DxeDriverBindingStart (
> > > IN EFI_DRIVER_BINDING_PROTOCOL *This,
> > > IN EFI_HANDLE ControllerHandle,
> > > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> > > )
> > > {
> > > ...
> > > Status = gBS->InstallProtocolInterface (
> > > &ControllerHandle,
> > > &gEfiCallerIdGuid,
> > > EFI_NATIVE_INTERFACE,
> > > &Private->Id
> > > );
> > > if (EFI_ERROR (Status)) {
> > > goto ON_ERROR;
> > > }
> > >
> > > }
> > > +
> > > + //
> > > + // Set IPv6 available flag.
> > > + //
> > > + Status = HttpBootCheckIpv6Support (ControllerHandle,
> > > -This->DriverBindingHandle, &Ipv6Available);
> > > +Private, &Ipv6Available);
> > > + if (EFI_ERROR (Status)) {
> > > + //
> > > + // Fail to get the data whether UNDI supports IPv6.
> > > + // Set default value to TRUE.
> > > + //
> > > + Ipv6Available = TRUE;
> > > + }
> > > +
> > > + if (!Ipv6Available) {
> > > + return EFI_UNSUPPORTED;
> > > + }
> > >
> > > if (Private->Ip6Nic != NULL) {
> > > //
> > > ...
> > >
> > > ON_ERROR:
> > >
> > > HttpBootDestroyIp6Children(This, Private);
> > > HttpBootConfigFormUnload (Private);
> > > FreePool (Private); // If I comment
> this
> > > line ASSERT is not happening
> > >
> > > return Status;
> > > }
> > >
> > > 4. At your end could you please verify this IP6 Condition check
> > > for HTTP
> > >
> > > Please correct if anything is wrong, Thanks for your support
> > >
> > >
> > > Thank You,
> > > Karunakar
> > >
> > > -----Original Message-----
> > > From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> > > Sent: Tuesday, October 17, 2017 7:32 AM
> > > To: Wu, Jiaxin; edk2-devel@lists.01.org
> > > Cc: Karunakar P; Ye, Ting; Fu, Siyuan
> > > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check
> > > for HTTP/ISCSI.
> > >
> > > Hello Karunakar,
> > >
> > > Base on your original changes attached in Bugzilla 701
> > > (https://bugzilla.tianocore.org/show_bug.cgi?id=710), I created
> > > the formal series patches to support the IPv6 condition check for HTTP/ISCSI.
> > >
> > > Please help to review/verify it.
> > >
> > > BTW, To review the ISCSI part, please apply the "[Patch v2 0/2]
> > > NetworkPkg/IScsiDxe: Display InitiatorInfo correctly" first to
> > > avoid any
> > patch
> > > conflict.
> > >
> > > Thanks,
> > > Jiaxin
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> > > > Behalf Of Jiaxin Wu
> > > > Sent: Tuesday, October 17, 2017 9:58 AM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Karunakar P <karunakarp@amiindia.co.in>; Ye, Ting
> > > > <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu,
> > > > Jiaxin <jiaxin.wu@intel.com>
> > > > Subject: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > > HTTP/ISCSI.
> > > >
> > > > Base on the request of
> > > > https://bugzilla.tianocore.org/show_bug.cgi?id=710,
> > > > we provide this patch to IPv6 condition check by leveraging AIP Protocol.
> > > >
> > > > Cc: Karunakar P <karunakarp@amiindia.co.in>
> > > > Cc: Ye Ting <ting.ye@intel.com>
> > > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > > Signed-off-by: Karunakar P <karunakarp@amiindia.co.in>
> > > > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > > >
> > > > Jiaxin Wu (2):
> > > > NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> > > > NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> > > >
> > > > NetworkPkg/HttpBootDxe/HttpBootDxe.c | 115
> > > > +++++++++++++++++++++++++++-
> > > > NetworkPkg/HttpBootDxe/HttpBootDxe.h | 2 +
> > > > NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 4 +-
> > > > NetworkPkg/IScsiDxe/IScsiConfig.c | 18 +++++
> > > > NetworkPkg/IScsiDxe/IScsiDriver.c | 2 +-
> > > > NetworkPkg/IScsiDxe/IScsiDriver.h | 1 +
> > > > NetworkPkg/IScsiDxe/IScsiDxe.inf | 2 +
> > > > NetworkPkg/IScsiDxe/IScsiImpl.h | 1 +
> > > > NetworkPkg/IScsiDxe/IScsiMisc.c | 135
> > > > ++++++++++++++++++++++++++++++++-
> > > > NetworkPkg/IScsiDxe/IScsiMisc.h | 4 +-
> > > > 10 files changed, 278 insertions(+), 6 deletions(-)
> > > >
> > > > --
> > > > 1.9.5.msysgit.1
> > > >
> > > > _______________________________________________
> > > > edk2-devel mailing list
> > > > edk2-devel@lists.01.org
> > > > https://lists.01.org/mailman/listinfo/edk2-devel
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
2017-10-24 7:13 ` Karunakar P
@ 2017-10-24 7:20 ` Wu, Jiaxin
2017-10-24 7:29 ` Karunakar P
0 siblings, 1 reply; 15+ messages in thread
From: Wu, Jiaxin @ 2017-10-24 7:20 UTC (permalink / raw)
To: Karunakar P, edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan
Hi Karunakar,
If you have no more comments, can you reply the below series patches with Reviewed-by/Tested-By tags, then I can go on the commit process?
[Patch v3 0/3] NetworkPkg/IScsiDxe: Display InitiatorInfo correctly.
NetworkPkg/IScsiDxe: Fix the incorrect/needless DHCP process.
NetworkPkg/IScsiDxe: Clean the previous ConfigData when switching the IP mode. /// This one is to fix the issue A.
NetworkPkg/IScsiDxe: Display InitiatorInfo in attempt page even DHCP enabled.
[Patch v2 0/2] Add IPv6 support condition check.
NetworkPkg/HttpBootDxe: Add IPv6 support condition check. /// B && C have been fixed in version 2.
NetworkPkg/IScsiDxe: Add IPv6 support condition check.
Thanks,
Jiaxin
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Karunakar P
> Sent: Tuesday, October 24, 2017 3:13 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: Re: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hi Jiaxin,
>
> Yes, I believe this issue(#) is not the regression for previous issues we have
> fixed.
>
> I created a new bug For #3 and below are the details
> https://bugzilla.tianocore.org/show_bug.cgi?id=743
>
> Could you please let me know the scheduled check in date for previously
> reported issue/Features
>
> Thank You,
> Karunakar
>
> -----Original Message-----
> From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> Sent: Tuesday, October 24, 2017 8:45 AM
> To: Karunakar P; edk2-devel@lists.01.org
> Cc: Ye, Ting; Fu, Siyuan
> Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hi Karunakar,
>
> For #3, it's the non-regression issue that will cause the target IP is invalid if it's
> the same with previous one. And I'd like to fix it by another patch since it's
> not related to the InitiatorInfo display.
>
> Can you please report another Bugzilla for the ISCSI target address issue?
>
> Thanks,
> Jiaxin
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Karunakar P
> > Sent: Tuesday, October 24, 2017 12:41 AM
> > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> > Subject: Re: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> >
> > Hi Jiaxin,
> >
> > 1. I've verified, IPv6 support condition check for HTTP/ISCSI, It
> > works fine and the ASSERT issue also resolved.
> > 2. Regards Display Initiator IP, It resolves the issues the issues
> > which I've mentioned previously.
> > 3. I found some other issue in ISCSI, Below are the details A. Add
> > Attempt1 with Target Info via Static and provide Target Name & Target
> > IP, Save changes.
> > B. If we are trying to add another attempt , It is taking Target IP as
> > default IP which is same Target IP given in Attempt1.
> >
> > Could you please check at your end provide your comments on 3rd one.
> >
> > Thank You,
> > Karunakar
> >
> > -----Original Message-----
> > From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> > Sent: Wednesday, October 18, 2017 2:30 PM
> > To: Karunakar P; edk2-devel@lists.01.org
> > Cc: Ye, Ting; Fu, Siyuan
> > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> >
> > Hi Karunakar,
> >
> > Actually, in my part, I didn't meet the ASSERT no matter the "Ipv6Available"
> > returned from HttpBootCheckIpv6Support is true or false. According
> > your ASSERT case that happened in Ip4DxeDriverBindingStart, which is
> > caused by the FreePool of Private in Ip6DxeDriverBindingStart, so I
> > guess the Ip6DxeDriverBindingStart may be involved ahead of
> > Ip4DxeDriverBindingStart, then something wrong happened in
> > Ip6DxeDriverBindingStart and goto the ON_ERROR (Private is freed
> > here!). so, you can add the breakpoint within
> > Ip6DxeDriverBindingStart/Ip4DxeDriverBindingStart to check it.
> >
> > Per my analysis above, it does the issue that may trigger the potential
> ASSERT.
> > So, I refined the patch as version2. The principle of patch v2 is that
> > IPv6 and
> > IPv4 should not affect each other even any failure happen, but the
> > original code doesn't follow that:).
> >
> >
> > Thanks,
> > Jiaxin
> >
> >
> > > -----Original Message-----
> > > From: Karunakar P [mailto:karunakarp@amiindia.co.in]
> > > Sent: Wednesday, October 18, 2017 4:06 PM
> > > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> > > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > > HTTP/ISCSI.
> > >
> > > Hi Jiaxin,
> > >
> > > 1. Cleaning the ConfigData when switching the mode will resolve the
> > > Issue
> > A.
> > > 2. Will also verify the ASSERT issue and update you.
> > >
> > > Could you please help in clarifying the below items, 1. Could you
> > > please let us know why the ASSERT happens?
> > > 2. I've not faced any ASSERT with the changes attached in Bugzilla,
> > > Did you find any issues/drawbacks with that?
> > > -> In HttpBootCheckIpv6Support () definition Instead of getting
> > > -> Ipv6Support
> > > from Private->Nii->Ipv6Supported if we open the protocol there
> > > itself, Then there will not be issues in Destroying Children or
> > > FreePool(Private)
> > right.
> > > Because we're going to check HttpBootCheckIpv6Support() before
> > > opening any instances in HttpBootIp6DxeDriverBindingStart().
> > >
> > > Thanks for your great support.
> > >
> > > Thank You,
> > > Karunakar
> > > ________________________________________
> > > From: Wu, Jiaxin [jiaxin.wu@intel.com]
> > > Sent: 18 October 2017 12:35
> > > To: Karunakar P; edk2-devel@lists.01.org
> > > Cc: Ye, Ting; Fu, Siyuan
> > > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > > HTTP/ISCSI.
> > >
> > > Hi Karunakar,
> > >
> > > Thanks your verification. Base on your comments, I refined the
> > > series patches as below to fix the issues:
> > >
> > > [Patch v3 0/3] NetworkPkg/IScsiDxe: Display InitiatorInfo correctly.
> > > NetworkPkg/IScsiDxe: Fix the incorrect/needless DHCP process.
> > > NetworkPkg/IScsiDxe: Clean the previous ConfigData when
> > > switching the IP mode. /// This one is to fix the issue A.
> > > NetworkPkg/IScsiDxe: Display InitiatorInfo in attempt
> > > page even DHCP enabled.
> > >
> > > [Patch v2 0/2] Add IPv6 support condition check.
> > > NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> > > /// B && C have been fixed in version 2.
> > > NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> > >
> > > Please help to verify them.
> > >
> > > Best Regards,
> > > Jiaxin
> > >
> > >
> > > > -----Original Message-----
> > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> > > > Behalf Of Karunakar P
> > > > Sent: Tuesday, October 17, 2017 6:13 PM
> > > > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > > > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> > > > Subject: Re: [edk2] [Patch 0/2] Add IPv6 support condition check
> > > > for HTTP/ISCSI.
> > > >
> > > > Hi Jiaxin,
> > > >
> > > > I Reviewed the changes for 3 features/Bugs and verified the same,
> > > > Please find my below comments and issues faced
> > > >
> > > > A. Display InitiatorInfo in attempt page even DHCP enabled
> > > > ------------------------------------------------------------------
> > > > --
> > > > -----------------------
> > > ---
> > > > --------------------------------------------
> > > > 1. I applied IScsiConfigVfr.vfr changes and as well IScsiMisc.c
> > > > changes 2. It displays initiator info properly when it's Enabled
> > > > for DHCP 3. But, I found some different behavior in below case
> > > > a. Add an Attempt (Attempt1 -> Initiator Info Enabled for DHCP)
> > > > b. On reboot iSCSI attempt was success and Initiator Details
> > > > shown properly ==> This is as expected
> > > > c. Edit the same Attempt1 details to IP6 and save changes and reset
> > > > d. Now Iscsi connection with IP6 ==> This is as Expected
> > > > e. Now if we again Change the Attempt1 to IP4, It is
> > > > Displaying Subnet Mask ==> I guess we are not clearing It
> > > >
> > > > I guess we need to do ZeroMem for initiator details before.
> > > >
> > > >
> > > > B. [Patch 2/2] NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> > > > ------------------------------------------------------------------
> > > > --
> > > > -----------------------
> > > ---
> > > > -------------------------------------------
> > > > -> This changes looks similar whatever I attached in Bugzilla, and
> > > > -> verified
> > > the
> > > > same with off board card witch doesn't support IP6
> > > > -> It works fine, I didn't find any issues on it.
> > > >
> > > >
> > > > C. [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support condition
> > check.
> > > > ------------------------------------------------------------------
> > > > --
> > > > -----------------------
> > > ---
> > > > ------------------------------------------------------------------
> > > > --
> > > > ---- I found some issues in this changes, please find my below
> > > > comments 1. HttpBootCheckIpv6Support() function definition and
> > > > function call parameter differs , To correct this I've done 1
> > > > insertion(+), 1 deletion(-) like below ...
> > > > +HttpBootCheckIpv6Support (
> > > > + IN EFI_HANDLE ControllerHandle,
> > > > + IN HTTP_BOOT_PRIVATE_DATA *Private,
> > > > + OUT BOOLEAN *Ipv6Support
> > > > + )
> > > > ...
> > > > + // Set IPv6 available flag.
> > > > + //
> > > > + Status = HttpBootCheckIpv6Support (ControllerHandle,
> > > > -This->DriverBindingHandle, &Ipv6Available);
> > > > +Private, &Ipv6Available);
> > > > ...
> > > >
> > > > 2. With the above changes I've verified with Off board card which
> > > > doesn't support IP6, But I'm facing below ASSERT
> > > > (324): CR has Bad Signature
> > > >
> > > > EFI_STATUS
> > > > EFIAPI
> > > > HttpBootIp4DxeDriverBindingStart (
> > > > IN EFI_DRIVER_BINDING_PROTOCOL *This,
> > > > IN EFI_HANDLE ControllerHandle,
> > > > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> > > > )
> > > > {
> > > > ...
> > > > if (!EFI_ERROR (Status)) {
> > > > Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); //
> > ASSERTs
> > > > here
> > > > } else {
> > > > .....
> > > >
> > > > 3. I would like add some points and info about the this ASSERT,
> > > > which I've found The ASSERT is happening because of FreePool
> > > > (Private), mentioned exact line no below
> > > >
> > > > EFI_STATUS
> > > > EFIAPI
> > > > HttpBootIp6DxeDriverBindingStart (
> > > > IN EFI_DRIVER_BINDING_PROTOCOL *This,
> > > > IN EFI_HANDLE ControllerHandle,
> > > > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> > > > )
> > > > {
> > > > ...
> > > > Status = gBS->InstallProtocolInterface (
> > > > &ControllerHandle,
> > > > &gEfiCallerIdGuid,
> > > > EFI_NATIVE_INTERFACE,
> > > > &Private->Id
> > > > );
> > > > if (EFI_ERROR (Status)) {
> > > > goto ON_ERROR;
> > > > }
> > > >
> > > > }
> > > > +
> > > > + //
> > > > + // Set IPv6 available flag.
> > > > + //
> > > > + Status = HttpBootCheckIpv6Support (ControllerHandle,
> > > > -This->DriverBindingHandle, &Ipv6Available);
> > > > +Private, &Ipv6Available);
> > > > + if (EFI_ERROR (Status)) {
> > > > + //
> > > > + // Fail to get the data whether UNDI supports IPv6.
> > > > + // Set default value to TRUE.
> > > > + //
> > > > + Ipv6Available = TRUE;
> > > > + }
> > > > +
> > > > + if (!Ipv6Available) {
> > > > + return EFI_UNSUPPORTED;
> > > > + }
> > > >
> > > > if (Private->Ip6Nic != NULL) {
> > > > //
> > > > ...
> > > >
> > > > ON_ERROR:
> > > >
> > > > HttpBootDestroyIp6Children(This, Private);
> > > > HttpBootConfigFormUnload (Private);
> > > > FreePool (Private); // If I comment
> > this
> > > > line ASSERT is not happening
> > > >
> > > > return Status;
> > > > }
> > > >
> > > > 4. At your end could you please verify this IP6 Condition check
> > > > for HTTP
> > > >
> > > > Please correct if anything is wrong, Thanks for your support
> > > >
> > > >
> > > > Thank You,
> > > > Karunakar
> > > >
> > > > -----Original Message-----
> > > > From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> > > > Sent: Tuesday, October 17, 2017 7:32 AM
> > > > To: Wu, Jiaxin; edk2-devel@lists.01.org
> > > > Cc: Karunakar P; Ye, Ting; Fu, Siyuan
> > > > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check
> > > > for HTTP/ISCSI.
> > > >
> > > > Hello Karunakar,
> > > >
> > > > Base on your original changes attached in Bugzilla 701
> > > > (https://bugzilla.tianocore.org/show_bug.cgi?id=710), I created
> > > > the formal series patches to support the IPv6 condition check for
> HTTP/ISCSI.
> > > >
> > > > Please help to review/verify it.
> > > >
> > > > BTW, To review the ISCSI part, please apply the "[Patch v2 0/2]
> > > > NetworkPkg/IScsiDxe: Display InitiatorInfo correctly" first to
> > > > avoid any
> > > patch
> > > > conflict.
> > > >
> > > > Thanks,
> > > > Jiaxin
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> > > > > Behalf Of Jiaxin Wu
> > > > > Sent: Tuesday, October 17, 2017 9:58 AM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Karunakar P <karunakarp@amiindia.co.in>; Ye, Ting
> > > > > <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu,
> > > > > Jiaxin <jiaxin.wu@intel.com>
> > > > > Subject: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > > > HTTP/ISCSI.
> > > > >
> > > > > Base on the request of
> > > > > https://bugzilla.tianocore.org/show_bug.cgi?id=710,
> > > > > we provide this patch to IPv6 condition check by leveraging AIP
> Protocol.
> > > > >
> > > > > Cc: Karunakar P <karunakarp@amiindia.co.in>
> > > > > Cc: Ye Ting <ting.ye@intel.com>
> > > > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > > > Signed-off-by: Karunakar P <karunakarp@amiindia.co.in>
> > > > > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > > > >
> > > > > Jiaxin Wu (2):
> > > > > NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> > > > > NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> > > > >
> > > > > NetworkPkg/HttpBootDxe/HttpBootDxe.c | 115
> > > > > +++++++++++++++++++++++++++-
> > > > > NetworkPkg/HttpBootDxe/HttpBootDxe.h | 2 +
> > > > > NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 4 +-
> > > > > NetworkPkg/IScsiDxe/IScsiConfig.c | 18 +++++
> > > > > NetworkPkg/IScsiDxe/IScsiDriver.c | 2 +-
> > > > > NetworkPkg/IScsiDxe/IScsiDriver.h | 1 +
> > > > > NetworkPkg/IScsiDxe/IScsiDxe.inf | 2 +
> > > > > NetworkPkg/IScsiDxe/IScsiImpl.h | 1 +
> > > > > NetworkPkg/IScsiDxe/IScsiMisc.c | 135
> > > > > ++++++++++++++++++++++++++++++++-
> > > > > NetworkPkg/IScsiDxe/IScsiMisc.h | 4 +-
> > > > > 10 files changed, 278 insertions(+), 6 deletions(-)
> > > > >
> > > > > --
> > > > > 1.9.5.msysgit.1
> > > > >
> > > > > _______________________________________________
> > > > > edk2-devel mailing list
> > > > > edk2-devel@lists.01.org
> > > > > https://lists.01.org/mailman/listinfo/edk2-devel
> > > > _______________________________________________
> > > > edk2-devel mailing list
> > > > edk2-devel@lists.01.org
> > > > https://lists.01.org/mailman/listinfo/edk2-devel
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
2017-10-24 7:20 ` Wu, Jiaxin
@ 2017-10-24 7:29 ` Karunakar P
0 siblings, 0 replies; 15+ messages in thread
From: Karunakar P @ 2017-10-24 7:29 UTC (permalink / raw)
To: 'Wu, Jiaxin', edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan
Reviewed-by: Karunakar p <karunakarp@amiindia.co.in>
Thanks,
Karunakar
-----Original Message-----
From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
Sent: Tuesday, October 24, 2017 12:50 PM
To: Karunakar P; edk2-devel@lists.01.org
Cc: Ye, Ting; Fu, Siyuan
Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI.
Hi Karunakar,
If you have no more comments, can you reply the below series patches with Reviewed-by/Tested-By tags, then I can go on the commit process?
[Patch v3 0/3] NetworkPkg/IScsiDxe: Display InitiatorInfo correctly.
NetworkPkg/IScsiDxe: Fix the incorrect/needless DHCP process.
NetworkPkg/IScsiDxe: Clean the previous ConfigData when switching the IP mode. /// This one is to fix the issue A.
NetworkPkg/IScsiDxe: Display InitiatorInfo in attempt page even DHCP enabled.
[Patch v2 0/2] Add IPv6 support condition check.
NetworkPkg/HttpBootDxe: Add IPv6 support condition check. /// B && C have been fixed in version 2.
NetworkPkg/IScsiDxe: Add IPv6 support condition check.
Thanks,
Jiaxin
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Karunakar P
> Sent: Tuesday, October 24, 2017 3:13 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: Re: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hi Jiaxin,
>
> Yes, I believe this issue(#) is not the regression for previous issues
> we have fixed.
>
> I created a new bug For #3 and below are the details
> https://bugzilla.tianocore.org/show_bug.cgi?id=743
>
> Could you please let me know the scheduled check in date for
> previously reported issue/Features
>
> Thank You,
> Karunakar
>
> -----Original Message-----
> From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> Sent: Tuesday, October 24, 2017 8:45 AM
> To: Karunakar P; edk2-devel@lists.01.org
> Cc: Ye, Ting; Fu, Siyuan
> Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> HTTP/ISCSI.
>
> Hi Karunakar,
>
> For #3, it's the non-regression issue that will cause the target IP is
> invalid if it's the same with previous one. And I'd like to fix it by
> another patch since it's not related to the InitiatorInfo display.
>
> Can you please report another Bugzilla for the ISCSI target address issue?
>
> Thanks,
> Jiaxin
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> > Of Karunakar P
> > Sent: Tuesday, October 24, 2017 12:41 AM
> > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> > Subject: Re: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> >
> > Hi Jiaxin,
> >
> > 1. I've verified, IPv6 support condition check for HTTP/ISCSI, It
> > works fine and the ASSERT issue also resolved.
> > 2. Regards Display Initiator IP, It resolves the issues the issues
> > which I've mentioned previously.
> > 3. I found some other issue in ISCSI, Below are the details A. Add
> > Attempt1 with Target Info via Static and provide Target Name &
> > Target IP, Save changes.
> > B. If we are trying to add another attempt , It is taking Target IP
> > as default IP which is same Target IP given in Attempt1.
> >
> > Could you please check at your end provide your comments on 3rd one.
> >
> > Thank You,
> > Karunakar
> >
> > -----Original Message-----
> > From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> > Sent: Wednesday, October 18, 2017 2:30 PM
> > To: Karunakar P; edk2-devel@lists.01.org
> > Cc: Ye, Ting; Fu, Siyuan
> > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for
> > HTTP/ISCSI.
> >
> > Hi Karunakar,
> >
> > Actually, in my part, I didn't meet the ASSERT no matter the "Ipv6Available"
> > returned from HttpBootCheckIpv6Support is true or false. According
> > your ASSERT case that happened in Ip4DxeDriverBindingStart, which is
> > caused by the FreePool of Private in Ip6DxeDriverBindingStart, so I
> > guess the Ip6DxeDriverBindingStart may be involved ahead of
> > Ip4DxeDriverBindingStart, then something wrong happened in
> > Ip6DxeDriverBindingStart and goto the ON_ERROR (Private is freed
> > here!). so, you can add the breakpoint within
> > Ip6DxeDriverBindingStart/Ip4DxeDriverBindingStart to check it.
> >
> > Per my analysis above, it does the issue that may trigger the
> > potential
> ASSERT.
> > So, I refined the patch as version2. The principle of patch v2 is
> > that
> > IPv6 and
> > IPv4 should not affect each other even any failure happen, but the
> > original code doesn't follow that:).
> >
> >
> > Thanks,
> > Jiaxin
> >
> >
> > > -----Original Message-----
> > > From: Karunakar P [mailto:karunakarp@amiindia.co.in]
> > > Sent: Wednesday, October 18, 2017 4:06 PM
> > > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> > > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check
> > > for HTTP/ISCSI.
> > >
> > > Hi Jiaxin,
> > >
> > > 1. Cleaning the ConfigData when switching the mode will resolve
> > > the Issue
> > A.
> > > 2. Will also verify the ASSERT issue and update you.
> > >
> > > Could you please help in clarifying the below items, 1. Could you
> > > please let us know why the ASSERT happens?
> > > 2. I've not faced any ASSERT with the changes attached in
> > > Bugzilla, Did you find any issues/drawbacks with that?
> > > -> In HttpBootCheckIpv6Support () definition Instead of getting
> > > -> Ipv6Support
> > > from Private->Nii->Ipv6Supported if we open the protocol there
> > > itself, Then there will not be issues in Destroying Children or
> > > FreePool(Private)
> > right.
> > > Because we're going to check HttpBootCheckIpv6Support() before
> > > opening any instances in HttpBootIp6DxeDriverBindingStart().
> > >
> > > Thanks for your great support.
> > >
> > > Thank You,
> > > Karunakar
> > > ________________________________________
> > > From: Wu, Jiaxin [jiaxin.wu@intel.com]
> > > Sent: 18 October 2017 12:35
> > > To: Karunakar P; edk2-devel@lists.01.org
> > > Cc: Ye, Ting; Fu, Siyuan
> > > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check
> > > for HTTP/ISCSI.
> > >
> > > Hi Karunakar,
> > >
> > > Thanks your verification. Base on your comments, I refined the
> > > series patches as below to fix the issues:
> > >
> > > [Patch v3 0/3] NetworkPkg/IScsiDxe: Display InitiatorInfo correctly.
> > > NetworkPkg/IScsiDxe: Fix the incorrect/needless DHCP process.
> > > NetworkPkg/IScsiDxe: Clean the previous ConfigData
> > > when switching the IP mode. /// This one is to fix the issue A.
> > > NetworkPkg/IScsiDxe: Display InitiatorInfo in attempt
> > > page even DHCP enabled.
> > >
> > > [Patch v2 0/2] Add IPv6 support condition check.
> > > NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> > > /// B && C have been fixed in version 2.
> > > NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> > >
> > > Please help to verify them.
> > >
> > > Best Regards,
> > > Jiaxin
> > >
> > >
> > > > -----Original Message-----
> > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> > > > Behalf Of Karunakar P
> > > > Sent: Tuesday, October 17, 2017 6:13 PM
> > > > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > > > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan
> > > > <siyuan.fu@intel.com>
> > > > Subject: Re: [edk2] [Patch 0/2] Add IPv6 support condition check
> > > > for HTTP/ISCSI.
> > > >
> > > > Hi Jiaxin,
> > > >
> > > > I Reviewed the changes for 3 features/Bugs and verified the
> > > > same, Please find my below comments and issues faced
> > > >
> > > > A. Display InitiatorInfo in attempt page even DHCP enabled
> > > > ----------------------------------------------------------------
> > > > --
> > > > --
> > > > -----------------------
> > > ---
> > > > --------------------------------------------
> > > > 1. I applied IScsiConfigVfr.vfr changes and as well IScsiMisc.c
> > > > changes 2. It displays initiator info properly when it's Enabled
> > > > for DHCP 3. But, I found some different behavior in below case
> > > > a. Add an Attempt (Attempt1 -> Initiator Info Enabled for DHCP)
> > > > b. On reboot iSCSI attempt was success and Initiator
> > > > Details shown properly ==> This is as expected
> > > > c. Edit the same Attempt1 details to IP6 and save changes and reset
> > > > d. Now Iscsi connection with IP6 ==> This is as Expected
> > > > e. Now if we again Change the Attempt1 to IP4, It is
> > > > Displaying Subnet Mask ==> I guess we are not clearing It
> > > >
> > > > I guess we need to do ZeroMem for initiator details before.
> > > >
> > > >
> > > > B. [Patch 2/2] NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> > > > ----------------------------------------------------------------
> > > > --
> > > > --
> > > > -----------------------
> > > ---
> > > > -------------------------------------------
> > > > -> This changes looks similar whatever I attached in Bugzilla,
> > > > -> and verified
> > > the
> > > > same with off board card witch doesn't support IP6
> > > > -> It works fine, I didn't find any issues on it.
> > > >
> > > >
> > > > C. [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support
> > > > condition
> > check.
> > > > ----------------------------------------------------------------
> > > > --
> > > > --
> > > > -----------------------
> > > ---
> > > > ----------------------------------------------------------------
> > > > --
> > > > --
> > > > ---- I found some issues in this changes, please find my below
> > > > comments 1. HttpBootCheckIpv6Support() function definition and
> > > > function call parameter differs , To correct this I've done 1
> > > > insertion(+), 1 deletion(-) like below ...
> > > > +HttpBootCheckIpv6Support (
> > > > + IN EFI_HANDLE ControllerHandle,
> > > > + IN HTTP_BOOT_PRIVATE_DATA *Private,
> > > > + OUT BOOLEAN *Ipv6Support
> > > > + )
> > > > ...
> > > > + // Set IPv6 available flag.
> > > > + //
> > > > + Status = HttpBootCheckIpv6Support (ControllerHandle,
> > > > -This->DriverBindingHandle, &Ipv6Available);
> > > > +Private, &Ipv6Available);
> > > > ...
> > > >
> > > > 2. With the above changes I've verified with Off board card
> > > > which doesn't support IP6, But I'm facing below ASSERT
> > > > (324): CR has Bad Signature
> > > >
> > > > EFI_STATUS
> > > > EFIAPI
> > > > HttpBootIp4DxeDriverBindingStart (
> > > > IN EFI_DRIVER_BINDING_PROTOCOL *This,
> > > > IN EFI_HANDLE ControllerHandle,
> > > > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> > > > )
> > > > {
> > > > ...
> > > > if (!EFI_ERROR (Status)) {
> > > > Private = HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); //
> > ASSERTs
> > > > here
> > > > } else {
> > > > .....
> > > >
> > > > 3. I would like add some points and info about the this ASSERT,
> > > > which I've found The ASSERT is happening because of FreePool
> > > > (Private), mentioned exact line no below
> > > >
> > > > EFI_STATUS
> > > > EFIAPI
> > > > HttpBootIp6DxeDriverBindingStart (
> > > > IN EFI_DRIVER_BINDING_PROTOCOL *This,
> > > > IN EFI_HANDLE ControllerHandle,
> > > > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> > > > )
> > > > {
> > > > ...
> > > > Status = gBS->InstallProtocolInterface (
> > > > &ControllerHandle,
> > > > &gEfiCallerIdGuid,
> > > > EFI_NATIVE_INTERFACE,
> > > > &Private->Id
> > > > );
> > > > if (EFI_ERROR (Status)) {
> > > > goto ON_ERROR;
> > > > }
> > > >
> > > > }
> > > > +
> > > > + //
> > > > + // Set IPv6 available flag.
> > > > + //
> > > > + Status = HttpBootCheckIpv6Support (ControllerHandle,
> > > > -This->DriverBindingHandle, &Ipv6Available);
> > > > +Private, &Ipv6Available);
> > > > + if (EFI_ERROR (Status)) {
> > > > + //
> > > > + // Fail to get the data whether UNDI supports IPv6.
> > > > + // Set default value to TRUE.
> > > > + //
> > > > + Ipv6Available = TRUE;
> > > > + }
> > > > +
> > > > + if (!Ipv6Available) {
> > > > + return EFI_UNSUPPORTED;
> > > > + }
> > > >
> > > > if (Private->Ip6Nic != NULL) {
> > > > //
> > > > ...
> > > >
> > > > ON_ERROR:
> > > >
> > > > HttpBootDestroyIp6Children(This, Private);
> > > > HttpBootConfigFormUnload (Private);
> > > > FreePool (Private); // If I comment
> > this
> > > > line ASSERT is not happening
> > > >
> > > > return Status;
> > > > }
> > > >
> > > > 4. At your end could you please verify this IP6 Condition check
> > > > for HTTP
> > > >
> > > > Please correct if anything is wrong, Thanks for your support
> > > >
> > > >
> > > > Thank You,
> > > > Karunakar
> > > >
> > > > -----Original Message-----
> > > > From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> > > > Sent: Tuesday, October 17, 2017 7:32 AM
> > > > To: Wu, Jiaxin; edk2-devel@lists.01.org
> > > > Cc: Karunakar P; Ye, Ting; Fu, Siyuan
> > > > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check
> > > > for HTTP/ISCSI.
> > > >
> > > > Hello Karunakar,
> > > >
> > > > Base on your original changes attached in Bugzilla 701
> > > > (https://bugzilla.tianocore.org/show_bug.cgi?id=710), I created
> > > > the formal series patches to support the IPv6 condition check
> > > > for
> HTTP/ISCSI.
> > > >
> > > > Please help to review/verify it.
> > > >
> > > > BTW, To review the ISCSI part, please apply the "[Patch v2 0/2]
> > > > NetworkPkg/IScsiDxe: Display InitiatorInfo correctly" first to
> > > > avoid any
> > > patch
> > > > conflict.
> > > >
> > > > Thanks,
> > > > Jiaxin
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> > > > > Behalf Of Jiaxin Wu
> > > > > Sent: Tuesday, October 17, 2017 9:58 AM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Karunakar P <karunakarp@amiindia.co.in>; Ye, Ting
> > > > > <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu,
> > > > > Jiaxin <jiaxin.wu@intel.com>
> > > > > Subject: [edk2] [Patch 0/2] Add IPv6 support condition check
> > > > > for
> > > > HTTP/ISCSI.
> > > > >
> > > > > Base on the request of
> > > > > https://bugzilla.tianocore.org/show_bug.cgi?id=710,
> > > > > we provide this patch to IPv6 condition check by leveraging
> > > > > AIP
> Protocol.
> > > > >
> > > > > Cc: Karunakar P <karunakarp@amiindia.co.in>
> > > > > Cc: Ye Ting <ting.ye@intel.com>
> > > > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > > > Signed-off-by: Karunakar P <karunakarp@amiindia.co.in>
> > > > > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > > > >
> > > > > Jiaxin Wu (2):
> > > > > NetworkPkg/HttpBootDxe: Add IPv6 support condition check.
> > > > > NetworkPkg/IScsiDxe: Add IPv6 support condition check.
> > > > >
> > > > > NetworkPkg/HttpBootDxe/HttpBootDxe.c | 115
> > > > > +++++++++++++++++++++++++++-
> > > > > NetworkPkg/HttpBootDxe/HttpBootDxe.h | 2 +
> > > > > NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 4 +-
> > > > > NetworkPkg/IScsiDxe/IScsiConfig.c | 18 +++++
> > > > > NetworkPkg/IScsiDxe/IScsiDriver.c | 2 +-
> > > > > NetworkPkg/IScsiDxe/IScsiDriver.h | 1 +
> > > > > NetworkPkg/IScsiDxe/IScsiDxe.inf | 2 +
> > > > > NetworkPkg/IScsiDxe/IScsiImpl.h | 1 +
> > > > > NetworkPkg/IScsiDxe/IScsiMisc.c | 135
> > > > > ++++++++++++++++++++++++++++++++-
> > > > > NetworkPkg/IScsiDxe/IScsiMisc.h | 4 +-
> > > > > 10 files changed, 278 insertions(+), 6 deletions(-)
> > > > >
> > > > > --
> > > > > 1.9.5.msysgit.1
> > > > >
> > > > > _______________________________________________
> > > > > edk2-devel mailing list
> > > > > edk2-devel@lists.01.org
> > > > > https://lists.01.org/mailman/listinfo/edk2-devel
> > > > _______________________________________________
> > > > edk2-devel mailing list
> > > > edk2-devel@lists.01.org
> > > > https://lists.01.org/mailman/listinfo/edk2-devel
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-10-24 7:25 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-17 1:58 [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI Jiaxin Wu
2017-10-17 1:58 ` [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support condition check Jiaxin Wu
2017-10-17 1:58 ` [Patch 2/2] NetworkPkg/IScsiDxe: " Jiaxin Wu
2017-10-17 2:01 ` [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI Wu, Jiaxin
2017-10-17 10:12 ` Karunakar P
2017-10-17 11:35 ` Karunakar P
2017-10-18 7:05 ` Wu, Jiaxin
2017-10-18 8:05 ` Karunakar P
2017-10-18 9:00 ` Wu, Jiaxin
2017-10-18 11:43 ` Karunakar P
2017-10-23 16:41 ` Karunakar P
2017-10-24 3:14 ` Wu, Jiaxin
2017-10-24 7:13 ` Karunakar P
2017-10-24 7:20 ` Wu, Jiaxin
2017-10-24 7:29 ` Karunakar P
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox