Re: MdeModulePkg/UefiPxeBcDxe: Question about IcmpErrorListenHandler in PxeBcImpl.c

[Heyi Guo <heyi.guo@linaro.org>]

Hi Jiaxin,

Thanks for your reply.

You say "IcmpErrorRcvToken is only used to get ICMP error from IP 
layer", does that mean only ICMP error packets will go to 
IcmpErrorListenHandler?

If it is the case, how do we make it? I can only find a simple 
Ip4->Receive is called to receive IP4 packets; how are other types of 
IP4 packets filtered out?

If it is not, why don't we need to filter the packets in 
IcmpErrorListenHandler? If we recycle the packets in 
IcmpErrorListenHandler, will it cause the upper layer of protocols fail 
to fetch RxData?

Please forgive me if my questions are too stupid :)

Regards,

Gary (Heyi Guo)


在 12/7/2017 3:48 PM, Wu, Jiaxin 写道:
> Hi Gary,
>
> IcmpErrorRcvToken is only used to get ICMP error from IP layer, and the data will be copied to Mode->IcmpError. So, I think the RxData should be recycled.
>
> Besides, EFI_IP_PROTO_ICMP should be also checked in the call function but currently it's not:
>
>    if (!EFI_IP4_EQUAL (&RxData->Header->DestinationAddress, &Mode->StationIp.v4)) {
>      //
>      // The dest address is not equal to Station Ip address, discard it.
>      //
>      goto CleanUp;
>    }
>
>    +if (&RxData->Header->Protocol != EFI_IP_PROTO_ICMP) {
>    +//
>    +// The protocol value in the header of the receveid packet should be EFI_IP_PROTO_ICMP.
>    +//
>    +goto CleanUp;
>    +}
>
> Thanks the report.
>
> Thanks,
> Jiaxin
>
>
>
>> -----Original Message-----
>> From: Guo Heyi [mailto:heyi.guo@linaro.org]
>> Sent: Thursday, December 7, 2017 12:07 PM
>> To: edk2-devel@lists.01.org
>> Cc: Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni,
>> Ruiyu <ruiyu.ni@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin
>> <jiaxin.wu@intel.com>
>> Subject: MdeModulePkg/UefiPxeBcDxe: Question about
>> IcmpErrorListenHandler in PxeBcImpl.c
>>
>> Hi folks,
>>
>> In PxeBcImpl.c, we have IcmpErrorListenHandler which seems to process
>> ICMP errors. But in EfiPxeBcStart function, we can see Private-
>>> IcmpErrorRcvToken.Event is only a common event and Ip4->Receive is
>> called to receive IP4 packets. So will IcmpErrorListenHandler receive all IP4
>> packets belonging to this network interface, or will it only receive ICMP error
>> packets? If it is the latter situation, how do we make it?
>>
>> The background of this question is that when we flush the network with
>> deprecated ICMP packets (type 15, 16, ...), RxData will not be recycled and
>> the list of UEFI events becomes longer and longer, which finally impacts
>> system performance a lot. If only error ICMP will be received by
>> IcmpErrorListenHandler, we'd like to patch it as below:
>>
>> diff --git a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.c
>> b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.c
>> index 6d4f33f..f74b264 100644
>> --- a/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.c
>> +++ b/MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcImpl.c
>> @@ -216,8 +216,6 @@ IcmpErrorListenHandlerDpc (
>>       CopiedPointer += CopiedLen;
>>     }
>>
>> -  goto Resume;
>> -
>>   CleanUp:
>>     gBS->SignalEvent (RxData->RecycleSignal);
>>
>> We tested and it worked, but we are still not sure whether it will impact
>> other code in the network stack.
>>
>> Please let me know your comments.
>>
>> Thanks,
>>
>> Gary (Heyi Guo)