From 3d2904cdf2371e26c0465184436bd063979a5d97 Mon Sep 17 00:00:00 2001 From: Tamas Berghammer Date: Mon, 13 Jul 2015 19:12:28 +0100 Subject: [PATCH] Increase size of the the adb packets sent over the wire The reason behing this change is to increase the adb push/pull speed with reduceing the number of packets sent between the host and the device because the communication is heavily bound by packet latency. The change maintains two way compatibility in the communication protocol with negotiating a packet size between the target and the host with the CONNECT packets. After this change the push/pull speeds improved significantly (measured from Linux-x86_64 with 100MB of data): | Old push | Old pull || New push | New pull | ----------------------------------------------------------- Hammerhead | 4.6 MB/s | 3.9 MB/s || 13.1 MB/s | 16.5 MB/s | ----------------------------------------------------------- Volantis | 6.0 MB/s | 6.2 MS/s || 25.9 MB/s | 29.0 MB/s | ----------------------------------------------------------- Fugu | 6.0 MB/s | 5.1 MB/s || 27.9 MB/s | 33.2 MB/s | ----------------------------------------------------------- Change-Id: Id9625de31266e43394289e325c7e7e473379c5d8 --- adb/adb.cpp | 8 ++++---- adb/adb.h | 15 ++++++++++++++- adb/adb_auth.cpp | 2 +- adb/adb_auth_client.cpp | 4 ++-- adb/adb_auth_host.cpp | 2 +- adb/jdwp_service.cpp | 4 ++-- adb/protocol.txt | 7 +++++-- adb/sockets.cpp | 26 +++++++++++++++++++------- adb/transport.cpp | 20 +++++++++++++++++--- adb/transport.h | 2 +- adb/transport_local.cpp | 2 +- adb/transport_usb.cpp | 2 +- adb/usb_linux.cpp | 39 ++++++++++++--------------------------- adb/usb_linux_client.cpp | 20 +++++++++++++------- 14 files changed, 93 insertions(+), 60 deletions(-) diff --git a/adb/adb.cpp b/adb/adb.cpp index 17272258e..97ce12562 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -334,10 +334,10 @@ void send_connect(atransport *t) D("Calling send_connect \n"); apacket *cp = get_apacket(); cp->msg.command = A_CNXN; - cp->msg.arg0 = A_VERSION; - cp->msg.arg1 = MAX_PAYLOAD; + cp->msg.arg0 = t->get_protocol_version(); + cp->msg.arg1 = t->get_max_payload(); cp->msg.data_length = fill_connect_data((char *)cp->data, - sizeof(cp->data)); + MAX_PAYLOAD_V1); send_packet(cp, t); } @@ -424,12 +424,12 @@ void handle_packet(apacket *p, atransport *t) return; case A_CNXN: /* CONNECT(version, maxdata, "system-id-string") */ - /* XXX verify version, etc */ if(t->connection_state != kCsOffline) { t->connection_state = kCsOffline; handle_offline(t); } + t->update_version(p->msg.arg0, p->msg.arg1); parse_banner(reinterpret_cast(p->data), t); if (HOST || !auth_required) { diff --git a/adb/adb.h b/adb/adb.h index 31fe3a5e0..357be513f 100644 --- a/adb/adb.h +++ b/adb/adb.h @@ -25,7 +25,9 @@ #include "adb_trace.h" #include "fdevent.h" -#define MAX_PAYLOAD 4096 +constexpr size_t MAX_PAYLOAD_V1 = 4 * 1024; +constexpr size_t MAX_PAYLOAD_V2 = 256 * 1024; +constexpr size_t MAX_PAYLOAD = MAX_PAYLOAD_V2; #define A_SYNC 0x434e5953 #define A_CNXN 0x4e584e43 @@ -137,6 +139,8 @@ struct asocket { /* A socket is bound to atransport */ atransport *transport; + + size_t get_max_payload() const; }; @@ -193,6 +197,8 @@ public: atransport() { auth_fde = {}; transport_fde = {}; + protocol_version = A_VERSION; + max_payload = MAX_PAYLOAD; } virtual ~atransport() {} @@ -234,7 +240,14 @@ public: const char* connection_state_name() const; + void update_version(int version, size_t payload); + int get_protocol_version() const; + size_t get_max_payload() const; + private: + int protocol_version; + size_t max_payload; + DISALLOW_COPY_AND_ASSIGN(atransport); }; diff --git a/adb/adb_auth.cpp b/adb/adb_auth.cpp index cff26d619..8a6b156df 100644 --- a/adb/adb_auth.cpp +++ b/adb/adb_auth.cpp @@ -75,7 +75,7 @@ void send_auth_publickey(atransport *t) apacket *p = get_apacket(); int ret; - ret = adb_auth_get_userkey(p->data, sizeof(p->data)); + ret = adb_auth_get_userkey(p->data, MAX_PAYLOAD_V1); if (!ret) { D("Failed to get user public key\n"); put_apacket(p); diff --git a/adb/adb_auth_client.cpp b/adb/adb_auth_client.cpp index 884d5be20..be28202a4 100644 --- a/adb/adb_auth_client.cpp +++ b/adb/adb_auth_client.cpp @@ -54,7 +54,7 @@ static bool needs_retry = false; static void read_keys(const char *file, struct listnode *list) { FILE *f; - char buf[MAX_PAYLOAD]; + char buf[MAX_PAYLOAD_V1]; char *sep; int ret; @@ -191,7 +191,7 @@ static void adb_auth_event(int fd, unsigned events, void *data) void adb_auth_confirm_key(unsigned char *key, size_t len, atransport *t) { - char msg[MAX_PAYLOAD]; + char msg[MAX_PAYLOAD_V1]; int ret; if (!usb_transport) { diff --git a/adb/adb_auth_host.cpp b/adb/adb_auth_host.cpp index 8085c1af8..b6bb00cb2 100644 --- a/adb/adb_auth_host.cpp +++ b/adb/adb_auth_host.cpp @@ -157,7 +157,7 @@ static int write_public_keyfile(RSA *private_key, const char *private_key_path) { RSAPublicKey pkey; FILE *outfile = NULL; - char path[PATH_MAX], info[MAX_PAYLOAD]; + char path[PATH_MAX], info[MAX_PAYLOAD_V1]; uint8_t* encoded = nullptr; size_t encoded_length; int ret = 0; diff --git a/adb/jdwp_service.cpp b/adb/jdwp_service.cpp index 23af6c970..a2e0f88fd 100644 --- a/adb/jdwp_service.cpp +++ b/adb/jdwp_service.cpp @@ -608,7 +608,7 @@ jdwp_socket_ready( asocket* s ) */ if (jdwp->pass == 0) { apacket* p = get_apacket(); - p->len = jdwp_process_list((char*)p->data, MAX_PAYLOAD); + p->len = jdwp_process_list((char*)p->data, s->get_max_payload()); peer->enqueue(peer, p); jdwp->pass = 1; } @@ -695,7 +695,7 @@ jdwp_tracker_ready( asocket* s ) if (t->need_update) { apacket* p = get_apacket(); t->need_update = 0; - p->len = jdwp_process_list_msg((char*)p->data, sizeof(p->data)); + p->len = jdwp_process_list_msg((char*)p->data, s->get_max_payload()); s->peer->enqueue(s->peer, p); } } diff --git a/adb/protocol.txt b/adb/protocol.txt index 333e7e7d8..5c7c6ba81 100644 --- a/adb/protocol.txt +++ b/adb/protocol.txt @@ -60,11 +60,14 @@ The version is used to ensure protocol compatibility and maxdata declares the maximum message body size that the remote system is willing to accept. -Currently, version=0x01000000 and maxdata=4096 +Currently, version=0x01000000 and maxdata=256*1024. Older versions of adb +hard-coded maxdata=4096, so CONNECT and AUTH packets sent to a device must not +be larger than that because they're sent before the CONNECT from the device +that tells the adb server what maxdata the device can support. Both sides send a CONNECT message when the connection between them is established. Until a CONNECT message is received no other messages may -be sent. Any messages received before a CONNECT message MUST be ignored. +be sent. Any messages received before a CONNECT message MUST be ignored. If a CONNECT message is received with an unknown version or insufficiently large maxdata value, the connection with the other side must be closed. diff --git a/adb/sockets.cpp b/adb/sockets.cpp index 621944e71..d8ea2ee5c 100644 --- a/adb/sockets.cpp +++ b/adb/sockets.cpp @@ -330,8 +330,9 @@ static void local_socket_event_func(int fd, unsigned ev, void* _s) if (ev & FDE_READ) { apacket *p = get_apacket(); unsigned char *x = p->data; - size_t avail = MAX_PAYLOAD; - int r; + const size_t max_payload = s->get_max_payload(); + size_t avail = max_payload; + int r = 0; int is_eof = 0; while (avail > 0) { @@ -354,10 +355,10 @@ static void local_socket_event_func(int fd, unsigned ev, void* _s) } D("LS(%d): fd=%d post avail loop. r=%d is_eof=%d forced_eof=%d\n", s->id, s->fd, r, is_eof, s->fde.force_eof); - if ((avail == MAX_PAYLOAD) || (s->peer == 0)) { + if ((avail == max_payload) || (s->peer == 0)) { put_apacket(p); } else { - p->len = MAX_PAYLOAD - avail; + p->len = max_payload - avail; r = s->peer->enqueue(s->peer, p); D("LS(%d): fd=%d post peer->enqueue(). r=%d\n", s->id, s->fd, @@ -569,9 +570,9 @@ void connect_to_remote(asocket *s, const char *destination) { D("Connect_to_remote call RS(%d) fd=%d\n", s->id, s->fd); apacket *p = get_apacket(); - int len = strlen(destination) + 1; + size_t len = strlen(destination) + 1; - if(len > (MAX_PAYLOAD-1)) { + if(len > (s->get_max_payload()-1)) { fatal("destination oversized"); } @@ -700,7 +701,7 @@ static int smart_socket_enqueue(asocket *s, apacket *p) s->pkt_first = p; s->pkt_last = p; } else { - if((s->pkt_first->len + p->len) > MAX_PAYLOAD) { + if((s->pkt_first->len + p->len) > s->get_max_payload()) { D("SS(%d): overflow\n", s->id); put_apacket(p); goto fail; @@ -901,3 +902,14 @@ void connect_to_smartsocket(asocket *s) ss->peer = s; s->ready(s); } + +size_t asocket::get_max_payload() const { + size_t max_payload = MAX_PAYLOAD; + if (transport) { + max_payload = std::min(max_payload, transport->get_max_payload()); + } + if (peer && peer->transport) { + max_payload = std::min(max_payload, peer->transport->get_max_payload()); + } + return max_payload; +} diff --git a/adb/transport.cpp b/adb/transport.cpp index 87ca5accf..87aff88e8 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -835,6 +835,19 @@ const char* atransport::connection_state_name() const { } } +void atransport::update_version(int version, size_t payload) { + protocol_version = std::min(version, A_VERSION); + max_payload = std::min(payload, MAX_PAYLOAD); +} + +int atransport::get_protocol_version() const { + return protocol_version; +} + +size_t atransport::get_max_payload() const { + return max_payload; +} + #if ADB_HOST static void append_transport_info(std::string* result, const char* key, @@ -1020,15 +1033,16 @@ void unregister_usb_transport(usb_handle *usb) { #undef TRACE_TAG #define TRACE_TAG TRACE_RWX -int check_header(apacket *p) +int check_header(apacket *p, atransport *t) { if(p->msg.magic != (p->msg.command ^ 0xffffffff)) { D("check_header(): invalid magic\n"); return -1; } - if(p->msg.data_length > MAX_PAYLOAD) { - D("check_header(): %d > MAX_PAYLOAD\n", p->msg.data_length); + if(p->msg.data_length > t->get_max_payload()) { + D("check_header(): %u > atransport::max_payload = %zu\n", + p->msg.data_length, t->get_max_payload()); return -1; } diff --git a/adb/transport.h b/adb/transport.h index 538f63ecb..edcc99ded 100644 --- a/adb/transport.h +++ b/adb/transport.h @@ -57,7 +57,7 @@ void unregister_usb_transport(usb_handle* usb); void unregister_transport(atransport* t); void unregister_all_tcp_transports(); -int check_header(apacket* p); +int check_header(apacket* p, atransport* t); int check_data(apacket* p); /* for MacOS X cleanup */ diff --git a/adb/transport_local.cpp b/adb/transport_local.cpp index 97e3d50cc..1adc69e13 100644 --- a/adb/transport_local.cpp +++ b/adb/transport_local.cpp @@ -53,7 +53,7 @@ static int remote_read(apacket *p, atransport *t) return -1; } - if(check_header(p)) { + if(check_header(p, t)) { D("bad header: terminated (data)\n"); return -1; } diff --git a/adb/transport_usb.cpp b/adb/transport_usb.cpp index eb3454d1c..2c975a9a7 100644 --- a/adb/transport_usb.cpp +++ b/adb/transport_usb.cpp @@ -32,7 +32,7 @@ static int remote_read(apacket *p, atransport *t) return -1; } - if(check_header(p)) { + if(check_header(p, t)) { D("remote usb: check_header failed\n"); return -1; } diff --git a/adb/usb_linux.cpp b/adb/usb_linux.cpp index c6f712b86..439826a02 100644 --- a/adb/usb_linux.cpp +++ b/adb/usb_linux.cpp @@ -439,36 +439,21 @@ fail: int usb_write(usb_handle *h, const void *_data, int len) { - unsigned char *data = (unsigned char*) _data; - int n; - int need_zero = 0; - D("++ usb_write ++\n"); - if(h->zero_mask) { + + unsigned char *data = (unsigned char*) _data; + int n = usb_bulk_write(h, data, len); + if(n != len) { + D("ERROR: n = %d, errno = %d (%s)\n", + n, errno, strerror(errno)); + return -1; + } + + if(h->zero_mask && !(len & h->zero_mask)) { /* if we need 0-markers and our transfer ** is an even multiple of the packet size, - ** we make note of it + ** then send the zero markers. */ - if(!(len & h->zero_mask)) { - need_zero = 1; - } - } - - while(len > 0) { - int xfer = (len > 4096) ? 4096 : len; - - n = usb_bulk_write(h, data, xfer); - if(n != xfer) { - D("ERROR: n = %d, errno = %d (%s)\n", - n, errno, strerror(errno)); - return -1; - } - - len -= xfer; - data += xfer; - } - - if(need_zero){ n = usb_bulk_write(h, _data, 0); return n; } @@ -484,7 +469,7 @@ int usb_read(usb_handle *h, void *_data, int len) D("++ usb_read ++\n"); while(len > 0) { - int xfer = (len > 4096) ? 4096 : len; + int xfer = len; D("[ usb read %d fd = %d], fname=%s\n", xfer, h->desc, h->fname); n = usb_bulk_read(h, data, xfer); diff --git a/adb/usb_linux_client.cpp b/adb/usb_linux_client.cpp index cfd95ddce..b1b353856 100644 --- a/adb/usb_linux_client.cpp +++ b/adb/usb_linux_client.cpp @@ -213,14 +213,20 @@ static int usb_adb_write(usb_handle *h, const void *data, int len) static int usb_adb_read(usb_handle *h, void *data, int len) { - int n; - D("about to read (fd=%d, len=%d)\n", h->fd, len); - n = unix_read(h->fd, data, len); - if(n != len) { - D("ERROR: fd = %d, n = %d, errno = %d (%s)\n", - h->fd, n, errno, strerror(errno)); - return -1; + while (len > 0) { + // The kernel implementation of adb_read in f_adb.c doesn't support + // reads larger then 4096 bytes. Read the data in 4096 byte chunks to + // avoid the issue. (The ffs implementation doesn't have this limit.) + int bytes_to_read = len < 4096 ? len : 4096; + int n = unix_read(h->fd, data, bytes_to_read); + if (n != bytes_to_read) { + D("ERROR: fd = %d, n = %d, errno = %d (%s)\n", + h->fd, n, errno, strerror(errno)); + return -1; + } + len -= n; + data = ((char*)data) + n; } D("[ done fd=%d ]\n", h->fd); return 0;