public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch] ShellPkg: update ping to use timer service instead of timer arch protocol .
@ 2016-10-17  6:21 Fu Siyuan
  2016-10-19 10:37 ` Hegde, Nagaraj P
       [not found] ` <B1FF2E9001CE9041BD10B825821D5BC58A831439@shsmsx102.ccr.corp.intel.com>
  0 siblings, 2 replies; 5+ messages in thread
From: Fu Siyuan @ 2016-10-17  6:21 UTC (permalink / raw)
  To: edk2-devel; +Cc: Ni Ruiyu, Ye Ting, Zhang Lubo

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



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Patch] ShellPkg: update ping to use timer service instead of timer arch protocol .
  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
       [not found] ` <B1FF2E9001CE9041BD10B825821D5BC58A831439@shsmsx102.ccr.corp.intel.com>
  1 sibling, 1 reply; 5+ messages in thread
From: Hegde, Nagaraj P @ 2016-10-19 10:37 UTC (permalink / raw)
  To: Fu Siyuan, edk2-devel@lists.01.org; +Cc: Ni Ruiyu, Ye Ting, Zhang Lubo

Hi Siyuan,

Few comments:

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 time as 0~2ms for 0.2 ms. Why not use "." itself? Any reason for using tild (~)?
3. We print ttl=%d in STR_PING_REPLY_INFO, which is always 0 for IPv4 Ping. We need to fix that.

I have tested your code and looks fine.

Regards,
Nagaraj.

-----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 <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/UefiShellNetwork1Com
+++ mandsLib.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/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=%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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Patch] ShellPkg: update ping to use timer service instead of timer arch protocol .
  2016-10-19 10:37 ` Hegde, Nagaraj P
@ 2016-10-20  3:02   ` Fu, Siyuan
  2016-10-20  4:12     ` Hegde, Nagaraj P
  0 siblings, 1 reply; 5+ messages in thread
From: Fu, Siyuan @ 2016-10-20  3:02 UTC (permalink / raw)
  To: Hegde, Nagaraj P, edk2-devel@lists.01.org
  Cc: Ni, Ruiyu, Ye, Ting, Zhang,  Lubo

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 the system timer period, see the new added GetTimerPeriod() function. It first 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 the 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 service 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 standard way to get the real time period only by UEFI service. It's a little tricky, 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 update. 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 <siyuan.fu@intel.com>; 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: Re: [edk2] [Patch] ShellPkg: update ping to use timer service
> instead of timer arch protocol .
> 
> Hi Siyuan,
> 
> Few comments:
> 
> 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 time
> as 0~2ms for 0.2 ms. Why not use "." itself? Any reason for using tild (~)?
> 3. We print ttl=%d in STR_PING_REPLY_INFO, which is always 0 for IPv4 Ping.
> We need to fix that.
> 
> I have tested your code and looks fine.
> 
> Regards,
> Nagaraj.
> 
> -----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 <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/UefiShellNetwork1CommandsL
> ib.inf
> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsL
> ib.inf
> index 92d47d1..25b2e14 100644
> ---
> a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsL
> 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. <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/UefiShellNetwork1CommandsL
> ib.uni
> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsL
> ib.uni
> index c8ab64f..76b6188 100644
> ---
> a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsL
> 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=%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
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch] ShellPkg: update ping to use timer service instead of timer arch protocol .
  2016-10-20  3:02   ` Fu, Siyuan
@ 2016-10-20  4:12     ` Hegde, Nagaraj P
  0 siblings, 0 replies; 5+ messages in thread
From: Hegde, Nagaraj P @ 2016-10-20  4:12 UTC (permalink / raw)
  To: Fu, Siyuan, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu, Ye, Ting, Zhang,  Lubo

Thanks Siyuan.
Reviewed-by: Hegde, Nagaraj P <nagaraj-p.hegde@hpe.com>

Regards,
Nagaraj.

-----Original Message-----
From: Fu, Siyuan [mailto:siyuan.fu@intel.com] 
Sent: Thursday, October 20, 2016 8:32 AM
To: Hegde, Nagaraj P <nagaraj-p.hegde@hpe.com>; 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: RE: [edk2] [Patch] ShellPkg: update ping to use timer service instead of timer arch protocol .

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 the system timer period, see the new added GetTimerPeriod() function. It first 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 the 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 service 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 standard way to get the real time period only by UEFI service. It's a little tricky, 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 update. 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 <siyuan.fu@intel.com>; 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: Re: [edk2] [Patch] ShellPkg: update ping to use timer service 
> instead of timer arch protocol .
> 
> Hi Siyuan,
> 
> Few comments:
> 
> 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 
> time as 0~2ms for 0.2 ms. Why not use "." itself? Any reason for using tild (~)?
> 3. We print ttl=%d in STR_PING_REPLY_INFO, which is always 0 for IPv4 Ping.
> We need to fix that.
> 
> I have tested your code and looks fine.
> 
> Regards,
> Nagaraj.
> 
> -----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 <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/UefiShellNetwork1Comma
> ndsL
> ib.inf
> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Comma
> ndsL
> ib.inf
> index 92d47d1..25b2e14 100644
> ---
> a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Comma
> ndsL
> ib.inf
> +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1C
> +++ om
> +++ mandsLib.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/UefiShellNetwork1Comma
> ndsL
> ib.uni
> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Comma
> ndsL
> ib.uni
> index c8ab64f..76b6188 100644
> ---
> a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Comma
> ndsL
> ib.uni
> +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1C
> +++ om
> +++ 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=%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
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch] ShellPkg: update ping to use timer service instead of timer arch protocol .
       [not found] ` <B1FF2E9001CE9041BD10B825821D5BC58A831439@shsmsx102.ccr.corp.intel.com>
@ 2016-10-25  7:05   ` Ni, Ruiyu
  0 siblings, 0 replies; 5+ messages in thread
From: Ni, Ruiyu @ 2016-10-25  7:05 UTC (permalink / raw)
  To: edk2-devel@lists.01.org; +Cc: Fu, Siyuan



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


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-10-25  7:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox