From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (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 05707803D2 for ; Mon, 13 Mar 2017 18:55:25 -0700 (PDT) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Mar 2017 18:55:25 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,162,1486454400"; d="scan'208";a="67118009" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga004.jf.intel.com with ESMTP; 13 Mar 2017 18:55:25 -0700 Received: from fmsmsx124.amr.corp.intel.com (10.18.125.39) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 13 Mar 2017 18:55:19 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx124.amr.corp.intel.com (10.18.125.39) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 13 Mar 2017 18:55:18 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.88]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.204]) with mapi id 14.03.0248.002; Tue, 14 Mar 2017 09:55:16 +0800 From: "Zhang, Lubo" To: "Wu, Jiaxin" , "edk2-devel@lists.01.org" CC: "Ye, Ting" , "Fu, Siyuan" Thread-Topic: [edk2] [patch] NetworkPkg: Fix driver binding issue in TCP dxe. Thread-Index: AQHSmYVyX9ixQJtNQ0Kc8zjhfaHvRqGSVDeAgAFEjYA= Date: Tue, 14 Mar 2017 01:55:15 +0000 Message-ID: <7619447B08B8F74DA4FF2A813B79803B39B01E7C@shsmsx102.ccr.corp.intel.com> References: <1489140137-29456-1-git-send-email-lubo.zhang@intel.com> <895558F6EA4E3B41AC93A00D163B7274162A1BD5@SHSMSX103.ccr.corp.intel.com> In-Reply-To: <895558F6EA4E3B41AC93A00D163B7274162A1BD5@SHSMSX103.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: 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: Tue, 14 Mar 2017 01:55:26 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thanks for your comments, will send another patch to update this. Best regards Lubo -----Original Message----- From: Wu, Jiaxin=20 Sent: Monday, March 13, 2017 3:44 PM To: Zhang, Lubo ; edk2-devel@lists.01.org Cc: Ye, Ting ; Fu, Siyuan Subject: RE: [edk2] [patch] NetworkPkg: Fix driver binding issue in TCP dxe= . 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=20 > 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=20 > block, we need to first close the parent protocol, then remove the=20 > protocol from childHandle and last to free any data structures that=20 > allocated in CreateChild. But currently, we free the socket data=20 > (Socket ConfigureState) before removing the protocol form the=20 > childhandle. So if the up layer want to send the tcp reset packet in=20 > 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=20 > 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=20 > reserved.
> + Copyright (c) 2009 - 2017, Intel Corporation. All rights=20 > + reserved.
>=20 > This program and the accompanying materials > are licensed and made available under the terms and conditions of=20 > the BSD License > which accompanies this distribution. The full text of the license=20 > 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 @@=20 > 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=20 > 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=20 > reserved.
> + Copyright (c) 2009 - 2017, Intel Corporation. All rights=20 > + reserved.
>=20 > This program and the accompanying materials > are licensed and made available under the terms and conditions of=20 > the BSD License > which accompanies this distribution. The full text of the license=20 > 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=20 > reserved.
> + Copyright (c) 2009 - 2017, Intel Corporation. All rights=20 > + reserved.
>=20 > This program and the accompanying materials > are licensed and made available under the terms and conditions of=20 > the BSD License > which accompanies this distribution. The full text of the license=20 > may be found at > http://opensource.org/licenses/bsd-license.php. > @@ -140,20 +140,82 @@ SockBufferToken ( EFI_STATUS SockDestroyChild=20 > ( > 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=20 > + (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=20 > + 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 // =20 > + 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=20 > reserved.
> + Copyright (c) 2009 - 2017, Intel Corporation. All rights=20 > + reserved.
>=20 > This program and the accompanying materials > are licensed and made available under the terms and conditions of=20 > the BSD License > which accompanies this distribution. The full text of the license=20 > 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