From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:400e:c05::22a; helo=mail-pg0-x22a.google.com; envelope-from=heyi.guo@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-pg0-x22a.google.com (mail-pg0-x22a.google.com [IPv6:2607:f8b0:400e:c05::22a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 0DCBC220FB322 for ; Mon, 11 Dec 2017 02:38:46 -0800 (PST) Received: by mail-pg0-x22a.google.com with SMTP id g7so10651242pgs.0 for ; Mon, 11 Dec 2017 02:43:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:date:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=smTI3fKeluNxwCG+rMp+dH0bERogZo9dnS3dlS0BxjA=; b=QbYd3pvPq9X71W5WgaxN79x9THBGzw/XcPnQ1zzZTswPNc+ZHfw0hYUrNt3PNRiXkV 5aYuIqOeSv9YKyOfEj9Z9F9Q+Qs6C+Q5uslkQJ/AGdFLE4R4LHtt3c161RuLpuYeIe63 HTEt9N9khTup0mDqoQNdMaDr3cjo3b5+Vbn+A= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:date:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=smTI3fKeluNxwCG+rMp+dH0bERogZo9dnS3dlS0BxjA=; b=rPDGhANBfmqnE6XxRnvOJKgASDGiMmRZP7giiMfisxc4/KHnmoY36svNT7uJ2pkshD /JauaQv5V/kuHhmTHRQ5o/uRgWt35ijzyssWXZt9y6sdLPhgxlXmmYDYIznIx7w+r2FP sahX39eJA9d+/fZRFx3bWSyRjKaLkqxYg2/7RsPaRGLubKJbcuoE7AbMM0lT4K60VfgV CE765jlpCUHcQnnq/S8GZF0zOM7BxVg5HedTfJ9vOi5wB3MYuqALNHKQYt2AOtnkdFf2 ckEit+9XMq80gMsG1Voqf85N30dKAiBykua37V9kMI+EQ8PwPWamNhohXlUV/sbziPwc p7YQ== X-Gm-Message-State: AJaThX5qkoITWAJf6tcNM68FRh7AKSz+gsc7rBl/W1c5mOYZNfK5lHlz q3y3mS1TBuqn2KuQaSWv9WHBhA== X-Google-Smtp-Source: AGs4zMa6kmJbEiOsyx8+JrFnQhKfN0G01zKL44kD4emaWIhwMCKB9ACHnlh6Pv06YGgzhTzeOcBNLg== X-Received: by 10.99.120.10 with SMTP id t10mr37740338pgc.393.1512989003975; Mon, 11 Dec 2017 02:43:23 -0800 (PST) Received: from SZX1000114654 ([45.56.152.71]) by smtp.gmail.com with ESMTPSA id p10sm27106907pfl.32.2017.12.11.02.43.21 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 11 Dec 2017 02:43:23 -0800 (PST) From: Guo Heyi X-Google-Original-From: Guo Heyi Date: Mon, 11 Dec 2017 18:43:19 +0800 To: "Wu, Jiaxin" Cc: "Wu, Jiaxin" , "edk2-devel@lists.01.org" , "Ni, Ruiyu" , "Fu, Siyuan" , "Dong, Eric" , "Zeng, Star" Message-ID: <20171211104319.GA67395@SZX1000114654> References: <20171207040650.GA62959@SZX1000114654> <895558F6EA4E3B41AC93A00D163B727416350F0E@SHSMSX103.ccr.corp.intel.com> <4d306e68-626f-c943-3d34-64ec181e7398@linaro.org> <895558F6EA4E3B41AC93A00D163B7274163510BC@SHSMSX103.ccr.corp.intel.com> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Subject: Re: MdeModulePkg/UefiPxeBcDxe: Question about IcmpErrorListenHandler in PxeBcImpl.c X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 Dec 2017 10:38:47 -0000 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit Hi Jiaxin, We tested and the patch worked. We also tested PXE boot and no new issue was observed. Thanks and regards, Gary (Heyi Guo) On Thu, Dec 07, 2017 at 08:20:28PM +0800, Heyi Guo wrote: > > > 在 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-bounces@lists.01.org] On Behalf Of > >>Heyi Guo > >>Sent: Thursday, December 7, 2017 4:18 PM > >>To: Wu, Jiaxin ; edk2-devel@lists.01.org > >>Cc: Ni, Ruiyu ; Fu, Siyuan ; Dong, > >>Eric ; Zeng, Star > >>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.guo@linaro.org] > >>>>Sent: Thursday, December 7, 2017 12:07 PM > >>>>To: edk2-devel@lists.01.org > >>>>Cc: Zeng, Star ; Dong, Eric ; > >>Ni, > >>>>Ruiyu ; Fu, Siyuan ; Wu, Jiaxin > >>>> > >>>>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 >