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...@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-boun...@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 <karunak...@amiindia.co.in>; Ye, Ting > <ting...@intel.com>; Fu, Siyuan <siyuan...@intel.com>; Wu, Jiaxin > <jiaxin...@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 <karunak...@amiindia.co.in> > Cc: Ye Ting <ting...@intel.com> > Cc: Fu Siyuan <siyuan...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Karunakar P <karunak...@amiindia.co.in> > Signed-off-by: Wu Jiaxin <jiaxin...@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