From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1 with cipher CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7244F1A1E6B for ; Wed, 19 Oct 2016 20:02:08 -0700 (PDT) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga101.fm.intel.com with ESMTP; 19 Oct 2016 20:02:07 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,516,1473145200"; d="scan'208";a="891884654" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga003.jf.intel.com with ESMTP; 19 Oct 2016 20:02:08 -0700 Received: from fmsmsx101.amr.corp.intel.com (10.18.124.199) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 19 Oct 2016 20:02:07 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx101.amr.corp.intel.com (10.18.124.199) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 19 Oct 2016 20:02:06 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.206]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.96]) with mapi id 14.03.0248.002; Thu, 20 Oct 2016 11:02:03 +0800 From: "Fu, Siyuan" To: "Hegde, Nagaraj P" , "edk2-devel@lists.01.org" CC: "Ni, Ruiyu" , "Ye, Ting" , "Zhang, Lubo" Thread-Topic: [edk2] [Patch] ShellPkg: update ping to use timer service instead of timer arch protocol . Thread-Index: AQHSKD69fzoQ8hWav0+YximwsZ9yVaCvEwwAgAGTiPA= Date: Thu, 20 Oct 2016 03:02:02 +0000 Message-ID: References: <1476685290-11536-1-git-send-email-siyuan.fu@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYTE3OWMwNGMtOWRhYy00OTIwLWIzNTUtZDNiMjY0ZDExYTE3IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IkRZc3BidWVKZGJCOWVOMzBYTXQwSGtTK0txM3ZcL1wvUjhJVWxPQkFFeXNpST0ifQ== x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [Patch] ShellPkg: update ping to use timer service instead of timer arch protocol . X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Oct 2016 03:02:08 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi, Nagaraj The 0~2ms doesn't mean 0.2ms, it means 0 to 2 ms. It's not an accurate time= because we are using the timer event service and gBS->Stall to estimate th= e system timer period, see the new added GetTimerPeriod() function. It firs= t create a period timer with 1ms interval, and use a Stall(1m) to wait in a= while loop until the 1ms timer is triggered 10 times, and finally return t= he estimate system timer period by StallCounter / RttTimerTick. You got "0~2ms" result because you platform may set the timer period to 2ms= (or bigger), and the minimum time interval we could count by the timer ser= vice is 2ms, so we use 0~2ms, means the reply is arrived within 2ms. The reason we make these change is because the UEFI shell is not allowed to= use neither PI protocol nor platform specific library, so there is no stan= dard way to get the real time period only by UEFI service. It's a little tr= icky, if you have better idea I'm glad to hear that. I will make another patch to fix the ttl print issue, and also the ping6 up= date. Thanks. BestRegards Fu Siyuan > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Hegde, Nagaraj P > Sent: Wednesday, October 19, 2016 6:38 PM > To: Fu, Siyuan ; edk2-devel@lists.01.org > Cc: Ni, Ruiyu ; Ye, Ting ; Zhang, > Lubo > Subject: Re: [edk2] [Patch] ShellPkg: update ping to use timer service > instead of timer arch protocol . >=20 > Hi Siyuan, >=20 > Few comments: >=20 > 1. We need the same code change for Ping6 > (ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c) > 2. From the patch that I could view, in STR_PING_REPLY_INFO, we print tim= e > as 0~2ms for 0.2 ms. Why not use "." itself? Any reason for using tild (~= )? > 3. We print ttl=3D%d in STR_PING_REPLY_INFO, which is always 0 for IPv4 P= ing. > We need to fix that. >=20 > I have tested your code and looks fine. >=20 > Regards, > Nagaraj. >=20 > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Fu > Siyuan > Sent: Monday, October 17, 2016 11:52 AM > To: edk2-devel@lists.01.org > Cc: Ni Ruiyu ; Ye Ting ; Zhang Lub= o > > Subject: [edk2] [Patch] ShellPkg: update ping to use timer service instea= d > of timer arch protocol . >=20 > This patch update the shell ping command to use timer service to calculat= e > the RTT time, instead of using the timer arch protocol. >=20 > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Fu Siyuan > Cc: Ni Ruiyu > Cc: Ye Ting > Cc: Zhang Lubo > --- > .../Library/UefiShellNetwork1CommandsLib/Ping.c | 233 +++++++++++++++= - > ----- > .../UefiShellNetwork1CommandsLib.inf | 3 +- > .../UefiShellNetwork1CommandsLib.uni | 4 +- > 3 files changed, 171 insertions(+), 69 deletions(-) >=20 > diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c > b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c > index 38625fe..2e1e878 100644 > --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c > +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c > @@ -86,7 +86,7 @@ typedef struct _ICMPX_ECHO_REQUEST_REPLY { > UINT16 Checksum; > UINT16 Identifier; > UINT16 SequenceNum; > - UINT64 TimeStamp; > + UINT32 TimeStamp; > UINT8 Data[1]; > } ICMPX_ECHO_REQUEST_REPLY; > #pragma pack() > @@ -94,7 +94,7 @@ typedef struct _ICMPX_ECHO_REQUEST_REPLY { typedef > struct _PING_ICMP_TX_INFO { > LIST_ENTRY Link; > UINT16 SequenceNum; > - UINT64 TimeStamp; > + UINT32 TimeStamp; > PING_IPX_COMPLETION_TOKEN *Token; > } PING_ICMPX_TX_INFO; >=20 > @@ -109,6 +109,7 @@ typedef struct _PING_ICMP_TX_INFO { > #define DEFAULT_BUFFER_SIZE 16 > #define ICMP_V4_ECHO_REQUEST 0x8 > #define ICMP_V4_ECHO_REPLY 0x0 > +#define STALL_1_MILLI_SECOND 1000 >=20 > #define PING_PRIVATE_DATA_SIGNATURE SIGNATURE_32 ('P', 'i', 'n', 'g') > typedef struct _PING_PRIVATE_DATA { @@ -117,6 +118,10 @@ typedef struct > _PING_PRIVATE_DATA { > EFI_HANDLE IpChildHandle; > EFI_EVENT Timer; >=20 > + UINT32 TimerPeriod; > + UINT32 RttTimerTick; > + EFI_EVENT RttTimer; > + > EFI_STATUS Status; > LIST_ENTRY TxList; > UINT16 RxCount; > @@ -221,93 +226,186 @@ STATIC CONST SHELL_PARAM_ITEM PingParamList[] = =3D > { > // > STATIC CONST CHAR16 *mDstString; > STATIC CONST CHAR16 *mSrcString; > -STATIC UINT64 mFrequency =3D 0; > EFI_CPU_ARCH_PROTOCOL *gCpu =3D NULL; >=20 > /** > - Read the current time. > + RTT timer tick routine. > + > + @param[in] Event A EFI_EVENT type event. > + @param[in] Context The pointer to Context. >=20 > - @retval the current tick value. > **/ > -UINT64 > -ReadTime ( > +VOID > +EFIAPI > +RttTimerTickRoutine ( > + IN EFI_EVENT Event, > + IN VOID *Context > + ) > +{ > + UINT32 *RttTimerTick; > + > + RttTimerTick =3D (UINT32*) Context; > + (*RttTimerTick)++; > +} > + > +/** > + Get the timer period of the system. > + > + This function tries to get the system timer period by creating an > + 1ms period timer. > + > + @return System timer period in MS, or 0 if operation failed. > + > +**/ > +UINT32 > +GetTimerPeriod( > VOID > ) > { > - UINT64 TimerPeriod; > - EFI_STATUS Status; > + EFI_STATUS Status; > + UINT32 RttTimerTick; > + EFI_EVENT TimerEvent; > + UINT32 StallCounter; > + EFI_TPL OldTpl; >=20 > - ASSERT (gCpu !=3D NULL); > + RttTimerTick =3D 0; > + StallCounter =3D 0; >=20 > - Status =3D gCpu->GetTimerValue (gCpu, 0, &mCurrentTick, &TimerPeriod); > + Status =3D gBS->CreateEvent ( > + EVT_TIMER | EVT_NOTIFY_SIGNAL, > + TPL_NOTIFY, > + RttTimerTickRoutine, > + &RttTimerTick, > + &TimerEvent > + ); > if (EFI_ERROR (Status)) { > - // > - // The WinntGetTimerValue will return EFI_UNSUPPORTED. Set the > - // TimerPeriod by ourselves. > - // > - mCurrentTick +=3D 1000000; > + return 0; > + } > + > + OldTpl =3D gBS->RaiseTPL (TPL_CALLBACK); Status =3D gBS->SetTimer ( > + TimerEvent, > + TimerPeriodic, > + TICKS_PER_MS > + ); > + if (EFI_ERROR (Status)) { > + gBS->CloseEvent (TimerEvent); > + return 0; > + } > + > + while (RttTimerTick < 10) { > + gBS->Stall (STALL_1_MILLI_SECOND); > + ++StallCounter; > } > - > - return mCurrentTick; > -} >=20 > + gBS->RestoreTPL (OldTpl); > + > + gBS->SetTimer (TimerEvent, TimerCancel, 0); gBS->CloseEvent > + (TimerEvent); > + > + return StallCounter / RttTimerTick; > +} >=20 > /** > - Get and calculate the frequency in ticks/ms. > - The result is saved in the global variable mFrequency > + Initialize the timer event for RTT (round trip time). >=20 > - @retval EFI_SUCCESS Calculated the frequency successfully. > - @retval Others Failed to calculate the frequency. > + @param[in] Private The pointer to PING_PRIVATE_DATA. > + > + @retval EFI_SUCCESS RTT timer is started. > + @retval Others Failed to start the RTT timer. >=20 > **/ > EFI_STATUS > -GetFrequency ( > - VOID > +PingInitRttTimer ( > + PING_PRIVATE_DATA *Private > ) > { > - EFI_STATUS Status; > - UINT64 CurrentTick; > - UINT64 TimerPeriod; > + EFI_STATUS Status; > + > + Private->TimerPeriod =3D GetTimerPeriod (); if (Private->TimerPeriod > + =3D=3D 0) { > + return EFI_ABORTED; > + } > + > + Private->RttTimerTick =3D 0; > + Status =3D gBS->CreateEvent ( > + EVT_TIMER | EVT_NOTIFY_SIGNAL, > + TPL_NOTIFY, > + RttTimerTickRoutine, > + &Private->RttTimerTick, > + &Private->RttTimer > + ); > + if (EFI_ERROR (Status)) { > + return Status; > + } >=20 > - Status =3D gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID = **) > &gCpu); > + Status =3D gBS->SetTimer ( > + Private->RttTimer, > + TimerPeriodic, > + TICKS_PER_MS > + ); > if (EFI_ERROR (Status)) { > + gBS->CloseEvent (Private->RttTimer); > return Status; > } >=20 > - Status =3D gCpu->GetTimerValue (gCpu, 0, &CurrentTick, &TimerPeriod); > + return EFI_SUCCESS; > +} >=20 > - if (EFI_ERROR (Status)) { > - TimerPeriod =3D DEFAULT_TIMER_PERIOD; > +/** > + Free RTT timer event resource. > + > + @param[in] Private The pointer to PING_PRIVATE_DATA. > + > +**/ > +VOID > +PingFreeRttTimer ( > + PING_PRIVATE_DATA *Private > + ) > +{ > + if (Private->RttTimer !=3D NULL) { > + gBS->SetTimer (Private->RttTimer, TimerCancel, 0); > + gBS->CloseEvent (Private->RttTimer); > } > +} >=20 > - // > - // The timer period is in femtosecond (1 femtosecond is 1e-15 second). > - // So 1e+12 is divided by timer period to produce the freq in ticks/ms= . > - // > - mFrequency =3D DivU64x64Remainder (1000000000000ULL, TimerPeriod, NULL= ); > +/** > + Read the current time. > + > + @param[in] Private The pointer to PING_PRIVATE_DATA. >=20 > - return EFI_SUCCESS; > + @retval the current tick value. > +**/ > +UINT32 > +ReadTime ( > + PING_PRIVATE_DATA *Private > + ) > +{ > + return Private->RttTimerTick; > } >=20 > /** > Calculate a duration in ms. >=20 > - @param[in] Begin The start point of time. > - @param[in] End The end point of time. > + @param[in] Private The pointer to PING_PRIVATE_DATA. > + @param[in] Begin The start point of time. > + @param[in] End The end point of time. >=20 > @return The duration in ms. > @retval 0 The parameters were not valid. > **/ > -UINT64 > +UINT32 > CalculateTick ( > - IN UINT64 Begin, > - IN UINT64 End > + PING_PRIVATE_DATA *Private, > + IN UINT32 Begin, > + IN UINT32 End > ) > { > - if (End <=3D Begin) { > + if (End < Begin) { > return (0); > } > - return DivU64x64Remainder (End - Begin, mFrequency, NULL); > + > + return (End - Begin) * Private->TimerPeriod; > } >=20 > /** > @@ -448,8 +546,7 @@ Ping6OnEchoReplyReceived ( > PING_PRIVATE_DATA *Private; > ICMPX_ECHO_REQUEST_REPLY *Reply; > UINT32 PayLoad; > - UINT64 Rtt; > - CHAR8 Near; > + UINT32 Rtt; >=20 > Private =3D (PING_PRIVATE_DATA *) Context; >=20 > @@ -502,12 +599,7 @@ Ping6OnEchoReplyReceived ( > // > // Display statistics on this icmp6 echo reply packet. > // > - Rtt =3D CalculateTick (Reply->TimeStamp, ReadTime ()); > - if (Rtt !=3D 0) { > - Near =3D (CHAR8) '=3D'; > - } else { > - Near =3D (CHAR8) '<'; > - } > + Rtt =3D CalculateTick (Private, Reply->TimeStamp, ReadTime (Private))= ; >=20 > Private->RttSum +=3D Rtt; > Private->RttMin =3D Private->RttMin > Rtt ? Rtt : Private->RttMin; @@= - > 523,8 +615,8 @@ Ping6OnEchoReplyReceived ( > mDstString, > Reply->SequenceNum, > Private->IpChoice =3D=3D > PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private- > >RxToken.Packet.RxData)->Header->HopLimit:0, > - Near, > - Rtt > + Rtt, > + Rtt + Private->TimerPeriod > ); >=20 > ON_EXIT: > @@ -565,7 +657,7 @@ ON_EXIT: > PING_IPX_COMPLETION_TOKEN * > PingGenerateToken ( > IN PING_PRIVATE_DATA *Private, > - IN UINT64 TimeStamp, > + IN UINT32 TimeStamp, > IN UINT16 SequenceNum > ) > { > @@ -678,7 +770,7 @@ PingSendEchoRequest ( > return EFI_OUT_OF_RESOURCES; > } >=20 > - TxInfo->TimeStamp =3D ReadTime (); > + TxInfo->TimeStamp =3D ReadTime (Private); > TxInfo->SequenceNum =3D (UINT16) (Private->TxCount + 1); > TxInfo->Token =3D PingGenerateToken ( > Private, > @@ -784,7 +876,7 @@ Ping6OnTimerRoutine ( > // > NET_LIST_FOR_EACH_SAFE (Entry, NextEntry, &Private->TxList) { > TxInfo =3D BASE_CR (Entry, PING_ICMPX_TX_INFO, Link); > - Time =3D CalculateTick (TxInfo->TimeStamp, ReadTime ()); > + Time =3D CalculateTick (Private, TxInfo->TimeStamp, ReadTime > (Private)); >=20 > // > // Remove the timeout echo request from txlist. > @@ -1239,6 +1331,7 @@ Ping6DestroyIp6Instance ( > } > } >=20 > + > /** > The Ping Process. >=20 > @@ -1323,6 +1416,16 @@ ShellPing ( > ShellStatus =3D SHELL_ACCESS_DENIED; > goto ON_EXIT; > } > + > + // > + // Start a timer to calculate the RTT. > + // > + Status =3D PingInitRttTimer (Private); > + if (EFI_ERROR (Status)) { > + ShellStatus =3D SHELL_ACCESS_DENIED; > + goto ON_EXIT; > + } > + > // > // Create a ipv6 token to send the first icmp6 echo request packet. > // > @@ -1397,8 +1500,11 @@ ON_STAT: > STRING_TOKEN (STR_PING_RTT), > gShellNetwork1HiiHandle, > Private->RttMin, > + Private->RttMin + Private->TimerPeriod, > Private->RttMax, > - DivU64x64Remainder (Private->RttSum, (Private->RxCount - Private- > >FailedCount), NULL) > + Private->RttMax + Private->TimerPeriod, > + DivU64x64Remainder (Private->RttSum, (Private->RxCount - Private- > >FailedCount), NULL), > + DivU64x64Remainder (Private->RttSum, (Private->RxCount - > + Private->FailedCount), NULL) + Private->TimerPeriod > ); > } >=20 > @@ -1417,6 +1523,8 @@ ON_EXIT: > PingDestroyTxInfo (TxInfo, Private->IpChoice); > } >=20 > + PingFreeRttTimer (Private); > + > if (Private->Timer !=3D NULL) { > gBS->CloseEvent (Private->Timer); > } > @@ -1580,14 +1688,7 @@ ShellCommandRunPing ( > goto ON_EXIT; > } > } > - // > - // Get frequency to calculate the time from ticks. > - // > - Status =3D GetFrequency (); >=20 > - if (EFI_ERROR(Status)) { > - goto ON_EXIT; > - } > // > // Enter into ping process. > // > diff --git > a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Commands= L > ib.inf > b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Commands= L > ib.inf > index 92d47d1..25b2e14 100644 > --- > a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Commands= L > ib.inf > +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Com > +++ mandsLib.inf > @@ -1,7 +1,7 @@ > ## @file > # Provides shell network1 functions > # > -# Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved. > +# Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved. > +
> # > # This program and the accompanying materials # are licensed and made > available under the terms and conditions of the BSD License @@ -55,6 +55,= 7 > @@ >=20 > [Protocols] > gEfiCpuArchProtocolGuid ## CONSUMES > + gEfiTimerArchProtocolGuid > gEfiIp6ProtocolGuid ## SOMETIMES_CONSUMES > gEfiIp6ServiceBindingProtocolGuid ## SOMETIMES_CONSUMES > gEfiIp6ConfigProtocolGuid ## SOMETIMES_CONSUMES > diff --git > a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Commands= L > ib.uni > b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Commands= L > ib.uni > index c8ab64f..76b6188 100644 > --- > a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Commands= L > ib.uni > +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Com > +++ mandsLib.uni > @@ -56,9 +56,9 @@ > #string STR_PING_NOROUTE_FOUND #language en-US "There is no route > to the destination '%B%s%N' from the source '%B%s%N' was found.\r\n" > #string STR_PING_START #language en-US "Ping %s %d data > bytes.\r\n" > #string STR_PING_TIMEOUT #language en-US "Echo request > sequence %d timeout.\r\n" > -#string STR_PING_REPLY_INFO #language en-US "%d bytes from %s : > icmp_seq=3D%d ttl=3D%d time%c%dms\r\n" > +#string STR_PING_REPLY_INFO #language en-US "%d bytes from %s : > icmp_seq=3D%d ttl=3D%d time%d~%dms\r\n" > #string STR_PING_STAT #language en-US "\n%d packets > transmitted, %d received, %d%% packet loss, time %dms\r\n" > -#string STR_PING_RTT #language en-US "\nRtt(round trip > time) min=3D%dms max=3D%dms avg=3D%dms\r\n" > +#string STR_PING_RTT #language en-US "\nRtt(round trip > time) min=3D%d~%dms max=3D%d~%dms avg=3D%d~%dms\r\n" >=20 > #string STR_IFCONFIG_UNSUPPORTED_OPTION #language en-US "The > option '%H%s%N' is unsupported now.\n" > #string STR_IFCONFIG_LACK_OPTION #language en-US "Flags > lack.\n" > -- > 2.7.4.windows.1 >=20 > _______________________________________________ > 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