Jaben , Do you agree to support no source IP specified case? After update, Ping will select the first both connected and configured interface automatically instead of always the first NIC. If no objection, patch will be sent out for review.
Thanks. Jiaxin > -----Original Message----- > From: David Van Arnem [mailto:dvanar...@cmlab.biz] > Sent: Friday, April 15, 2016 1:32 AM > To: Wu, Jiaxin <jiaxin...@intel.com>; Bhupesh Sharma > <bhupesh.sha...@nxp.com>; Laszlo Ersek <ler...@redhat.com>; edk2- > de...@lists.01.org > Cc: Ye, Ting <ting...@intel.com>; Carsey, Jaben <jaben.car...@intel.com>; > Fu, Siyuan <siyuan...@intel.com> > Subject: Re: [edk2] [Patch] ShellPkg: Update ping command options to sync > with Spec > > On 04/13/2016 09:13 PM, Wu, Jiaxin wrote: > > Hi David and Laszlo, > > > > This patch is not focused on the discussion about the multiple NICs existed > case, just use to update the ping command option. So, please don't > misunderstand. > > > > Ah, ok. Sorry to clutter up this patch discussion then, I wanted to make sure > the multiple NIC case wasn't lost in the discussion about "-s"/"-_s". Thanks! > > > For the discussion in > <http://thread.gmane.org/gmane.comp.bios.edk2.devel/10046>, If there > are multiple NICs existed in the platform and no source IP is specified (no > '-s' > option), the first NIC will be selected as default interface to ping the > target. '- > s SourceIp' can be used to specify any interface you want. That's the current > implementation choice. > > > > I agree your and Bhupesh 's option to support no source IP specified case, > that's meaningful to improve the command's friendliness(Windows and Linus > do this resolution). How about selecting the first both connected and > configured interface automatically? If so, I can create another patch to > support that solution. > > > > That sounds great to me, as long as it's not too much trouble :-) > Alternately, a > more useful error message in that case might suffice (e.g. "Multiple > configured interfaces found, please specify one with '-s'"). But, since > Windows and Linux both do the automatic resolution, I think consistency > would be beneficial. > > Thanks again! > > David > > > Thanks. > > Jiaxin > > > >> -----Original Message----- > >> From: David Van Arnem [mailto:dvanar...@cmlab.biz] > >> Sent: Thursday, April 14, 2016 4:02 AM > >> To: Wu, Jiaxin <jiaxin...@intel.com>; edk2-devel@lists.01.org > >> Cc: Ye, Ting <ting...@intel.com>; Carsey, Jaben > >> <jaben.car...@intel.com>; Fu, Siyuan <siyuan...@intel.com> > >> Subject: Re: [edk2] [Patch] ShellPkg: Update ping command options to > >> sync with Spec > >> > >> On 04/12/2016 09:16 PM, Jiaxin Wu wrote: > >>> This patch is used to update ping command options to sync with > >>> shell2.2 Spec. > >>> Considering the backward compatible issue, the patch keeps ‘-_s’ > >>> command option unchanged, only add the new option '-s' > >>> and make the old option '-_s' function same as new one. > >>> > >>> Cc: Ye Ting <ting...@intel.com> > >>> Cc: Fu Siyuan <siyuan...@intel.com> > >>> Cc: Jaben Carsey <jaben.car...@intel.com> > >>> Contributed-under: TianoCore Contribution Agreement 1.0 > >>> Signed-off-by: Jiaxin Wu <jiaxin...@intel.com> > >>> --- > >>> .../Library/UefiShellNetwork1CommandsLib/Ping.c | 12 ++++++++++- > - > >>> .../UefiShellNetwork1CommandsLib.uni | 22 > >>> +++++++++--------- > ---- > >>> 2 files changed, 19 insertions(+), 15 deletions(-) > >>> > >>> diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c > >>> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c > >>> index dbee764..13bcdde 100644 > >>> --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c > >>> +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c > >>> @@ -1,10 +1,10 @@ > >>> /** @file > >>> The implementation for Ping shell command. > >>> > >>> (C) Copyright 2015 Hewlett-Packard Development Company, > >>> L.P.<BR> > >>> - Copyright (c) 2009 - 2015, Intel Corporation. All rights > >>> reserved.<BR> > >>> + Copyright (c) 2009 - 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 > >>> which accompanies this distribution. The full text of the > >>> license may be > >> found at > >>> http://opensource.org/licenses/bsd-license.php. > >>> @@ -196,10 +196,14 @@ STATIC CONST SHELL_PARAM_ITEM > >> PingParamList[] = { > >>> { > >>> L"-n", > >>> TypeValue > >>> }, > >>> { > >>> + L"-s", > >>> + TypeValue > >>> + }, > >>> + { > >>> L"-_s", > >>> TypeValue > >>> }, > >>> { > >>> L"-_ip6", > >>> @@ -1510,11 +1514,15 @@ ShellCommandRunPing ( > >>> ZeroMem (&DstAddress, sizeof (EFI_IPv6_ADDRESS)); > >>> > >>> // > >>> // Parse the paramter of source ip address. > >>> // > >>> - ValueStr = ShellCommandLineGetValue (ParamPackage, L"-_s"); > >>> + ValueStr = ShellCommandLineGetValue (ParamPackage, L"-s"); if > >>> + (ValueStr == NULL) { > >>> + ValueStr = ShellCommandLineGetValue (ParamPackage, L"-_s"); } > >>> + > >> > >> What happens to ValueStr when neither "-s" nor "-_s" are specified > >> from the command line, but there are multiple network interfaces > >> active on the system? Will ValueStr be resolved to one of their IP's > automatically? > >> > >> In other words, if the system has two active network interfaces, will > >> ping work with just the command: > >> > >> ping <target ip> > >> > >> (I would test here but I do not have a platform that will work, > unfortunately). > >> > >> I think this was (part of) Bhupesh's original issue (below from > >> <http://thread.gmane.org/gmane.comp.bios.edk2.devel/10046>): > >> > >> > We are facing an issue when we enable two SNP (Simple Network) > >> drivers in our EDK2 setup over a ARMv8 based > SoC - NXP LS2080A. > >> > > >> > <snip> > >> > > >> > Shell> ifconfig -l > >> > > >> > ----------------------------------------------------------------- > >> > > >> > name : eth0 > >> > Media State : Media present > >> > policy : dhcp > >> > mac addr : 68:05:CA:16:8C:5E > >> > > >> > <snip> > >> > ----------------------------------------------------------------- > >> > > >> > name : eth1 > >> > Media State : Media present > >> > policy : static > >> > mac addr : 6E:70:FE:EC:00:06 > >> > > >> > <snip> > >> > ----------------------------------------------------------------- > >> > C) Now, when I try to ping a server over a LAN cable, I get the > >> following error message: > >> > > >> > Shell> ping 192.168.1.1 > >> > InstallProtocolInterface: 41D94CD2-35B6-455A-8258-D4E51334AADD > >> 8370DFD9E0 > InstallProtocolInterface: 8A219718-4EF5-4761-91C8- > >> C0F04BDA9E56 8370DFF8A0 > > Snp->undi.transmit() 8000h:Ah > > >> Config No mapping > >> > >> Note that he did not specify an interface in the ping command, but > >> the issue was resolved when he did (which started the "-_s"/"-s" > >> discussion). In my experience, Linux's ping will choose an active > >> interface when there are multiple active interfaces but the "-I" flag > >> is unused. I don't know if automatic interface resolution is desired > >> in the Shell's ping (perhaps Windows doesn't do this resolution), and > >> if not that's fine, but I figure it's worth mentioning. > >> > >> Thanks! > >> > >> David > >> > >>> if (ValueStr != NULL) { > >>> mSrcString = ValueStr; > >>> if (IpChoice == PING_IP_CHOICE_IP6) { > >>> Status = NetLibStrToIp6 (ValueStr, &SrcAddress); > >>> } else { > >>> diff --git > >>> > >> > a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Com > >> ma > >>> ndsLib.uni > >>> > >> > b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Com > >> ma > >>> ndsLib.uni > >>> index bc6acac..7d6f2da 100644 > >>> --- > >>> > >> > a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Com > >> ma > >>> ndsLib.uni > >>> +++ > >> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1C > >>> +++ ommandsLib.uni > >>> @@ -1,9 +1,9 @@ > >>> // /** > >>> // > >>> // (C) Copyright 2013-2015 Hewlett-Packard Development Company, > >>> L.P.<BR> -// 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 > >>> // which accompanies this distribution. The full text of the > >>> license may be > >> found at > >>> // http://opensource.org/licenses/bsd-license.php > >>> // > >>> @@ -86,28 +86,27 @@ > >>> #string STR_IFCONFIG_INFO_GATEWAY_HEAD #language en-US > >> "\n%Hdefault gateway: %N" > >>> #string STR_IFCONFIG_INFO_DNS_ADDR_HEAD #language en-US > >> "\n%HDNS server : %N\n" > >>> #string STR_IFCONFIG_INFO_IP_ADDR_BODY #language en-US > >> "%d.%d.%d.%d\n" > >>> > >>> #string STR_GET_HELP_PING #language en-US "" > >>> -".TH ping 0 "Pings the target host with an IPv4 or IPv6 stack."\r\n" > >>> +".TH ping 0 "Ping the target host with an IPv4 stack."\r\n" > >>> ".SH NAME\r\n" > >>> -"Pings the target host with an IPv4 or IPv6 stack.\r\n" > >>> +"Ping the target host with an IPv4 stack.\r\n" > >>> ".SH SYNOPSIS\r\n" > >>> " \r\n" > >>> -"PING [-_ip6] [-_s SourceIp] [-n count] [-l size] TargetIp\r\n" > >>> +"PING [-n count] [-l size] [-s SourceIp] TargetIp\r\n" > >>> ".SH OPTIONS\r\n" > >>> " \r\n" > >>> " -n - Specifies the number of echo request datagrams to be > sent.\r\n" > >>> " -l - Specifies the size of the data buffer in the echo request > >> datagram.\r\n" > >>> -" -_ip6 - Specifies the IPv6 stack usage mode (Default is IPv4 > stack).\r\n" > >>> -" -_s - Specifies the source adapter as IPv4 or IPv6 address.\r\n" > >>> -" SourceIp - Specifies the IPv4 or IPv6 address of the source > machine.\r\n" > >>> -" TargetIp - Specifies the IPv4 or IPv6 address of the target > machine.\r\n" > >>> +" -s - Specifies the source adapter as IPv4 address.\r\n" > >>> +" SourceIp - Specifies the IPv4 address of the source machine.\r\n" > >>> +" TargetIp - Specifies the IPv4 address of the target machine.\r\n" > >>> ".SH DESCRIPTION\r\n" > >>> " \r\n" > >>> "NOTES:\r\n" > >>> -" 1. This command uses the ICMPv4 or ICMPv6 ECHO_REQUEST > datagram > >> to elicit an\r\n" > >>> +" 1. This command uses the ICMPv4 ECHO_REQUEST datagram to elicit > >> an\r\n" > >>> " ECHO_REPLY from a host.\r\n" > >>> ".SH EXAMPLES\r\n" > >>> " \r\n" > >>> "EXAMPLES:\r\n" > >>> " * To ping the target host with 64 bytes data:\r\n" > >>> @@ -115,14 +114,11 @@ > >>> " \r\n" > >>> " * To ping the target host by sending 20 echo request datagrams:\r\n" > >>> " fs0:\> ping -n 20 202.120.100.1\r\n" > >>> " \r\n" > >>> " * To ping the target host by specifying the source adapter as > >>> IPv4 > >> address:\r\n" > >>> -" fs0:\> ping -_s 202.120.100.12 202.120.100.1\r\n" > >>> -" \r\n" > >>> -" * To ping the target host by specifying the IPv6 stack usage > mode:\r\n" > >>> -" fs0:\> ping -_ip6 2000:bbbb::8\r\n" > >>> +" fs0:\> ping -s 202.120.100.12 202.120.100.1\r\n" > >>> ".SH RETURNVALUES\r\n" > >>> " \r\n" > >>> "RETURN VALUES:\r\n" > >>> " SHELL_SUCCESS The action was completed as > >>> requested.\r\n" > >>> " SHELL_INVALID_PARAMETER One of the passed-in parameters was > >> incorrectly\r\n" > >>> > >> > >> > >> -- > >> Thanks, > >> David Van Arnem > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > > > > > -- > Thanks, > David Van Arnem _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
