[PATCH 2/3] net: add fastboot TCP support

Simon Glass sjg at chromium.org
Sat Apr 1 08:32:23 CEST 2023


Hi Dmitrii,

On Wed, 29 Mar 2023 at 09:31, Dmitrii Merkurev <dimorinny at google.com> wrote:
>
> Known limitations are
> 1. fastboot reboot doesn't work (answering OK but not rebooting)
> 2. flashing isn't supported (TCP transport only limitation)
>
> The command syntax is
> fastboot tcp
>
> Signed-off-by: Dmitrii Merkurev <dimorinny at google.com>
> Cc: Ying-Chun Liu (PaulLiu) <paul.liu at linaro.org>
> Cc: Simon Glass <sjg at chromium.org>
> Сс: Joe Hershberger <joe.hershberger at ni.com>
> Сс: Ramon Fried <rfried.dev at gmail.com>
> ---
>
>  MAINTAINERS                                |   6 +-
>  cmd/fastboot.c                             |  25 +++-
>  drivers/fastboot/Kconfig                   |   7 +
>  drivers/fastboot/fb_common.c               |   1 -
>  include/net.h                              |   3 +-
>  include/net/fastboot_tcp.h                 |  14 ++
>  include/net/{fastboot.h => fastboot_udp.h} |   2 +-
>  net/Makefile                               |   3 +-
>  net/fastboot_tcp.c                         | 143 +++++++++++++++++++++
>  net/{fastboot.c => fastboot_udp.c}         |   4 +-
>  net/net.c                                  |  17 ++-
>  11 files changed, 210 insertions(+), 15 deletions(-)
>  create mode 100644 include/net/fastboot_tcp.h
>  rename include/net/{fastboot.h => fastboot_udp.h} (91%)
>  create mode 100644 net/fastboot_tcp.c
>  rename net/{fastboot.c => fastboot_udp.c} (99%)

Reviewed-by: Simon Glass <sjg at chromium.org>

nits and a question below

