astlpc: Make MTU configurable at binding instantiation
Make the MTU value provided to mctp_astlpc_init() stick. Previously we
just printed a warning and forced the MTU to the baseline transmission
unit. Now that MTU negotiation is in place, accept the provided value.
Change-Id: I0c026ba0a94a26a6d99755d3debf048b6e0b4aca
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
diff --git a/astlpc.c b/astlpc.c
index 0011777..d8129d1 100644
--- a/astlpc.c
+++ b/astlpc.c
@@ -56,6 +56,7 @@
uint8_t mode;
uint16_t version;
+ uint32_t requested_mtu;
/* direct ops data */
struct mctp_binding_astlpc_ops ops;
@@ -379,6 +380,13 @@
sz &= ~0xfUL;
sz = ASTLPC_PACKET_SIZE(MCTP_PACKET_SIZE(sz));
+ if (astlpc->requested_mtu) {
+ size_t r;
+
+ r = ASTLPC_PACKET_SIZE(MCTP_PACKET_SIZE(astlpc->requested_mtu));
+ sz = MIN(sz, r);
+ }
+
/* Flip the buffers as the names are defined in terms of the host */
astlpc->layout.tx.offset = control_size;
astlpc->layout.tx.size = sz;
@@ -386,8 +394,10 @@
astlpc->layout.tx.offset + astlpc->layout.tx.size;
astlpc->layout.rx.size = sz;
- /* Sanity check that can be eliminated if asserts are off */
- assert(mctp_astlpc_layout_validate(&astlpc->layout));
+ if (!mctp_astlpc_layout_validate(&astlpc->layout)) {
+ astlpc_prerr(astlpc, "Cannot support an MTU of %zu", sz);
+ return -EINVAL;
+ }
hdr = (struct mctp_lpcmap_hdr){
.magic = htobe32(ASTLPC_MCTP_MAGIC),
@@ -470,7 +480,10 @@
return -EINVAL;
}
- sz = ASTLPC_PACKET_SIZE(MCTP_PACKET_SIZE(MCTP_BTU));
+ astlpc_prinfo(astlpc, "Desire an MTU of %" PRIu32 " bytes",
+ astlpc->requested_mtu);
+
+ sz = ASTLPC_PACKET_SIZE(MCTP_PACKET_SIZE(astlpc->requested_mtu));
layout.rx.size = sz;
if (!mctp_astlpc_layout_validate(&layout)) {
@@ -484,7 +497,8 @@
return -EINVAL;
}
- astlpc_prinfo(astlpc, "Requesting MTU of %" PRIu32 " bytes", MCTP_BTU);
+ astlpc_prinfo(astlpc, "Requesting MTU of %" PRIu32 " bytes",
+ astlpc->requested_mtu);
return mctp_astlpc_layout_write(astlpc, &layout);
}
@@ -666,6 +680,12 @@
high = MAX(astlpc->layout.rx.offset, astlpc->layout.tx.offset);
limit = high - low;
+ /* Determine the largest MTU the BMC _wants_ to support */
+ if (astlpc->requested_mtu) {
+ uint32_t req = astlpc->requested_mtu;
+
+ limit = MIN(limit, ASTLPC_PACKET_SIZE(MCTP_PACKET_SIZE(req)));
+ }
/* Determine the accepted MTU, applied both directions by convention */
return MCTP_BODY_SIZE(ASTLPC_BODY_SIZE(MIN(limit, layout->tx.size)));
@@ -950,6 +970,7 @@
memset(astlpc, 0, sizeof(*astlpc));
astlpc->mode = mode;
astlpc->lpc_map = NULL;
+ astlpc->requested_mtu = mtu;
astlpc->binding.name = "astlpc";
astlpc->binding.version = 1;
astlpc->binding.pkt_size = MCTP_PACKET_SIZE(mtu);
@@ -985,12 +1006,6 @@
return NULL;
}
- if (mtu != MCTP_BTU) {
- mctp_prwarn("Unable to negotiate the MTU, using %u instead",
- MCTP_BTU);
- mtu = MCTP_BTU;
- }
-
astlpc = __mctp_astlpc_init(mode, mtu);
if (!astlpc)
return NULL;
diff --git a/tests/test_astlpc.c b/tests/test_astlpc.c
index 6de249a..9ed99c2 100644
--- a/tests/test_astlpc.c
+++ b/tests/test_astlpc.c
@@ -165,7 +165,8 @@
};
static int endpoint_init(struct astlpc_endpoint *ep, mctp_eid_t eid,
- uint8_t mode, uint8_t (*kcs)[2], void *lpc_mem)
+ uint8_t mode, uint32_t mtu, uint8_t (*kcs)[2],
+ void *lpc_mem)
{
/*
* Configure the direction of the KCS interface so we know whether to
@@ -180,7 +181,7 @@
ep->mmio.kcs = kcs;
/* Initialise the binding */
- ep->astlpc = mctp_astlpc_init(mode, MCTP_BTU, lpc_mem,
+ ep->astlpc = mctp_astlpc_init(mode, mtu, lpc_mem,
&astlpc_direct_mmio_ops, &ep->mmio);
assert(ep->astlpc);
@@ -201,14 +202,14 @@
assert(ctx->lpc_mem);
/* BMC initialisation */
- rc = endpoint_init(&ctx->bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC,
+ rc = endpoint_init(&ctx->bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, MCTP_BTU,
&ctx->kcs, ctx->lpc_mem);
assert(!rc);
assert(ctx->kcs[MCTP_ASTLPC_KCS_REG_STATUS] & KCS_STATUS_BMC_READY);
/* Host initialisation */
rc = endpoint_init(&ctx->host, 9, MCTP_BINDING_ASTLPC_MODE_HOST,
- &ctx->kcs, ctx->lpc_mem);
+ MCTP_BTU, &ctx->kcs, ctx->lpc_mem);
assert(!rc);
/* BMC processes host channel init request, alerts host */
@@ -440,8 +441,8 @@
assert(lpc_mem);
/* BMC initialisation */
- rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &kcs,
- lpc_mem);
+ rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, MCTP_BTU,
+ &kcs, lpc_mem);
assert(!rc);
/* Now the BMC is initialised, break its version announcement */
@@ -449,8 +450,8 @@
hdr->bmc_ver_cur = ASTLPC_VER_BAD;
/* Host initialisation */
- rc = endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, &kcs,
- lpc_mem);
+ rc = endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, MCTP_BTU,
+ &kcs, lpc_mem);
assert(rc < 0);
endpoint_destroy(&bmc);
@@ -471,13 +472,13 @@
assert(lpc_mem);
/* BMC initialisation */
- rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &kcs,
- lpc_mem);
+ rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, MCTP_BTU,
+ &kcs, lpc_mem);
assert(!rc);
/* Host initialisation */
- rc = endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, &kcs,
- lpc_mem);
+ rc = endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, MCTP_BTU,
+ &kcs, lpc_mem);
assert(!rc);
/* Now the host is initialised, break its version announcement */
@@ -509,16 +510,16 @@
assert(lpc_mem);
/* BMC initialisation */
- rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &kcs,
- lpc_mem);
+ rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, MCTP_BTU,
+ &kcs, lpc_mem);
assert(!rc);
/* Verify the BMC binding was initialised */
assert(kcs[MCTP_ASTLPC_KCS_REG_STATUS] & KCS_STATUS_BMC_READY);
/* Host initialisation */
- rc = endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, &kcs,
- lpc_mem);
+ rc = endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, MCTP_BTU,
+ &kcs, lpc_mem);
assert(!rc);
/* Host sends channel init command */
@@ -637,8 +638,8 @@
astlpc_assert_tx_packet(&ctx.host, &unwritten[0], MCTP_BTU);
/* BMC comes back */
- rc = endpoint_init(&ctx.bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &ctx.kcs,
- ctx.lpc_mem);
+ rc = endpoint_init(&ctx.bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, MCTP_BTU,
+ &ctx.kcs, ctx.lpc_mem);
assert(!rc);
mctp_set_rx_all(ctx.bmc.mctp, rx_message, &ctx);
@@ -670,8 +671,8 @@
assert(lpc_mem);
/* BMC initialisation */
- rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &kcs,
- lpc_mem);
+ rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, MCTP_BTU,
+ &kcs, lpc_mem);
assert(!rc);
/* Check for a command despite none present */
@@ -696,8 +697,8 @@
assert(lpc_mem);
/* BMC initialisation */
- rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &kcs,
- lpc_mem);
+ rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, MCTP_BTU,
+ &kcs, lpc_mem);
assert(!rc);
/* 0x5a isn't legal in v1 or v2 */
@@ -842,16 +843,21 @@
struct mctp_lpcmap_hdr *hdr;
uint8_t kcs[2] = { 0 };
void *lpc_mem;
+ int rc;
/* Test harness initialisation */
lpc_mem = calloc(1, 1 * 1024 * 1024);
assert(lpc_mem);
/* BMC initialisation */
- endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &kcs, lpc_mem);
+ rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, MCTP_BTU,
+ &kcs, lpc_mem);
+ assert(!rc);
/* Host initialisation */
- endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, &kcs, lpc_mem);
+ rc = endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, MCTP_BTU,
+ &kcs, lpc_mem);
+ assert(!rc);
/*
* Now that the host has initialised the control area, break
@@ -883,7 +889,9 @@
assert(lpc_mem);
/* BMC initialisation */
- endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &kcs, lpc_mem);
+ rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, MCTP_BTU,
+ &kcs, lpc_mem);
+ assert(!rc);
/*
* Now that the BMC has initialised the control area, break something
@@ -893,8 +901,8 @@
hdr->layout.rx_size = 0;
/* Host initialisation: Fails due to bad layout */
- rc = endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, &kcs,
- lpc_mem);
+ rc = endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, MCTP_BTU,
+ &kcs, lpc_mem);
assert(rc < 0);
endpoint_destroy(&host);
@@ -915,10 +923,14 @@
assert(lpc_mem);
/* BMC initialisation */
- endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &kcs, lpc_mem);
+ rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, MCTP_BTU,
+ &kcs, lpc_mem);
+ assert(!rc);
/* Host initialisation */
- endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, &kcs, lpc_mem);
+ rc = endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, MCTP_BTU,
+ &kcs, lpc_mem);
+ assert(!rc);
mctp_astlpc_poll(bmc.astlpc);
@@ -937,6 +949,126 @@
free(lpc_mem);
}
+static void astlpc_test_buffers_bad_host_init(void)
+{
+ struct astlpc_endpoint host;
+ uint8_t kcs[2] = { 0 };
+ void *lpc_mem;
+ int rc;
+
+ /* Test harness initialisation */
+ lpc_mem = calloc(1, 1 * 1024 * 1024);
+ assert(lpc_mem);
+
+ host.mctp = mctp_init();
+ assert(host.mctp);
+ host.mmio.kcs = &kcs;
+ host.mmio.bmc = false;
+
+ /* Set the MTU to 0 to provoke a failure */
+ host.astlpc =
+ mctp_astlpc_init(MCTP_BINDING_ASTLPC_MODE_HOST, 0, lpc_mem,
+ &astlpc_direct_mmio_ops, &host.mmio);
+
+ rc = mctp_register_bus(host.mctp, &host.astlpc->binding, 8);
+ assert(rc < 0);
+
+ mctp_astlpc_destroy(host.astlpc);
+ mctp_destroy(host.mctp);
+ free(lpc_mem);
+}
+
+static void astlpc_test_negotiate_increased_mtu(void)
+{
+ struct astlpc_endpoint bmc, host;
+ uint8_t kcs[2] = { 0 };
+ void *lpc_mem;
+ int rc;
+
+ /* Test harness initialisation */
+ lpc_mem = calloc(1, 1 * 1024 * 1024);
+ assert(lpc_mem);
+
+ /* BMC initialisation */
+ rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, 3 * MCTP_BTU,
+ &kcs, lpc_mem);
+ assert(!rc);
+
+ /* Host initialisation */
+ rc = endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST,
+ 2 * MCTP_BTU, &kcs, lpc_mem);
+ assert(!rc);
+
+ rc = mctp_astlpc_poll(bmc.astlpc);
+ assert(rc == 0);
+
+ rc = mctp_astlpc_poll(host.astlpc);
+ assert(rc == 0);
+
+ endpoint_destroy(&host);
+ endpoint_destroy(&bmc);
+ free(lpc_mem);
+}
+
+static void astlpc_test_negotiate_mtu_low_high(void)
+{
+ struct astlpc_endpoint bmc, host;
+ uint8_t kcs[2] = { 0 };
+ void *lpc_mem;
+ int rc;
+
+ /* Test harness initialisation */
+ lpc_mem = calloc(1, 1 * 1024 * 1024);
+ assert(lpc_mem);
+
+ /* BMC initialisation */
+ rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, 3 * MCTP_BTU,
+ &kcs, lpc_mem);
+ assert(!rc);
+
+ /* Host initialisation with low MTU */
+ rc = endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST,
+ 2 * MCTP_BTU, &kcs, lpc_mem);
+ assert(!rc);
+
+ /* Process low MTU proposal */
+ rc = mctp_astlpc_poll(bmc.astlpc);
+ assert(rc == 0);
+
+ /* Accept low MTU proposal */
+ rc = mctp_astlpc_poll(host.astlpc);
+ assert(rc == 0);
+
+ assert(host.astlpc->layout.rx.size ==
+ ASTLPC_PACKET_SIZE(MCTP_PACKET_SIZE(2 * MCTP_BTU)));
+
+ /* Tear-down the host so we can bring up a new one */
+ endpoint_destroy(&host);
+
+ /*
+ * Bring up a new host endpoint with a higher MTU than we previously
+ * negotiated
+ */
+ rc = endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST,
+ 3 * MCTP_BTU, &kcs, lpc_mem);
+ assert(!rc);
+
+ /* Process high MTU proposal */
+ rc = mctp_astlpc_poll(bmc.astlpc);
+ assert(rc == 0);
+
+ /* Accept high MTU proposal */
+ rc = mctp_astlpc_poll(host.astlpc);
+ assert(rc == 0);
+
+ assert(host.astlpc->layout.rx.size ==
+ ASTLPC_PACKET_SIZE(MCTP_PACKET_SIZE(3 * MCTP_BTU)));
+
+ endpoint_destroy(&host);
+ endpoint_destroy(&bmc);
+ free(lpc_mem);
+}
+
/* clang-format off */
#define TEST_CASE(test) { #test, test }
static const struct {
@@ -973,6 +1105,9 @@
TEST_CASE(astlpc_test_buffers_bad_bmc_negotiation),
TEST_CASE(astlpc_test_buffers_overlap_exact),
TEST_CASE(astlpc_test_buffers_overlap_control),
+ TEST_CASE(astlpc_test_buffers_bad_host_init),
+ TEST_CASE(astlpc_test_negotiate_increased_mtu),
+ TEST_CASE(astlpc_test_negotiate_mtu_low_high),
};
/* clang-format on */