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:[email protected]] On Behalf Of
> Karunakar P
> Sent: Tuesday, October 17, 2017 6:13 PM
> To: Wu, Jiaxin <[email protected]>; [email protected]
> Cc: Ye, Ting <[email protected]>; Fu, Siyuan <[email protected]>
> 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:[email protected]]
> Sent: Tuesday, October 17, 2017 7:32 AM
> To: Wu, Jiaxin; [email protected]
> 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:[email protected]] On Behalf Of
> > Jiaxin Wu
> > Sent: Tuesday, October 17, 2017 9:58 AM
> > To: [email protected]
> > Cc: Karunakar P <[email protected]>; Ye, Ting
> > <[email protected]>; Fu, Siyuan <[email protected]>; Wu, Jiaxin
> > <[email protected]>
> > 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 <[email protected]>
> > Cc: Ye Ting <[email protected]>
> > Cc: Fu Siyuan <[email protected]>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Karunakar P <[email protected]>
> > Signed-off-by: Wu Jiaxin <[email protected]>
> >
> > 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
> > [email protected]
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> [email protected]
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