>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 91d40ea4b6..501d1147d9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -978,10 +978,12 @@ F:        cmd/fastboot.c
>  F:     doc/android/fastboot*.rst
>  F:     include/fastboot.h
>  F:     include/fastboot-internal.h
> -F:     include/net/fastboot.h
> +F:     include/net/fastboot_tcp.h
> +F:     include/net/fastboot_udp.h
>  F:     drivers/fastboot/
>  F:     drivers/usb/gadget/f_fastboot.c
> -F:     net/fastboot.c
> +F:     net/fastboot_tcp.c
> +F:     net/fastboot_udp.c
>  F:     test/dm/fastboot.c
>
>  FPGA
> diff --git a/cmd/fastboot.c b/cmd/fastboot.c
> index 97dc02ce74..3d5ff951eb 100644
> --- a/cmd/fastboot.c
> +++ b/cmd/fastboot.c
> @@ -26,7 +26,7 @@ static int do_fastboot_udp(int argc, char *const argv[],
>                 return CMD_RET_FAILURE;
>         }
>
> -       err = net_loop(FASTBOOT);
> +       err = net_loop(FASTBOOT_UDP);
>
>         if (err < 0) {
>                 printf("fastboot udp error: %d\n", err);
> @@ -36,6 +36,26 @@ static int do_fastboot_udp(int argc, char *const argv[],
>         return CMD_RET_SUCCESS;
>  }
>
> +static int do_fastboot_tcp(int argc, char *const argv[],
> +                          uintptr_t buf_addr, size_t buf_size)
> +{
> +       int err;
> +
> +       if (!IS_ENABLED(CONFIG_TCP_FUNCTION_FASTBOOT)) {
> +               pr_err("Fastboot TCP not enabled\n");
> +               return CMD_RET_FAILURE;
> +       }
> +
> +       err = net_loop(FASTBOOT_TCP);
> +
> +       if (err < 0) {
> +               printf("fastboot tcp error: %d\n", err);
> +               return CMD_RET_FAILURE;
> +       }
> +
> +       return CMD_RET_SUCCESS;
> +}
> +
>  static int do_fastboot_usb(int argc, char *const argv[],
>                            uintptr_t buf_addr, size_t buf_size)
>  {
> @@ -141,7 +161,8 @@ NXTARG:
>
>         if (!strcmp(argv[1], "udp"))
>                 return do_fastboot_udp(argc, argv, buf_addr, buf_size);
> -
> +       if (!strcmp(argv[1], "tcp"))
> +               return do_fastboot_tcp(argc, argv, buf_addr, buf_size);
>         if (!strcmp(argv[1], "usb")) {
>                 argv++;
>                 argc--;
> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> index eefa34779c..c07df8369e 100644
> --- a/drivers/fastboot/Kconfig
> +++ b/drivers/fastboot/Kconfig
> @@ -28,6 +28,13 @@ config UDP_FUNCTION_FASTBOOT_PORT
>         help
>           The fastboot protocol requires a UDP port number.
>
> +config TCP_FUNCTION_FASTBOOT
> +       depends on NET
> +       select FASTBOOT
> +       bool "Enable fastboot protocol over TCP"
> +       help
> +         This enables the fastboot protocol over TCP.

Please can you add some more help, like a link to the protocol and
what it is used for?

> +
>  if FASTBOOT
>
>  config FASTBOOT_BUF_ADDR
> diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c
> index 57b6182c46..dde3cda78f 100644
> --- a/drivers/fastboot/fb_common.c
> +++ b/drivers/fastboot/fb_common.c
> @@ -15,7 +15,6 @@
>  #include <command.h>
>  #include <env.h>
>  #include <fastboot.h>
> -#include <net/fastboot.h>
>
>  /**
>   * fastboot_buf_addr - base address of the fastboot download buffer
> diff --git a/include/net.h b/include/net.h
> index 399af5e064..63daab3731 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -505,7 +505,8 @@ extern int          net_restart_wrap;       /* Tried all network devices */
>
>  enum proto_t {
>         BOOTP, RARP, ARP, TFTPGET, DHCP, PING, PING6, DNS, NFS, CDP, NETCONS,
> -       SNTP, TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP, NCSI, WGET
> +       SNTP, TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT_UDP, FASTBOOT_TCP, WOL,
> +       UDP, NCSI, WGET
>  };
>
>  extern char    net_boot_file_name[1024];/* Boot File name */
> diff --git a/include/net/fastboot_tcp.h b/include/net/fastboot_tcp.h
> new file mode 100644
> index 0000000000..6cf29d52e9
> --- /dev/null
> +++ b/include/net/fastboot_tcp.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (C) 2023 The Android Open Source Project
> + */
> +
> +#ifndef __NET_FASTBOOT_TCP_H__
> +#define __NET_FASTBOOT_TCP_H__
> +
> +/**
> + * Wait for incoming tcp fastboot comands.
> + */
> +void fastboot_tcp_start_server(void);
> +
> +#endif /* __NET_FASTBOOT_TCP_H__ */
> diff --git a/include/net/fastboot.h b/include/net/fastboot_udp.h
> similarity index 91%
> rename from include/net/fastboot.h
> rename to include/net/fastboot_udp.h
> index 68602095d2..82145bcb9c 100644
> --- a/include/net/fastboot.h
> +++ b/include/net/fastboot_udp.h
> @@ -14,7 +14,7 @@
>  /**
>   * Wait for incoming fastboot comands.
>   */
> -void fastboot_start_server(void);
> +void fastboot_udp_start_server(void);

Please add a comment

>
>  /**********************************************************************/
>
> diff --git a/net/Makefile b/net/Makefile
> index bea000b206..67fc77257d 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -26,7 +26,8 @@ obj-$(CONFIG_CMD_PCAP) += pcap.o
>  obj-$(CONFIG_CMD_RARP) += rarp.o
>  obj-$(CONFIG_CMD_SNTP) += sntp.o
>  obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
> -obj-$(CONFIG_UDP_FUNCTION_FASTBOOT)  += fastboot.o
> +obj-$(CONFIG_UDP_FUNCTION_FASTBOOT)  += fastboot_udp.o
> +obj-$(CONFIG_TCP_FUNCTION_FASTBOOT)  += fastboot_tcp.o
>  obj-$(CONFIG_CMD_WOL)  += wol.o
>  obj-$(CONFIG_PROT_UDP) += udp.o
>  obj-$(CONFIG_PROT_TCP) += tcp.o
> diff --git a/net/fastboot_tcp.c b/net/fastboot_tcp.c
> new file mode 100644
> index 0000000000..b4bbd815c8
> --- /dev/null
> +++ b/net/fastboot_tcp.c
> @@ -0,0 +1,143 @@
> +// SPDX-License-Identifier: BSD-2-Clause
> +/*
> + * Copyright (C) 2023 The Android Open Source Project
> + */
> +
> +#include <common.h>
> +#include <fastboot.h>
> +#include <net.h>
> +#include <net/fastboot_tcp.h>
> +#include <net/tcp.h>
> +
> +static char command[FASTBOOT_COMMAND_LEN] = {0};
> +static char response[FASTBOOT_RESPONSE_LEN] = {0};
> +
> +static const unsigned short handshake_length = 4;
> +static const uchar *handshake = "FB01";
> +
> +static u16 curr_sport;
> +static u16 curr_dport;
> +static u32 curr_tcp_seq_num;
> +static u32 curr_tcp_ack_num;
> +static unsigned int curr_request_len;
> +static enum fastboot_tcp_state {
> +       FASTBOOT_CLOSED,
> +       FASTBOOT_CONNECTED,
> +       FASTBOOT_DISCONNECTING
> +} state = FASTBOOT_CLOSED;
> +
> +static void fastboot_tcp_answer(u8 action, unsigned int len)
> +{
> +       const u32 response_seq_num = curr_tcp_ack_num;
> +       const u32 response_ack_num = curr_tcp_seq_num +
> +                 (curr_request_len > 0 ? curr_request_len : 1);
> +
> +       net_send_tcp_packet(len, htons(curr_sport), htons(curr_dport),
> +                           action, response_seq_num, response_ack_num);
> +}
> +
> +static void fastboot_tcp_reset(void)
> +{
> +       fastboot_tcp_answer(TCP_RST, 0);
> +       state = FASTBOOT_CLOSED;
> +}
> +
> +static void fastboot_tcp_send_packet(u8 action, const uchar *data, unsigned int len)
> +{
> +       uchar *pkt = net_get_async_tx_pkt_buf();
> +
> +       memset((void *)pkt, 0, PKTSIZE);

'\0'

Can you drop cast on these?

> +       pkt += net_eth_hdr_size() + IP_TCP_HDR_SIZE + TCP_TSOPT_SIZE + 2;
> +       memcpy(pkt, data, len);
> +       fastboot_tcp_answer(action, len);
> +       memset((void *)pkt, 0, PKTSIZE);
> +}
> +
> +static void fastboot_tcp_send_message(const char *message, unsigned int len)
> +{
> +       __be64 len_be = __cpu_to_be64(len);
> +       uchar *pkt = net_get_async_tx_pkt_buf();
> +
> +       memset((void *)pkt, 0, PKTSIZE);
> +       pkt += net_eth_hdr_size() + IP_TCP_HDR_SIZE + TCP_TSOPT_SIZE + 2;
> +       // Put first 8 bytes as a big endian message length
> +       memcpy(pkt, &len_be, 8);
> +       pkt += 8;
> +       memcpy(pkt, message, len);
> +       fastboot_tcp_answer(TCP_ACK | TCP_PUSH, len + 8);
> +       memset((void *)pkt, 0, PKTSIZE);
> +}
> +
> +static void fastboot_tcp_handler_ipv4(uchar *pkt, u16 dport,
> +                                     struct in_addr sip, u16 sport,
> +                                     u32 tcp_seq_num, u32 tcp_ack_num,
> +                                     u8 action, unsigned int len)
> +{
> +       u64 command_size;

Wow, that can really be that big? Or are you using u64 just because
that is the size of the field?

> +       u8 tcp_fin = action & TCP_FIN;
> +       u8 tcp_push = action & TCP_PUSH;
> +
> +       curr_sport = sport;
> +       curr_dport = dport;
> +       curr_tcp_seq_num = tcp_seq_num;
> +       curr_tcp_ack_num = tcp_ack_num;
> +       curr_request_len = len;
> +
> +       switch (state) {
> +       case FASTBOOT_CLOSED:
> +               if (tcp_push) {
> +                       if (len != handshake_length ||
> +                           strlen(pkt) != handshake_length ||
> +                           memcmp(pkt, handshake, handshake_length) != 0) {
> +                               fastboot_tcp_reset();
> +                               break;
> +                       }
> +                       fastboot_tcp_send_packet(TCP_ACK | TCP_PUSH,
> +                                                handshake, handshake_length);
> +                       state = FASTBOOT_CONNECTED;
> +               }
> +               break;
> +       case FASTBOOT_CONNECTED:
> +               if (tcp_fin) {
> +                       fastboot_tcp_answer(TCP_FIN | TCP_ACK, 0);
> +                       state = FASTBOOT_DISCONNECTING;
> +                       break;
> +               }
> +               if (tcp_push) {
> +                       // First 8 bytes is big endian message length
> +                       command_size = __be64_to_cpu(*(u64 *)pkt);
> +                       len -= 8;
> +                       pkt += 8;
> +
> +                       // Only single packet messages are supported ATM
> +                       if (strlen(pkt) != command_size) {
> +                               fastboot_tcp_reset();
> +                               break;
> +                       }
> +                       strlcpy(command, pkt, len + 1);
> +                       fastboot_handle_command(command, response);
> +                       fastboot_tcp_send_message(response, strlen(response));
> +               }
> +               break;
> +       case FASTBOOT_DISCONNECTING:
> +               if (tcp_push)
> +                       state = FASTBOOT_CLOSED;
> +               break;
> +       }
> +
> +       memset(command, 0, FASTBOOT_COMMAND_LEN);
> +       memset(response, 0, FASTBOOT_RESPONSE_LEN);
> +       curr_sport = 0;
> +       curr_dport = 0;
> +       curr_tcp_seq_num = 0;
> +       curr_tcp_ack_num = 0;
> +       curr_request_len = 0;
> +}
> +
> +void fastboot_tcp_start_server(void)
> +{
> +       printf("Using %s device\n", eth_get_name());
> +       printf("Listening for fastboot command on tcp %pI4\n", &net_ip);
> +
> +       tcp_set_tcp_handler(fastboot_tcp_handler_ipv4);
> +}
> diff --git a/net/fastboot.c b/net/fastboot_udp.c
> similarity index 99%
> rename from net/fastboot.c
> rename to net/fastboot_udp.c
> index e9569d88d2..27e779d8e0 100644
> --- a/net/fastboot.c
> +++ b/net/fastboot_udp.c
> @@ -7,7 +7,7 @@
>  #include <command.h>
>  #include <fastboot.h>
>  #include <net.h>
> -#include <net/fastboot.h>
> +#include <net/fastboot_udp.h>
>
>  enum {
>         FASTBOOT_ERROR = 0,
> @@ -300,7 +300,7 @@ static void fastboot_handler(uchar *packet, unsigned int dport,
>         }
>  }
>
> -void fastboot_start_server(void)
> +void fastboot_udp_start_server(void)
>  {
>         printf("Using %s device\n", eth_get_name());
>         printf("Listening for fastboot command on %pI4\n", &net_ip);
> diff --git a/net/net.c b/net/net.c
> index c9a749f6cc..41c3cac26e 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -93,7 +93,8 @@
>  #include <net.h>
>  #include <net6.h>
>  #include <ndisc.h>
> -#include <net/fastboot.h>
> +#include <net/fastboot_udp.h>
> +#include <net/fastboot_tcp.h>
>  #include <net/tftp.h>
>  #include <net/ncsi.h>
>  #if defined(CONFIG_CMD_PCAP)
> @@ -498,9 +499,14 @@ restart:
>                         tftp_start_server();
>                         break;
>  #endif
> -#ifdef CONFIG_UDP_FUNCTION_FASTBOOT
> -               case FASTBOOT:
> -                       fastboot_start_server();
> +#if defined(CONFIG_UDP_FUNCTION_FASTBOOT)
> +               case FASTBOOT_UDP:
> +                       fastboot_udp_start_server();
> +                       break;
> +#endif
> +#if defined(CONFIG_TCP_FUNCTION_FASTBOOT)
> +               case FASTBOOT_TCP:
> +                       fastboot_tcp_start_server();
>                         break;
>  #endif
>  #if defined(CONFIG_CMD_DHCP)
> @@ -1491,7 +1497,8 @@ common:
>                 /* Fall through */
>
>         case NETCONS:
> -       case FASTBOOT:
> +       case FASTBOOT_UDP:
> +       case FASTBOOT_TCP:
>         case TFTPSRV:
>                 if (IS_ENABLED(CONFIG_IPV6) && use_ip6) {
>                         if (!memcmp(&net_link_local_ip6, &net_null_addr_ip6,
> --
> 2.40.0.348.gf938b09366-goog
>

Regards,
Simon


More information about the U-Boot mailing list