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 F40A7202E5CFC for ; Wed, 18 Oct 2017 01:02:00 -0700 (PDT) Received: from IMSVA.IN.MEGATRENDS.COM (IMSVA.IN.MEGATRENDS.COM [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 7AFDF82047; Wed, 18 Oct 2017 13:38:27 +0530 (IST) Received: from IMSVA.IN.MEGATRENDS.COM (IMSVA.IN.MEGATRENDS.COM [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 6434682046; Wed, 18 Oct 2017 13:38:26 +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 13:38:26 +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 13:35:32 +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 Date: Wed, 18 Oct 2017 08:05:32 +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> In-Reply-To: <895558F6EA4E3B41AC93A00D163B72741633ECFC@SHSMSX103.ccr.corp.intel.com> Accept-Language: en-GB, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [1.23.74.162] MIME-Version: 1.0 X-TM-AS-GCONF: 00 X-TM-AS-Product-Ver: IMSVA-9.1.0.1600-8.1.0.1062-23402.005 X-TM-AS-Result: No--38.255-5.0-31-10 X-imss-scan-details: No--38.255-5.0-31-10 X-TMASE-Version: IMSVA-9.1.0.1600-8.1.1062-23402.005 X-TMASE-Result: 10--38.254600-10.000000 X-TMASE-MatchedRID: Rp71wniPtoP6htMwDTl4DUKcYi5Qw/RVVNyumldWne1R6SMusLeVML2+ Pt89anuuyJ8r9CxNAUCuB6WcjahkUWxVP8CEcZNUALBxQHkJtWqC7C2rJeUToVpbYq2f4jz+KUX f6njAjsjoHgrZ+UltMZj4oSfJftZ+iNCj8jDazVITDJnK4xDjXfDfYikgJILntXl9IxEPXOq4jX ewl4zopwUqOzL4alHyZrx0VP+xF3u6x2PC25NEZszWN98iBBeGfjJOgArMOCY4YKAM3oRt9omlc KusaK4hmNVEdxRO2BKbiQYnOnNMEOu/fUuQFeBG6ivQ8oO6nUIwLjM7t3iRo7w2tvOM+/MnAHed lY3p0SzHFMvks3o0Z57cfdWOY+ZrWK/5Im3X9NCVSBCoZUyqbCDQvhfLY4eVBlt4RZwvTdVwF0f 4C2g8Gq441foHWiRK2wh79NdRsvUKm5kt79XbMTCIlN/eSPB9QrO4XR6BRQMrNO+0WTIWLbSmaH 8EDRtKFrGaGuAHjPF7e2pDn+7ufbmvMSppeWbNsyw+ZJnFumTljSRvSGpq3P0C6Ox5RDvcN5Xi8 c6G52SvForNCx3W4i/6ijZ11WSPEbjpuHFTtb4XGp2ggKr4hguK1hIitSIH51ONI4UL++qjxYyR Ba/qJR8AKgKWeNGhKVv+AHCw0Uqx5amWK2anSPoLR4+zsDTtAqYBE3k9Mpw= 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 08:02:02 -0000 Content-Language: en-GB Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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 you= find any issues/drawbacks with that?=0A= -> In HttpBootCheckIpv6Support () definition Instead of getting Ipv6Support= from Private->Nii->Ipv6Supported if we open the protocol there itself, Th= en there will not be issues in Destroying Children or FreePool(Private) rig= ht. Because we're going to check HttpBootCheckIpv6Support() before opening= 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 HTTP/I= SCSI.=0A= =0A= Hi Karunakar,=0A= =0A= Thanks your verification. Base on your comments, I refined the series patch= es 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 switchi= ng the IP mode. /// This one is to fix the issue A.=0A= NetworkPkg/IScsiDxe: Display InitiatorInfo in attempt page even= DHCP enabled.=0A= =0A= [Patch v2 0/2] Add IPv6 support condition check.=0A= NetworkPkg/HttpBootDxe: Add IPv6 support condition check. /// = B && 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, Please= =0A= > find my below comments and issues faced=0A= >=0A= > A. Display InitiatorInfo in attempt page even DHCP enabled=0A= > -------------------------------------------------------------------------= ---------------------=0A= > --------------------------------------------=0A= > 1. I applied IScsiConfigVfr.vfr changes and as well IScsiMisc.c changes= =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 Subne= t=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= > -> This changes looks similar whatever I attached in Bugzilla, and verifi= ed 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= > 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 deletion(= -) 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); // ASSE= RTs=0A= > here=0A= > } else {=0A= > .....=0A= >=0A= > 3. I would like add some points and info about the this ASSERT, which I'v= e=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 HTTP= =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 for= mal=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 any = 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 Of= =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 Protoco= l.=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=