core: Reuse buffers for tx, allow message pools
Use new m_msg_alloc/m_msg_free operations for whole-message
MCTP buffers. m_realloc is no longer used, instead the maximum
sized buffer is allocated for each reassembly.
This allows applications to keep a pool of MCTP message buffers.
Don't create a queue of packets to transmit, instead reuse a single
binding-provided tx_storage buffer for each transmitted packet, which
can be static for bindings that have a known maximum packet size.
Asynchronous users/bindings can no longer rely on the core for queueing
TX packets, instead they should test mctp_is_tx_ready() prior to calling
mctp_message_tx(). The stack will return -EBUSY from mctp_message_tx()
if there is already a message pending to send.
Bindings must be updated to add the tx_storage member, and the core will
no longer free packets passed to mctp_bus_rx().
Change-Id: I2598bb91026ccef01b268c52b06c0f8e20bebb1e
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
diff --git a/core.c b/core.c
index 33f5093..dd8376a 100644
--- a/core.c
+++ b/core.c
@@ -17,6 +17,7 @@
#include "libmctp-log.h"
#include "libmctp-cmds.h"
#include "range.h"
+#include "compiler.h"
/* Internal data structures */
@@ -30,19 +31,33 @@
mctp_eid_t eid;
struct mctp_binding *binding;
enum mctp_bus_state state;
+ struct mctp *mctp;
- struct mctp_pktbuf *tx_queue_head;
- struct mctp_pktbuf *tx_queue_tail;
+ /* Current message to transmit */
+ void *tx_msg;
+ /* Position in tx_msg */
+ size_t tx_msgpos;
+ /* Length of tx_msg */
+ size_t tx_msglen;
+ /* Length of current packet payload */
+ size_t tx_pktlen;
+ uint8_t tx_seq;
+ uint8_t tx_src;
+ uint8_t tx_dest;
+ bool tx_to;
+ uint8_t tx_tag;
/* todo: routing */
};
struct mctp_msg_ctx {
+ /* NULL buf indicates an unused mctp_msg_ctx */
+ void *buf;
+
uint8_t src;
uint8_t dest;
uint8_t tag;
uint8_t last_seq;
- void *buf;
size_t buf_size;
size_t buf_alloc_size;
size_t fragment_size;
@@ -70,6 +85,8 @@
ROUTE_BRIDGE,
} route_policy;
size_t max_message_size;
+
+ void *alloc_ctx;
};
#ifndef ARRAY_SIZE
@@ -88,32 +105,44 @@
struct mctp_pktbuf *mctp_pktbuf_alloc(struct mctp_binding *binding, size_t len)
{
- struct mctp_pktbuf *buf;
- size_t size;
-
- size = binding->pkt_size + binding->pkt_header + binding->pkt_trailer;
+ size_t size =
+ binding->pkt_size + binding->pkt_header + binding->pkt_trailer;
if (len > size) {
return NULL;
}
- /* todo: pools */
- buf = __mctp_alloc(sizeof(*buf) + size);
-
- if (!buf)
+ void *storage = __mctp_alloc(size + sizeof(struct mctp_pktbuf));
+ if (!storage) {
return NULL;
-
- buf->size = size;
- buf->start = binding->pkt_header;
- buf->end = buf->start + len;
- buf->mctp_hdr_off = buf->start;
- buf->next = NULL;
-
- return buf;
+ }
+ struct mctp_pktbuf *pkt = mctp_pktbuf_init(binding, storage);
+ pkt->alloc = true;
+ pkt->end = pkt->start + len;
+ return pkt;
}
void mctp_pktbuf_free(struct mctp_pktbuf *pkt)
{
- __mctp_free(pkt);
+ if (pkt->alloc) {
+ __mctp_free(pkt);
+ } else {
+ mctp_prdebug("pktbuf_free called for non-alloced");
+ }
+}
+
+struct mctp_pktbuf *mctp_pktbuf_init(struct mctp_binding *binding,
+ void *storage)
+{
+ size_t size =
+ binding->pkt_size + binding->pkt_header + binding->pkt_trailer;
+ struct mctp_pktbuf *buf = (struct mctp_pktbuf *)storage;
+ buf->size = size;
+ buf->start = binding->pkt_header;
+ buf->end = buf->start;
+ buf->mctp_hdr_off = buf->start;
+ buf->alloc = false;
+
+ return buf;
}
struct mctp_hdr *mctp_pktbuf_hdr(struct mctp_pktbuf *pkt)
@@ -126,7 +155,7 @@
return pkt->data + pkt->mctp_hdr_off + sizeof(struct mctp_hdr);
}
-size_t mctp_pktbuf_size(struct mctp_pktbuf *pkt)
+size_t mctp_pktbuf_size(const struct mctp_pktbuf *pkt)
{
return pkt->end - pkt->start;
}
@@ -172,6 +201,19 @@
return pkt->data + pkt->end;
}
+/* Allocate a duplicate of the message and copy it */
+static void *mctp_msg_dup(const void *msg, size_t msg_len, struct mctp *mctp)
+{
+ void *copy = __mctp_msg_alloc(msg_len, mctp);
+ if (!copy) {
+ mctp_prdebug("msg dup len %zu failed", msg_len);
+ return NULL;
+ }
+
+ memcpy(copy, msg, msg_len);
+ return copy;
+}
+
/* Message reassembly */
static struct mctp_msg_ctx *mctp_msg_ctx_lookup(struct mctp *mctp, uint8_t src,
uint8_t dest, uint8_t tag)
@@ -182,7 +224,8 @@
* message contexts */
for (i = 0; i < ARRAY_SIZE(mctp->msg_ctxs); i++) {
struct mctp_msg_ctx *ctx = &mctp->msg_ctxs[i];
- if (ctx->src == src && ctx->dest == dest && ctx->tag == tag)
+ if (ctx->buf && ctx->src == src && ctx->dest == dest &&
+ ctx->tag == tag)
return ctx;
}
@@ -197,7 +240,7 @@
for (i = 0; i < ARRAY_SIZE(mctp->msg_ctxs); i++) {
struct mctp_msg_ctx *tmp = &mctp->msg_ctxs[i];
- if (!tmp->src) {
+ if (!tmp->buf) {
ctx = tmp;
break;
}
@@ -209,14 +252,22 @@
ctx->src = src;
ctx->dest = dest;
ctx->tag = tag;
+
ctx->buf_size = 0;
+ ctx->buf_alloc_size = mctp->max_message_size;
+ ctx->buf = __mctp_msg_alloc(ctx->buf_alloc_size, mctp);
+ if (!ctx->buf) {
+ return NULL;
+ }
return ctx;
}
-static void mctp_msg_ctx_drop(struct mctp_msg_ctx *ctx)
+static void mctp_msg_ctx_drop(struct mctp_bus *bus, struct mctp_msg_ctx *ctx)
{
- ctx->src = 0;
+ /* Free and mark as unused */
+ __mctp_msg_free(ctx->buf, bus->mctp);
+ ctx->buf = NULL;
}
static void mctp_msg_ctx_reset(struct mctp_msg_ctx *ctx)
@@ -226,7 +277,7 @@
}
static int mctp_msg_ctx_add_pkt(struct mctp_msg_ctx *ctx,
- struct mctp_pktbuf *pkt, size_t max_size)
+ struct mctp_pktbuf *pkt)
{
size_t len;
@@ -237,29 +288,7 @@
}
if (ctx->buf_size + len > ctx->buf_alloc_size) {
- size_t new_alloc_size;
- void *lbuf;
-
- /* @todo: finer-grained allocation */
- if (!ctx->buf_alloc_size) {
- new_alloc_size = MAX(len, 4096UL);
- } else {
- new_alloc_size = MAX(ctx->buf_alloc_size * 2,
- len + ctx->buf_size);
- }
-
- /* Don't allow heap to grow beyond a limit */
- if (new_alloc_size > max_size)
- return -1;
-
- lbuf = __mctp_realloc(ctx->buf, new_alloc_size);
- if (lbuf) {
- ctx->buf = lbuf;
- ctx->buf_alloc_size = new_alloc_size;
- } else {
- __mctp_free(ctx->buf);
- return -1;
- }
+ return -1;
}
memcpy((uint8_t *)ctx->buf + ctx->buf_size, mctp_pktbuf_data(pkt), len);
@@ -295,13 +324,11 @@
mctp->capture_data = user;
}
-static void mctp_bus_destroy(struct mctp_bus *bus)
+static void mctp_bus_destroy(struct mctp_bus *bus, struct mctp *mctp)
{
- while (bus->tx_queue_head) {
- struct mctp_pktbuf *curr = bus->tx_queue_head;
-
- bus->tx_queue_head = curr->next;
- mctp_pktbuf_free(curr);
+ if (bus->tx_msg) {
+ __mctp_msg_free(bus->tx_msg, mctp);
+ bus->tx_msg = NULL;
}
}
@@ -314,11 +341,11 @@
for (i = 0; i < ARRAY_SIZE(mctp->msg_ctxs); i++) {
struct mctp_msg_ctx *tmp = &mctp->msg_ctxs[i];
if (tmp->buf)
- __mctp_free(tmp->buf);
+ __mctp_msg_free(tmp->buf, mctp);
}
while (mctp->n_busses--)
- mctp_bus_destroy(&mctp->busses[mctp->n_busses]);
+ mctp_bus_destroy(&mctp->busses[mctp->n_busses], mctp);
__mctp_free(mctp->busses);
__mctp_free(mctp);
@@ -351,11 +378,14 @@
assert(mctp->n_busses == 0);
mctp->n_busses = 1;
+ assert(binding->tx_storage);
+
mctp->busses = __mctp_alloc(sizeof(struct mctp_bus));
if (!mctp->busses)
return -ENOMEM;
memset(mctp->busses, 0, sizeof(struct mctp_bus));
+ mctp->busses[0].mctp = mctp;
mctp->busses[0].binding = binding;
mctp->busses[0].eid = eid;
binding->bus = &mctp->busses[0];
@@ -393,6 +423,9 @@
{
int rc = 0;
+ assert(b1->tx_storage);
+ assert(b2->tx_storage);
+
assert(mctp->n_busses == 0);
mctp->busses = __mctp_alloc(2 * sizeof(struct mctp_bus));
if (!mctp->busses)
@@ -524,8 +557,13 @@
if (dest_bus == bus)
continue;
+ void *copy = mctp_msg_dup(buf, len, mctp);
+ if (!copy) {
+ return;
+ }
+
mctp_message_tx_on_bus(dest_bus, src, dest, tag_owner,
- msg_tag, buf, len);
+ msg_tag, copy, len);
}
}
}
@@ -571,8 +609,14 @@
/* single-packet message - send straight up to rx function,
* no need to create a message context */
len = pkt->end - pkt->mctp_hdr_off - sizeof(struct mctp_hdr);
- p = pkt->data + pkt->mctp_hdr_off + sizeof(struct mctp_hdr);
- mctp_rx(mctp, bus, hdr->src, hdr->dest, tag_owner, tag, p, len);
+ p = mctp_msg_dup(pkt->data + pkt->mctp_hdr_off +
+ sizeof(struct mctp_hdr),
+ len, mctp);
+ if (p) {
+ mctp_rx(mctp, bus, hdr->src, hdr->dest, tag_owner, tag,
+ p, len);
+ __mctp_msg_free(p, mctp);
+ }
break;
case MCTP_HDR_FLAG_SOM:
@@ -597,9 +641,9 @@
* should of the same size */
ctx->fragment_size = mctp_pktbuf_size(pkt);
- rc = mctp_msg_ctx_add_pkt(ctx, pkt, mctp->max_message_size);
+ rc = mctp_msg_ctx_add_pkt(ctx, pkt);
if (rc) {
- mctp_msg_ctx_drop(ctx);
+ mctp_msg_ctx_drop(bus, ctx);
} else {
ctx->last_seq = seq;
}
@@ -617,7 +661,7 @@
mctp_prdebug(
"Sequence number %d does not match expected %d",
seq, exp_seq);
- mctp_msg_ctx_drop(ctx);
+ mctp_msg_ctx_drop(bus, ctx);
goto out;
}
@@ -627,16 +671,16 @@
mctp_prdebug("Unexpected fragment size. Expected"
" less than %zu, received = %zu",
ctx->fragment_size, len);
- mctp_msg_ctx_drop(ctx);
+ mctp_msg_ctx_drop(bus, ctx);
goto out;
}
- rc = mctp_msg_ctx_add_pkt(ctx, pkt, mctp->max_message_size);
+ rc = mctp_msg_ctx_add_pkt(ctx, pkt);
if (!rc)
mctp_rx(mctp, bus, ctx->src, ctx->dest, tag_owner, tag,
ctx->buf, ctx->buf_size);
- mctp_msg_ctx_drop(ctx);
+ mctp_msg_ctx_drop(bus, ctx);
break;
case 0:
@@ -650,7 +694,7 @@
mctp_prdebug(
"Sequence number %d does not match expected %d",
seq, exp_seq);
- mctp_msg_ctx_drop(ctx);
+ mctp_msg_ctx_drop(bus, ctx);
goto out;
}
@@ -660,13 +704,13 @@
mctp_prdebug("Unexpected fragment size. Expected = %zu "
"received = %zu",
ctx->fragment_size, len);
- mctp_msg_ctx_drop(ctx);
+ mctp_msg_ctx_drop(bus, ctx);
goto out;
}
- rc = mctp_msg_ctx_add_pkt(ctx, pkt, mctp->max_message_size);
+ rc = mctp_msg_ctx_add_pkt(ctx, pkt);
if (rc) {
- mctp_msg_ctx_drop(ctx);
+ mctp_msg_ctx_drop(bus, ctx);
goto out;
}
ctx->last_seq = seq;
@@ -674,15 +718,17 @@
break;
}
out:
- mctp_pktbuf_free(pkt);
+ return;
}
static int mctp_packet_tx(struct mctp_bus *bus, struct mctp_pktbuf *pkt)
{
struct mctp *mctp = bus->binding->mctp;
- if (bus->state != mctp_bus_state_tx_enabled)
+ if (bus->state != mctp_bus_state_tx_enabled) {
+ mctp_prdebug("tx with bus disabled");
return -1;
+ }
if (mctp->capture)
mctp->capture(pkt, MCTP_MESSAGE_CAPTURE_OUTGOING,
@@ -691,36 +737,95 @@
return bus->binding->tx(bus->binding, pkt);
}
+/* Returns a pointer to the binding's tx_storage */
+static struct mctp_pktbuf *mctp_next_tx_pkt(struct mctp_bus *bus)
+{
+ if (!bus->tx_msg) {
+ return NULL;
+ }
+
+ size_t p = bus->tx_msgpos;
+ size_t msg_len = bus->tx_msglen;
+ size_t payload_len = msg_len - p;
+ size_t max_payload_len = MCTP_BODY_SIZE(bus->binding->pkt_size);
+ if (payload_len > max_payload_len)
+ payload_len = max_payload_len;
+
+ struct mctp_pktbuf *pkt =
+ mctp_pktbuf_init(bus->binding, bus->binding->tx_storage);
+ struct mctp_hdr *hdr = mctp_pktbuf_hdr(pkt);
+
+ hdr->ver = bus->binding->version & 0xf;
+ hdr->dest = bus->tx_dest;
+ hdr->src = bus->tx_src;
+ hdr->flags_seq_tag = (bus->tx_to << MCTP_HDR_TO_SHIFT) |
+ (bus->tx_tag << MCTP_HDR_TAG_SHIFT);
+
+ if (p == 0)
+ hdr->flags_seq_tag |= MCTP_HDR_FLAG_SOM;
+ if (p + payload_len >= msg_len)
+ hdr->flags_seq_tag |= MCTP_HDR_FLAG_EOM;
+ hdr->flags_seq_tag |= bus->tx_seq << MCTP_HDR_SEQ_SHIFT;
+
+ memcpy(mctp_pktbuf_data(pkt), (uint8_t *)bus->tx_msg + p, payload_len);
+ pkt->end = pkt->start + sizeof(*hdr) + payload_len;
+ bus->tx_pktlen = payload_len;
+
+ mctp_prdebug(
+ "tx dst %d tag %d payload len %zu seq %d. msg pos %zu len %zu",
+ hdr->dest, bus->tx_tag, payload_len, bus->tx_seq, p, msg_len);
+
+ return pkt;
+}
+
+/* Called when a packet has successfully been sent */
+static void mctp_tx_complete(struct mctp_bus *bus)
+{
+ if (!bus->tx_msg) {
+ mctp_prdebug("tx complete no message");
+ return;
+ }
+
+ bus->tx_seq = (bus->tx_seq + 1) & MCTP_HDR_SEQ_MASK;
+ bus->tx_msgpos += bus->tx_pktlen;
+
+ if (bus->tx_msgpos >= bus->tx_msglen) {
+ __mctp_msg_free(bus->tx_msg, bus->binding->mctp);
+ bus->tx_msg = NULL;
+ }
+}
+
static void mctp_send_tx_queue(struct mctp_bus *bus)
{
struct mctp_pktbuf *pkt;
- while ((pkt = bus->tx_queue_head)) {
+ while (bus->tx_msg && bus->state == mctp_bus_state_tx_enabled) {
int rc;
+ pkt = mctp_next_tx_pkt(bus);
+
rc = mctp_packet_tx(bus, pkt);
switch (rc) {
- /* If transmission succeded, or */
+ /* If transmission succeded */
case 0:
- /* If the packet is somehow too large */
- case -EMSGSIZE:
/* Drop the packet */
- bus->tx_queue_head = pkt->next;
- mctp_pktbuf_free(pkt);
+ mctp_tx_complete(bus);
break;
- /* If the binding was busy, or */
+ /* If the binding was busy */
case -EBUSY:
+ /* Keep the packet for next try */
+ mctp_prdebug("tx EBUSY");
+ return;
+
/* Some other unknown error occurred */
default:
- /* Make sure the tail pointer is consistent and retry later */
- goto cleanup_tail;
+ /* Drop the packet */
+ mctp_prdebug("tx drop %d", rc);
+ mctp_tx_complete(bus);
+ return;
};
}
-
-cleanup_tail:
- if (!bus->tx_queue_head)
- bus->tx_queue_tail = NULL;
}
void mctp_binding_set_tx_enabled(struct mctp_binding *binding, bool enable)
@@ -765,74 +870,60 @@
mctp_eid_t dest, bool tag_owner,
uint8_t msg_tag, void *msg, size_t msg_len)
{
- size_t max_payload_len, payload_len, p;
- struct mctp_pktbuf *pkt;
- struct mctp_hdr *hdr;
- int i;
+ size_t max_payload_len;
+ int rc;
- if (bus->state == mctp_bus_state_constructed)
- return -ENXIO;
+ if (bus->state == mctp_bus_state_constructed) {
+ rc = -ENXIO;
+ goto err;
+ }
- if ((msg_tag & MCTP_HDR_TAG_MASK) != msg_tag)
- return -EINVAL;
+ if ((msg_tag & MCTP_HDR_TAG_MASK) != msg_tag) {
+ rc = -EINVAL;
+ goto err;
+ }
max_payload_len = MCTP_BODY_SIZE(bus->binding->pkt_size);
{
const bool valid_mtu = max_payload_len >= MCTP_BTU;
assert(valid_mtu);
- if (!valid_mtu)
- return -EINVAL;
+ if (!valid_mtu) {
+ rc = -EINVAL;
+ goto err;
+ }
}
mctp_prdebug(
"%s: Generating packets for transmission of %zu byte message from %hhu to %hhu",
__func__, msg_len, src, dest);
- /* queue up packets, each of max MCTP_MTU size */
- for (p = 0, i = 0; p < msg_len; i++) {
- payload_len = msg_len - p;
- if (payload_len > max_payload_len)
- payload_len = max_payload_len;
-
- pkt = mctp_pktbuf_alloc(bus->binding,
- payload_len + sizeof(*hdr));
- hdr = mctp_pktbuf_hdr(pkt);
-
- hdr->ver = bus->binding->version & 0xf;
- hdr->dest = dest;
- hdr->src = src;
- hdr->flags_seq_tag = (tag_owner << MCTP_HDR_TO_SHIFT) |
- (msg_tag << MCTP_HDR_TAG_SHIFT);
-
- if (i == 0)
- hdr->flags_seq_tag |= MCTP_HDR_FLAG_SOM;
- if (p + payload_len >= msg_len)
- hdr->flags_seq_tag |= MCTP_HDR_FLAG_EOM;
- hdr->flags_seq_tag |= (i & MCTP_HDR_SEQ_MASK)
- << MCTP_HDR_SEQ_SHIFT;
-
- memcpy(mctp_pktbuf_data(pkt), (uint8_t *)msg + p, payload_len);
-
- /* add to tx queue */
- if (bus->tx_queue_tail)
- bus->tx_queue_tail->next = pkt;
- else
- bus->tx_queue_head = pkt;
- bus->tx_queue_tail = pkt;
-
- p += payload_len;
+ if (bus->tx_msg) {
+ mctp_prdebug("Bus busy");
+ rc = -EBUSY;
+ goto err;
}
- mctp_prdebug("%s: Enqueued %d packets", __func__, i);
+ /* Take the message to send */
+ bus->tx_msg = msg;
+ bus->tx_msglen = msg_len;
+ bus->tx_msgpos = 0;
+ /* bus->tx_seq is allowed to continue from previous message */
+ bus->tx_src = src;
+ bus->tx_dest = dest;
+ bus->tx_to = tag_owner;
+ bus->tx_tag = msg_tag;
mctp_send_tx_queue(bus);
-
return 0;
+
+err:
+ __mctp_msg_free(msg, bus->binding->mctp);
+ return rc;
}
-int mctp_message_tx(struct mctp *mctp, mctp_eid_t eid, bool tag_owner,
- uint8_t msg_tag, void *msg, size_t msg_len)
+int mctp_message_tx_alloced(struct mctp *mctp, mctp_eid_t eid, bool tag_owner,
+ uint8_t msg_tag, void *msg, size_t msg_len)
{
struct mctp_bus *bus;
@@ -840,13 +931,49 @@
* different callers */
if ((msg_tag & MCTP_HDR_TAG_MASK) != msg_tag) {
mctp_prerr("Incorrect message tag %u passed.", msg_tag);
+ __mctp_msg_free(msg, mctp);
return -EINVAL;
}
bus = find_bus_for_eid(mctp, eid);
- if (!bus)
+ if (!bus) {
+ __mctp_msg_free(msg, mctp);
return 0;
+ }
return mctp_message_tx_on_bus(bus, bus->eid, eid, tag_owner, msg_tag,
msg, msg_len);
}
+
+int mctp_message_tx(struct mctp *mctp, mctp_eid_t eid, bool tag_owner,
+ uint8_t msg_tag, const void *msg, size_t msg_len)
+{
+ void *copy = mctp_msg_dup(msg, msg_len, mctp);
+ if (!copy) {
+ return -ENOMEM;
+ }
+
+ return mctp_message_tx_alloced(mctp, eid, tag_owner, msg_tag, copy,
+ msg_len);
+}
+
+bool mctp_is_tx_ready(struct mctp *mctp, mctp_eid_t eid)
+{
+ struct mctp_bus *bus;
+
+ bus = find_bus_for_eid(mctp, eid);
+ if (!bus) {
+ return true;
+ }
+ return bus->tx_msg == NULL;
+}
+
+void *mctp_get_alloc_ctx(struct mctp *mctp)
+{
+ return mctp->alloc_ctx;
+}
+
+void mctp_set_alloc_ctx(struct mctp *mctp, void *ctx)
+{
+ mctp->alloc_ctx = ctx;
+}