[PATCH 2/3] net: add fastboot TCP support
Dmitrii Merkurev
dimorinny at google.com
Wed Apr 12 20:55:35 CEST 2023
Thank you for the comments. Fixed everything you mentioned in uploaded v2.
> Wow, that can really be that big? Or are you using u64 just because
> that is the size of the field?
I don't think any fastboot message can be that big. You're right,
using u64 to fit the field. Here is more information about packet
format https://chromium.googlesource.com/aosp/platform/system/core/+/refs/heads/upstream/fastboot/#fastboot-data
On Sat, Apr 1, 2023 at 7:32 AM Simon Glass <sjg at chromium.org> wrote:
>
> 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