That sounds like a good enhancement to me. -Jaben
> -----Original Message----- > From: Wu, Jiaxin > Sent: Thursday, April 14, 2016 8:52 PM > To: Carsey, Jaben <jaben.car...@intel.com>; David Van Arnem > <dvanar...@cmlab.biz>; Bhupesh Sharma <bhupesh.sha...@nxp.com>; > Laszlo Ersek <ler...@redhat.com>; edk2-devel@lists.01.org > Cc: Ye, Ting <ting...@intel.com>; Fu, Siyuan <siyuan...@intel.com> > Subject: RE: [edk2] [Patch] ShellPkg: Update ping command options to sync > with Spec > Importance: High > > 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
