public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Fu, Siyuan" <siyuan.fu@intel.com>
To: "Zhang, Lubo" <lubo.zhang@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Ni, Ruiyu" <ruiyu.ni@intel.com>, "Ye, Ting" <ting.ye@intel.com>
Subject: Re: [patch] ShellPkg: update ping6 to use timer service instead of timer arch protocol .
Date: Wed, 9 Nov 2016 02:28:23 +0000	[thread overview]
Message-ID: <B1FF2E9001CE9041BD10B825821D5BC58A838725@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <1478603284-27560-1-git-send-email-lubo.zhang@intel.com>

Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>


> -----Original Message-----
> From: Zhang, Lubo
> Sent: Tuesday, November 8, 2016 7:08 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu,
> Siyuan <siyuan.fu@intel.com>
> Subject: [patch] ShellPkg: update ping6 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: Zhang Lubo <lubo.zhang@intel.com>
> Cc: Ni Ruiyu <ruiyu.ni@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> ---
>  .../Library/UefiShellNetwork2CommandsLib/Ping6.c   | 241 ++++++++++++++--
> -----
>  .../UefiShellNetwork2CommandsLib.inf               |   1 +
>  .../UefiShellNetwork2CommandsLib.uni               |   4 +-
>  3 files changed, 170 insertions(+), 76 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
> b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
> index 90a2604..a30c064 100644
> --- a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
> +++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
> @@ -17,45 +17,44 @@
> 
>  #define PING6_DEFAULT_TIMEOUT      5000
>  #define PING6_MAX_SEND_NUMBER      10000
>  #define PING6_MAX_BUFFER_SIZE      32768
>  #define PING6_ONE_SECOND           10000000
> -
> -//
> -// A similar amount of time that passes in femtoseconds
> -// for each increment of TimerValue. It is for NT32 only.
> -//
> -#define NTTIMERPERIOD    358049
> +#define STALL_1_MILLI_SECOND  1000
> 
>  #pragma pack(1)
> 
>  typedef struct _ICMP6_ECHO_REQUEST_REPLY {
>    UINT8                       Type;
>    UINT8                       Code;
>    UINT16                      Checksum;
>    UINT16                      Identifier;
>    UINT16                      SequenceNum;
> -  UINT64                      TimeStamp;
> +  UINT32                      TimeStamp;
>    UINT8                       Data[1];
>  } ICMP6_ECHO_REQUEST_REPLY;
> 
>  #pragma pack()
> 
>  typedef struct _PING6_ICMP6_TX_INFO {
>    LIST_ENTRY                  Link;
>    UINT16                      SequenceNum;
> -  UINT64                      TimeStamp;
> +  UINT32                      TimeStamp;
>    EFI_IP6_COMPLETION_TOKEN    *Token;
>  } PING6_ICMP6_TX_INFO;
> 
>  typedef struct _PING6_PRIVATE_DATA {
>    EFI_HANDLE                  ImageHandle;
>    EFI_HANDLE                  NicHandle;
>    EFI_HANDLE                  Ip6ChildHandle;
>    EFI_IP6_PROTOCOL            *Ip6;
>    EFI_EVENT                   Timer;
> 
> +  UINT32                      TimerPeriod;
> +  UINT32                      RttTimerTick;
> +  EFI_EVENT                   RttTimer;
> +
>    EFI_STATUS                  Status;
>    LIST_ENTRY                  TxList;
>    EFI_IP6_COMPLETION_TOKEN    RxToken;
>    UINT16                      RxCount;
>    UINT16                      TxCount;
> @@ -97,100 +96,193 @@ SHELL_PARAM_ITEM    Ping6ParamList[] = {
>  //
>  // Global Variables in Ping6 application.
>  //
>  CONST CHAR16            *mIp6DstString;
>  CONST CHAR16            *mIp6SrcString;
> -UINT64                  mFrequency = 0;
> -UINT64                  mIp6CurrentTick = 0;
>  EFI_CPU_ARCH_PROTOCOL   *Cpu = NULL;
> 
> +/**
> +  RTT timer tick routine.
> 
> +  @param[in]    Event    A EFI_EVENT type event.
> +  @param[in]    Context  The pointer to Context.
> +
> +**/
> +VOID
> +EFIAPI
> +Ping6RttTimerTickRoutine (
> +  IN EFI_EVENT    Event,
> +  IN VOID         *Context
> +  )
> +{
> +  UINT32     *RttTimerTick;
> +
> +  RttTimerTick = (UINT32*) Context;
> +  (*RttTimerTick)++;
> +}
> 
>  /**
> -  Reads and returns the current value of the Time.
> +  Get the timer period of the system.
> +
> +  This function tries to get the system timer period by creating
> +  an 1ms period timer.
> 
> -  @return The current tick value.
> +  @return     System timer period in MS, or 0 if operation failed.
> 
>  **/
> -UINT64
> -Ping6ReadTime ()
> +UINT32
> +Ping6GetTimerPeriod(
> +  VOID
> +  )
>  {
> -  UINT64                 TimerPeriod;
> -  EFI_STATUS             Status;
> +  EFI_STATUS                 Status;
> +  UINT32                     RttTimerTick;
> +  EFI_EVENT                  TimerEvent;
> +  UINT32                     StallCounter;
> +  EFI_TPL                    OldTpl;
> 
> -  ASSERT (Cpu != NULL);
> +  RttTimerTick = 0;
> +  StallCounter   = 0;
> 
> -  Status = Cpu->GetTimerValue (Cpu, 0, &mIp6CurrentTick, &TimerPeriod);
> +  Status = gBS->CreateEvent (
> +                  EVT_TIMER | EVT_NOTIFY_SIGNAL,
> +                  TPL_NOTIFY,
> +                  Ping6RttTimerTickRoutine,
> +                  &RttTimerTick,
> +                  &TimerEvent
> +                  );
>    if (EFI_ERROR (Status)) {
> -    //
> -    // The WinntGetTimerValue will return EFI_UNSUPPORTED. Set the
> -    // TimerPeriod by ourselves.
> -    //
> -    mIp6CurrentTick += 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 mIp6CurrentTick;
> +  gBS->RestoreTPL (OldTpl);
> +
> +  gBS->SetTimer (TimerEvent, TimerCancel, 0);
> +  gBS->CloseEvent (TimerEvent);
> +
> +  return StallCounter / RttTimerTick;
>  }
> 
> +
>  /**
> -  Get and calculate the frequency in tick/ms.
> -  The result is saved in the globle 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 PING6_PRIVATE_DATA.
> +
> +  @retval EFI_SUCCESS      RTT timer is started.
> +  @retval Others           Failed to start the RTT timer.
> 
>  **/
>  EFI_STATUS
> -Ping6GetFrequency (
> -  VOID
> +Ping6InitRttTimer (
> +  IN  PING6_PRIVATE_DATA      *Private
>    )
>  {
> -  EFI_STATUS               Status;
> -  UINT64                   CurrentTick;
> -  UINT64                   TimerPeriod;
> -
> -  Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)
> &Cpu);
> +  EFI_STATUS                 Status;
> 
> +  Private->TimerPeriod = Ping6GetTimerPeriod ();
> +  if (Private->TimerPeriod == 0) {
> +    return EFI_ABORTED;
> +  }
> +
> +  Private->RttTimerTick = 0;
> +  Status = gBS->CreateEvent (
> +                  EVT_TIMER | EVT_NOTIFY_SIGNAL,
> +                  TPL_NOTIFY,
> +                  Ping6RttTimerTickRoutine,
> +                  &Private->RttTimerTick,
> +                  &Private->RttTimer
> +                  );
>    if (EFI_ERROR (Status)) {
>      return Status;
>    }
> 
> -  Status = Cpu->GetTimerValue (Cpu, 0, &CurrentTick, &TimerPeriod);
> -
> +  Status = gBS->SetTimer (
> +                  Private->RttTimer,
> +                  TimerPeriodic,
> +                  TICKS_PER_MS
> +                  );
>    if (EFI_ERROR (Status)) {
> -    //
> -    // For NT32 Simulator only. 358049 is a similar value to keep timer
> granularity.
> -    // Set the timer period by ourselves.
> -    //
> -    TimerPeriod = (UINT64) NTTIMERPERIOD;
> +    gBS->CloseEvent (Private->RttTimer);
> +    return Status;
>    }
> -  //
> -  // 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 tick/ms.
> -  //
> -  mFrequency = DivU64x64Remainder (1000000000000ULL, TimerPeriod, NULL);
> 
>    return EFI_SUCCESS;
> +
> +}
> +
> +/**
> +  Free RTT timer event resource.
> +
> +  @param[in]    Private    The pointer to PING6_PRIVATE_DATA.
> +
> +**/
> +VOID
> +Ping6FreeRttTimer (
> +  IN  PING6_PRIVATE_DATA      *Private
> +  )
> +{
> +  if (Private->RttTimer != NULL) {
> +    gBS->SetTimer (Private->RttTimer, TimerCancel, 0);
> +    gBS->CloseEvent (Private->RttTimer);
> +  }
> +}
> +
> +/**
> +  Read the current time.
> +
> +  @param[in]    Private    The pointer to PING6_PRIVATE_DATA.
> +
> +  @retval the current tick value.
> +**/
> +UINT32
> +Ping6ReadTime (
> +  IN  PING6_PRIVATE_DATA      *Private
> +  )
> +{
> +  return Private->RttTimerTick;
>  }
> 
>  /**
>    Get and calculate the duration in ms.
> 
> +  @param[in]  Private  The pointer to PING6_PRIVATE_DATA.
>    @param[in]  Begin    The start point of time.
>    @param[in]  End      The end point of time.
> 
>    @return The duration in ms.
> 
>  **/
> -UINT64
> +UINT32
>  Ping6CalculateTick (
> -  IN UINT64    Begin,
> -  IN UINT64    End
> +  IN PING6_PRIVATE_DATA      *Private,
> +  IN UINT32    Begin,
> +  IN UINT32    End
>    )
>  {
> -  ASSERT (End > Begin);
> -  return DivU64x64Remainder (End - Begin, mFrequency, NULL);
> +  if (End < Begin) {
> +    return (0);
> +  }
> +
> +  return (End - Begin) * Private->TimerPeriod;
> +
>  }
> 
>  /**
>    Destroy IPING6_ICMP6_TX_INFO, and recollect the memory.
> 
> @@ -310,12 +402,11 @@ Ping6OnEchoReplyReceived6 (
>    PING6_PRIVATE_DATA          *Private;
>    EFI_IP6_COMPLETION_TOKEN    *RxToken;
>    EFI_IP6_RECEIVE_DATA        *RxData;
>    ICMP6_ECHO_REQUEST_REPLY    *Reply;
>    UINT32                      PayLoad;
> -  UINT64                      Rtt;
> -  CHAR8                       Near;
> +  UINT32                      Rtt;
> 
>    Private = (PING6_PRIVATE_DATA *) Context;
> 
>    if (Private->Status == EFI_ABORTED) {
>      return;
> @@ -350,16 +441,11 @@ Ping6OnEchoReplyReceived6 (
>      goto ON_EXIT;
>    }
>    //
>    // Display statistics on this icmp6 echo reply packet.
>    //
> -  Rtt  = Ping6CalculateTick (Reply->TimeStamp, Ping6ReadTime ());
> -  if (Rtt != 0) {
> -    Near = (CHAR8) '=';
> -  } else {
> -    Near = (CHAR8) '<';
> -  }
> +  Rtt  = Ping6CalculateTick (Private, Reply->TimeStamp, Ping6ReadTime
> (Private));
> 
>    Private->RttSum += Rtt;
>    Private->RttMin  = Private->RttMin > Rtt ? Rtt : Private->RttMin;
>    Private->RttMax  = Private->RttMax < Rtt ? Rtt : Private->RttMax;
> 
> @@ -371,12 +457,12 @@ Ping6OnEchoReplyReceived6 (
>      gShellNetwork2HiiHandle,
>      PayLoad,
>      mIp6DstString,
>      Reply->SequenceNum,
>      RxData->Header->HopLimit,
> -    Near,
> -    Rtt
> +    Rtt,
> +    Rtt + Private->TimerPeriod
>      );
> 
>  ON_EXIT:
> 
>    if (Private->RxCount < Private->SendNum) {
> @@ -413,11 +499,11 @@ ON_EXIT:
> 
>  **/
>  EFI_IP6_COMPLETION_TOKEN *
>  Ping6GenerateToken (
>    IN PING6_PRIVATE_DATA    *Private,
> -  IN UINT64                TimeStamp,
> +  IN UINT32                TimeStamp,
>    IN UINT16                SequenceNum
>    )
>  {
>    EFI_STATUS                  Status;
>    EFI_IP6_COMPLETION_TOKEN    *Token;
> @@ -511,11 +597,11 @@ Ping6SendEchoRequest (
> 
>    if (TxInfo == NULL) {
>      return EFI_OUT_OF_RESOURCES;
>    }
> 
> -  TxInfo->TimeStamp   = Ping6ReadTime ();
> +  TxInfo->TimeStamp   = Ping6ReadTime (Private);
>    TxInfo->SequenceNum = (UINT16) (Private->TxCount + 1);
> 
>    TxInfo->Token       = Ping6GenerateToken (
>                            Private,
>                            TxInfo->TimeStamp,
> @@ -613,11 +699,11 @@ Ping6OnTimerRoutine6 (
>    //
>    // Check whether any icmp6 echo request in the list timeout.
>    //
>    NET_LIST_FOR_EACH_SAFE (Entry, NextEntry, &Private->TxList) {
>      TxInfo = BASE_CR (Entry, PING6_ICMP6_TX_INFO, Link);
> -    Time   = Ping6CalculateTick (TxInfo->TimeStamp, Ping6ReadTime ());
> +    Time   = Ping6CalculateTick (Private, TxInfo->TimeStamp,
> Ping6ReadTime (Private));
> 
>      //
>      // Remove the timeout echo request from txlist.
>      //
>      if (Time > PING6_DEFAULT_TIMEOUT) {
> @@ -1013,10 +1099,20 @@ ShellPing6 (
> 
>    if (EFI_ERROR (Status)) {
>      ShellStatus = SHELL_ACCESS_DENIED;
>      goto ON_EXIT;
>    }
> +
> +  //
> +  // Start a timer to calculate the RTT.
> +  //
> +  Status = Ping6InitRttTimer (Private);
> +  if (EFI_ERROR (Status)) {
> +    ShellStatus = SHELL_ACCESS_DENIED;
> +    goto ON_EXIT;
> +  }
> +
>    //
>    // Create a ipv6 token to send the first icmp6 echo request packet.
>    //
>    Status = Ping6SendEchoRequest (Private);
>    //
> @@ -1091,12 +1187,15 @@ ON_STAT:
>        -1,
>        NULL,
>        STRING_TOKEN (STR_PING6_RTT),
>        gShellNetwork2HiiHandle,
>        Private->RttMin,
> +      Private->RttMin + Private->TimerPeriod,
>        Private->RttMax,
> -      DivU64x64Remainder (Private->RttSum, Private->RxCount, NULL)
> +      Private->RttMax + Private->TimerPeriod,
> +      DivU64x64Remainder (Private->RttSum, Private->RxCount, NULL),
> +      DivU64x64Remainder (Private->RttSum, Private->RxCount, NULL) +
> Private->TimerPeriod
>        );
>    }
> 
>  ON_EXIT:
> 
> @@ -1110,10 +1209,12 @@ ON_EXIT:
> 
>        RemoveEntryList (&TxInfo->Link);
>        Ping6DestroyTxInfo (TxInfo);
>      }
> 
> +    Ping6FreeRttTimer (Private);
> +
>      if (Private->Timer != NULL) {
>        gBS->CloseEvent (Private->Timer);
>      }
> 
>      if (Private->Ip6 != NULL) {
> @@ -1246,19 +1347,11 @@ ShellCommandRunPing6 (
>        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_PING6_INVALID_IP),
> gShellNetwork2HiiHandle, ValueStr);
>        ShellStatus = SHELL_INVALID_PARAMETER;
>        goto ON_EXIT;
>      }
>    }
> -  //
> -  // Get frequency to calculate the time from ticks.
> -  //
> -  Status = Ping6GetFrequency ();
> 
> -  if (EFI_ERROR(Status)) {
> -    ShellStatus = SHELL_ACCESS_DENIED;
> -    goto ON_EXIT;
> -  }
>    //
>    // Enter into ping6 process.
>    //
>    ShellStatus = ShellPing6 (
>                ImageHandle,
> diff --git
> a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsL
> ib.inf
> b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsL
> ib.inf
> index 426efcc..8f253d2 100644
> ---
> a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsL
> ib.inf
> +++
> b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsL
> ib.inf
> @@ -53,10 +53,11 @@
>  [Pcd]
>    gEfiShellPkgTokenSpaceGuid.PcdShellProfileMask ## CONSUMES
> 
>  [Protocols]
>    gEfiCpuArchProtocolGuid                       ## CONSUMES
> +  gEfiTimerArchProtocolGuid
>    gEfiIp6ProtocolGuid                           ## SOMETIMES_CONSUMES
>    gEfiIp6ServiceBindingProtocolGuid             ## SOMETIMES_CONSUMES
>    gEfiIp6ConfigProtocolGuid                     ## SOMETIMES_CONSUMES
> 
>  [Guids]
> diff --git
> a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsL
> ib.uni
> b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsL
> ib.uni
> index 89e8e37..c3445bb 100644
> ---
> a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsL
> ib.uni
> +++
> b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsL
> ib.uni
> @@ -36,13 +36,13 @@
>  #string STR_PING6_SEND_REQUEST             #language en-US  "Echo request
> sequence %d fails.\r\n"
>  #string STR_PING6_CONFIGD_NIC_NF           #language en-US  "%Ping6: No
> configured interfaces were found.\r\n"
>  #string STR_PING6_NOSOURCE_INDOMAIN        #language en-US  "No sources
> in %s's multicast domain.\r\n"
>  #string STR_PING6_START                    #language en-US  "Ping %s %d
> data bytes\r\n"
>  #string STR_PING6_TIMEOUT                  #language en-US  "Echo request
> sequence %d timeout.\r\n"
> -#string STR_PING6_REPLY_INFO               #language en-US  "%d bytes
> from %s : icmp_seq=%d ttl=%d time%c%dms\r\n"
> +#string STR_PING6_REPLY_INFO               #language en-US  "%d bytes
> from %s : icmp_seq=%d ttl=%d time%d~%dms\r\n"
>  #string STR_PING6_STAT                     #language en-US  "\n%d packets
> transmitted, %d received, %d%% packet loss, time %dms\r\n"
> -#string STR_PING6_RTT                      #language en-US  "\nRtt(round
> trip time) min=%dms max=%dms avg=%dms\r\n"
> +#string STR_PING6_RTT                      #language en-US  "\nRtt(round
> trip time) min=%d~%dms max=%d~%dms avg=%d~%dms\r\n"
> 
>  #string STR_IFCONFIG6_ERR_IP6CFG_GETDATA       #language en-US    "Get
> data of the interface information %hr\r\n"
>  #string STR_IFCONFIG6_INFO_BREAK               #language en-US    "------
> -----------------------------------------------------------"
>  #string STR_IFCONFIG6_INFO_COLON               #language en-US    ":"
>  #string STR_IFCONFIG6_INFO_JOINT               #language en-US    " >> "
> --
> 1.9.5.msysgit.1



      reply	other threads:[~2016-11-09  2:28 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-08 11:08 [patch] ShellPkg: update ping6 to use timer service instead of timer arch protocol Zhang Lubo
2016-11-09  2:28 ` Fu, Siyuan [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=B1FF2E9001CE9041BD10B825821D5BC58A838725@shsmsx102.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