From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (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 A4965803B2 for ; Mon, 13 Mar 2017 00:44:35 -0700 (PDT) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Mar 2017 00:44:35 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,157,1486454400"; d="scan'208";a="1141405214" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga002.fm.intel.com with ESMTP; 13 Mar 2017 00:44:27 -0700 Received: from fmsmsx119.amr.corp.intel.com (10.18.124.207) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 13 Mar 2017 00:44:26 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX119.amr.corp.intel.com (10.18.124.207) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 13 Mar 2017 00:44:25 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.20]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.177]) with mapi id 14.03.0248.002; Mon, 13 Mar 2017 15:44:23 +0800 From: "Wu, Jiaxin" To: "Zhang, Lubo" , "edk2-devel@lists.01.org" CC: "Ye, Ting" , "Fu, Siyuan" Thread-Topic: [edk2] [patch] NetworkPkg: Fix driver binding issue in TCP dxe. Thread-Index: AQHSmYVyX9ixQJtNQ0Kc8zjhfaHvRqGSVDeA Date: Mon, 13 Mar 2017 07:44:23 +0000 Message-ID: <895558F6EA4E3B41AC93A00D163B7274162A1BD5@SHSMSX103.ccr.corp.intel.com> References: <1489140137-29456-1-git-send-email-lubo.zhang@intel.com> In-Reply-To: <1489140137-29456-1-git-send-email-lubo.zhang@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMjQyYzhiNWItNGM4NC00M2ViLTlhYmItYTgzOGM2ZmFiNjFhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX1BVQkxJQyJ9XX1dfSwiU3ViamVjdExhYmVscyI6W10sIlRNQ1ZlcnNpb24iOiIxNS45LjYuNiIsIlRydXN0ZWRMYWJlbEhhc2giOiIxOXg0NjY4QUxneXc1VDNPb2V0OFZvTmdtUDkwRjBlQWIwN0E2cXJjY2dFPSJ9 x-ctpclassification: CTP_PUBLIC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [patch] NetworkPkg: Fix driver binding issue in TCP dxe. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 Mar 2017 07:44:35 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable My comments as below: 1. SockDestroyChild: The sock should be locked to access first, then we ca= n update it (close/uninstall). 2. SockDestroyChild: Debug info should be "SockDestroyChild:" instead of "= SockDestroy:" 3. One concern in SockCreateChild:=20 Since the " CloseProtocol/Uninstall" operation is removed from SockDestro= y, we need handle it alone while the ERROR may happened in in SockCreateChi= ld.=20 Thanks, Jiaxin > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Zhang Lubo > Sent: Friday, March 10, 2017 6:02 PM > To: edk2-devel@lists.01.org > Cc: Ye, Ting ; Fu, Siyuan > Subject: [edk2] [patch] NetworkPkg: Fix driver binding issue in TCP dxe. >=20 > when we destroy the socket Sock and its associated > protocol control block, we need to first close the > parent protocol, then remove the protocol from childHandle > and last to free any data structures that allocated in > CreateChild. But currently, we free the socket data (Socket ConfigureStat= e) > before removing the protocol form the childhandle. So if the up layer > want to send the tcp reset packet in it's driver binding stop > function, it will failed. >=20 > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Zhang Lubo > Cc: Ye Ting > Cc: Fu Siyuan > --- > NetworkPkg/TcpDxe/SockImpl.c | 56 +-------------------------------- > NetworkPkg/TcpDxe/SockImpl.h | 3 +- > NetworkPkg/TcpDxe/SockInterface.c | 66 > +++++++++++++++++++++++++++++++++++++-- > NetworkPkg/TcpDxe/TcpDispatcher.c | 19 +---------- > 4 files changed, 68 insertions(+), 76 deletions(-) >=20 > diff --git a/NetworkPkg/TcpDxe/SockImpl.c > b/NetworkPkg/TcpDxe/SockImpl.c > index 4eb42fb..c5fb176 100644 > --- a/NetworkPkg/TcpDxe/SockImpl.c > +++ b/NetworkPkg/TcpDxe/SockImpl.c > @@ -1,9 +1,9 @@ > /** @file > Implementation of the Socket. >=20 > - Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
> + Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
>=20 > 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. > @@ -826,20 +826,12 @@ OnError: > VOID > SockDestroy ( > IN OUT SOCKET *Sock > ) > { > - VOID *SockProtocol; > - EFI_GUID *TcpProtocolGuid; > - EFI_STATUS Status; > - > ASSERT (SockStream =3D=3D Sock->Type); >=20 > - if (Sock->DestroyCallback !=3D NULL) { > - Sock->DestroyCallback (Sock, Sock->Context); > - } > - > // > // Flush the completion token buffered > // by sock and rcv, snd buffer > // > if (!SOCK_IS_UNCONFIGURED (Sock)) { > @@ -870,56 +862,10 @@ SockDestroy ( > ); >=20 > Sock->Parent =3D NULL; > } >=20 > - // > - // Set the protocol guid and driver binding handle > - // in the light of Sock->SockType > - // > - if (Sock->IpVersion =3D=3D IP_VERSION_4) { > - TcpProtocolGuid =3D &gEfiTcp4ProtocolGuid; > - } else { > - TcpProtocolGuid =3D &gEfiTcp6ProtocolGuid; > - } > - > - // > - // Retrieve the protocol installed on this sock > - // > - Status =3D gBS->OpenProtocol ( > - Sock->SockHandle, > - TcpProtocolGuid, > - &SockProtocol, > - Sock->DriverBinding, > - Sock->SockHandle, > - EFI_OPEN_PROTOCOL_GET_PROTOCOL > - ); > - > - if (EFI_ERROR (Status)) { > - > - DEBUG ( > - (EFI_D_ERROR, > - "SockDestroy: Open protocol installed on socket failed with %r\n", > - Status) > - ); > - > - goto FreeSock; > - } > - > - // > - // Uninstall the protocol installed on this sock > - // in the light of Sock->SockType > - // > - gBS->UninstallMultipleProtocolInterfaces ( > - Sock->SockHandle, > - TcpProtocolGuid, > - SockProtocol, > - NULL > - ); > - > -FreeSock: > - > FreePool (Sock); > } >=20 > /** > Flush the sndBuffer and rcvBuffer of socket. > diff --git a/NetworkPkg/TcpDxe/SockImpl.h > b/NetworkPkg/TcpDxe/SockImpl.h > index 5a067de..80692b1 100644 > --- a/NetworkPkg/TcpDxe/SockImpl.h > +++ b/NetworkPkg/TcpDxe/SockImpl.h > @@ -1,9 +1,9 @@ > /** @file > The function declaration that provided for Socket Interface. >=20 > - Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
> + Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
>=20 > 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. > @@ -15,10 +15,11 @@ >=20 > #ifndef _SOCK_IMPL_H_ > #define _SOCK_IMPL_H_ >=20 > #include "Socket.h" > +#include "TcpMain.h" >=20 > /** > Signal a event with the given status. >=20 > @param[in] Token The token's event is to be signaled. > diff --git a/NetworkPkg/TcpDxe/SockInterface.c > b/NetworkPkg/TcpDxe/SockInterface.c > index 21ce643..5269c56 100644 > --- a/NetworkPkg/TcpDxe/SockInterface.c > +++ b/NetworkPkg/TcpDxe/SockInterface.c > @@ -1,9 +1,9 @@ > /** @file > Interface function of the Socket. >=20 > - Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
> + Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
>=20 > 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. > @@ -140,20 +140,82 @@ SockBufferToken ( > EFI_STATUS > SockDestroyChild ( > IN OUT SOCKET *Sock > ) > { > - EFI_STATUS Status; > + EFI_STATUS Status; > + TCP_PROTO_DATA *ProtoData; > + TCP_CB *Tcb; > + EFI_GUID *IpProtocolGuid; > + EFI_GUID *TcpProtocolGuid; > + VOID *SockProtocol; >=20 > ASSERT ((Sock !=3D NULL) && (Sock->ProtoHandler !=3D NULL)); >=20 > if (Sock->InDestroy) { > return EFI_SUCCESS; > } >=20 > Sock->InDestroy =3D TRUE; >=20 > + if (Sock->IpVersion =3D=3D IP_VERSION_4) { > + IpProtocolGuid =3D &gEfiIp4ProtocolGuid; > + TcpProtocolGuid =3D &gEfiTcp4ProtocolGuid; > + } else { > + IpProtocolGuid =3D &gEfiIp6ProtocolGuid; > + TcpProtocolGuid =3D &gEfiTcp6ProtocolGuid; > + } > + ProtoData =3D (TCP_PROTO_DATA *) Sock->ProtoReserved; > + Tcb =3D ProtoData->TcpPcb; > + > + ASSERT (Tcb !=3D NULL); > + > + // > + // Close the IP protocol. > + // > + gBS->CloseProtocol ( > + Tcb->IpInfo->ChildHandle, > + IpProtocolGuid, > + ProtoData->TcpService->IpIo->Image, > + Sock->SockHandle > + ); > + > + if (Sock->DestroyCallback !=3D NULL) { > + Sock->DestroyCallback (Sock, Sock->Context); > + } > + > + // > + // Retrieve the protocol installed on this sock > + // > + Status =3D gBS->OpenProtocol ( > + Sock->SockHandle, > + TcpProtocolGuid, > + &SockProtocol, > + Sock->DriverBinding, > + Sock->SockHandle, > + EFI_OPEN_PROTOCOL_GET_PROTOCOL > + ); > + > + if (EFI_ERROR (Status)) { > + > + DEBUG ( > + (EFI_D_ERROR, > + "SockDestroy: Open protocol installed on socket failed with %r\n", > + Status) > + ); > + } > + > + // > + // Uninstall the protocol installed on this sock > + // > + gBS->UninstallMultipleProtocolInterfaces ( > + Sock->SockHandle, > + TcpProtocolGuid, > + SockProtocol, > + NULL > + ); > + > Status =3D EfiAcquireLockOrFail (&(Sock->Lock)); > if (EFI_ERROR (Status)) { >=20 > DEBUG ( > (EFI_D_ERROR, > diff --git a/NetworkPkg/TcpDxe/TcpDispatcher.c > b/NetworkPkg/TcpDxe/TcpDispatcher.c > index d4bc8ac..9a352b1 100644 > --- a/NetworkPkg/TcpDxe/TcpDispatcher.c > +++ b/NetworkPkg/TcpDxe/TcpDispatcher.c > @@ -1,10 +1,10 @@ > /** @file > The implementation of a dispatch routine for processing TCP requests. >=20 > (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
> - Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.
> + Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
>=20 > 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. > @@ -421,34 +421,17 @@ TcpDetachPcb ( > IN OUT SOCKET *Sk > ) > { > TCP_PROTO_DATA *ProtoData; > TCP_CB *Tcb; > - EFI_GUID *IpProtocolGuid; >=20 > - if (Sk->IpVersion =3D=3D IP_VERSION_4) { > - IpProtocolGuid =3D &gEfiIp4ProtocolGuid; > - } else { > - IpProtocolGuid =3D &gEfiIp6ProtocolGuid; > - } > - > ProtoData =3D (TCP_PROTO_DATA *) Sk->ProtoReserved; > Tcb =3D ProtoData->TcpPcb; >=20 > ASSERT (Tcb !=3D NULL); >=20 > TcpFlushPcb (Tcb); > - > - // > - // Close the IP protocol. > - // > - gBS->CloseProtocol ( > - Tcb->IpInfo->ChildHandle, > - IpProtocolGuid, > - ProtoData->TcpService->IpIo->Image, > - Sk->SockHandle > - ); >=20 > IpIoRemoveIp (ProtoData->TcpService->IpIo, Tcb->IpInfo); >=20 > FreePool (Tcb); >=20 > -- > 1.9.5.msysgit.1 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel