[PATCH v1] MdeModulePkg/Mtftp4Dxe: Fix invalid configuration of MTFTP local port.

[PATCH v1] MdeModulePkg/Mtftp4Dxe: Fix invalid configuration of MTFTP local port.

[Jiaxin Wu]

This patch is to fix the invalid setting of MTFTP local port. The
issue can be reproduced by tftp shell command by using [-l port]
option.

Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
index f442e6d7ac..793ad77b1e 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
@@ -309,11 +309,11 @@ Mtftp4ConfigUnicastPort (
   UdpConfig.ReceiveTimeout     = 0;
   UdpConfig.TransmitTimeout    = 0;
   UdpConfig.UseDefaultAddress  = Config->UseDefaultSetting;
   IP4_COPY_ADDRESS (&UdpConfig.StationAddress, &Config->StationIp);
   IP4_COPY_ADDRESS (&UdpConfig.SubnetMask, &Config->SubnetMask);
-  UdpConfig.StationPort        = 0;
+  UdpConfig.StationPort        = Config->LocalPort;
   UdpConfig.RemotePort         = 0;
 
   Ip = HTONL (Instance->ServerIp);
   IP4_COPY_ADDRESS (&UdpConfig.RemoteAddress, &Ip);
 
-- 
2.17.1.windows.2

[PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry count option in command.

[Jiaxin Wu]

[-c <retry count>] is to define the number of times to transmit request
packets and wait for a response. The default value is 6. But it doesn't
specify the behavior of zero value. Here, The patch is to clear that:
Set to zero also means to use the default value.

Cc: Carsey Jaben <jaben.carsey@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c   | 6 +++++-
 ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni | 3 ++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
index ac2813efc3..028686e1ff 100644
--- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
+++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
@@ -216,11 +216,11 @@ EFI_MTFTP4_CONFIG_DATA DefaultMtftp4ConfigData = {
   { { 0, 0, 0, 0 } },               // SubnetMask        - Not relevant as UseDefaultSetting=TRUE
   0,                                // LocalPort         - Automatically assigned port number.
   { { 0, 0, 0, 0 } },               // GatewayIp         - Not relevant as UseDefaultSetting=TRUE
   { { 0, 0, 0, 0 } },               // ServerIp          - Not known yet
   69,                               // InitialServerPort - Standard TFTP server port
-  6,                                // TryCount          - Max number of retransmissions.
+  6,                                // TryCount          - The number of times to transmit request packets and wait for a response.
   4                                 // TimeoutValue      - Retransmission timeout in seconds.
 };
 
 STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
   {L"-i", TypeValue},
@@ -419,10 +419,14 @@ RunTftp (
   ValueStr = ShellCommandLineGetValue (CheckPackage, L"-c");
   if (ValueStr != NULL) {
     if (!StringToUint16 (ValueStr, &Mtftp4ConfigData.TryCount)) {
       goto Error;
     }
+
+    if (Mtftp4ConfigData.TryCount == 0) {
+      Mtftp4ConfigData.TryCount = 6;
+    }
   }
 
   ValueStr = ShellCommandLineGetValue (CheckPackage, L"-t");
   if (ValueStr != NULL) {
     if (!StringToUint16 (ValueStr, &Mtftp4ConfigData.TimeoutValue)) {
diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
index 654e42ad23..ff64912564 100644
--- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
+++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
@@ -56,11 +56,12 @@
 "  -i interface     - Specifies an adapter name, i.e., eth0.\r\n"
 "  -l port          - Specifies the local port number. Default value is 0\r\n"
 "                     and the port number is automatically assigned.\r\n"
 "  -r port          - Specifies the remote port number. Default value is 69.\r\n"
 "  -c <retry count> - The number of times to transmit request packets and\r\n"
-"                     wait for a response. The default value is 6.\r\n"
+"                     wait for a response. The default value is 6. Set to zero\r\n"
+"                     also means to use the default value.\r\n"
 "  -t <timeout>     - The number of seconds to wait for a response after\r\n"
 "                     sending a request packet. Default value is 4s.\r\n"
 "  -s <block size>  - Specifies the TFTP blksize option as defined in RFC 2348.\r\n"
 "                     Valid range is between 8 and 65464, default value is 512.\r\n"
 "  -w <window size> - Specifies the TFTP windowsize option as defined in RFC 7440.\r\n"
-- 
2.17.1.windows.2

Re: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry count option in command.

[Carsey, Jaben]

Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Sunday, November 04, 2018 10:59 PM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ye, Ting <ting.ye@intel.com>;
> Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry count
> option in command.
> Importance: High
> 
> [-c <retry count>] is to define the number of times to transmit request
> packets and wait for a response. The default value is 6. But it doesn't
> specify the behavior of zero value. Here, The patch is to clear that:
> Set to zero also means to use the default value.
> 
> Cc: Carsey Jaben <jaben.carsey@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c   | 6 +++++-
>  ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni | 3 ++-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> index ac2813efc3..028686e1ff 100644
> --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> @@ -216,11 +216,11 @@ EFI_MTFTP4_CONFIG_DATA
> DefaultMtftp4ConfigData = {
>    { { 0, 0, 0, 0 } },               // SubnetMask        - Not relevant as
> UseDefaultSetting=TRUE
>    0,                                // LocalPort         - Automatically assigned port number.
>    { { 0, 0, 0, 0 } },               // GatewayIp         - Not relevant as
> UseDefaultSetting=TRUE
>    { { 0, 0, 0, 0 } },               // ServerIp          - Not known yet
>    69,                               // InitialServerPort - Standard TFTP server port
> -  6,                                // TryCount          - Max number of retransmissions.
> +  6,                                // TryCount          - The number of times to transmit
> request packets and wait for a response.
>    4                                 // TimeoutValue      - Retransmission timeout in seconds.
>  };
> 
>  STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
>    {L"-i", TypeValue},
> @@ -419,10 +419,14 @@ RunTftp (
>    ValueStr = ShellCommandLineGetValue (CheckPackage, L"-c");
>    if (ValueStr != NULL) {
>      if (!StringToUint16 (ValueStr, &Mtftp4ConfigData.TryCount)) {
>        goto Error;
>      }
> +
> +    if (Mtftp4ConfigData.TryCount == 0) {
> +      Mtftp4ConfigData.TryCount = 6;
> +    }
>    }
> 
>    ValueStr = ShellCommandLineGetValue (CheckPackage, L"-t");
>    if (ValueStr != NULL) {
>      if (!StringToUint16 (ValueStr, &Mtftp4ConfigData.TimeoutValue)) {
> diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> index 654e42ad23..ff64912564 100644
> --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> @@ -56,11 +56,12 @@
>  "  -i interface     - Specifies an adapter name, i.e., eth0.\r\n"
>  "  -l port          - Specifies the local port number. Default value is 0\r\n"
>  "                     and the port number is automatically assigned.\r\n"
>  "  -r port          - Specifies the remote port number. Default value is 69.\r\n"
>  "  -c <retry count> - The number of times to transmit request packets
> and\r\n"
> -"                     wait for a response. The default value is 6.\r\n"
> +"                     wait for a response. The default value is 6. Set to zero\r\n"
> +"                     also means to use the default value.\r\n"
>  "  -t <timeout>     - The number of seconds to wait for a response after\r\n"
>  "                     sending a request packet. Default value is 4s.\r\n"
>  "  -s <block size>  - Specifies the TFTP blksize option as defined in RFC
> 2348.\r\n"
>  "                     Valid range is between 8 and 65464, default value is 512.\r\n"
>  "  -w <window size> - Specifies the TFTP windowsize option as defined in
> RFC 7440.\r\n"
> --
> 2.17.1.windows.2

Re: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry count option in command.

[Fu, Siyuan]

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



> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Monday, November 5, 2018 2:59 PM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ye, Ting <ting.ye@intel.com>;
> Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry count
> option in command.
> 
> [-c <retry count>] is to define the number of times to transmit request
> packets and wait for a response. The default value is 6. But it doesn't
> specify the behavior of zero value. Here, The patch is to clear that:
> Set to zero also means to use the default value.
> 
> Cc: Carsey Jaben <jaben.carsey@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c   | 6 +++++-
>  ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni | 3 ++-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> index ac2813efc3..028686e1ff 100644
> --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> @@ -216,11 +216,11 @@ EFI_MTFTP4_CONFIG_DATA DefaultMtftp4ConfigData = {
>    { { 0, 0, 0, 0 } },               // SubnetMask        - Not relevant
> as UseDefaultSetting=TRUE
>    0,                                // LocalPort         - Automatically
> assigned port number.
>    { { 0, 0, 0, 0 } },               // GatewayIp         - Not relevant
> as UseDefaultSetting=TRUE
>    { { 0, 0, 0, 0 } },               // ServerIp          - Not known yet
>    69,                               // InitialServerPort - Standard TFTP
> server port
> -  6,                                // TryCount          - Max number of
> retransmissions.
> +  6,                                // TryCount          - The number of
> times to transmit request packets and wait for a response.
>    4                                 // TimeoutValue      - Retransmission
> timeout in seconds.
>  };
> 
>  STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
>    {L"-i", TypeValue},
> @@ -419,10 +419,14 @@ RunTftp (
>    ValueStr = ShellCommandLineGetValue (CheckPackage, L"-c");
>    if (ValueStr != NULL) {
>      if (!StringToUint16 (ValueStr, &Mtftp4ConfigData.TryCount)) {
>        goto Error;
>      }
> +
> +    if (Mtftp4ConfigData.TryCount == 0) {
> +      Mtftp4ConfigData.TryCount = 6;
> +    }
>    }
> 
>    ValueStr = ShellCommandLineGetValue (CheckPackage, L"-t");
>    if (ValueStr != NULL) {
>      if (!StringToUint16 (ValueStr, &Mtftp4ConfigData.TimeoutValue)) {
> diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> index 654e42ad23..ff64912564 100644
> --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> @@ -56,11 +56,12 @@
>  "  -i interface     - Specifies an adapter name, i.e., eth0.\r\n"
>  "  -l port          - Specifies the local port number. Default value is
> 0\r\n"
>  "                     and the port number is automatically assigned.\r\n"
>  "  -r port          - Specifies the remote port number. Default value is
> 69.\r\n"
>  "  -c <retry count> - The number of times to transmit request packets
> and\r\n"
> -"                     wait for a response. The default value is 6.\r\n"
> +"                     wait for a response. The default value is 6. Set to
> zero\r\n"
> +"                     also means to use the default value.\r\n"
>  "  -t <timeout>     - The number of seconds to wait for a response
> after\r\n"
>  "                     sending a request packet. Default value is 4s.\r\n"
>  "  -s <block size>  - Specifies the TFTP blksize option as defined in RFC
> 2348.\r\n"
>  "                     Valid range is between 8 and 65464, default value
> is 512.\r\n"
>  "  -w <window size> - Specifies the TFTP windowsize option as defined in
> RFC 7440.\r\n"
> --
> 2.17.1.windows.2

Re: [PATCH v1] MdeModulePkg/Mtftp4Dxe: Fix invalid configuration of MTFTP local port.

[Fu, Siyuan]

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



> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Monday, November 5, 2018 2:58 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> Subject: [PATCH v1] MdeModulePkg/Mtftp4Dxe: Fix invalid configuration of
> MTFTP local port.
> 
> This patch is to fix the invalid setting of MTFTP local port. The
> issue can be reproduced by tftp shell command by using [-l port]
> option.
> 
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> index f442e6d7ac..793ad77b1e 100644
> --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> @@ -309,11 +309,11 @@ Mtftp4ConfigUnicastPort (
>    UdpConfig.ReceiveTimeout     = 0;
>    UdpConfig.TransmitTimeout    = 0;
>    UdpConfig.UseDefaultAddress  = Config->UseDefaultSetting;
>    IP4_COPY_ADDRESS (&UdpConfig.StationAddress, &Config->StationIp);
>    IP4_COPY_ADDRESS (&UdpConfig.SubnetMask, &Config->SubnetMask);
> -  UdpConfig.StationPort        = 0;
> +  UdpConfig.StationPort        = Config->LocalPort;
>    UdpConfig.RemotePort         = 0;
> 
>    Ip = HTONL (Instance->ServerIp);
>    IP4_COPY_ADDRESS (&UdpConfig.RemoteAddress, &Ip);
> 
> --
> 2.17.1.windows.2

Re: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry count option in command.

[Carsey, Jaben]

Wu,

I plan to push this patch tomorrow, but I would like to add this to the commit message.  What do you think?

"This fixes the bug where parameter value 0 causes failure."

-Jaben

> -----Original Message-----
> From: Fu, Siyuan
> Sent: Monday, November 05, 2018 10:51 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ye, Ting <ting.ye@intel.com>
> Subject: RE: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry
> count option in command.
> Importance: High
> 
> Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> 
> 
> 
> > -----Original Message-----
> > From: Wu, Jiaxin
> > Sent: Monday, November 5, 2018 2:59 PM
> > To: edk2-devel@lists.01.org
> > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ye, Ting
> <ting.ye@intel.com>;
> > Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> > Subject: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry
> count
> > option in command.
> >
> > [-c <retry count>] is to define the number of times to transmit request
> > packets and wait for a response. The default value is 6. But it doesn't
> > specify the behavior of zero value. Here, The patch is to clear that:
> > Set to zero also means to use the default value.
> >
> > Cc: Carsey Jaben <jaben.carsey@intel.com>
> > Cc: Ye Ting <ting.ye@intel.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > ---
> >  ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c   | 6 +++++-
> >  ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni | 3 ++-
> >  2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> > b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> > index ac2813efc3..028686e1ff 100644
> > --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> > +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> > @@ -216,11 +216,11 @@ EFI_MTFTP4_CONFIG_DATA
> DefaultMtftp4ConfigData = {
> >    { { 0, 0, 0, 0 } },               // SubnetMask        - Not relevant
> > as UseDefaultSetting=TRUE
> >    0,                                // LocalPort         - Automatically
> > assigned port number.
> >    { { 0, 0, 0, 0 } },               // GatewayIp         - Not relevant
> > as UseDefaultSetting=TRUE
> >    { { 0, 0, 0, 0 } },               // ServerIp          - Not known yet
> >    69,                               // InitialServerPort - Standard TFTP
> > server port
> > -  6,                                // TryCount          - Max number of
> > retransmissions.
> > +  6,                                // TryCount          - The number of
> > times to transmit request packets and wait for a response.
> >    4                                 // TimeoutValue      - Retransmission
> > timeout in seconds.
> >  };
> >
> >  STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
> >    {L"-i", TypeValue},
> > @@ -419,10 +419,14 @@ RunTftp (
> >    ValueStr = ShellCommandLineGetValue (CheckPackage, L"-c");
> >    if (ValueStr != NULL) {
> >      if (!StringToUint16 (ValueStr, &Mtftp4ConfigData.TryCount)) {
> >        goto Error;
> >      }
> > +
> > +    if (Mtftp4ConfigData.TryCount == 0) {
> > +      Mtftp4ConfigData.TryCount = 6;
> > +    }
> >    }
> >
> >    ValueStr = ShellCommandLineGetValue (CheckPackage, L"-t");
> >    if (ValueStr != NULL) {
> >      if (!StringToUint16 (ValueStr, &Mtftp4ConfigData.TimeoutValue)) {
> > diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> > b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> > index 654e42ad23..ff64912564 100644
> > --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> > +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> > @@ -56,11 +56,12 @@
> >  "  -i interface     - Specifies an adapter name, i.e., eth0.\r\n"
> >  "  -l port          - Specifies the local port number. Default value is
> > 0\r\n"
> >  "                     and the port number is automatically assigned.\r\n"
> >  "  -r port          - Specifies the remote port number. Default value is
> > 69.\r\n"
> >  "  -c <retry count> - The number of times to transmit request packets
> > and\r\n"
> > -"                     wait for a response. The default value is 6.\r\n"
> > +"                     wait for a response. The default value is 6. Set to
> > zero\r\n"
> > +"                     also means to use the default value.\r\n"
> >  "  -t <timeout>     - The number of seconds to wait for a response
> > after\r\n"
> >  "                     sending a request packet. Default value is 4s.\r\n"
> >  "  -s <block size>  - Specifies the TFTP blksize option as defined in RFC
> > 2348.\r\n"
> >  "                     Valid range is between 8 and 65464, default value
> > is 512.\r\n"
> >  "  -w <window size> - Specifies the TFTP windowsize option as defined in
> > RFC 7440.\r\n"
> > --
> > 2.17.1.windows.2

Re: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry count option in command.

[Wu, Jiaxin]

Hi Jaben,

The patch already has been pushed after received your/Siyuan reviewed-by tag.

Thanks,
Jiaxin


> -----Original Message-----
> From: Carsey, Jaben
> Sent: Thursday, November 8, 2018 9:09 AM
> To: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>;
> edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>
> Subject: RE: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry
> count option in command.
> 
> Wu,
> 
> I plan to push this patch tomorrow, but I would like to add this to the commit
> message.  What do you think?
> 
> "This fixes the bug where parameter value 0 causes failure."
> 
> -Jaben
> 
> > -----Original Message-----
> > From: Fu, Siyuan
> > Sent: Monday, November 05, 2018 10:51 PM
> > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ye, Ting <ting.ye@intel.com>
> > Subject: RE: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry
> > count option in command.
> > Importance: High
> >
> > Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> >
> >
> >
> > > -----Original Message-----
> > > From: Wu, Jiaxin
> > > Sent: Monday, November 5, 2018 2:59 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ye, Ting
> > <ting.ye@intel.com>;
> > > Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> > > Subject: [PATCH v1] ShellPkg/TftpDynamicCommand: Clarify the retry
> > count
> > > option in command.
> > >
> > > [-c <retry count>] is to define the number of times to transmit request
> > > packets and wait for a response. The default value is 6. But it doesn't
> > > specify the behavior of zero value. Here, The patch is to clear that:
> > > Set to zero also means to use the default value.
> > >
> > > Cc: Carsey Jaben <jaben.carsey@intel.com>
> > > Cc: Ye Ting <ting.ye@intel.com>
> > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > > ---
> > >  ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c   | 6 +++++-
> > >  ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni | 3 ++-
> > >  2 files changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> > > b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> > > index ac2813efc3..028686e1ff 100644
> > > --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> > > +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> > > @@ -216,11 +216,11 @@ EFI_MTFTP4_CONFIG_DATA
> > DefaultMtftp4ConfigData = {
> > >    { { 0, 0, 0, 0 } },               // SubnetMask        - Not relevant
> > > as UseDefaultSetting=TRUE
> > >    0,                                // LocalPort         - Automatically
> > > assigned port number.
> > >    { { 0, 0, 0, 0 } },               // GatewayIp         - Not relevant
> > > as UseDefaultSetting=TRUE
> > >    { { 0, 0, 0, 0 } },               // ServerIp          - Not known yet
> > >    69,                               // InitialServerPort - Standard TFTP
> > > server port
> > > -  6,                                // TryCount          - Max number of
> > > retransmissions.
> > > +  6,                                // TryCount          - The number of
> > > times to transmit request packets and wait for a response.
> > >    4                                 // TimeoutValue      - Retransmission
> > > timeout in seconds.
> > >  };
> > >
> > >  STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
> > >    {L"-i", TypeValue},
> > > @@ -419,10 +419,14 @@ RunTftp (
> > >    ValueStr = ShellCommandLineGetValue (CheckPackage, L"-c");
> > >    if (ValueStr != NULL) {
> > >      if (!StringToUint16 (ValueStr, &Mtftp4ConfigData.TryCount)) {
> > >        goto Error;
> > >      }
> > > +
> > > +    if (Mtftp4ConfigData.TryCount == 0) {
> > > +      Mtftp4ConfigData.TryCount = 6;
> > > +    }
> > >    }
> > >
> > >    ValueStr = ShellCommandLineGetValue (CheckPackage, L"-t");
> > >    if (ValueStr != NULL) {
> > >      if (!StringToUint16 (ValueStr, &Mtftp4ConfigData.TimeoutValue)) {
> > > diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> > > b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> > > index 654e42ad23..ff64912564 100644
> > > --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> > > +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.uni
> > > @@ -56,11 +56,12 @@
> > >  "  -i interface     - Specifies an adapter name, i.e., eth0.\r\n"
> > >  "  -l port          - Specifies the local port number. Default value is
> > > 0\r\n"
> > >  "                     and the port number is automatically assigned.\r\n"
> > >  "  -r port          - Specifies the remote port number. Default value is
> > > 69.\r\n"
> > >  "  -c <retry count> - The number of times to transmit request packets
> > > and\r\n"
> > > -"                     wait for a response. The default value is 6.\r\n"
> > > +"                     wait for a response. The default value is 6. Set to
> > > zero\r\n"
> > > +"                     also means to use the default value.\r\n"
> > >  "  -t <timeout>     - The number of seconds to wait for a response
> > > after\r\n"
> > >  "                     sending a request packet. Default value is 4s.\r\n"
> > >  "  -s <block size>  - Specifies the TFTP blksize option as defined in RFC
> > > 2348.\r\n"
> > >  "                     Valid range is between 8 and 65464, default value
> > > is 512.\r\n"
> > >  "  -w <window size> - Specifies the TFTP windowsize option as defined in
> > > RFC 7440.\r\n"
> > > --
> > > 2.17.1.windows.2