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
diff --git a/adb/adb.cpp b/adb/adb.cpp
index 1727225..97ce125 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -334,10 +334,10 @@
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 @@
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<const char*>(p->data), t);
if (HOST || !auth_required) {
diff --git a/adb/adb.h b/adb/adb.h
index 31fe3a5..357be51 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 @@
/* A socket is bound to atransport */
atransport *transport;
+
+ size_t get_max_payload() const;
};
@@ -193,6 +197,8 @@
atransport() {
auth_fde = {};
transport_fde = {};
+ protocol_version = A_VERSION;
+ max_payload = MAX_PAYLOAD;
}
virtual ~atransport() {}
@@ -234,7 +240,14 @@
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 cff26d6..8a6b156 100644
--- a/adb/adb_auth.cpp
+++ b/adb/adb_auth.cpp
@@ -75,7 +75,7 @@
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 884d5be..be28202 100644
--- a/adb/adb_auth_client.cpp
+++ b/adb/adb_auth_client.cpp
@@ -54,7 +54,7 @@
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 @@
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 8085c1a..b6bb00c 100644
--- a/adb/adb_auth_host.cpp
+++ b/adb/adb_auth_host.cpp
@@ -157,7 +157,7 @@
{
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 23af6c9..a2e0f88 100644
--- a/adb/jdwp_service.cpp
+++ b/adb/jdwp_service.cpp
@@ -608,7 +608,7 @@
*/
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 @@
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 333e7e7..5c7c6ba 100644
--- a/adb/protocol.txt
+++ b/adb/protocol.txt
@@ -60,11 +60,14 @@
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 621944e..d8ea2ee 100644
--- a/adb/sockets.cpp
+++ b/adb/sockets.cpp
@@ -330,8 +330,9 @@
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 @@
}
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 @@
{
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 @@
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 @@
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 87ca5ac..87aff88 100644
--- a/adb/transport.cpp
+++ b/adb/transport.cpp
@@ -835,6 +835,19 @@
}
}
+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 @@
#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 538f63e..edcc99d 100644
--- a/adb/transport.h
+++ b/adb/transport.h
@@ -57,7 +57,7 @@
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 97e3d50..1adc69e 100644
--- a/adb/transport_local.cpp
+++ b/adb/transport_local.cpp
@@ -53,7 +53,7 @@
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 eb3454d..2c975a9 100644
--- a/adb/transport_usb.cpp
+++ b/adb/transport_usb.cpp
@@ -32,7 +32,7 @@
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 c6f712b..439826a 100644
--- a/adb/usb_linux.cpp
+++ b/adb/usb_linux.cpp
@@ -439,36 +439,21 @@
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 @@
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 cfd95dd..b1b3538 100644
--- a/adb/usb_linux_client.cpp
+++ b/adb/usb_linux_client.cpp
@@ -213,14 +213,20 @@
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;
