From: "Ni, Ruiyu" <ruiyu.ni@intel.com>
To: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Fu, Siyuan" <siyuan.fu@intel.com>
Subject: Re: [Patch] ShellPkg: update ping to use timer service instead of timer arch protocol .
Date: Tue, 25 Oct 2016 07:05:08 +0000 [thread overview]
Message-ID: <734D49CCEBEEF84792F5B80ED585239D58E3F86E@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <B1FF2E9001CE9041BD10B825821D5BC58A831439@shsmsx102.ccr.corp.intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
>
>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Fu Siyuan
>Sent: Monday, October 17, 2016 2:22 PM
>To: edk2-devel@lists.01.org
>Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Ye, Ting <ting.ye@intel.com>; Zhang, Lubo <lubo.zhang@intel.com>
>Subject: [edk2] [Patch] ShellPkg: update ping to use timer service instead of timer arch protocol .
>
>This patch update the shell ping command to use timer service to calculate the
>RTT time, instead of using the timer arch protocol.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
>Cc: Ni Ruiyu <ruiyu.ni@intel.com>
>Cc: Ye Ting <ting.ye@intel.com>
>Cc: Zhang Lubo <lubo.zhang@intel.com>
>---
> .../Library/UefiShellNetwork1CommandsLib/Ping.c | 233 +++++++++++++++------
> .../UefiShellNetwork1CommandsLib.inf | 3 +-
> .../UefiShellNetwork1CommandsLib.uni | 4 +-
> 3 files changed, 171 insertions(+), 69 deletions(-)
>
>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;
>
>@@ -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
>
> #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;
>
>+ 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[] = {
> //
> STATIC CONST CHAR16 *mDstString;
> STATIC CONST CHAR16 *mSrcString;
>-STATIC UINT64 mFrequency = 0;
> EFI_CPU_ARCH_PROTOCOL *gCpu = NULL;
>
> /**
>- Read the current time.
>+ RTT timer tick routine.
>+
>+ @param[in] Event A EFI_EVENT type event.
>+ @param[in] Context The pointer to Context.
>
>- @retval the current tick value.
> **/
>-UINT64
>-ReadTime (
>+VOID
>+EFIAPI
>+RttTimerTickRoutine (
>+ IN EFI_EVENT Event,
>+ IN VOID *Context
>+ )
>+{
>+ UINT32 *RttTimerTick;
>+
>+ RttTimerTick = (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;
>
>- ASSERT (gCpu != NULL);
>+ RttTimerTick = 0;
>+ StallCounter = 0;
>
>- Status = gCpu->GetTimerValue (gCpu, 0, &mCurrentTick, &TimerPeriod);
>+ Status = 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 += 1000000;
>+ return 0;
>+ }
>+
>+ OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
>+ Status = 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;
>-}
>
>+ gBS->RestoreTPL (OldTpl);
>+
>+ gBS->SetTimer (TimerEvent, TimerCancel, 0);
>+ gBS->CloseEvent (TimerEvent);
>+
>+ return StallCounter / RttTimerTick;
>+}
>
> /**
>- 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).
>
>- @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.
>
> **/
> EFI_STATUS
>-GetFrequency (
>- VOID
>+PingInitRttTimer (
>+ PING_PRIVATE_DATA *Private
> )
> {
>- EFI_STATUS Status;
>- UINT64 CurrentTick;
>- UINT64 TimerPeriod;
>+ EFI_STATUS Status;
>+
>+ Private->TimerPeriod = GetTimerPeriod ();
>+ if (Private->TimerPeriod == 0) {
>+ return EFI_ABORTED;
>+ }
>+
>+ Private->RttTimerTick = 0;
>+ Status = gBS->CreateEvent (
>+ EVT_TIMER | EVT_NOTIFY_SIGNAL,
>+ TPL_NOTIFY,
>+ RttTimerTickRoutine,
>+ &Private->RttTimerTick,
>+ &Private->RttTimer
>+ );
>+ if (EFI_ERROR (Status)) {
>+ return Status;
>+ }
>
>- Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **) &gCpu);
>+ Status = gBS->SetTimer (
>+ Private->RttTimer,
>+ TimerPeriodic,
>+ TICKS_PER_MS
>+ );
> if (EFI_ERROR (Status)) {
>+ gBS->CloseEvent (Private->RttTimer);
> return Status;
> }
>
>- Status = gCpu->GetTimerValue (gCpu, 0, &CurrentTick, &TimerPeriod);
>+ return EFI_SUCCESS;
>+}
>
>- if (EFI_ERROR (Status)) {
>- TimerPeriod = 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 != NULL) {
>+ gBS->SetTimer (Private->RttTimer, TimerCancel, 0);
>+ gBS->CloseEvent (Private->RttTimer);
> }
>+}
>
>- //
>- // 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 = DivU64x64Remainder (1000000000000ULL, TimerPeriod, NULL);
>+/**
>+ Read the current time.
>+
>+ @param[in] Private The pointer to PING_PRIVATE_DATA.
>
>- return EFI_SUCCESS;
>+ @retval the current tick value.
>+**/
>+UINT32
>+ReadTime (
>+ PING_PRIVATE_DATA *Private
>+ )
>+{
>+ return Private->RttTimerTick;
> }
>
> /**
> Calculate a duration in ms.
>
>- @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.
>
> @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 <= Begin) {
>+ if (End < Begin) {
> return (0);
> }
>- return DivU64x64Remainder (End - Begin, mFrequency, NULL);
>+
>+ return (End - Begin) * Private->TimerPeriod;
> }
>
> /**
>@@ -448,8 +546,7 @@ Ping6OnEchoReplyReceived (
> PING_PRIVATE_DATA *Private;
> ICMPX_ECHO_REQUEST_REPLY *Reply;
> UINT32 PayLoad;
>- UINT64 Rtt;
>- CHAR8 Near;
>+ UINT32 Rtt;
>
> Private = (PING_PRIVATE_DATA *) Context;
>
>@@ -502,12 +599,7 @@ Ping6OnEchoReplyReceived (
> //
> // Display statistics on this icmp6 echo reply packet.
> //
>- Rtt = CalculateTick (Reply->TimeStamp, ReadTime ());
>- if (Rtt != 0) {
>- Near = (CHAR8) '=';
>- } else {
>- Near = (CHAR8) '<';
>- }
>+ Rtt = CalculateTick (Private, Reply->TimeStamp, ReadTime (Private));
>
> Private->RttSum += Rtt;
> Private->RttMin = Private->RttMin > Rtt ? Rtt : Private->RttMin;
>@@ -523,8 +615,8 @@ Ping6OnEchoReplyReceived (
> mDstString,
> Reply->SequenceNum,
> Private->IpChoice ==
>PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private->RxToken.Packet.RxData)->Header->HopLimit:0,
>- Near,
>- Rtt
>+ Rtt,
>+ Rtt + Private->TimerPeriod
> );
>
> 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;
> }
>
>- TxInfo->TimeStamp = ReadTime ();
>+ TxInfo->TimeStamp = ReadTime (Private);
> TxInfo->SequenceNum = (UINT16) (Private->TxCount + 1);
> TxInfo->Token = PingGenerateToken (
> Private,
>@@ -784,7 +876,7 @@ Ping6OnTimerRoutine (
> //
> NET_LIST_FOR_EACH_SAFE (Entry, NextEntry, &Private->TxList) {
> TxInfo = BASE_CR (Entry, PING_ICMPX_TX_INFO, Link);
>- Time = CalculateTick (TxInfo->TimeStamp, ReadTime ());
>+ Time = CalculateTick (Private, TxInfo->TimeStamp, ReadTime (Private));
>
> //
> // Remove the timeout echo request from txlist.
>@@ -1239,6 +1331,7 @@ Ping6DestroyIp6Instance (
> }
> }
>
>+
> /**
> The Ping Process.
>
>@@ -1323,6 +1416,16 @@ ShellPing (
> ShellStatus = SHELL_ACCESS_DENIED;
> goto ON_EXIT;
> }
>+
>+ //
>+ // Start a timer to calculate the RTT.
>+ //
>+ Status = PingInitRttTimer (Private);
>+ if (EFI_ERROR (Status)) {
>+ ShellStatus = 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
> );
> }
>
>@@ -1417,6 +1523,8 @@ ON_EXIT:
> PingDestroyTxInfo (TxInfo, Private->IpChoice);
> }
>
>+ PingFreeRttTimer (Private);
>+
> if (Private->Timer != NULL) {
> gBS->CloseEvent (Private->Timer);
> }
>@@ -1580,14 +1688,7 @@ ShellCommandRunPing (
> goto ON_EXIT;
> }
> }
>- //
>- // Get frequency to calculate the time from ticks.
>- //
>- Status = GetFrequency ();
>
>- if (EFI_ERROR(Status)) {
>- goto ON_EXIT;
>- }
> //
> // Enter into ping process.
> //
>diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
>b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
>index 92d47d1..25b2e14 100644
>--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
>+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
>@@ -1,7 +1,7 @@
> ## @file
> # Provides shell network1 functions
> #
>-# Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved. <BR>
>+# Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved. <BR>
> #
> # This program and the accompanying materials
> # are licensed and made available under the terms and conditions of the BSD License
>@@ -55,6 +55,7 @@
>
> [Protocols]
> gEfiCpuArchProtocolGuid ## CONSUMES
>+ gEfiTimerArchProtocolGuid
> gEfiIp6ProtocolGuid ## SOMETIMES_CONSUMES
> gEfiIp6ServiceBindingProtocolGuid ## SOMETIMES_CONSUMES
> gEfiIp6ConfigProtocolGuid ## SOMETIMES_CONSUMES
>diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
>b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
>index c8ab64f..76b6188 100644
>--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
>+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.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=%d ttl=%d time%c%dms\r\n"
>+#string STR_PING_REPLY_INFO #language en-US "%d bytes from %s : icmp_seq=%d ttl=%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=%dms max=%dms avg=%dms\r\n"
>+#string STR_PING_RTT #language en-US "\nRtt(round trip time) min=%d~%dms max=%d~%dms
>avg=%d~%dms\r\n"
>
> #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
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
prev parent reply other threads:[~2016-10-25 7:05 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-17 6:21 [Patch] ShellPkg: update ping to use timer service instead of timer arch protocol Fu Siyuan
2016-10-19 10:37 ` Hegde, Nagaraj P
2016-10-20 3:02 ` Fu, Siyuan
2016-10-20 4:12 ` Hegde, Nagaraj P
[not found] ` <B1FF2E9001CE9041BD10B825821D5BC58A831439@shsmsx102.ccr.corp.intel.com>
2016-10-25 7:05 ` Ni, Ruiyu [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=734D49CCEBEEF84792F5B80ED585239D58E3F86E@SHSMSX104.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox