Replied too soon. I saw you had already closed mine.

Thanks
Ashish

________________________________
From: Ashish Singhal <ashishsin...@nvidia.com>
Sent: Friday, January 19, 2024 7:23 AM
To: Kinney, Michael D <michael.d.kin...@intel.com>; Kasbekar, Saloni 
<saloni.kasbe...@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>; 
Clark-williams, Zachary <zachary.clark-willi...@intel.com>; Jeff Brasen 
<jbra...@nvidia.com>; Gao, Liming <gaolim...@byosoft.com.cn>
Subject: Re: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

Hi Michael,

If you are going to create a new PR yourself instead of using the one I already 
created (https://github.com/tianocore/edk2/pull/5150), should I close this one?

Thanks
Ashish
________________________________
From: Kinney, Michael D <michael.d.kin...@intel.com>
Sent: Friday, January 19, 2024 4:57 AM
To: Kasbekar, Saloni <saloni.kasbe...@intel.com>; Ashish Singhal 
<ashishsin...@nvidia.com>; devel@edk2.groups.io <devel@edk2.groups.io>; 
Clark-williams, Zachary <zachary.clark-willi...@intel.com>; Jeff Brasen 
<jbra...@nvidia.com>; Gao, Liming <gaolim...@byosoft.com.cn>
Cc: Kinney, Michael D <michael.d.kin...@intel.com>
Subject: RE: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

External email: Use caution opening links or attachments


Acked-by: Michael D Kinney <michael.d.kin...@intel.com>

I will prepare PR for merge




From: Kasbekar, Saloni <saloni.kasbe...@intel.com>
Sent: Wednesday, January 17, 2024 9:27 AM
To: Ashish Singhal <ashishsin...@nvidia.com>; devel@edk2.groups.io; 
Clark-williams, Zachary <zachary.clark-willi...@intel.com>; Jeff Brasen 
<jbra...@nvidia.com>; Kinney, Michael D <michael.d.kin...@intel.com>; Gao, 
Liming <gaolim...@byosoft.com.cn>
Subject: RE: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

Liming, Mike,

Could you please help merge this PR?

Thanks,
Saloni

From: Ashish Singhal <mailto:ashishsin...@nvidia.com>
Sent: Wednesday, January 17, 2024 6:08 AM
To: Kasbekar, Saloni <mailto:saloni.kasbe...@intel.com>; 
mailto:devel@edk2.groups.io; Clark-williams, Zachary 
<mailto:zachary.clark-willi...@intel.com>; Jeff Brasen 
<mailto:jbra...@nvidia.com>
Subject: Re: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

Hello,

Checking back for an update on when this PR can be merged or if there are any 
other changes you recommend.

Thanks
Ashish

________________________________________
From: Ashish Singhal <mailto:ashishsin...@nvidia.com>
Sent: Saturday, January 6, 2024 5:53 AM
To: Kasbekar, Saloni <mailto:saloni.kasbe...@intel.com>; 
mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Clark-williams, 
Zachary <mailto:zachary.clark-willi...@intel.com>; Jeff Brasen 
<mailto:jbra...@nvidia.com>
Subject: Re: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

Thanks Saloni. PR for getting this merged is available at 
https://github.com/tianocore/edk2/pull/5150

Thanks
Ashish

________________________________________
From: Kasbekar, Saloni <mailto:saloni.kasbe...@intel.com>
Sent: Saturday, January 6, 2024 1:31 AM
To: Ashish Singhal <mailto:ashishsin...@nvidia.com>; 
mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Clark-williams, 
Zachary <mailto:zachary.clark-willi...@intel.com>; Jeff Brasen 
<mailto:jbra...@nvidia.com>
Subject: RE: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

External email: Use caution opening links or attachments

Yes, SetData does reset the previous configuration.

Reviewed-by: Saloni Kasbekar <mailto:saloni.kasbe...@intel.com>

Thanks,
Saloni

From: Ashish Singhal <mailto:ashishsin...@nvidia.com>
Sent: Friday, January 5, 2024 2:34 AM
To: Kasbekar, Saloni <mailto:saloni.kasbe...@intel.com>; 
mailto:devel@edk2.groups.io; Clark-williams, Zachary 
<mailto:zachary.clark-willi...@intel.com>; Jeff Brasen 
<mailto:jbra...@nvidia.com>
Subject: Re: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

I do not recommend doing that. Setting policy via SetData does enough to wipe 
out any previous manual configuration and that is the goal for reset to default.
________________________________________
From: Kasbekar, Saloni <mailto:saloni.kasbe...@intel.com>
Sent: Friday, January 5, 2024 2:30 AM
To: Ashish Singhal <mailto:ashishsin...@nvidia.com>; 
mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Clark-williams, 
Zachary <mailto:zachary.clark-willi...@intel.com>; Jeff Brasen 
<mailto:jbra...@nvidia.com>
Subject: RE: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

External email: Use caution opening links or attachments

Makes sense. Should we also set IfrNvData->DhcpEnable = TRUE when updating the 
Policy then?

From: Ashish Singhal <mailto:ashishsin...@nvidia.com>
Sent: Wednesday, January 3, 2024 8:52 AM
To: Kasbekar, Saloni <mailto:saloni.kasbe...@intel.com>; 
mailto:devel@edk2.groups.io; Clark-williams, Zachary 
<mailto:zachary.clark-willi...@intel.com>; Jeff Brasen 
<mailto:jbra...@nvidia.com>
Subject: Re: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

Hello Saloni,

Thanks for the feedback. After the reset, or when we disable configure from 
menu, GetData returns policy to static as the enum value is 0. However, setting 
value as static does not have any benefit as it forces to reuse the old network 
settings. Using DHCP really mimics the reset behavior that we see without any 
configuration done manually.

Thanks
Ashish

________________________________________
From: Kasbekar, Saloni <mailto:saloni.kasbe...@intel.com>
Sent: Tuesday, January 2, 2024 1:47 PM
To: Ashish Singhal <mailto:ashishsin...@nvidia.com>; 
mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Clark-williams, 
Zachary <mailto:zachary.clark-willi...@intel.com>; Jeff Brasen 
<mailto:jbra...@nvidia.com>
Subject: RE: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

External email: Use caution opening links or attachments

Hi Ashish,

+    Ip4NvData->Policy = Ip4Config2PolicyDhcp;
+    Status            = Ip4Cfg2->SetData (
+                                   Ip4Cfg2,
+                                   Ip4Config2DataTypePolicy,
+                                   sizeof (EFI_IP4_CONFIG2_POLICY),
+                                   &Ip4NvData->Policy
+                                   );

Here we're assuming IfrFormNvData->DhcpEnable is TRUE. Should we check it 
before setting the policy and calling SetData()?

Thanks,
Saloni


From: Ashish Singhal <mailto:ashishsin...@nvidia.com>
Sent: Monday, January 1, 2024 8:48 AM
To: mailto:devel@edk2.groups.io; Kasbekar, Saloni 
<mailto:saloni.kasbe...@intel.com>; Clark-williams, Zachary 
<mailto:zachary.clark-willi...@intel.com>; Jeff Brasen 
<mailto:jbra...@nvidia.com>
Subject: Re: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

Hello,

Checking again for some feedback on this.

Thanks
Ashish

________________________________________
From: Ashish Singhal <mailto:ashishsin...@nvidia.com>
Sent: Thursday, December 14, 2023 4:42 PM
To: mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>; 
mailto:saloni.kasbe...@intel.com <mailto:saloni.kasbe...@intel.com>; 
mailto:zachary.clark-willi...@intel.com 
<mailto:zachary.clark-willi...@intel.com>; Jeff Brasen 
<mailto:jbra...@nvidia.com>
Cc: Ashish Singhal <mailto:ashishsin...@nvidia.com>
Subject: [PATCH] NetworkPkg/Ip4Dxe: Fix Reset To Default

Exercising reset to default does not reset the settings.
Add handler code for the case where configuration is
disabled.

Signed-off-by: Ashish Singhal <mailto:ashishsin...@nvidia.com>
---
 NetworkPkg/Ip4Dxe/Ip4Config2Nv.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/NetworkPkg/Ip4Dxe/Ip4Config2Nv.c b/NetworkPkg/Ip4Dxe/Ip4Config2Nv.c
index e0b6a4d4a9..dac5817b7c 100644
--- a/NetworkPkg/Ip4Dxe/Ip4Config2Nv.c
+++ b/NetworkPkg/Ip4Dxe/Ip4Config2Nv.c
@@ -586,6 +586,31 @@ Ip4Config2ConvertIfrNvDataToConfigNvData (
   }

   if (IfrFormNvData->Configure != TRUE) {
+    if (Ip4NvData->DnsAddress != NULL) {
+      FreePool (Ip4NvData->DnsAddress);
+      Ip4NvData->DnsAddress      = NULL;
+      Ip4NvData->DnsAddressCount = 0;
+    }
+
+    if (Ip4NvData->GatewayAddress != NULL) {
+      FreePool (Ip4NvData->GatewayAddress);
+      Ip4NvData->GatewayAddress      = NULL;
+      Ip4NvData->GatewayAddressCount = 0;
+    }
+
+    if (Ip4NvData->ManualAddress != NULL) {
+      FreePool (Ip4NvData->ManualAddress);
+      Ip4NvData->ManualAddress      = NULL;
+      Ip4NvData->ManualAddressCount = 0;
+    }
+
+    Ip4NvData->Policy = Ip4Config2PolicyDhcp;
+    Status            = Ip4Cfg2->SetData (
+                                   Ip4Cfg2,
+                                   Ip4Config2DataTypePolicy,
+                                   sizeof (EFI_IP4_CONFIG2_POLICY),
+                                   &Ip4NvData->Policy
+                                   );
     return EFI_SUCCESS;
   }

--
2.17.1
Hello,
Hello Saloni,
Thanks


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114022): https://edk2.groups.io/g/devel/message/114022
Mute This Topic: https://groups.io/mt/103181314/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to