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::22d; helo=mail-pf0-x22d.google.com; envelope-from=heyi.guo@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-pf0-x22d.google.com (mail-pf0-x22d.google.com [IPv6:2607:f8b0:400e:c00::22d]) (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 1664521B02834 for ; Thu, 7 Dec 2017 04:16:02 -0800 (PST) Received: by mail-pf0-x22d.google.com with SMTP id a90so4565734pfk.1 for ; Thu, 07 Dec 2017 04:20:35 -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=KW9NAufRAuF23MT8LQUpEQl2EOtaV1g7GHXWfj9ueww=; b=ZPkqQCKkOMw/kJoYZXJRppV6jxBmV/iSqUNPzNFuzFL+MocqlEiRdICuHbazg2NAU8 QmJ8vhlqEprvXJ512xdc2h+FGqX0I5CjT7SyI7O9xQUagezHb696sh+bxuvbc/icLFjP AVAIQWVzctAGRyFFM1P6JVHxhLBt79jvaGsTU= 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=KW9NAufRAuF23MT8LQUpEQl2EOtaV1g7GHXWfj9ueww=; b=nt9Op9oyKrKe7yR7KYXTfvqRRITzFFN1XTXkxUiHWfZP8voU3/8XxY6XekOucHmyky 0k5uGm97NWRqbwtVVhoWS6Gy/ODKcCTAyA2YzWMftPU/zwAQ5PrYzUcsKBIieRW2ZY0I +ENqz9Fwa8Gx/4DPW2W+NJilwO0FEFa5GffsgyThb94pVBSvS6KO3pd2L4eb1wR1Q9NR udkE5wsCvmcsWAHGJy0XHDXR1oUAI6OEIj0RtWydZNOFKztBjf3ua1Saz1GIsnjrAK4O 9YeqpZhLazqB5LduwwA+0bB9Qs28hHwuEVedQdFBYAQ8KrVYeG4sQzWVBLbZm45zaJMD 12lQ== X-Gm-Message-State: AJaThX6kVMgAI/Zv3X8DrGdqZ5YwlCN9isNvidv7UQbfjM9dsHIo69zR il9NZdVzHPG/cnAH+WJLLDY1oQ== X-Google-Smtp-Source: AGs4zMYRoCGIcO45P7jnrJtZtVtY837y8PoZmTDfuNM1ZlyPJZRxkuvD2ITsEnsZmICXyAgahZSC3Q== X-Received: by 10.84.128.76 with SMTP id 70mr25451116pla.171.1512649235264; Thu, 07 Dec 2017 04:20:35 -0800 (PST) Received: from [10.171.242.110] ([45.56.155.91]) by smtp.gmail.com with ESMTPSA id 126sm5628582pfe.16.2017.12.07.04.20.32 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 07 Dec 2017 04:20:34 -0800 (PST) To: "Wu, Jiaxin" , "edk2-devel@lists.01.org" Cc: "Ni, Ruiyu" , "Fu, Siyuan" , "Dong, Eric" , "Zeng, Star" References: <20171207040650.GA62959@SZX1000114654> <895558F6EA4E3B41AC93A00D163B727416350F0E@SHSMSX103.ccr.corp.intel.com> <4d306e68-626f-c943-3d34-64ec181e7398@linaro.org> <895558F6EA4E3B41AC93A00D163B7274163510BC@SHSMSX103.ccr.corp.intel.com> From: Heyi Guo Message-ID: Date: Thu, 7 Dec 2017 20:20:28 +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: <895558F6EA4E3B41AC93A00D163B7274163510BC@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 12:16:03 -0000 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US 在 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