From 6c85e38775c06786556f1ed6f3ce878164b350b7 Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Fri, 23 Jun 2017 21:24:58 +0900
Subject: [PATCH] Rewrite using rob instead of framebuf
---
lib/Makefile.am | 4 +-
lib/ngtcp2_conn.c | 94 ++++----------
lib/ngtcp2_conn.h | 13 +-
lib/ngtcp2_framebuf.c | 49 --------
lib/ngtcp2_rob.c | 155 ++++++++++++++++++++++++
lib/{ngtcp2_framebuf.h => ngtcp2_rob.h} | 41 +++++--
6 files changed, 221 insertions(+), 135 deletions(-)
delete mode 100644 lib/ngtcp2_framebuf.c
create mode 100644 lib/ngtcp2_rob.c
rename lib/{ngtcp2_framebuf.h => ngtcp2_rob.h} (57%)
diff --git a/lib/Makefile.am b/lib/Makefile.am
index d0291f59..f930ea46 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -40,7 +40,7 @@ OBJECTS = \
ngtcp2_conn.c \
ngtcp2_mem.c \
ngtcp2_pq.c \
- ngtcp2_framebuf.c
+ ngtcp2_rob.c
HFILES = \
ngtcp2_pkt.h \
@@ -51,7 +51,7 @@ HFILES = \
ngtcp2_conn.h \
ngtcp2_mem.h \
ngtcp2_pq.h \
- ngtcp2_framebuf.h \
+ ngtcp2_rob.h \
ngtcp2_macro.h
libngtcp2_la_SOURCES = $(HFILES) $(OBJECTS)
diff --git a/lib/ngtcp2_conn.c b/lib/ngtcp2_conn.c
index 110d739f..d55b1392 100644
--- a/lib/ngtcp2_conn.c
+++ b/lib/ngtcp2_conn.c
@@ -25,7 +25,6 @@
#include "ngtcp2_conn.h"
#include "ngtcp2_upe.h"
#include "ngtcp2_pkt.h"
-#include "ngtcp2_framebuf.h"
#include "ngtcp2_macro.h"
static int ngtcp2_conn_new(ngtcp2_conn **pconn, uint64_t conn_id,
@@ -98,7 +97,7 @@ void ngtcp2_conn_del(ngtcp2_conn *conn) {
return;
}
- ngtcp2_strm_free(&conn->strm0, conn->mem);
+ ngtcp2_strm_free(&conn->strm0);
ngtcp2_mem_free(conn->mem, conn);
}
@@ -352,7 +351,7 @@ static int ngtcp2_conn_recv_cleartext(ngtcp2_conn *conn, const uint8_t *pkt,
pktlen -= (size_t)nread;
if (fr.type != NGTCP2_FRAME_STREAM || fr.stream.stream_id != 0 ||
- conn->strm0.offset > fr.stream.offset) {
+ conn->strm0.offset >= fr.stream.offset + fr.stream.datalen) {
continue;
}
@@ -370,7 +369,7 @@ static int ngtcp2_conn_recv_cleartext(ngtcp2_conn *conn, const uint8_t *pkt,
return rv;
}
} else {
- rv = ngtcp2_conn_recv_reordering(conn, &conn->strm0, &fr.stream);
+ rv = ngtcp2_strm_recv_reordering(&conn->strm0, &fr.stream);
if (rv != 0) {
return rv;
}
@@ -430,91 +429,46 @@ int ngtcp2_conn_recv(ngtcp2_conn *conn, const uint8_t *pkt, size_t pktlen) {
return -1;
}
-int ngtcp2_conn_recv_reordering(ngtcp2_conn *conn, ngtcp2_strm *strm,
- ngtcp2_stream *fr) {
- ngtcp2_framebuf *fb;
- int rv;
-
- if (strm->nbuffered >= 65536) {
- return NGTCP2_ERR_INTERNAL_ERROR;
- }
-
- rv = ngtcp2_framebuf_new(&fb, fr, conn->mem);
- if (rv != 0) {
- return rv;
- }
-
- /* TODO This is not efficient. Invent new way to store duplicated
- buffered data */
- strm->nbuffered += fr->datalen;
-
- return ngtcp2_pq_push(&strm->pq, &fb->pq_entry);
-}
-
int ngtcp2_conn_emit_pending_recv_handshake(ngtcp2_conn *conn,
ngtcp2_strm *strm) {
- ngtcp2_framebuf *fb;
- uint64_t delta;
+ size_t datalen;
+ const uint8_t *data;
int rv;
- for (; !ngtcp2_pq_empty(&strm->pq);) {
- fb = ngtcp2_struct_of(ngtcp2_pq_top(&strm->pq), ngtcp2_framebuf, pq_entry);
-
- if (strm->offset < fb->fr.stream.offset) {
+ for (;;) {
+ datalen = ngtcp2_rob_data_at(&strm->rob, &data, strm->offset);
+ if (datalen == 0) {
return 0;
}
- ngtcp2_pq_pop(&strm->pq);
-
- delta = strm->offset - fb->fr.stream.offset;
-
- if (delta < fb->fr.stream.datalen) {
- rv = conn->callbacks.recv_handshake_data(conn, fb->fr.stream.data + delta,
- fb->fr.stream.datalen - delta,
- conn->user_data);
- if (rv != 0) {
- return rv;
- }
+ rv = conn->callbacks.recv_handshake_data(conn, data, datalen,
+ conn->user_data);
+ if (rv != 0) {
+ return rv;
}
- ngtcp2_framebuf_del(fb, conn->mem);
+ ngtcp2_rob_pop(&strm->rob);
}
-
- return 0;
-}
-
-static int ngtcp2_stream_offset_less(const void *lhsx, const void *rhsx) {
- const ngtcp2_framebuf *lhs, *rhs;
-
- lhs = ngtcp2_struct_of(lhsx, ngtcp2_framebuf, pq_entry);
- rhs = ngtcp2_struct_of(rhsx, ngtcp2_framebuf, pq_entry);
-
- return lhs->fr.stream.offset < rhs->fr.stream.offset;
}
int ngtcp2_strm_init(ngtcp2_strm *strm, ngtcp2_mem *mem) {
strm->offset = 0;
strm->nbuffered = 0;
- return ngtcp2_pq_init(&strm->pq, ngtcp2_stream_offset_less, mem);
+ strm->mem = mem;
+ return ngtcp2_rob_init(&strm->rob, mem);
}
-static int ngtcp2_framebuf_item_free(ngtcp2_pq_entry *item, void *arg) {
- ngtcp2_framebuf *fb;
- ngtcp2_mem *mem;
-
- fb = ngtcp2_struct_of(item, ngtcp2_framebuf, pq_entry);
- mem = arg;
-
- ngtcp2_framebuf_del(fb, mem);
-
- return 0;
-}
-
-void ngtcp2_strm_free(ngtcp2_strm *strm, ngtcp2_mem *mem) {
+void ngtcp2_strm_free(ngtcp2_strm *strm) {
if (strm == NULL) {
return;
}
+ ngtcp2_rob_free(&strm->rob);
+}
+
+int ngtcp2_strm_recv_reordering(ngtcp2_strm *strm, ngtcp2_stream *fr) {
+ if (strm->rob.bufferedlen >= 128 * 1024) {
+ return NGTCP2_ERR_INTERNAL_ERROR;
+ }
- ngtcp2_pq_each(&strm->pq, ngtcp2_framebuf_item_free, mem);
- ngtcp2_pq_free(&strm->pq);
+ return ngtcp2_rob_push(&strm->rob, fr->offset, fr->data, fr->datalen);
}
diff --git a/lib/ngtcp2_conn.h b/lib/ngtcp2_conn.h
index 89de28c2..07c6021e 100644
--- a/lib/ngtcp2_conn.h
+++ b/lib/ngtcp2_conn.h
@@ -32,7 +32,8 @@
#include <ngtcp2/ngtcp2.h>
#include "ngtcp2_mem.h"
-#include "ngtcp2_pq.h"
+#include "ngtcp2_buf.h"
+#include "ngtcp2_rob.h"
typedef enum {
/* Client specific handshake states */
@@ -52,13 +53,16 @@ typedef enum {
typedef struct {
uint64_t offset;
- ngtcp2_pq pq;
+ ngtcp2_rob rob;
+ ngtcp2_mem *mem;
size_t nbuffered;
} ngtcp2_strm;
int ngtcp2_strm_init(ngtcp2_strm *strm, ngtcp2_mem *mem);
-void ngtcp2_strm_free(ngtcp2_strm *strm, ngtcp2_mem *mem);
+void ngtcp2_strm_free(ngtcp2_strm *strm);
+
+int ngtcp2_strm_recv_reordering(ngtcp2_strm *strm, ngtcp2_stream *fr);
struct ngtcp2_conn {
int state;
@@ -72,9 +76,6 @@ struct ngtcp2_conn {
int server;
};
-int ngtcp2_conn_recv_reordering(ngtcp2_conn *conn, ngtcp2_strm *strm,
- ngtcp2_stream *fr);
-
int ngtcp2_conn_emit_pending_recv_handshake(ngtcp2_conn *conn,
ngtcp2_strm *strm);
diff --git a/lib/ngtcp2_framebuf.c b/lib/ngtcp2_framebuf.c
deleted file mode 100644
index 8bca8d3b..00000000
--- a/lib/ngtcp2_framebuf.c
+++ /dev/null
@@ -1,49 +0,0 @@
-/*
- * ngtcp2
- *
- * Copyright (c) 2017 ngtcp2 contributors
- *
- * Permission is hereby granted, free of charge, to any person obtaining
- * a copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sublicense, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice shall be
- * included in all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
- * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
- * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
- * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
- */
-#include "ngtcp2_framebuf.h"
-
-#include <string.h>
-
-int ngtcp2_framebuf_new(ngtcp2_framebuf **pfb, ngtcp2_stream *fr,
- ngtcp2_mem *mem) {
- uint8_t *data;
-
- *pfb = ngtcp2_mem_malloc(mem, sizeof(ngtcp2_framebuf) + fr->datalen);
- if (*pfb == NULL) {
- return NGTCP2_ERR_NOMEM;
- }
-
- data = ((uint8_t *)*pfb) + sizeof(ngtcp2_framebuf);
- memcpy(data, fr->data, fr->datalen);
-
- (*pfb)->fr.stream = *fr;
- (*pfb)->fr.stream.data = data;
-
- return 0;
-}
-
-void ngtcp2_framebuf_del(ngtcp2_framebuf *fb, ngtcp2_mem *mem) {
- ngtcp2_mem_free(mem, fb);
-}
diff --git a/lib/ngtcp2_rob.c b/lib/ngtcp2_rob.c
new file mode 100644
index 00000000..7d25d1fa
--- /dev/null
+++ b/lib/ngtcp2_rob.c
@@ -0,0 +1,155 @@
+/*
+ * ngtcp2
+ *
+ * Copyright (c) 2017 ngtcp2 contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+ * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+ * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+#include "ngtcp2_rob.h"
+
+#include <string.h>
+
+#include "ngtcp2_macro.h"
+
+int ngtcp2_rob_data_new(ngtcp2_rob_data **prdat, uint64_t offset,
+ const uint8_t *data, size_t datalen, ngtcp2_mem *mem) {
+ uint8_t *dest;
+
+ *prdat = ngtcp2_mem_malloc(mem, sizeof(ngtcp2_rob_data) + datalen);
+ if (*prdat == NULL) {
+ return NGTCP2_ERR_NOMEM;
+ }
+
+ dest = ((uint8_t *)*prdat) + sizeof(ngtcp2_rob_data);
+ memcpy(dest, data, datalen);
+
+ (*prdat)->offset = offset;
+ (*prdat)->data = dest;
+ (*prdat)->datalen = datalen;
+
+ return 0;
+}
+
+void ngtcp2_rob_data_del(ngtcp2_rob_data *rdat, ngtcp2_mem *mem) {
+ ngtcp2_mem_free(mem, rdat);
+}
+
+static int offset_less(const void *lhsx, const void *rhsx) {
+ const ngtcp2_rob_data *lhs, *rhs;
+
+ lhs = ngtcp2_struct_of(lhsx, ngtcp2_rob_data, pq_entry);
+ rhs = ngtcp2_struct_of(rhsx, ngtcp2_rob_data, pq_entry);
+
+ return lhs->offset < rhs->offset;
+}
+
+int ngtcp2_rob_init(ngtcp2_rob *rob, ngtcp2_mem *mem) {
+ int rv;
+
+ rv = ngtcp2_pq_init(&rob->pq, offset_less, mem);
+ if (rv != 0) {
+ return rv;
+ }
+
+ rob->bufferedlen = 0;
+ rob->mem = mem;
+
+ return 0;
+}
+
+static int pq_rob_data_free(ngtcp2_pq_entry *item, void *arg) {
+ ngtcp2_rob_data *rdat;
+ ngtcp2_rob *rob;
+
+ rdat = ngtcp2_struct_of(item, ngtcp2_rob_data, pq_entry);
+ rob = arg;
+
+ ngtcp2_rob_data_del(rdat, rob->mem);
+
+ return 0;
+}
+
+void ngtcp2_rob_free(ngtcp2_rob *rob) {
+ ngtcp2_pq_each(&rob->pq, pq_rob_data_free, rob);
+ ngtcp2_pq_free(&rob->pq);
+}
+
+int ngtcp2_rob_push(ngtcp2_rob *rob, uint64_t offset, const uint8_t *data,
+ size_t datalen) {
+ ngtcp2_rob_data *rdat;
+ int rv;
+
+ rv = ngtcp2_rob_data_new(&rdat, offset, data, datalen, rob->mem);
+ if (rv != 0) {
+ return rv;
+ }
+
+ rv = ngtcp2_pq_push(&rob->pq, &rdat->pq_entry);
+ if (rv != 0) {
+ ngtcp2_rob_data_del(rdat, rob->mem);
+ return rv;
+ }
+
+ rob->bufferedlen += datalen;
+
+ return 0;
+}
+
+size_t ngtcp2_rob_data_at(ngtcp2_rob *rob, const uint8_t **pdest,
+ uint64_t offset) {
+ ngtcp2_rob_data *rdat;
+ uint64_t delta;
+
+ for (; !ngtcp2_pq_empty(&rob->pq);) {
+ rdat = ngtcp2_struct_of(ngtcp2_pq_top(&rob->pq), ngtcp2_rob_data, pq_entry);
+ if (offset < rdat->offset) {
+ return 0;
+ }
+
+ delta = offset - rdat->offset;
+ if (delta >= rdat->datalen) {
+ ngtcp2_pq_pop(&rob->pq);
+ rob->bufferedlen -= rdat->datalen;
+
+ ngtcp2_rob_data_del(rdat, rob->mem);
+
+ continue;
+ }
+
+ *pdest = rdat->data + delta;
+ return rdat->datalen - delta;
+ }
+
+ return 0;
+}
+
+void ngtcp2_rob_pop(ngtcp2_rob *rob) {
+ ngtcp2_rob_data *rdat;
+
+ if (ngtcp2_pq_empty(&rob->pq)) {
+ return;
+ }
+
+ rdat = ngtcp2_struct_of(ngtcp2_pq_top(&rob->pq), ngtcp2_rob_data, pq_entry);
+ ngtcp2_pq_pop(&rob->pq);
+ rob->bufferedlen -= rdat->datalen;
+
+ ngtcp2_rob_data_del(rdat, rob->mem);
+}
diff --git a/lib/ngtcp2_framebuf.h b/lib/ngtcp2_rob.h
similarity index 57%
rename from lib/ngtcp2_framebuf.h
rename to lib/ngtcp2_rob.h
index 2cafd493..d10ee471 100644
--- a/lib/ngtcp2_framebuf.h
+++ b/lib/ngtcp2_rob.h
@@ -22,8 +22,8 @@
* OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
-#ifndef NGTCP2_FRAMEBUF_H
-#define NGTCP2_FRAMEBUF_H
+#ifndef NGTCP2_ROB_H
+#define NGTCP2_ROB_H
#ifdef HAVE_CONFIG_H
#include <config.h>
@@ -36,13 +36,38 @@
typedef struct {
ngtcp2_pq_entry pq_entry;
- ngtcp2_frame fr;
+ uint64_t offset;
uint8_t *data;
-} ngtcp2_framebuf;
+ size_t datalen;
+} ngtcp2_rob_data;
-int ngtcp2_framebuf_new(ngtcp2_framebuf **pfb, ngtcp2_stream *fr,
- ngtcp2_mem *mem);
+int ngtcp2_rob_data_new(ngtcp2_rob_data **prdat, uint64_t offset,
+ const uint8_t *data, size_t datalen, ngtcp2_mem *mem);
-void ngtcp2_framebuf_del(ngtcp2_framebuf *fb, ngtcp2_mem *mem);
+void ngtcp2_rob_data_del(ngtcp2_rob_data *rdat, ngtcp2_mem *mem);
-#endif /* NGTCP2_FRAMEBUF_H */
+/*
+ * ngtcp2_rob reassembles stream data received in out of order.
+ *
+ * TODO The current implementation is very inefficient. It should be
+ * redesigned to reduce memory foot print, and avoid dead lock issue.
+ */
+typedef struct {
+ ngtcp2_pq pq;
+ ngtcp2_mem *mem;
+ uint64_t bufferedlen;
+} ngtcp2_rob;
+
+int ngtcp2_rob_init(ngtcp2_rob *rob, ngtcp2_mem *mem);
+
+void ngtcp2_rob_free(ngtcp2_rob *rob);
+
+int ngtcp2_rob_push(ngtcp2_rob *rob, uint64_t offset, const uint8_t *data,
+ size_t datalen);
+
+size_t ngtcp2_rob_data_at(ngtcp2_rob *rob, const uint8_t **pdest,
+ uint64_t offset);
+
+void ngtcp2_rob_pop(ngtcp2_rob *rob);
+
+#endif /* NGTCP2_ROB_H */
--
GitLab