From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=203.199.198.232; helo=imsva.in.megatrends.com; envelope-from=karunakarp@amiindia.co.in; receiver=edk2-devel@lists.01.org Received: from IMSVA.IN.MEGATRENDS.COM (Webmail.amiindia.co.in [203.199.198.232]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 247192095B06F for ; Wed, 18 Oct 2017 04:40:12 -0700 (PDT) Received: from IMSVA.IN.MEGATRENDS.COM (IMSVA.IN.MEGATRENDS.COM [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 75AED82055; Wed, 18 Oct 2017 17:16:40 +0530 (IST) Received: from IMSVA.IN.MEGATRENDS.COM (IMSVA.IN.MEGATRENDS.COM [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 598D78204A; Wed, 18 Oct 2017 17:16:40 +0530 (IST) Received: from webmail.amiindia.co.in (venus2.in.megatrends.com [10.0.0.7]) by IMSVA.IN.MEGATRENDS.COM (Postfix) with ESMTPS; Wed, 18 Oct 2017 17:16:40 +0530 (IST) Received: from VENUS1.in.megatrends.com ([fe80::951:7975:6ecf:eae5]) by Venus2.in.megatrends.com ([fe80::2002:4a07:4f17:c09b%14]) with mapi id 14.03.0248.002; Wed, 18 Oct 2017 17:13:46 +0530 From: Karunakar P To: "Wu, Jiaxin" , "edk2-devel@lists.01.org" CC: "Ye, Ting" , "Fu, Siyuan" Thread-Topic: [edk2] [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI. Thread-Index: AQHTRutzgYZnUvS1QUq9ZfByiwGsxKLm7cOAgADPqOCAAReLAIAAZq7p//+5RgCAAIiiog== Date: Wed, 18 Oct 2017 11:43:46 +0000 Message-ID: References: <1508205501-13832-1-git-send-email-jiaxin.wu@intel.com> <895558F6EA4E3B41AC93A00D163B72741633E5F1@SHSMSX103.ccr.corp.intel.com> , <895558F6EA4E3B41AC93A00D163B72741633ECFC@SHSMSX103.ccr.corp.intel.com> , <895558F6EA4E3B41AC93A00D163B72741633EE3A@SHSMSX103.ccr.corp.intel.com> In-Reply-To: <895558F6EA4E3B41AC93A00D163B72741633EE3A@SHSMSX103.ccr.corp.intel.com> Accept-Language: en-GB, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [113.193.149.58] MIME-Version: 1.0 X-TM-AS-GCONF: 00 X-TM-AS-Product-Ver: IMSVA-9.1.0.1600-8.1.0.1062-23402.006 X-TM-AS-Result: No--36.556-5.0-31-10 X-imss-scan-details: No--36.556-5.0-31-10 X-TMASE-Version: IMSVA-9.1.0.1600-8.1.1062-23402.006 X-TMASE-Result: 10--36.555600-10.000000 X-TMASE-MatchedRID: 1ZHks2aQIkinykMun0J1wpmug812qIbzloU71ctjXZSdCqKtxM6bh3aS g7fiy0fIJmLqKu2yqdkhcnM8qCdEagMWCgdCzp+aw69AIwXJn0Yg0L4Xy2OHlQbYcy9YQl6eIA6 D0Tft3i9SH69NMIroytZcWV9eukqmcc8G9KynN+MwiJTf3kjwfQXXmzqmsIi74uxAgOavdLm18F rWtLzsrQAo+theewrr8gTKtovq3jS/KVPPO2NgiPHkpkyUphL9ktxvVMaWMbCe9toQ6h6LE4GNl v3Gd/fJKAJ3dGG6zd+qkcRfq/BgBa/V8/cgjzjSVP9jNEQH04oW+kaqYhU/87KeTtOdjMy6rRGi aFY8lpAP9u1aMLZYQMr9lWA66QxJXRTdE9b6Is+BlNt4VSGvIbWTdtEh1dU07mlrq0Hz6U0f11g S5FO37ujomakYLm6YkHn0x3GfDuIe+1Z02u2sanTnOygHVQpOeFyzQYzPQ+Tn0JXek/DSxQRv1H Ajr33tpp4BnujcFllEeQ/cABZIKpSL8e/MGApZt1AhvyEKdj5CX8V1FiRRktVTZaI6TuNo273xs KOXSkqOFyV5ZkmZ+YVBl/yyOM8RECriNTRBmUEVglQa/gMvfHrMPEZwURsK8cWgFw6wp7NSLrfV wP0q0HwcG8wLcEkYRgTrLnQo+cwh1MYHBgDafH4f9De+CyQCJnRXy0H5W63M7zpEspqG/zpbfdT AcNXm4vM1YF6AJbZYoPZAqTBHwi/tubwHkCSX+gD2vYtOFhgqtq5d3cxkNQP90fJP9eHt X-TMASE-SNAP-Result: 1.821001.0001-0-1-12:0,22:0,33:0,34:0,39:0-0 Subject: Re: [Patch 0/2] Add IPv6 support condition check for HTTP/ISCSI. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Oct 2017 11:40:13 -0000 Content-Language: en-GB Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Jixian,=0A= =0A= Thanks for your Detailed explanation.=0A= Will review/Verify the changes and update you on Friday.=0A= =0A= Thank You,=0A= Karunakar=0A= ________________________________________=0A= From: Wu, Jiaxin [jiaxin.wu@intel.com]=0A= Sent: 18 October 2017 14:30=0A= To: Karunakar P; edk2-devel@lists.01.org=0A= Cc: Ye, Ting; Fu, Siyuan=0A= Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for HTTP/I= SCSI.=0A= =0A= Hi Karunakar,=0A= =0A= Actually, in my part, I didn't meet the ASSERT no matter the "Ipv6Available= " returned from HttpBootCheckIpv6Support is true or false. According your A= SSERT case that happened in Ip4DxeDriverBindingStart, which is caused by th= e FreePool of Private in Ip6DxeDriverBindingStart, so I guess the Ip6DxeDri= verBindingStart may be involved ahead of Ip4DxeDriverBindingStart, then som= ething wrong happened in Ip6DxeDriverBindingStart and goto the ON_ERROR (Pr= ivate is freed here!). so, you can add the breakpoint within Ip6DxeDriverBi= ndingStart/Ip4DxeDriverBindingStart to check it.=0A= =0A= Per my analysis above, it does the issue that may trigger the potential ASS= ERT. 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 th= e original code doesn't follow that:).=0A= =0A= =0A= Thanks,=0A= Jiaxin=0A= =0A= =0A= > -----Original Message-----=0A= > From: Karunakar P [mailto:karunakarp@amiindia.co.in]=0A= > Sent: Wednesday, October 18, 2017 4:06 PM=0A= > To: Wu, Jiaxin ; edk2-devel@lists.01.org=0A= > Cc: Ye, Ting ; Fu, Siyuan =0A= > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for=0A= > HTTP/ISCSI.=0A= >=0A= > Hi Jiaxin,=0A= >=0A= > 1. Cleaning the ConfigData when switching the mode will resolve the Issue= A.=0A= > 2. Will also verify the ASSERT issue and update you.=0A= >=0A= > Could you please help in clarifying the below items,=0A= > 1. Could you please let us know why the ASSERT happens?=0A= > 2. I've not faced any ASSERT with the changes attached in Bugzilla, Did y= ou=0A= > find any issues/drawbacks with that?=0A= > -> In HttpBootCheckIpv6Support () definition Instead of getting Ipv6Suppo= rt=0A= > from Private->Nii->Ipv6Supported if we open the protocol there itself, Th= en=0A= > there will not be issues in Destroying Children or FreePool(Private) righ= t.=0A= > Because we're going to check HttpBootCheckIpv6Support() before opening= =0A= > any instances in HttpBootIp6DxeDriverBindingStart().=0A= >=0A= > Thanks for your great support.=0A= >=0A= > Thank You,=0A= > Karunakar=0A= > ________________________________________=0A= > From: Wu, Jiaxin [jiaxin.wu@intel.com]=0A= > Sent: 18 October 2017 12:35=0A= > To: Karunakar P; edk2-devel@lists.01.org=0A= > Cc: Ye, Ting; Fu, Siyuan=0A= > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for=0A= > HTTP/ISCSI.=0A= >=0A= > Hi Karunakar,=0A= >=0A= > Thanks your verification. Base on your comments, I refined the series=0A= > patches as below to fix the issues:=0A= >=0A= > [Patch v3 0/3] NetworkPkg/IScsiDxe: Display InitiatorInfo correctly.=0A= > NetworkPkg/IScsiDxe: Fix the incorrect/needless DHCP process.= =0A= > NetworkPkg/IScsiDxe: Clean the previous ConfigData when switc= hing=0A= > the IP mode. /// This one is to fix the issue A.=0A= > NetworkPkg/IScsiDxe: Display InitiatorInfo in attempt page ev= en DHCP=0A= > enabled.=0A= >=0A= > [Patch v2 0/2] Add IPv6 support condition check.=0A= > NetworkPkg/HttpBootDxe: Add IPv6 support condition check. //= / B=0A= > && C have been fixed in version 2.=0A= > NetworkPkg/IScsiDxe: Add IPv6 support condition check.=0A= >=0A= > Please help to verify them.=0A= >=0A= > Best Regards,=0A= > Jiaxin=0A= >=0A= >=0A= > > -----Original Message-----=0A= > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of= =0A= > > Karunakar P=0A= > > Sent: Tuesday, October 17, 2017 6:13 PM=0A= > > To: Wu, Jiaxin ; edk2-devel@lists.01.org=0A= > > Cc: Ye, Ting ; Fu, Siyuan =0A= > > Subject: Re: [edk2] [Patch 0/2] Add IPv6 support condition check for=0A= > > HTTP/ISCSI.=0A= > >=0A= > > Hi Jiaxin,=0A= > >=0A= > > I Reviewed the changes for 3 features/Bugs and verified the same, Pleas= e=0A= > > find my below comments and issues faced=0A= > >=0A= > > A. Display InitiatorInfo in attempt page even DHCP enabled=0A= > > -----------------------------------------------------------------------= --------------------=0A= > ---=0A= > > --------------------------------------------=0A= > > 1. I applied IScsiConfigVfr.vfr changes and as well IScsiMisc.c change= s=0A= > > 2. It displays initiator info properly when it's Enabled for DHCP=0A= > > 3. But, I found some different behavior in below case=0A= > > a. Add an Attempt (Attempt1 -> Initiator Info Enabled for DHCP)= =0A= > > b. On reboot iSCSI attempt was success and Initiator Details shown= =0A= > > properly =3D=3D> This is as expected=0A= > > c. Edit the same Attempt1 details to IP6 and save changes and reset= =0A= > > d. Now Iscsi connection with IP6 =3D=3D> This is as Expected=0A= > > e. Now if we again Change the Attempt1 to IP4, It is Displaying Sub= net=0A= > > Mask =3D=3D> I guess we are not clearing It=0A= > >=0A= > > I guess we need to do ZeroMem for initiator details before.=0A= > >=0A= > >=0A= > > B. [Patch 2/2] NetworkPkg/IScsiDxe: Add IPv6 support condition check.= =0A= > > -----------------------------------------------------------------------= --------------------=0A= > ---=0A= > > -------------------------------------------=0A= > > -> This changes looks similar whatever I attached in Bugzilla, and veri= fied=0A= > the=0A= > > same with off board card witch doesn't support IP6=0A= > > -> It works fine, I didn't find any issues on it.=0A= > >=0A= > >=0A= > > C. [Patch 1/2] NetworkPkg/HttpBootDxe: Add IPv6 support condition check= .=0A= > > -----------------------------------------------------------------------= --------------------=0A= > ---=0A= > > -----------------------------------------------------------------------= -=0A= > > I found some issues in this changes, please find my below comments=0A= > > 1. HttpBootCheckIpv6Support() function definition and function call=0A= > > parameter differs , To correct this I've done 1 insertion(+), 1 deletio= n(-) like=0A= > > below=0A= > > ...=0A= > > +HttpBootCheckIpv6Support (=0A= > > + IN EFI_HANDLE ControllerHandle,=0A= > > + IN HTTP_BOOT_PRIVATE_DATA *Private,=0A= > > + OUT BOOLEAN *Ipv6Support=0A= > > + )=0A= > > ...=0A= > > + // Set IPv6 available flag.=0A= > > + //=0A= > > + Status =3D HttpBootCheckIpv6Support (ControllerHandle,=0A= > > -This->DriverBindingHandle, &Ipv6Available);=0A= > > +Private, &Ipv6Available);=0A= > > ...=0A= > >=0A= > > 2. With the above changes I've verified with Off board card which doesn= 't=0A= > > support IP6, But I'm facing below ASSERT=0A= > > (324): CR has Bad Signature=0A= > >=0A= > > EFI_STATUS=0A= > > EFIAPI=0A= > > HttpBootIp4DxeDriverBindingStart (=0A= > > IN EFI_DRIVER_BINDING_PROTOCOL *This,=0A= > > IN EFI_HANDLE ControllerHandle,=0A= > > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL=0A= > > )=0A= > > {=0A= > > ...=0A= > > if (!EFI_ERROR (Status)) {=0A= > > Private =3D HTTP_BOOT_PRIVATE_DATA_FROM_ID(Id); // AS= SERTs=0A= > > here=0A= > > } else {=0A= > > .....=0A= > >=0A= > > 3. I would like add some points and info about the this ASSERT, which I= 've=0A= > > found=0A= > > The ASSERT is happening because of FreePool (Private), mentioned exact= =0A= > > line no below=0A= > >=0A= > > EFI_STATUS=0A= > > EFIAPI=0A= > > HttpBootIp6DxeDriverBindingStart (=0A= > > IN EFI_DRIVER_BINDING_PROTOCOL *This,=0A= > > IN EFI_HANDLE ControllerHandle,=0A= > > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL=0A= > > )=0A= > > {=0A= > > ...=0A= > > Status =3D gBS->InstallProtocolInterface (=0A= > > &ControllerHandle,=0A= > > &gEfiCallerIdGuid,=0A= > > EFI_NATIVE_INTERFACE,=0A= > > &Private->Id=0A= > > );=0A= > > if (EFI_ERROR (Status)) {=0A= > > goto ON_ERROR;=0A= > > }=0A= > >=0A= > > }=0A= > > +=0A= > > + //=0A= > > + // Set IPv6 available flag.=0A= > > + //=0A= > > + Status =3D HttpBootCheckIpv6Support (ControllerHandle,=0A= > > -This->DriverBindingHandle, &Ipv6Available);=0A= > > +Private, &Ipv6Available);=0A= > > + if (EFI_ERROR (Status)) {=0A= > > + //=0A= > > + // Fail to get the data whether UNDI supports IPv6.=0A= > > + // Set default value to TRUE.=0A= > > + //=0A= > > + Ipv6Available =3D TRUE;=0A= > > + }=0A= > > +=0A= > > + if (!Ipv6Available) {=0A= > > + return EFI_UNSUPPORTED;=0A= > > + }=0A= > >=0A= > > if (Private->Ip6Nic !=3D NULL) {=0A= > > //=0A= > > ...=0A= > >=0A= > > ON_ERROR:=0A= > >=0A= > > HttpBootDestroyIp6Children(This, Private);=0A= > > HttpBootConfigFormUnload (Private);=0A= > > FreePool (Private); = // If I comment this=0A= > > line ASSERT is not happening=0A= > >=0A= > > return Status;=0A= > > }=0A= > >=0A= > > 4. At your end could you please verify this IP6 Condition check for HTT= P=0A= > >=0A= > > Please correct if anything is wrong, Thanks for your support=0A= > >=0A= > >=0A= > > Thank You,=0A= > > Karunakar=0A= > >=0A= > > -----Original Message-----=0A= > > From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]=0A= > > Sent: Tuesday, October 17, 2017 7:32 AM=0A= > > To: Wu, Jiaxin; edk2-devel@lists.01.org=0A= > > Cc: Karunakar P; Ye, Ting; Fu, Siyuan=0A= > > Subject: RE: [edk2] [Patch 0/2] Add IPv6 support condition check for=0A= > > HTTP/ISCSI.=0A= > >=0A= > > Hello Karunakar,=0A= > >=0A= > > Base on your original changes attached in Bugzilla 701=0A= > > (https://bugzilla.tianocore.org/show_bug.cgi?id=3D710), I created the f= ormal=0A= > > series patches to support the IPv6 condition check for HTTP/ISCSI.=0A= > >=0A= > > Please help to review/verify it.=0A= > >=0A= > > BTW, To review the ISCSI part, please apply the "[Patch v2 0/2]=0A= > > NetworkPkg/IScsiDxe: Display InitiatorInfo correctly" first to avoid an= y=0A= > patch=0A= > > conflict.=0A= > >=0A= > > Thanks,=0A= > > Jiaxin=0A= > >=0A= > >=0A= > >=0A= > > > -----Original Message-----=0A= > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf O= f=0A= > > > Jiaxin Wu=0A= > > > Sent: Tuesday, October 17, 2017 9:58 AM=0A= > > > To: edk2-devel@lists.01.org=0A= > > > Cc: Karunakar P ; Ye, Ting=0A= > > > ; Fu, Siyuan ; Wu, Jiaxin=0A= > > > =0A= > > > Subject: [edk2] [Patch 0/2] Add IPv6 support condition check for=0A= > > HTTP/ISCSI.=0A= > > >=0A= > > > Base on the request of=0A= > > > https://bugzilla.tianocore.org/show_bug.cgi?id=3D710,=0A= > > > we provide this patch to IPv6 condition check by leveraging AIP Proto= col.=0A= > > >=0A= > > > Cc: Karunakar P =0A= > > > Cc: Ye Ting =0A= > > > Cc: Fu Siyuan =0A= > > > Contributed-under: TianoCore Contribution Agreement 1.0=0A= > > > Signed-off-by: Karunakar P =0A= > > > Signed-off-by: Wu Jiaxin =0A= > > >=0A= > > > Jiaxin Wu (2):=0A= > > > NetworkPkg/HttpBootDxe: Add IPv6 support condition check.=0A= > > > NetworkPkg/IScsiDxe: Add IPv6 support condition check.=0A= > > >=0A= > > > NetworkPkg/HttpBootDxe/HttpBootDxe.c | 115=0A= > > > +++++++++++++++++++++++++++-=0A= > > > NetworkPkg/HttpBootDxe/HttpBootDxe.h | 2 +=0A= > > > NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 4 +-=0A= > > > NetworkPkg/IScsiDxe/IScsiConfig.c | 18 +++++=0A= > > > NetworkPkg/IScsiDxe/IScsiDriver.c | 2 +-=0A= > > > NetworkPkg/IScsiDxe/IScsiDriver.h | 1 +=0A= > > > NetworkPkg/IScsiDxe/IScsiDxe.inf | 2 +=0A= > > > NetworkPkg/IScsiDxe/IScsiImpl.h | 1 +=0A= > > > NetworkPkg/IScsiDxe/IScsiMisc.c | 135=0A= > > > ++++++++++++++++++++++++++++++++-=0A= > > > NetworkPkg/IScsiDxe/IScsiMisc.h | 4 +-=0A= > > > 10 files changed, 278 insertions(+), 6 deletions(-)=0A= > > >=0A= > > > --=0A= > > > 1.9.5.msysgit.1=0A= > > >=0A= > > > _______________________________________________=0A= > > > edk2-devel mailing list=0A= > > > edk2-devel@lists.01.org=0A= > > > https://lists.01.org/mailman/listinfo/edk2-devel=0A= > > _______________________________________________=0A= > > edk2-devel mailing list=0A= > > edk2-devel@lists.01.org=0A= > > https://lists.01.org/mailman/listinfo/edk2-devel=0A=