Hi Siyuan, You are right, IsTransmitReceive flag looks redundant. Thanks. Jiaxin
-----Original Message----- From: Fu, Siyuan Sent: Thursday, July 16, 2015 10:47 AM To: Wu, Jiaxin; edk2-devel@lists.sourceforge.net Subject: RE: [Patch] MdeModulePkg: Remove TransmitReceive() and ActiveChild dependency Hi, Jiaxin 1. Some of the code path in DhcpOnTimerTick won't goto ON_EXIT, for example in if (DhcpSb->ExtraRefresh != 0) { return ; } Or goto END_SESSION; Should these case also need be considered to counter the TransmitReceive timer tick? 2. You didn't set the IsTransmitReceive flag to FALSE after timeout, and I also think this flag is not needed, just check Instance->Token != NULL is enough. Also "|| Instance == DhcpSb->ActiveChild" makes no sense, the TimeOut is only for a transmitreceive token, why should it also count a ActiveChild? Siyuan -----Original Message----- From: Wu, Jiaxin Sent: Wednesday, July 15, 2015 11:53 AM To: edk2-devel@lists.sourceforge.net; Fu, Siyuan Subject: [Patch] MdeModulePkg: Remove TransmitReceive() and ActiveChild dependency Fix git 59a8cfd4 (SVN r17869) removes DHCP4.TransmitReceive()and DORA process dependency, but it updated TransmitReceive() to take the ownership of DhcpSb->ActiveChild but never release it. This will break the retransmit and lease time out counter of DORA. To fix that, TransmitReceive() doesn't need to be the ActiveChild, and the timer routine should be updated to handle the TransmitReceive specially. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jiaxin Wu <jiaxin...@intel.com> --- MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Driver.c | 1 + MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.c | 6 +++++- MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.h | 4 +++- MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c | 18 ++++++++++++++---- 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Driver.c b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Driver.c index 9253df2..c9c3a4a 100644 --- a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Driver.c +++ b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Driver.c @@ -505,10 +505,11 @@ DhcpInitProtocol ( Instance->Signature = DHCP_PROTOCOL_SIGNATURE; CopyMem (&Instance->Dhcp4Protocol, &mDhcp4ProtocolTemplate, sizeof (Instance->Dhcp4Protocol)); InitializeListHead (&Instance->Link); Instance->Handle = NULL; Instance->Service = DhcpSb; + Instance->IsTransmitReceive = FALSE; Instance->InDestroy = FALSE; Instance->CompletionEvent = NULL; Instance->RenewRebindEvent = NULL; Instance->Token = NULL; Instance->UdpIo = NULL; diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.c b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.c index 821dfbb..7c7ed1f 100644 --- a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.c +++ b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.c @@ -1492,11 +1492,10 @@ EfiDhcp4TransmitReceive ( return EFI_INVALID_PARAMETER; } Instance = DHCP_INSTANCE_FROM_THIS (This); DhcpSb = Instance->Service; - DhcpSb->ActiveChild = Instance; if (Instance->Token != NULL) { // // The previous call to TransmitReceive is not finished. // @@ -1525,10 +1524,15 @@ EfiDhcp4TransmitReceive ( } OldTpl = gBS->RaiseTPL (TPL_CALLBACK); // + // Set the InTransmitReceive flag. + // + Instance->IsTransmitReceive = TRUE; + + // // Save the token and the timeout value. // Instance->Token = Token; Instance->Timeout = Token->TimeoutValue; diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.h b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.h index 44213cf..615e006 100644 --- a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.h +++ b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Impl.h @@ -4,11 +4,11 @@ RFC 2131: Dynamic Host Configuration Protocol RFC 2132: DHCP Options and BOOTP Vendor Extensions RFC 1534: Interoperation Between DHCP and BOOTP RFC 3396: Encoding Long Options in DHCP. -Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.<BR> +Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR> This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at http://opensource.org/licenses/bsd-license.php @@ -61,10 +61,12 @@ struct _DHCP_PROTOCOL { EFI_DHCP4_PROTOCOL Dhcp4Protocol; LIST_ENTRY Link; EFI_HANDLE Handle; DHCP_SERVICE *Service; + BOOLEAN IsTransmitReceive; + BOOLEAN InDestroy; EFI_EVENT CompletionEvent; EFI_EVENT RenewRebindEvent; diff --git a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c index 995bbcf..583e5bd 100644 --- a/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c +++ b/MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Io.c @@ -1516,10 +1516,12 @@ EFIAPI DhcpOnTimerTick ( IN EFI_EVENT Event, IN VOID *Context ) { + LIST_ENTRY *Entry; + LIST_ENTRY *Next; DHCP_SERVICE *DhcpSb; DHCP_PROTOCOL *Instance; EFI_STATUS Status; DhcpSb = (DHCP_SERVICE *) Context; @@ -1663,14 +1665,22 @@ DhcpOnTimerTick ( } } } ON_EXIT: - if ((Instance != NULL) && (Instance->Token != NULL)) { - Instance->Timeout--; - if (Instance->Timeout == 0) { - PxeDhcpDone (Instance); + // + // Iterate through all the DhcpSb Children. + // + NET_LIST_FOR_EACH_SAFE (Entry, Next, &DhcpSb->Children) { + Instance = NET_LIST_USER_STRUCT (Entry, DHCP_PROTOCOL, Link); + + if ((Instance != NULL) && (Instance->Token != NULL) && \ + (Instance->IsTransmitReceive || Instance == DhcpSb->ActiveChild)) { + Instance->Timeout--; + if (Instance->Timeout == 0) { + PxeDhcpDone (Instance); + } } } return ; -- 1.9.5.msysgit.1 ------------------------------------------------------------------------------ Don't Limit Your Business. Reach for the Cloud. GigeNET's Cloud Solutions provide you with the tools and support that you need to offload your IT needs and focus on growing your business. Configured For All Businesses. Start Your Cloud Today. https://www.gigenetcloud.com/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel