* [Patch 0/2] Fixed some issues in DxeIpIoLib
@ 2018-01-10 3:16 Wang Fan
2018-01-10 3:16 ` [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it is not expected Wang Fan
2018-01-10 3:16 ` [Patch 2/2] MdeModulePkg: Did some code enhancement for DxeIpIpLib Wang Fan
0 siblings, 2 replies; 8+ messages in thread
From: Wang Fan @ 2018-01-10 3:16 UTC (permalink / raw)
To: edk2-devel
See descriptions in each patch.
Wang Fan (2):
MdeModulePkg: Freed the received packet buffer if it is not expected.
MdeModulePkg: Did some code enhancement for DxeIpIpLib.
MdeModulePkg/Include/Library/IpIoLib.h | 21 ++++++++-
MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c | 70 +++++++++++++++++++---------
2 files changed, 66 insertions(+), 25 deletions(-)
--
1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it is not expected.
2018-01-10 3:16 [Patch 0/2] Fixed some issues in DxeIpIoLib Wang Fan
@ 2018-01-10 3:16 ` Wang Fan
2018-01-15 5:46 ` Fu, Siyuan
2018-01-16 0:44 ` Wu, Jiaxin
2018-01-10 3:16 ` [Patch 2/2] MdeModulePkg: Did some code enhancement for DxeIpIpLib Wang Fan
1 sibling, 2 replies; 8+ messages in thread
From: Wang Fan @ 2018-01-10 3:16 UTC (permalink / raw)
To: edk2-devel; +Cc: Jiaxin Wu, Ye Ting, Fu Siyuan
* When the packet is not normal packet or icmp error packet, the code
does not recycle it by signal RecycleSignal event, and this will
result some memory leak. This patch is to fix this issue.
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wang Fan <fan.wang@intel.com>
---
MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
index a06c0b6..c7bc1aa 100644
--- a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
+++ b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
@@ -1037,16 +1037,26 @@ IpIoListenHandlerDpc (
// The reception is actively aborted by the consumer, directly return.
//
return;
}
- if (((EFI_SUCCESS != Status) && (EFI_ICMP_ERROR != Status)) || (NULL == RxData)) {
+ if ((EFI_SUCCESS != Status) && (EFI_ICMP_ERROR != Status)) {
//
- // @bug Only process the normal packets and the icmp error packets, if RxData is NULL
- // @bug with Status == EFI_SUCCESS or EFI_ICMP_ERROR, just resume the receive although
- // @bug this should be a bug of the low layer (IP).
+ // Only process the normal packets and the icmp error packets.
//
+ if (RxData != NULL) {
+ goto CleanUp;
+ } else {
+ goto Resume;
+ }
+ }
+
+ //
+ // if RxData is NULL with Status == EFI_SUCCESS or EFI_ICMP_ERROR, this should be a code issue in the low layer (IP).
+ //
+ ASSERT (RxData != NULL);
+ if (RxData == NULL) {
goto Resume;
}
if (NULL == IpIo->PktRcvdNotify) {
goto CleanUp;
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Patch 2/2] MdeModulePkg: Did some code enhancement for DxeIpIpLib.
2018-01-10 3:16 [Patch 0/2] Fixed some issues in DxeIpIoLib Wang Fan
2018-01-10 3:16 ` [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it is not expected Wang Fan
@ 2018-01-10 3:16 ` Wang Fan
2018-01-10 4:51 ` Ni, Ruiyu
2018-01-15 5:48 ` Fu, Siyuan
1 sibling, 2 replies; 8+ messages in thread
From: Wang Fan @ 2018-01-10 3:16 UTC (permalink / raw)
To: edk2-devel; +Cc: Jiaxin Wu, Ye Ting, Fu Siyuan
* 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 <jiaxin.wu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wang Fan <fan.wang@intel.com>
---
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.<BR>
-Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2018, 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
@@ -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
)
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Patch 2/2] MdeModulePkg: Did some code enhancement for DxeIpIpLib.
2018-01-10 3:16 ` [Patch 2/2] MdeModulePkg: Did some code enhancement for DxeIpIpLib Wang Fan
@ 2018-01-10 4:51 ` Ni, Ruiyu
2018-01-10 4:58 ` Wang, Fan
2018-01-15 5:48 ` Fu, Siyuan
1 sibling, 1 reply; 8+ messages in thread
From: Ni, Ruiyu @ 2018-01-10 4:51 UTC (permalink / raw)
To: edk2-devel
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 <jiaxin.wu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan <fan.wang@intel.com>
> ---
> 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.<BR>
> -Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2018, 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
>
> @@ -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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch 2/2] MdeModulePkg: Did some code enhancement for DxeIpIpLib.
2018-01-10 4:51 ` Ni, Ruiyu
@ 2018-01-10 4:58 ` Wang, Fan
0 siblings, 0 replies; 8+ messages in thread
From: Wang, Fan @ 2018-01-10 4:58 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org
Thanks, Ray, will revise the title.
Best Regards
Fan
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ni, Ruiyu
Sent: Wednesday, January 10, 2018 12:51 PM
To: edk2-devel@lists.01.org
Subject: Re: [edk2] [Patch 2/2] MdeModulePkg: Did some code enhancement for DxeIpIpLib.
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 <jiaxin.wu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan <fan.wang@intel.com>
> ---
> 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.<BR>
> -Copyright (c) 2005 - 2017, Intel Corporation. All rights
> reserved.<BR>
> +Copyright (c) 2005 - 2018, 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
>
> @@ -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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it is not expected.
2018-01-10 3:16 ` [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it is not expected Wang Fan
@ 2018-01-15 5:46 ` Fu, Siyuan
2018-01-16 0:44 ` Wu, Jiaxin
1 sibling, 0 replies; 8+ messages in thread
From: Fu, Siyuan @ 2018-01-15 5:46 UTC (permalink / raw)
To: Wang, Fan, edk2-devel@lists.01.org; +Cc: Wu, Jiaxin, Ye, Ting
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> -----Original Message-----
> From: Wang, Fan
> Sent: Wednesday, January 10, 2018 11:16 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu,
> Siyuan <siyuan.fu@intel.com>
> Subject: [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it
> is not expected.
>
> * When the packet is not normal packet or icmp error packet, the code
> does not recycle it by signal RecycleSignal event, and this will
> result some memory leak. This patch is to fix this issue.
>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan <fan.wang@intel.com>
> ---
> MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> index a06c0b6..c7bc1aa 100644
> --- a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> +++ b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> @@ -1037,16 +1037,26 @@ IpIoListenHandlerDpc (
> // The reception is actively aborted by the consumer, directly return.
> //
> return;
> }
>
> - if (((EFI_SUCCESS != Status) && (EFI_ICMP_ERROR != Status)) || (NULL ==
> RxData)) {
> + if ((EFI_SUCCESS != Status) && (EFI_ICMP_ERROR != Status)) {
> //
> - // @bug Only process the normal packets and the icmp error packets,
> if RxData is NULL
> - // @bug with Status == EFI_SUCCESS or EFI_ICMP_ERROR, just resume the
> receive although
> - // @bug this should be a bug of the low layer (IP).
> + // Only process the normal packets and the icmp error packets.
> //
> + if (RxData != NULL) {
> + goto CleanUp;
> + } else {
> + goto Resume;
> + }
> + }
> +
> + //
> + // if RxData is NULL with Status == EFI_SUCCESS or EFI_ICMP_ERROR, this
> should be a code issue in the low layer (IP).
> + //
> + ASSERT (RxData != NULL);
> + if (RxData == NULL) {
> goto Resume;
> }
>
> if (NULL == IpIo->PktRcvdNotify) {
> goto CleanUp;
> --
> 1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch 2/2] MdeModulePkg: Did some code enhancement for DxeIpIpLib.
2018-01-10 3:16 ` [Patch 2/2] MdeModulePkg: Did some code enhancement for DxeIpIpLib Wang Fan
2018-01-10 4:51 ` Ni, Ruiyu
@ 2018-01-15 5:48 ` Fu, Siyuan
1 sibling, 0 replies; 8+ messages in thread
From: Fu, Siyuan @ 2018-01-15 5:48 UTC (permalink / raw)
To: Wang, Fan, edk2-devel@lists.01.org; +Cc: Ye, Ting, Wu, Jiaxin
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Wang Fan
> Sent: Wednesday, January 10, 2018 11:16 AM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> Subject: [edk2] [Patch 2/2] MdeModulePkg: Did some code enhancement for
> DxeIpIpLib.
>
> * 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 <jiaxin.wu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan <fan.wang@intel.com>
> ---
> 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.<BR>
> -Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2018, 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
>
> @@ -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
> )
> --
> 1.9.5.msysgit.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it is not expected.
2018-01-10 3:16 ` [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it is not expected Wang Fan
2018-01-15 5:46 ` Fu, Siyuan
@ 2018-01-16 0:44 ` Wu, Jiaxin
1 sibling, 0 replies; 8+ messages in thread
From: Wu, Jiaxin @ 2018-01-16 0:44 UTC (permalink / raw)
To: Wang, Fan, edk2-devel@lists.01.org; +Cc: Ye, Ting, Fu, Siyuan
Series Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
> -----Original Message-----
> From: Wang, Fan
> Sent: Wednesday, January 10, 2018 11:16 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu,
> Siyuan <siyuan.fu@intel.com>
> Subject: [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it is
> not expected.
>
> * When the packet is not normal packet or icmp error packet, the code
> does not recycle it by signal RecycleSignal event, and this will
> result some memory leak. This patch is to fix this issue.
>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan <fan.wang@intel.com>
> ---
> MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> index a06c0b6..c7bc1aa 100644
> --- a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> +++ b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> @@ -1037,16 +1037,26 @@ IpIoListenHandlerDpc (
> // The reception is actively aborted by the consumer, directly return.
> //
> return;
> }
>
> - if (((EFI_SUCCESS != Status) && (EFI_ICMP_ERROR != Status)) || (NULL ==
> RxData)) {
> + if ((EFI_SUCCESS != Status) && (EFI_ICMP_ERROR != Status)) {
> //
> - // @bug Only process the normal packets and the icmp error packets, if
> RxData is NULL
> - // @bug with Status == EFI_SUCCESS or EFI_ICMP_ERROR, just resume
> the receive although
> - // @bug this should be a bug of the low layer (IP).
> + // Only process the normal packets and the icmp error packets.
> //
> + if (RxData != NULL) {
> + goto CleanUp;
> + } else {
> + goto Resume;
> + }
> + }
> +
> + //
> + // if RxData is NULL with Status == EFI_SUCCESS or EFI_ICMP_ERROR, this
> should be a code issue in the low layer (IP).
> + //
> + ASSERT (RxData != NULL);
> + if (RxData == NULL) {
> goto Resume;
> }
>
> if (NULL == IpIo->PktRcvdNotify) {
> goto CleanUp;
> --
> 1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-01-16 0:39 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-10 3:16 [Patch 0/2] Fixed some issues in DxeIpIoLib Wang Fan
2018-01-10 3:16 ` [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it is not expected Wang Fan
2018-01-15 5:46 ` Fu, Siyuan
2018-01-16 0:44 ` Wu, Jiaxin
2018-01-10 3:16 ` [Patch 2/2] MdeModulePkg: Did some code enhancement for DxeIpIpLib Wang Fan
2018-01-10 4:51 ` Ni, Ruiyu
2018-01-10 4:58 ` Wang, Fan
2018-01-15 5:48 ` Fu, Siyuan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox