在 12/7/2017 6:40 PM, Wu, Jiaxin 写道:
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?
No, all of the *ICMP* packets with the same station IP address will go to 
IcmpErrorListenHandler() callback function. Because PXE driver has already 
configured the current IP protocol only receive the ICMP packets:
        ZeroMem (&Private->Ip4ConfigData, sizeof (EFI_IP4_CONFIG_DATA));
        Private->Ip4ConfigData.DefaultProtocol   = EFI_IP_PROTO_ICMP;
        Private->Ip4ConfigData.AcceptIcmpErrors  = TRUE;
        Ip4ConfigData.DefaultProtocol   = EFI_IP_PROTO_ICMP;
So, it is only used to capture background ICMP packets (Including the ICMP 
error message) with the same station IP address.
Many thanks; it explains my question clearly.


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?
IcmpErrorListenHandler() should filter the packets and only handle the ICMP 
error message. But currently, the code logic is incorrect. I generated one 
patch as attached one for your reference, can you help to verify whether it 
works or not? (Ignore my previous suggestion check).
Sure we can verify it.
In my opinion, the RxData should be recycled since it has been recorded in 
Mode->IcmpError.
Agree.

Thanks,

Gary (Heyi Guo)

Thanks,
Jiaxin










-----Original Message-----
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
Heyi Guo
Sent: Thursday, December 7, 2017 4:18 PM
To: Wu, Jiaxin <jiaxin...@intel.com>; edk2-devel@lists.01.org
Cc: Ni, Ruiyu <ruiyu...@intel.com>; Fu, Siyuan <siyuan...@intel.com>; Dong,
Eric <eric.d...@intel.com>; Zeng, Star <star.z...@intel.com>
Subject: Re: [edk2] MdeModulePkg/UefiPxeBcDxe: Question about
IcmpErrorListenHandler in PxeBcImpl.c

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....@linaro.org]
Sent: Thursday, December 7, 2017 12:07 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star <star.z...@intel.com>; Dong, Eric <eric.d...@intel.com>;
Ni,
Ruiyu <ruiyu...@intel.com>; Fu, Siyuan <siyuan...@intel.com>; Wu, Jiaxin
<jiaxin...@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)
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to