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:c00::231; helo=mail-pf0-x231.google.com; envelope-from=heyi.guo@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-pf0-x231.google.com (mail-pf0-x231.google.com [IPv6:2607:f8b0:400e:c00::231]) (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 B4B1F21B02833 for ; Thu, 7 Dec 2017 00:13:51 -0800 (PST) Received: by mail-pf0-x231.google.com with SMTP id v26so4131035pfl.7 for ; Thu, 07 Dec 2017 00:18:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding:content-language; bh=vSuoQ+q3n3IUkk7VgPYo74Mwn6ItMwdf1tNC725dOBk=; b=haiP9F6v1pAK4sllnJx5oAHOg5u1wEQfkRVNoFl8CZLU0d7/b2PSqhof4xRcEcmmjd XCNYxMJPKY2295mWhSpD/t/YVm/sE6rE6lugHjFXCQsP2VbfOeGZw84XU9FFQRnczHPS uo2J8x5FVRw2cX00RbI1DrBZ/aehpjo3CWgm4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=vSuoQ+q3n3IUkk7VgPYo74Mwn6ItMwdf1tNC725dOBk=; b=sHgO0Y5tV9hKOj2g1aCTgDrjHo/62v9Lyo3FIelMijIWAt/Qll9UyJMe8uFqPYJpFl EOWYhuADu7rHNNviI+UaMk1CAPej4xNYBgHDHGFgAEXHvYNsX7Rko5o5/5bvuhXDFcV6 q3pbzhh8xQCPnR1bUsJBhfCIEeq4KOPdnTpXVX2bfgTr8WxDOpPk+xxGmzHOwNbjH6yh ajjMlHI3X++icHWw8hKKNQUOSHIoPFzAul1MYNtv7D/TU9qkbiyeLmOIh2drNFVdaz/a qoJ7NtlNg1SPKGTQTELheQsG5duuhA4SZa+we/71hQGdXbPkA1m9wwgIanB+pJIpOptc NTgw== X-Gm-Message-State: AJaThX6OrwecL4R09g3lpHDEqpjCAof+0TW1ckdnhv1ohD9lu6WPMSoO mRw/oWQRcq+1qXPggo0bukfNig== X-Google-Smtp-Source: AGs4zMZDnHluFdckcTo1dX0PhBx+kk1Py47JrnblNQmRQJFNRjYxeB52df/b9XJsincfIPwJtNCmZg== X-Received: by 10.99.127.87 with SMTP id p23mr22957575pgn.400.1512634704277; Thu, 07 Dec 2017 00:18:24 -0800 (PST) Received: from [10.157.30.118] ([104.237.90.104]) by smtp.gmail.com with ESMTPSA id w19sm8705795pfa.127.2017.12.07.00.18.18 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 07 Dec 2017 00:18:23 -0800 (PST) To: "Wu, Jiaxin" , "edk2-devel@lists.01.org" Cc: "Zeng, Star" , "Dong, Eric" , "Ni, Ruiyu" , "Fu, Siyuan" References: <20171207040650.GA62959@SZX1000114654> <895558F6EA4E3B41AC93A00D163B727416350F0E@SHSMSX103.ccr.corp.intel.com> From: Heyi Guo Message-ID: <4d306e68-626f-c943-3d34-64ec181e7398@linaro.org> Date: Thu, 7 Dec 2017 16:18:12 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <895558F6EA4E3B41AC93A00D163B727416350F0E@SHSMSX103.ccr.corp.intel.com> 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: Thu, 07 Dec 2017 08:13:51 -0000 Content-Type: text/plain; charset=gbk; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US 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)