From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.31; helo=mga06.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (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 C2E4722227597 for ; Tue, 9 Jan 2018 20:45:51 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 09 Jan 2018 20:51:02 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,338,1511856000"; d="scan'208";a="193627187" Received: from ray-dev.ccr.corp.intel.com (HELO [10.239.9.19]) ([10.239.9.19]) by fmsmga005.fm.intel.com with ESMTP; 09 Jan 2018 20:51:01 -0800 To: edk2-devel@lists.01.org References: <1515554188-2560-1-git-send-email-fan.wang@intel.com> <1515554188-2560-3-git-send-email-fan.wang@intel.com> From: "Ni, Ruiyu" Message-ID: <39b3957e-19ba-c5aa-112b-2478752db5bd@Intel.com> Date: Wed, 10 Jan 2018 12:51:01 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <1515554188-2560-3-git-send-email-fan.wang@intel.com> Subject: Re: [Patch 2/2] MdeModulePkg: Did some code enhancement for DxeIpIpLib. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Jan 2018 04:45:52 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Patch title: DxeIpIoLib On 1/10/2018 11:16 AM, Wang Fan wrote: > * In DxeIpIo, there are several places use ASSERT() to check input > parameters without and descriptions or error handling. This patch > fixed this issue. > * Fixed some incorrect descriptions in code commence. > * Remove unneeded Exit tag in function IpIoOpen and IpIoConfigIp. > * Add EFIAPI tag for function IpIoRefreshNeighbor. > > Cc: Jiaxin Wu > Cc: Ye Ting > Cc: Fu Siyuan > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Wang Fan > --- > MdeModulePkg/Include/Library/IpIoLib.h | 21 +++++++++-- > MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c | 52 ++++++++++++++++++---------- > 2 files changed, 52 insertions(+), 21 deletions(-) > > diff --git a/MdeModulePkg/Include/Library/IpIoLib.h b/MdeModulePkg/Include/Library/IpIoLib.h > index 463bf95..61653b0 100644 > --- a/MdeModulePkg/Include/Library/IpIoLib.h > +++ b/MdeModulePkg/Include/Library/IpIoLib.h > @@ -308,10 +308,12 @@ typedef struct _IP_IO_IP_INFO { > UINT8 IpVersion; > } IP_IO_IP_INFO; > > /** > Create a new IP_IO instance. > + > + If IpVersion is not IP_VERSION_4 or IP_VERSION_6, then ASSERT(). > > This function uses IP4/IP6 service binding protocol in Controller to create > an IP4/IP6 child (aka IP4/IP6 instance). > > @param[in] Image The image handle of the driver or application that > @@ -351,10 +353,12 @@ IpIoDestroy ( > IN OUT IP_IO *IpIo > ); > > /** > Stop an IP_IO instance. > + > + If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT(). > > This function is paired with IpIoOpen(). The IP_IO will be unconfigured, and all > pending send/receive tokens will be canceled. > > @param[in, out] IpIo The pointer to the IP_IO instance that needs to stop. > @@ -370,11 +374,13 @@ IpIoStop ( > IN OUT IP_IO *IpIo > ); > > /** > Open an IP_IO instance for use. > - > + > + If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT(). > + > This function is called after IpIoCreate(). It is used for configuring the IP > instance and register the callbacks and their context data for sending and > receiving IP packets. > > @param[in, out] IpIo The pointer to an IP_IO instance that needs > @@ -399,11 +405,11 @@ IpIoOpen ( > ); > > /** > Send out an IP packet. > > - This function is called after IpIoOpen(). The data to be sent are wrapped in > + This function is called after IpIoOpen(). The data to be sent is wrapped in > Pkt. The IP instance wrapped in IpIo is used for sending by default but can be > overriden by Sender. Other sending configs, like source address and gateway > address etc., are specified in OverrideData. > > @param[in, out] IpIo Pointer to an IP_IO instance used for sending IP > @@ -437,10 +443,13 @@ IpIoSend ( > ); > > /** > Cancel the IP transmit token that wraps this Packet. > > + If IpIo is NULL, then ASSERT(). > + If Packet is NULL, then ASSERT(). > + > @param[in] IpIo The pointer to the IP_IO instance. > @param[in] Packet The pointer to the packet of NET_BUF to cancel. > > **/ > VOID > @@ -450,10 +459,13 @@ IpIoCancelTxToken ( > IN VOID *Packet > ); > > /** > Add a new IP instance for sending data. > + > + If IpIo is NULL, then ASSERT(). > + If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT(). > > The function is used to add the IP_IO to the IP_IO sending list. The caller > can later use IpIoFindSender() to get the IP_IO and call IpIoSend() to send > data. > > @@ -471,10 +483,12 @@ IpIoAddIp ( > > /** > Configure the IP instance of this IpInfo and start the receiving if IpConfigData > is not NULL. > > + If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT(). > + > @param[in, out] IpInfo The pointer to the IP_IO_IP_INFO instance. > @param[in, out] IpConfigData The IP4 or IP6 configure data used to configure > the IP instance. If NULL, the IP instance is reset. > If UseDefaultAddress is set to TRUE, and the configure > operation succeeds, the default address information > @@ -493,10 +507,12 @@ IpIoConfigIp ( > ); > > /** > Destroy an IP instance maintained in IpIo->IpList for > sending purpose. > + > + If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT(). > > This function pairs with IpIoAddIp(). The IpInfo is previously created by > IpIoAddIp(). The IP_IO_IP_INFO::RefCnt is decremented and the IP instance > will be dstroyed if the RefCnt is zero. > > @@ -584,10 +600,11 @@ IpIoGetIcmpErrStatus ( > @retval EFI_UNSUPPORTED IP version is IPv4, which doesn't support neighbor cache refresh. > @retval EFI_OUT_OF_RESOURCES Failed due to resource limitations. > > **/ > EFI_STATUS > +EFIAPI > IpIoRefreshNeighbor ( > IN IP_IO *IpIo, > IN EFI_IP_ADDRESS *Neighbor, > IN UINT32 Timeout > ); > diff --git a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c > index c7bc1aa..0c6681d 100644 > --- a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c > +++ b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c > @@ -1,10 +1,10 @@ > /** @file > IpIo Library. > > (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
> -Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.
> 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 > > @@ -632,12 +632,12 @@ IpIoExtFree ( > @param[in] Context Pointer to the context. > @param[in] NotifyData Pointer to the notify data. > @param[in] Dest Pointer to the destination IP address. > @param[in] Override Pointer to the overriden IP_IO data. > > - @return Pointer to the data structure created to wrap the packet. If NULL, > - @return resource limit occurred. > + @return Pointer to the data structure created to wrap the packet. If any error occurs, > + then return NULL. > > **/ > IP_IO_SEND_ENTRY * > IpIoCreateSndEntry ( > IN OUT IP_IO *IpIo, > @@ -1073,11 +1073,11 @@ IpIoListenHandlerDpc ( > if ((EFI_IP4 (RxData->Ip4RxData.Header->SourceAddress) != 0) && > (IpIo->SubnetMask != 0) && > IP4_NET_EQUAL (IpIo->StationIp, EFI_NTOHL (((EFI_IP4_RECEIVE_DATA *) RxData)->Header->SourceAddress), IpIo->SubnetMask) && > !NetIp4IsUnicast (EFI_NTOHL (((EFI_IP4_RECEIVE_DATA *) RxData)->Header->SourceAddress), IpIo->SubnetMask)) { > // > - // The source address is not zero and it's not a unicast IP address, discard it. > + // The source address doesn't match StationIp and it's not a unicast IP address, discard it. > // > goto CleanUp; > } > > if (RxData->Ip4RxData.DataLength == 0) { > @@ -1219,10 +1219,12 @@ IpIoListenHandler ( > } > > > /** > Create a new IP_IO instance. > + > + If IpVersion is not IP_VERSION_4 or IP_VERSION_6, then ASSERT(). > > This function uses IP4/IP6 service binding protocol in Controller to create > an IP4/IP6 child (aka IP4/IP6 instance). > > @param[in] Image The image handle of the driver or application that > @@ -1284,11 +1286,11 @@ IpIoCreate ( > Status = IpIoCreateIpChildOpenProtocol ( > Controller, > Image, > &IpIo->ChildHandle, > IpVersion, > - (VOID **)&(IpIo->Ip) > + (VOID **) & (IpIo->Ip) > ); > if (EFI_ERROR (Status)) { > goto ReleaseIpIo; > } > > @@ -1306,11 +1308,13 @@ ReleaseIpIo: > } > > > /** > Open an IP_IO instance for use. > - > + > + If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT(). > + > This function is called after IpIoCreate(). It is used for configuring the IP > instance and register the callbacks and their context data for sending and > receiving IP packets. > > @param[in, out] IpIo Pointer to an IP_IO instance that needs > @@ -1417,11 +1421,11 @@ IpIoOpen ( > IpIo->Ip.Ip4, > &(IpIo->RcvToken.Ip4Token) > ); > if (EFI_ERROR (Status)) { > IpIo->Ip.Ip4->Configure (IpIo->Ip.Ip4, NULL); > - goto ErrorExit; > + return Status; > } > > } else { > > IpIo->Protocol = OpenData->IpConfigData.Ip6CfgData.DefaultProtocol; > @@ -1429,25 +1433,25 @@ IpIoOpen ( > IpIo->Ip.Ip6, > &(IpIo->RcvToken.Ip6Token) > ); > if (EFI_ERROR (Status)) { > IpIo->Ip.Ip6->Configure (IpIo->Ip.Ip6, NULL); > - goto ErrorExit; > + return Status; > } > } > > IpIo->IsConfigured = TRUE; > InsertTailList (&mActiveIpIoList, &IpIo->Entry); > > -ErrorExit: > - > return Status; > } > > > /** > Stop an IP_IO instance. > + > + If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT(). > > This function is paired with IpIoOpen(). The IP_IO will be unconfigured and all > the pending send/receive tokens will be canceled. > > @param[in, out] IpIo Pointer to the IP_IO instance that needs to stop. > @@ -1575,11 +1579,11 @@ IpIoDestroy ( > > > /** > Send out an IP packet. > > - This function is called after IpIoOpen(). The data to be sent are wrapped in > + This function is called after IpIoOpen(). The data to be sent is wrapped in > Pkt. The IP instance wrapped in IpIo is used for sending by default but can be > overriden by Sender. Other sending configs, like source address and gateway > address etc., are specified in OverrideData. > > @param[in, out] IpIo Pointer to an IP_IO instance used for sending IP > @@ -1662,10 +1666,13 @@ IpIoSend ( > > > /** > Cancel the IP transmit token which wraps this Packet. > > + If IpIo is NULL, then ASSERT(). > + If Packet is NULL, then ASSERT(). > + > @param[in] IpIo Pointer to the IP_IO instance. > @param[in] Packet Pointer to the packet of NET_BUF to cancel. > > **/ > VOID > @@ -1708,10 +1715,13 @@ IpIoCancelTxToken ( > } > > > /** > Add a new IP instance for sending data. > + > + If IpIo is NULL, then ASSERT(). > + If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT(). > > The function is used to add the IP_IO to the IP_IO sending list. The caller > can later use IpIoFindSender() to get the IP_IO and call IpIoSend() to send > data. > > @@ -1735,10 +1745,11 @@ IpIoAddIp ( > > IpInfo = AllocatePool (sizeof (IP_IO_IP_INFO)); > if (IpInfo == NULL) { > return NULL; > } > + ASSERT ((IpInfo->IpVersion == IP_VERSION_4) || (IpInfo->IpVersion == IP_VERSION_6)); > > // > // Init this IpInfo, set the Addr and SubnetMask to 0 before we configure the IP > // instance. > // > @@ -1810,10 +1821,13 @@ ReleaseIpInfo: > > /** > Configure the IP instance of this IpInfo and start the receiving if IpConfigData > is not NULL. > > + If IpInfo is NULL, then ASSERT(). > + If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT(). > + > @param[in, out] IpInfo Pointer to the IP_IO_IP_INFO instance. > @param[in, out] IpConfigData The IP configure data used to configure the IP > instance, if NULL the IP instance is reset. If > UseDefaultAddress is set to TRUE, and the configure > operation succeeds, the default address information > @@ -1859,11 +1873,11 @@ IpIoConfigIp ( > } else { > Status = Ip.Ip6->Configure (Ip.Ip6, IpConfigData); > } > > if (EFI_ERROR (Status)) { > - goto OnExit; > + return Status; > } > > if (IpConfigData != NULL) { > if (IpInfo->IpVersion == IP_VERSION_4) { > > @@ -1874,11 +1888,11 @@ IpIoConfigIp ( > NULL, > NULL > ); > if (EFI_ERROR (Status)) { > Ip.Ip4->Configure (Ip.Ip4, NULL); > - goto OnExit; > + return Status; > } > > IP4_COPY_ADDRESS (&((EFI_IP4_CONFIG_DATA*) IpConfigData)->StationAddress, &Ip4ModeData.ConfigData.StationAddress); > IP4_COPY_ADDRESS (&((EFI_IP4_CONFIG_DATA*) IpConfigData)->SubnetMask, &Ip4ModeData.ConfigData.SubnetMask); > } > @@ -1908,11 +1922,11 @@ IpIoConfigIp ( > NULL, > NULL > ); > if (EFI_ERROR (Status)) { > Ip.Ip6->Configure (Ip.Ip6, NULL); > - goto OnExit; > + return Status; > } > > if (Ip6ModeData.IsConfigured) { > CopyMem ( > &((EFI_IP6_CONFIG_DATA *) IpConfigData)->StationAddress, > @@ -1944,11 +1958,11 @@ IpIoConfigIp ( > FreePool (Ip6ModeData.IcmpTypeList); > } > > } else { > Status = EFI_NO_MAPPING; > - goto OnExit; > + return Status; > } > > CopyMem ( > &IpInfo->Addr, > &Ip6ModeData.ConfigData.StationAddress, > @@ -1969,19 +1983,19 @@ IpIoConfigIp ( > // > ZeroMem (&IpInfo->Addr, sizeof (IpInfo->Addr)); > ZeroMem (&IpInfo->PreMask, sizeof (IpInfo->PreMask)); > } > > -OnExit: > - > return Status; > } > > > /** > Destroy an IP instance maintained in IpIo->IpList for > sending purpose. > + > + If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT(). > > This function pairs with IpIoAddIp(). The IpInfo is previously created by > IpIoAddIp(). The IP_IO_IP_INFO::RefCnt is decremented and the IP instance > will be dstroyed if the RefCnt is zero. > > @@ -2110,12 +2124,11 @@ IpIoFindSender ( > > if (EFI_IP6_EQUAL (&IpInfo->Addr.v6, &Src->v6)) { > *IpIo = IpIoPtr; > return IpInfo; > } > - } > - > + } > } > } > > // > // No match. > @@ -2258,10 +2271,11 @@ IpIoGetIcmpErrStatus ( > @retval EFI_UNSUPPORTED IP version is IPv4, which doesn't support neighbor cache refresh. > @retval EFI_OUT_OF_RESOURCES Failed due to resource limit. > > **/ > EFI_STATUS > +EFIAPI > IpIoRefreshNeighbor ( > IN IP_IO *IpIo, > IN EFI_IP_ADDRESS *Neighbor, > IN UINT32 Timeout > ) > -- Thanks, Ray