On 04/14/16 05:13, 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, okay. Although this patch is worthwhile without any particular reasoning too (of course), I associated it with that other discussion, and I thought your motivation was Bhupesh's use case. If your "only" goal was to improve the shell's conformance to the spec, that makes the patch 100% justified as well, naturally. And it just happens to fix Bhupesh's use case on the side. (I still think that it would be nice to hear back from Bhupesh.) > 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's very kind of you, thank you -- I personally didn't mean to submit a new feature request :) I'm neutral on the question whether such an automatism should be added to the ping command, in the UEFI shell. Thank you! Laszlo > 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
