[U-Boot] [PATCH 2/2] [RFC]new command: wol - enable command

[Joe Hershberger]

On Mon, Apr 23, 2018 at 12:47 PM, Lothar Felten <lothar.felten at gmail.com> wrote:
> This patch enables the WoL command
>
> Signed-off-by: Lothar Felten <lothar.felten at gmail.com>
> ---
>  cmd/Kconfig   |  5 +++++
>  cmd/net.c     | 14 ++++++++++++++
>  include/net.h |  3 ++-
>  net/Makefile  |  1 +
>  net/net.c     | 19 +++++++++++++++++++
>  5 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index bc1d2f31c0..ed9d82fe71 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1142,6 +1142,11 @@ config CMD_RARP
>         help
>           Boot image via network using RARP/TFTP protocol
>
> +config CMD_WOL

This should not depend on CONFIG_CMD_NET... please move it outside of
that guard. It does depend on CONFIG_NET, so make sure it stays in
there. I would place it directly after CMD_PXE.

> +       bool "wol"
> +       help
> +         Wait for wake-on-lan packages

packages -> Magic Packet

> +
>  config CMD_NFS
>         bool "nfs"
>         default y
> diff --git a/cmd/net.c b/cmd/net.c
> index 67888d4e18..2e963b19c2 100644
> --- a/cmd/net.c
> +++ b/cmd/net.c
> @@ -88,6 +88,20 @@ U_BOOT_CMD(
>  );
>  #endif
>
> +#if defined(CONFIG_CMD_WOL)
> +int do_wol(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +       return netboot_common(WOL, cmdtp, argc, argv);

You are passing the args into netboot_common, which interprets the
first (single) parameter as a load address. That's clearly not what
you want.

Instead of doing that, just parse out the timeout parameter in
do_wol() and store it in a variable to be used by wol_start() and call
net_loop(WOL);

If the net_loop returns < 0, then you should return CMD_RET_FAILURE,
otherwise CMD_RET_SUCCESS.

> +}
> +
> +U_BOOT_CMD(
> +       wol,    2,      1,      do_wol,
> +       "wait for an incoming wake-on-lan packet",
> +       "[timeout]\n"

The timeout should not be optional.

> +       "timeout is in seconds"
> +);

It seems that you are simply using cmd/net.c as a place to put this
basically unrelated (to netboot) command. Please move this to a
separate file (cmd/wol.c).

> +#endif
> +
>  #if defined(CONFIG_CMD_DHCP)
>  static int do_dhcp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  {
> diff --git a/include/net.h b/include/net.h
> index 3469811aa0..0b4c2438a5 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -344,6 +344,7 @@ struct vlan_ethernet_hdr {
>
>  #define PROT_IP                0x0800          /* IP protocol                  */
>  #define PROT_ARP       0x0806          /* IP ARP protocol              */
> +#define PROT_WOL       0x0842          /* IP WOL protocol              */

Technically this comment should say "ether-wake WoL protocol"

>  #define PROT_RARP      0x8035          /* IP ARP protocol              */
>  #define PROT_VLAN      0x8100          /* IEEE 802.1q protocol         */
>  #define PROT_IPV6      0x86dd          /* IPv6 over bluebook           */
> @@ -535,7 +536,7 @@ extern int          net_restart_wrap;       /* Tried all network devices */
>
>  enum proto_t {
>         BOOTP, RARP, ARP, TFTPGET, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
> -       TFTPSRV, TFTPPUT, LINKLOCAL
> +       TFTPSRV, TFTPPUT, LINKLOCAL, WOL
>  };
>
>  extern char    net_boot_file_name[1024];/* Boot File name */
> diff --git a/net/Makefile b/net/Makefile
> index ce6e5adfa5..993b18f24c 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_CMD_PING) += ping.o
>  obj-$(CONFIG_CMD_RARP) += rarp.o
>  obj-$(CONFIG_CMD_SNTP) += sntp.o
>  obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
> +obj-$(CONFIG_CMD_WOL)  += wol.o
>
>  # Disable this warning as it is triggered by:
>  # sprintf(buf, index ? "foo%d" : "foo", index)
> diff --git a/net/net.c b/net/net.c
> index 8a9b69c6b0..e0088d352c 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -78,6 +78,12 @@
>   *                     - own IP address
>   *     We want:        - network time
>   *     Next step:      none
> + *
> + * WOL:
> + *
> + *     Prerequisites:  - own ethernet address
> + *     We want:        - magic packet to initiate action
> + *     Next step:      none
>   */
>
>
> @@ -107,6 +113,9 @@
>  #if defined(CONFIG_CMD_SNTP)
>  #include "sntp.h"
>  #endif
> +#if defined(CONFIG_CMD_WOL)
> +#include "wol.h"
> +#endif
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -508,6 +517,11 @@ restart:
>                 case LINKLOCAL:
>                         link_local_start();
>                         break;
> +#endif
> +#if defined(CONFIG_CMD_WOL)
> +               case WOL:
> +                       wol_start();
> +                       break;
>  #endif
>                 default:
>                         break;
> @@ -1274,6 +1288,11 @@ void net_process_received_packet(uchar *in_packet, int len)
>                                       ntohs(ip->udp_src),
>                                       ntohs(ip->udp_len) - UDP_HDR_SIZE);
>                 break;
> +#ifdef CONFIG_CMD_WOL
> +       case PROT_WOL:

WoL can also be sent as a UDP broadcast packet to port 0, 6, or, 7 or
something (according to Wikipedia). This would match the wireshark
dissector.

> +               wol_receive(ip, len);
> +               break;
> +#endif
>         }
>  }
>
> --
> 2.14.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot