From ce09e96152977d2737abdb7b6781badb03be0a05 Mon Sep 17 00:00:00 2001 From: Andreas Smas Date: Mon, 7 Sep 2026 15:58:16 +0000 Subject: [PATCH 01/10] vllp: fix five buffer-management bugs reachable from the wire All of these are reachable by a peer on the bus, and none of them need malice -- a noisy CAN bus or a peer that has lost sync is enough. They were found by a new adversarial-peer test suite and by tightening the buffer accounting in the existing one. net/vllp.c: - vllp_channel_receive() leaked the whole reassembled chain when a message failed its CRC. STAILQ_INIT() has already detached it from the channel at that point, so pb is the only remaining reference and the channel teardown that follows cannot help. One corrupted message costs a message worth of buffers, permanently, so a device on a noisy bus bleeds its pbuf pool until it stops talking to anything. - The reassembly limit was enforced by closing the channel, which calls into the application bound to it -- but the management channel is the one channel with no application. A peer sending more fragments than the limit without ever setting the last-fragment bit dereferenced a NULL function pointer on the net thread. It now drops the link, which is the only sensible response to an oversized management message, and an assert documents the invariant in vllp_channel_net_close(). - vllp_disconnect() reset the management channel's transmit queue but not its reassembly queue. The management channel outlives the session, so a half-reassembled inbound message stayed queued and the next session's fragments were appended to it: the message then failed its CRC, which reset the link, which stranded another partial message. A link that resets under load never recovered. - The management-channel handler's default case returned without freeing the message it had been handed, leaking a buffer per frame carrying an opcode the spec reserves. net/pbuf.c: - pbuf_free_queue_irq_blocked() freed the chain but left the queue head pointing at the freed buffers. Most callers discard the containing object immediately and never notice; the ones that flush a queue and keep using it (vllp_disconnect() on the management channel, l2cap's reassembly flush) freed the same buffers again on the next flush and corrupted the pool free list, which then surfaces as a crash in whatever unrelated code allocates next. It now leaves the queue empty. Tests: - suite_vllp_frames.c: a new suite driving the server from a peer that hand-assembles frames, so the paths a correct client never asks for get exercised on purpose -- oversized management messages, reserved opcodes, and a partial message left behind by a session reset. Each phase then checks that the server still serves and that the pool is where it started. - suite_vllp.c: a heavy payload-corruption phase whose only assertion is buffer accounting, plus an end-of-run pool check. The existing corrupt-1 phase only ever loses a single one-fragment message, which is small enough to hide inside any sane tolerance. Every fix was verified by reverting it and confirming the suite fails and names the cause. Co-Authored-By: Claude --- src/net/pbuf.c | 10 + src/net/vllp.c | 42 ++- src/platform/host/host.mk | 1 + src/platform/host/suite_vllp.c | 79 +++++ src/platform/host/suite_vllp_frames.c | 448 ++++++++++++++++++++++++++ 5 files changed, 579 insertions(+), 1 deletion(-) create mode 100644 src/platform/host/suite_vllp_frames.c diff --git a/src/net/pbuf.c b/src/net/pbuf.c index 01f11d65..17547d2e 100644 --- a/src/net/pbuf.c +++ b/src/net/pbuf.c @@ -294,6 +294,16 @@ void pbuf_free_queue_irq_blocked(struct pbuf_queue *pq) { pbuf_free_irq_blocked(STAILQ_FIRST(pq)); + // Leave the queue empty, not dangling. Most callers throw the + // containing object away immediately afterwards and never notice, but + // some flush a queue and keep using it (vllp_disconnect() on the + // management channel, which outlives the session; l2cap's reassembly + // flush, which lets the channel starve out). Those used to be left + // pointing at freed buffers, so the next flush freed them a second + // time and corrupted the pool free list -- which then shows up as a + // crash somewhere else entirely, in whatever unlucky code allocated + // next. + STAILQ_INIT(pq); } diff --git a/src/net/vllp.c b/src/net/vllp.c index 1b7ce5b2..6b93939e 100644 --- a/src/net/vllp.c +++ b/src/net/vllp.c @@ -307,6 +307,8 @@ vllp_channel_net_close(vllp_t *v, vllp_channel_t *vc, if(vc->net_closed) return 0; + // Never called for the management channel, which has no app bound. + assert(vc->pp.app != NULL); vc->pp.app->close(vc->pp.app_opaque, reason); vc->net_closed = 1; LIST_REMOVE(vc, link); @@ -347,10 +349,21 @@ vllp_disconnect(vllp_t *v, const char *reason) vllp_channel_destroy(v, vc); } - // Anything queued on the CMC belongs to the old session + // Anything on the CMC belongs to the old session. The CMC outlives the + // session (it is never destroyed), so unlike every other channel it is + // not cleaned up by the loop above and has to be reset here -- + // including a half-reassembled *inbound* message. Leaving that in place + // does not just hold the buffers: the next session's first fragments + // are appended to the stale ones, so the message fails its CRC, which + // resets the link, which strands another partial message. { int q = irq_forbid(IRQ_LEVEL_NET); pbuf_free_queue_irq_blocked(&v->cmc->txq); + pbuf_free_queue_irq_blocked(&v->cmc->rxq); + if(v->cmc->stalled_tx) { + pbuf_free_irq_blocked(v->cmc->stalled_tx); + v->cmc->stalled_tx = NULL; + } irq_permit(q); } v->ack_pending = 0; @@ -524,6 +537,9 @@ handle_cmc(vllp_t *v, vllp_channel_t *cmc, pbuf_t *pb) return handle_cmc_close(v, cmc, pb, target_channel, u8 + 1, len - 1); default: + // Ownership of pb is ours (see vllp_channel_receive), and the + // ERR_BAD_STATE below drops the link, not the buffer. + pbuf_free(pb); return ERR_BAD_STATE; } } @@ -565,6 +581,20 @@ vllp_channel_receive(vllp_t *v, int channel_id, evlog(LOG_WARNING, "VLLP: channel %d: message larger than %d bytes, " "closing channel", vc->id, (int)vllp_max_message_size()); + + if(vc == v->cmc) { + // The management channel has no app bound, so it must not go + // through vllp_channel_net_close() -- that dereferences pp.app, + // which is NULL here. A peer that sends an oversized management + // message would otherwise crash us, and it takes nothing more + // than a few fragments with the last-fragment bit clear. + // + // An oversized management message means the peer is broken or we + // have lost sync with it, and neither is recoverable on this + // session, so drop the link and let it be rebuilt. + return ERR_BAD_STATE; + } + vc->close_error = ERR_MTU_EXCEEDED; vllp_channel_net_close(v, vc, "message too large"); return 0; // Fragment consumed (dropped); the rest follow suit @@ -607,6 +637,16 @@ vllp_channel_receive(vllp_t *v, int channel_id, STAILQ_INIT(&vc->rxq); if(calc_crc32(pb, vc->rx_crc_IV)) { + // STAILQ_INIT() above detached the chain from the channel, so pb is + // the only reference left to it -- returning without freeing loses + // every buffer in the message. The channel teardown that follows + // cannot help, because the chain is no longer on any queue. + // + // Reachable from the wire: a peer (or a noisy bus) that corrupts + // frames leaks a whole message worth of buffers per bad CRC, + // permanently, until the pool is empty and the device stops talking + // to anything at all. + pbuf_free(pb); return ERR_CHECKSUM_ERROR; } vc->rx_crc_IV++; diff --git a/src/platform/host/host.mk b/src/platform/host/host.mk index a2d9e9a0..96adc4a7 100644 --- a/src/platform/host/host.mk +++ b/src/platform/host/host.mk @@ -44,6 +44,7 @@ SRCS += ${P}/host.c \ ${P}/sim_dhcpd.c \ ${P}/suite_dhcp.c \ ${P}/suite_vllp.c \ + ${P}/suite_vllp_frames.c \ ${P}/suite_ota.c \ ${P}/vspiflash.c \ ${P}/host_ota.c \ diff --git a/src/platform/host/suite_vllp.c b/src/platform/host/suite_vllp.c index 6dc60ef6..8f33a1e8 100644 --- a/src/platform/host/suite_vllp.c +++ b/src/platform/host/suite_vllp.c @@ -485,6 +485,70 @@ phase_faults(scenario_t *sc, const char *name, faults_t f, int expect_resets) } +/* Buffer accounting across heavy payload corruption. + * + * A message that reassembles completely and only then fails its CRC takes + * a different path from a frame that is rejected on arrival: the chain has + * already been detached from the channel, so whoever holds it last has to + * free it. That path leaked the entire message, permanently, once per bad + * CRC. + * + * Made deliberately harsh -- a high corruption rate on multi-fragment + * messages -- because the gentle corrupt-1 phase only ever loses a single + * one-fragment message, which is small enough to hide inside any sane + * tolerance. This is the phase that makes such a leak obvious. + */ +static void +phase_corrupt_leak(scenario_t *sc) +{ + hosttest_log("-- corruption buffer accounting"); + + const int pool_before = pbuf_buffer_avail(); + const size_t len = echo_max_len() < 500 ? echo_max_len() : 500; + + hvllp_channel_t *ch = open_echo(sc); + SCHECK(sc, ch != NULL, "corrupt_leak: open failed"); + if(ch == NULL) + return; + + sc->ctx.f = (faults_t){ .corrupt = 10 }; + + int completed = 0; + for(int i = 0; i < 40; i++) { + if(ch == NULL) { + sc->ctx.f = (faults_t){0}; + if(!wait_connected(sc, 6 * SEC)) + break; + sc->ctx.f = (faults_t){ .corrupt = 10 }; + ch = open_echo(sc); + if(ch == NULL) + break; + } + if(echo_rt(sc, ch, len, i) == 0) + completed++; + else { + hvllp_channel_close(ch, 0, 0); + ch = NULL; + } + } + + sc->ctx.f = (faults_t){0}; + if(ch != NULL) + hvllp_channel_close(ch, 0, 1); + + /* Let the link settle so nothing is still legitimately in flight. */ + hvllp_sim_run(sc->v, clock_get() + 5 * SEC); + + const int pool_after = pbuf_buffer_avail(); + hosttest_log(" %d/40 round trips, pool %d -> %d", completed, pool_before, + pool_after); + SCHECK(sc, pool_after >= pool_before - 2, + "corrupt_leak: %d buffers lost across %d corrupted round trips -- " + "a message that fails its CRC after full reassembly is leaking " + "its whole chain", pool_before - pool_after, 40 - completed); +} + + static void scenario_fn(void *arg) { @@ -510,6 +574,7 @@ scenario_fn(void *arg) phase_faults(sc, "loss-20", (faults_t){ .drop = 20 }, 0); phase_faults(sc, "dup-10", (faults_t){ .dup = 10 }, 0); phase_faults(sc, "corrupt-1", (faults_t){ .corrupt = 1 }, 1); + phase_corrupt_leak(sc); SCHECK(sc, hvllp_is_connected(v), "link not up at end"); @@ -568,6 +633,20 @@ test_vllp(void) hosttest_log(" pool %d buffers, %u allocation failures", pool_total, fails); + // Every channel has been closed and the link is idle, so every buffer + // the stack took must be back. This is a blunt instrument on purpose: + // it does not care where a leak is, only that the run did not lose + // buffers, and it covers the paths the fault phases above reach that + // nothing else does. A leak on an error path is invisible in normal + // operation and then bricks a device that has been up for a month -- + // the corrupt-1 phase used to leak an entire message of buffers on + // every bad CRC, which is exactly the shape of thing this catches. + const int pool_now = pbuf_buffer_avail(); + CHECK(pool_now >= pool_total - 2, + "%d of %d buffers are still held after every channel closed and " + "the link went idle -- something on an error path is leaking them", + pool_total - pool_now, pool_total); + // A pool this small is only worth running to exercise what happens // when it runs dry, so require that it actually did. Without this the // starved variant silently degenerates into a duplicate of the others diff --git a/src/platform/host/suite_vllp_frames.c b/src/platform/host/suite_vllp_frames.c new file mode 100644 index 00000000..82ced015 --- /dev/null +++ b/src/platform/host/suite_vllp_frames.c @@ -0,0 +1,448 @@ +/* + * vllp-frames: the mios VLLP server against a peer that sends things no + * well-behaved client would. + * + * Every other VLLP suite drives the server with a real client, so it only + * ever reaches the paths a correct implementation asks for. The server is + * on a bus, though, and a bus carries whatever anyone puts on it: a peer + * mid-firmware-update, a peer that has lost sync, or simply noise. This + * suite hand-assembles frames so those paths get exercised on purpose. + * + * The peer here is deliberately not a VLLP implementation -- it is a few + * dozen lines that know the wire format from docs/vllp.txt. It mirrors the + * server's own SE bookkeeping (that part has to be right or the server + * rejects everything as out of sequence) and nothing else. + * + * Each phase ends by checking that the server still works and that the + * buffer pool is where it started, because the interesting failures here + * are not "wrong answer" but "crashed" and "quietly ate the pool". + */ + +#include +#include +#include +#include +#include + +#include + +#include "net/pbuf.h" +#include "util/crc32.h" + +#include "hosttest.h" +#include "sim.h" +#include "vcan.h" + +#define SEC 1000000ull + +/* The server, from the peer's point of view. */ +#define SRV_TX 0x300 +#define SRV_RX 0x301 +#define MTU 64 + +/* Wire constants, from docs/vllp.txt. Private to vllp.c, so restated. */ +#define F_SYN 0x0f +#define F_S 0x80 +#define F_E 0x40 +#define F_F 0x20 +#define F_L 0x10 + +#define CMC_CHANNEL 14 +#define CMC_OP_OPEN 0 +#define CMC_OP_OPEN_RESPONSE 2 +#define CMC_OP_UNUSED 1 /* reserved by the spec; nothing sends it */ + +static int fails; + +#define FCHECK(cond, ...) \ + do { if(!(cond)) { fails++; \ + hosttest_check(0, __FILE__, __LINE__, __VA_ARGS__); } } while(0) + + +typedef struct peer { + vcan_t *vcan; + uint32_t cookie; + uint8_t se; /* mirrors the server's own S/E bookkeeping */ + uint32_t iv_cnt; /* per-session channel IV counter */ + uint32_t cmc_tx_iv; /* IV for messages we send on the CMC */ + uint32_t cmc_rx_iv; /* ...and expect on */ + volatile int done; + volatile int failed_setup; +} peer_t; + + +/* ---- wire helpers ---- */ + +static int +pad_ladder(int len) +{ + if(len < 12) return 12; + if(len < 16) return 16; + if(len < 20) return 20; + if(len < 24) return 24; + if(len < 32) return 32; + if(len < 48) return 48; + return 64; +} + + +/* Send one frame, applying the FDCAN length adaptation the server + expects: anything over 8 bytes is padded up the DLC ladder with the pad + count in the final byte. */ +static void +tx(peer_t *p, const void *data, size_t len) +{ + uint8_t f[72]; + memcpy(f, data, len); + if(len > 8) { + const int total = pad_ladder(len); + const int pad = total - len; + memset(f + len, 0, pad); + f[total - 1] = pad; + len = total; + } + vcan_peer_send(p->vcan, SRV_RX, f, len); +} + + +/* Receive one frame from the server, stripping the pad. -1 on timeout. */ +static long +rx(peer_t *p, uint8_t *buf, size_t buflen, uint64_t deadline) +{ + while(1) { + uint32_t id; + long n = vcan_peer_recv(p->vcan, &id, buf, buflen, deadline); + if(n < 0) + return -1; + if(id != SRV_TX) + continue; + if(n > 8) { + const int pad = buf[n - 1]; + if(pad < n) + n -= pad; + } + return n; + } +} + + +/* Drain whatever the server has to say, keeping our SE in step. Returns + the number of data frames seen (ACKs do not count). */ +static int +drain(peer_t *p, uint64_t for_us) +{ + const uint64_t deadline = clock_get() + for_us; + uint8_t f[72]; + int data = 0; + + while(1) { + long n = rx(p, f, sizeof(f), deadline); + if(n < 1) + return data; + if((f[0] & 0x1f) == 0x1f) + continue; /* pure ACK: consumes no sequence */ + /* A data frame. Accept it and flip what we expect next, exactly as + the server does when it accepts one of ours. */ + p->se ^= F_E; + data++; + } +} + + +/* Open a session. Returns 0 on success. */ +static int +handshake(peer_t *p, uint32_t cookie) +{ + p->cookie = cookie; + p->se = F_E; + + uint8_t syn[7]; + syn[0] = F_SYN; + syn[1] = 2; /* version */ + syn[2] = MTU - 1; /* the server's adapted MTU */ + memcpy(syn + 3, &p->cookie, 4); + tx(p, syn, sizeof(syn)); + + uint8_t f[72]; + const uint64_t deadline = clock_get() + 3 * SEC; + while(1) { + long n = rx(p, f, sizeof(f), deadline); + if(n < 0) + return -1; + if(n == 7 && f[0] == (F_E | 0x1f)) + break; /* the ACK that answers our SYN */ + } + + /* Per-session channel IVs. The server derives the management channel's + from the cookie and its own counter, and mirrors the polarity: what + it transmits with is what we receive with. Keep the counter in step + -- the server advances it once per channel it is asked to open, + including ones it refuses. */ + p->iv_cnt = 1; + const uint32_t cmc_iv = crc32(p->cookie, &p->iv_cnt, sizeof(p->iv_cnt)); + p->cmc_rx_iv = cmc_iv; + p->cmc_tx_iv = ~cmc_iv; + return 0; +} + + +/* Send one fragment on a channel. `last` sets the end-of-message bit. */ +static void +tx_fragment(peer_t *p, int channel, const void *payload, size_t len, + int last) +{ + uint8_t f[72]; + p->se ^= F_S; /* a data frame consumes our sequence */ + f[0] = p->se | (last ? F_L : 0) | F_F | channel; + memcpy(f + 1, payload, len); + tx(p, f, len + 1); +} + + +/* Send a complete message on the management channel: fragment it, append + the message CRC to the last fragment, and keep the IV rolling. */ +static void +tx_cmc_message(peer_t *p, const void *msg, size_t len) +{ + uint8_t buf[256]; + memcpy(buf, msg, len); + + const uint32_t crc = ~crc32(p->cmc_tx_iv, buf, len); + buf[len + 0] = crc; + buf[len + 1] = crc >> 8; + buf[len + 2] = crc >> 16; + buf[len + 3] = crc >> 24; + len += 4; + p->cmc_tx_iv++; + + const size_t frag = MTU - 2; /* header, and one byte for the pad count */ + size_t off = 0; + while(off < len) { + const size_t n = MIN(frag, len - off); + tx_fragment(p, CMC_CHANNEL, buf + off, n, off + n == len); + off += n; + drain(p, 200000); /* let the server ack and advance */ + } +} + + +/* Ask the server to open `service`, and report the error code it answers + with. Returns the 16-bit code, or -1 if nothing came back. */ +static int +open_channel(peer_t *p, int channel, const char *service) +{ + uint8_t msg[64]; + const size_t namelen = strlen(service); + msg[0] = (CMC_OP_OPEN << 4) | channel; + memcpy(msg + 1, service, namelen); + + /* The server generates a channel IV per OPEN it receives. */ + p->iv_cnt++; + + tx_cmc_message(p, msg, 1 + namelen); + + /* The answer arrives as a management-channel message. We only need the + opcode and the code, and every CMC response fits one fragment. */ + uint8_t f[72]; + const uint64_t deadline = clock_get() + 5 * SEC; + while(1) { + long n = rx(p, f, sizeof(f), deadline); + if(n < 0) + return -1; + if((f[0] & 0x1f) == 0x1f) + continue; + if((f[0] & 0xf) != CMC_CHANNEL) + continue; + p->se ^= F_E; + /* [hdr][opcode|channel][err lo][err hi][crc32] */ + if(n >= 4 && (f[1] >> 4) == CMC_OP_OPEN_RESPONSE) + return f[2] | (f[3] << 8); + } +} + + +/* Is the server still able to do its job? Opens a real service on a fresh + session and checks the answer. */ +static int +server_healthy(peer_t *p, uint32_t cookie) +{ + if(handshake(p, cookie)) + return 0; + return open_channel(p, 0, "echo") == 0; +} + + +/* ---- phases ---- */ + +/* An oversized message on the *management* channel. + * + * The reassembly limit is enforced by closing the channel, and closing a + * channel calls into the application bound to it -- but the management + * channel is the one channel that has no application. A peer that sends + * more fragments than the limit without ever setting the last-fragment + * bit therefore used to dereference a NULL function pointer, from the net + * thread, on a frame anyone on the bus can send. + */ +static void +phase_oversize_cmc(peer_t *p) +{ + hosttest_log("-- oversized management message"); + + const int pool_before = pbuf_buffer_avail(); + + FCHECK(handshake(p, 0xa1b2c3d4) == 0, "oversize: handshake failed"); + + /* Fragments that never end. Enough to exceed the reassembly limit + whatever the configured buffer size is: the limit is a number of + buffers, so size the count from the buffer size. */ + uint8_t junk[MTU - 2]; + memset(junk, 0x5a, sizeof(junk)); + const int frags = (PBUF_DATA_SIZE * 6) / sizeof(junk) + 8; + + for(int i = 0; i < frags; i++) { + tx_fragment(p, CMC_CHANNEL, junk, sizeof(junk), 0); + drain(p, 100000); + } + + /* Surviving this at all is most of the point. */ + hosttest_log(" sent %d unterminated fragments, server still alive", + frags); + + /* The session is expected to be gone -- an oversized management + message is not recoverable -- but the server must still serve. */ + FCHECK(server_healthy(p, 0xa1b2c3d5), + "oversize: server no longer opens channels afterwards"); + + /* And it must not have eaten the pool on the way. */ + drain(p, SEC); + const int pool_after = pbuf_buffer_avail(); + FCHECK(pool_after >= pool_before - 2, + "oversize: %d buffers lost", pool_before - pool_after); +} + + +/* A management message with an opcode the spec reserves. Nothing sends + these, so the handler's default case never ran in normal operation -- + and it returned without freeing the message it had been handed. */ +static void +phase_unknown_opcode(peer_t *p) +{ + hosttest_log("-- reserved management opcode"); + + const int pool_before = pbuf_buffer_avail(); + + /* Repeated, because one leaked buffer hides inside any tolerance. */ + const int rounds = 20; + for(int i = 0; i < rounds; i++) { + if(handshake(p, 0xb0000000 + i)) { + FCHECK(0, "unknown_opcode: handshake %d failed", i); + return; + } + uint8_t msg[1] = { (CMC_OP_UNUSED << 4) | 0 }; + tx_cmc_message(p, msg, sizeof(msg)); + drain(p, 500000); + } + + FCHECK(server_healthy(p, 0xb0ffffff), + "unknown_opcode: server broken after %d reserved opcodes", rounds); + + drain(p, SEC); + const int pool_after = pbuf_buffer_avail(); + hosttest_log(" %d reserved opcodes, pool %d -> %d", rounds, pool_before, + pool_after); + FCHECK(pool_after >= pool_before - 2, + "unknown_opcode: %d buffers lost over %d messages -- the handler " + "is dropping the message without freeing it", + pool_before - pool_after, rounds); +} + + +/* A management message left half-reassembled when the session dies. + * + * Every other channel is destroyed on disconnect, which frees whatever it + * was holding. The management channel is not -- it outlives the session -- + * so a partial message stayed on its reassembly queue and the next + * session's fragments were appended to it. The result is a message that + * cannot pass its CRC, which resets the link, which strands another + * partial message: a link that never recovers, and a buffer lost each + * time round. + */ +static void +phase_partial_across_reset(peer_t *p) +{ + hosttest_log("-- partial management message across a session reset"); + + const int pool_before = pbuf_buffer_avail(); + + for(int i = 0; i < 3; i++) { + if(handshake(p, 0xc0000000 + i)) { + FCHECK(0, "partial: handshake %d failed", i); + return; + } + + /* One fragment of a message that never completes... */ + uint8_t junk[16]; + memset(junk, 0x33 + i, sizeof(junk)); + tx_fragment(p, CMC_CHANNEL, junk, sizeof(junk), 0); + drain(p, 200000); + + /* ...then go quiet for longer than the link timeout so the server + tears the session down with that fragment still in hand. */ + usleep(5 * SEC); + } + + /* A fresh session must work. If the stale fragments are still queued, + this message reassembles as [stale][ours] and fails its CRC. */ + FCHECK(server_healthy(p, 0xc0ffffff), + "partial: the server cannot open a channel on a new session -- a " + "half-reassembled management message from a dead session is " + "still queued and corrupting the new one"); + + drain(p, SEC); + const int pool_after = pbuf_buffer_avail(); + hosttest_log(" pool %d -> %d", pool_before, pool_after); + FCHECK(pool_after >= pool_before - 2, + "partial: %d buffers lost", pool_before - pool_after); +} + + +static void +peer_fn(void *arg) +{ + peer_t *p = arg; + + phase_oversize_cmc(p); + phase_unknown_opcode(p); + phase_partial_across_reset(p); + + p->done = 1; +} + + +static int +pred_done(void *arg) +{ + peer_t *p = arg; + return p->done; +} + + +static int +test_vllp_frames(void) +{ + hosttest_log("---- mios VLLP server vs a hand-assembled peer ----"); + + vcan_t *vcan = vcan_create("vcan0", MTU); + vllp_server_create(SRV_TX, SRV_RX, MTU, 3); + vcan_set_link(vcan, 1); + + peer_t *p = calloc(1, sizeof(peer_t)); + p->vcan = vcan; + + sim_thread_create("frame-peer", peer_fn, p, 1 << 18); + + CHECK(hosttest_wait(pred_done, p, 300 * SEC), "peer did not finish"); + return fails; +} + +HOSTTEST_SUITE("vllp-frames", test_vllp_frames, 0); From 30684496ef638583be198d1438b8abc7e8a82626 Mon Sep 17 00:00:00 2001 From: Andreas Smas Date: Mon, 7 Sep 2026 16:00:19 +0000 Subject: [PATCH 02/10] vllp: add the client role, and a way to bind a remote console to a vcon Lets a gateway on a CAN bus offer a console for each unit on the bus: the gateway is a VLLP client on one link per unit, each link keeps a channel open to the unit's ordinary "shell" service, and that channel is wired to a local virtual console. An operator telnets to the gateway and runs "attach unit3". Nothing changes on the units. The client role lives in src/net/vllp.c alongside the server rather than in a file of its own -- it is the same protocol, and the fragmentation, flow control, retransmission and reassembly code is shared verbatim. What the client adds is the handshake (a cookie, and a SYN once a second until the server answers), channel-id allocation, the OPEN/OPEN_RESPONSE half of channel management, and the mirrored per-channel CRC IVs. Behind ENABLE_VLLP_CLIENT, default no. This has to be a compile-time option rather than dead code the linker drops: the client paths hang off vllp_rx(), the keepalive timer and vllp_maybe_tx(), all of which a server needs, so --gc-sections cannot reach them. Measured on vexpress-a9, a server-only build pays 336 bytes of text and nothing at all in RAM -- sizeof(vllp_t) is unchanged, and cmd_show_vllp comes out the same size it was. With the client compiled in it is 3.5 KB and 48 bytes per link endpoint. The show_vllp additions are behind the flag as well, including the per-channel buffer counts. Those are useful on a server too -- they are what localises a draining pool to a channel and a queue -- but CLI commands live in a KEEP'd section, so anything added there is paid for by every build whether it is reachable or not. The bind timer is set up in vllp_client_create() rather than on the shared path, because assigning its callback is what makes the whole bind machinery reachable: doing it in vllp_create() linked ~600 bytes of it into images that only ever instantiate servers. mios ships two orthogonal primitives here and leaves the joining to the application, because which CAN ids, which console names, which remote service and how many units are all board policy: vcon_t *vc = vcon_create("unit1", 4096, 128); vllp_t *v = vllp_client_create(0x530, 0x531, 8, 3); vllp_client_bind(v, "shell", unit_console_open, vc); vllp_client_bind() keeps a channel open for the life of the system, re-opening it on every new session and, after a refused open, on a backoff. It asks the application to bind a fresh pushpull each time, so the pushpull contract ("after close() the network side calls nothing again") holds across reconnects without the application having to know about them. vcon_pushpull_open() is the vcon end of that. It spawns no thread for any number of consoles: push() and pull() run in net context, and keystrokes reach the engine through vcon_set_backend_notify(), which is new. Without that hook pull() only runs when the once-a-second keepalive next moves the engine, so a keystroke waits up to a full second -- measured 1 ms with the hook against 1 s without. The alternative was a poll thread per console, which on a nine-unit gateway is nine stacks. ENABLE_VCON was off everywhere and no in-tree platform set it, so none of src/util/vcon.c or src/shell/cmd_vcon.c had ever been compiled, let alone executed, by CI. host.mk now enables it and there are suites for it. Tests, all in virtual time: - vcon: the console itself. Scrollback replay, cursor resync when a client falls behind the window, multi-client mirroring, input merging, the ring's wraparound arithmetic, and the consoles/attach commands including the ^A escape handling. - vllp-client: the client against the server, over a looped-back virtual CAN bus. Includes the two invariants that fail silently and totally if they are wrong -- the mirrored CRC IVs, and that the client advances its per-channel IV counter for every OPEN it sends because the server advances its own for every OPEN it receives, including refused ones. - vcon-vllp: nine units end to end, with real remote shells. Crosstalk between links, both directions busy at once, detach/re-attach, keystroke latency, unit reboots, and loss/duplication/corruption. Also at a CAN-sized buffer pool and at a starved one with injected allocation failures. Supporting scaffolding: vcan_loop.{c,h} loops a vcan back on itself so many endpoints in one binary talk over the virtual bus, and carries the fault injection; testterm.{c,h} is a buffered stream standing in for an attached terminal. Also completes PBUF_ORIGIN_TRACE, which plumbed an origin string through the pool but never stored it. It now records the origin per arena slot and pbuf_origin_dump() lists what is checked out, which is what identified the CRC-failure leak fixed in the previous commit. Left switched off. Co-Authored-By: Claude --- Makefile | 7 + include/mios/vcon.h | 25 + include/mios/vcon_pushpull.h | 29 + include/mios/vllp.h | 45 ++ src/net/net.mk | 6 + src/net/pbuf.c | 55 +- src/net/pbuf.h | 7 + src/net/vcon_pushpull.c | 165 +++++ src/net/vllp.c | 650 ++++++++++++++++++- src/platform/host/host.mk | 13 + src/platform/host/suite_vcon.c | 676 ++++++++++++++++++++ src/platform/host/suite_vcon_vllp.c | 864 ++++++++++++++++++++++++++ src/platform/host/suite_vllp_client.c | 639 +++++++++++++++++++ src/platform/host/testterm.c | 203 ++++++ src/platform/host/testterm.h | 39 ++ src/platform/host/vcan.c | 8 +- src/platform/host/vcan_loop.c | 93 +++ src/platform/host/vcan_loop.h | 59 ++ src/util/vcon.c | 62 ++ 19 files changed, 3612 insertions(+), 33 deletions(-) create mode 100644 include/mios/vcon_pushpull.h create mode 100644 src/net/vcon_pushpull.c create mode 100644 src/platform/host/suite_vcon.c create mode 100644 src/platform/host/suite_vcon_vllp.c create mode 100644 src/platform/host/suite_vllp_client.c create mode 100644 src/platform/host/testterm.c create mode 100644 src/platform/host/testterm.h create mode 100644 src/platform/host/vcan_loop.c create mode 100644 src/platform/host/vcan_loop.h diff --git a/Makefile b/Makefile index d496a67b..31fc0dc6 100644 --- a/Makefile +++ b/Makefile @@ -93,6 +93,13 @@ ENABLE_PROFILE ?= no ENABLE_PERFTEST ?= no ENABLE_VCON ?= no +# The VLLP client role (vllp_client_create and friends). Only a client can +# establish a link and open channels, so a device that merely *serves* +# VLLP does not need it. Costs ~2 KB of flash and 48 bytes per link +# endpoint, and it cannot be linker-GC'd when compiled in because the +# client paths hang off the shared rx and timer callbacks. +ENABLE_VLLP_CLIENT ?= no + CONFIG_H := ${O}/include/config.h # diff --git a/include/mios/vcon.h b/include/mios/vcon.h index b025a2df..03abf011 100644 --- a/include/mios/vcon.h +++ b/include/mios/vcon.h @@ -59,6 +59,31 @@ void vcon_bind(vcon_t *vc, struct stream *term); // short if the input buffer is full). size_t vcon_input(vcon_t *vc, const void *buf, size_t len); +// Called when a client types something. For backends that are not a thread +// sitting in vcon_backend()'s read(): without this they have no way to +// learn a keystroke arrived, and the alternative is a thread per vcon +// whose only job is to poll. +// +// Runs on whichever thread called vcon_input(), with the vcon lock held, +// so it must not block and must not call back into vcon. Raising an event +// (pushpull_wakeup, net_task_raise) is what it is for. Holding the lock is +// deliberate: it is what lets a backend clear the callback and then free +// itself without racing a call already in flight. +void vcon_set_backend_notify(vcon_t *vc, void (*cb)(void *opaque), + void *opaque); + +// Look at up to `size` bytes of pending client input without consuming +// it, then consume what was actually used. Two steps so a backend that +// cannot take the bytes right now (no buffer, say) leaves them queued +// rather than dropping keystrokes. +size_t vcon_input_peek(vcon_t *vc, void *buf, size_t size); +void vcon_input_consume(vcon_t *vc, size_t len); + +// Discard pending client input. For a backend that has just reconnected: +// what is queued was typed at a session that no longer exists, and the +// tail of a half-typed command is worse than nothing in a fresh shell. +void vcon_input_flush(vcon_t *vc); + // Registry helpers (vcons are created at init and never destroyed). vcon_t *vcon_find(const char *name); vcon_t *vcon_first(void); diff --git a/include/mios/vcon_pushpull.h b/include/mios/vcon_pushpull.h new file mode 100644 index 00000000..18f55a24 --- /dev/null +++ b/include/mios/vcon_pushpull.h @@ -0,0 +1,29 @@ +#pragma once + +// Bind a virtual console to a pushpull channel. +// +// Console output arriving on the channel is appended to the vcon's +// scrollback; keystrokes from attached clients go back out on it. The +// intended use is a VLLP client channel opened against a remote unit's +// "shell" service, so `attach ` on a gateway gives a console on the +// unit: +// +// vcon_t *vc = vcon_create("unit1", 4096, 64); +// vllp_t *v = vllp_client_create(0x530, 0x531, 8, 3); +// vllp_client_bind(v, "shell", unit_console_open, vc); +// +// ...where unit_console_open() is a one-liner calling this. Nothing here +// is VLLP-specific though: it is a pushpull app, and works over any +// transport that speaks pushpull. +// +// Spawns no thread, for any number of consoles. + +#include + +struct pushpull; +typedef struct vcon vcon_t; + +// Suitable directly as a vllp_client_bind() open callback when the opaque +// is the vcon. Called again for every new session; each call binds a fresh +// app, and the previous one has already been closed. +error_t vcon_pushpull_open(vcon_t *vc, struct pushpull *pp); diff --git a/include/mios/vllp.h b/include/mios/vllp.h index 86a7401b..a4959100 100644 --- a/include/mios/vllp.h +++ b/include/mios/vllp.h @@ -3,10 +3,17 @@ #include #include +#include +#include + typedef struct vllp vllp_t; typedef struct vllp_channel vllp_channel_t; +#ifdef ENABLE_VLLP_CLIENT +typedef struct vllp_bind vllp_bind_t; +#endif + // Largest message the server can reassemble with the pbuf size this // build is configured for. A bigger message is dropped (and logged), and // because the sender gets no acknowledgement it will keep retrying, so @@ -16,3 +23,41 @@ size_t vllp_max_message_size(void); vllp_t *vllp_server_create(uint32_t txid, uint32_t rxid, uint8_t mtu, uint8_t timeout_seconds); + +#ifdef ENABLE_VLLP_CLIENT + +// The client end of a link. Only a client may establish a link and open +// channels (see docs/vllp.txt), so the two ends of an id pair must be +// created with different calls -- one server, one client. A client SYNs +// once a second until the server answers, and re-SYNs after a timeout, so +// this may be called before the peer exists or the bus is up. +// +// `mtu` and `timeout_seconds` must match the peer's; a mismatched MTU is +// rejected during the handshake and logged. +vllp_t *vllp_client_create(uint32_t txid, uint32_t rxid, uint8_t mtu, + uint8_t timeout_seconds); + +// Open one channel to the remote service `service`, binding it to the +// caller's app functions (fill pp->app and pp->app_opaque first; the rest +// of `pp` is filled in on return). Requires an established link, and the +// channel is not re-opened after link loss -- the app learns about that +// through its pushpull close() callback. Use vllp_client_bind() for +// anything that should simply stay connected. +// +// `service` is referenced, not copied. +error_t vllp_client_channel_open(vllp_t *v, const char *service, + pushpull_t *pp); + +// Keep a channel to `service` open for the life of the system: opened when +// the link comes up, re-opened on every later session, and retried on a +// backoff if the peer refuses. `open` is called from net context to bind a +// fresh app to each new channel; the previous app has already had its +// close() callback invoked by then, so each session starts clean. +// +// May be called before the link is up. `service` is referenced, not +// copied. Returns a handle for introspection, or NULL on failure. +vllp_bind_t *vllp_client_bind(vllp_t *v, const char *service, + error_t (*open)(void *opaque, pushpull_t *pp), + void *opaque); + +#endif // ENABLE_VLLP_CLIENT diff --git a/src/net/net.mk b/src/net/net.mk index d25bdab6..8b12856b 100644 --- a/src/net/net.mk +++ b/src/net/net.mk @@ -30,6 +30,12 @@ SRCS-${ENABLE_NET_DSIG} += \ ${SRC}/net/dsig.c \ ${SRC}/net/vllp.c \ +# Glue for binding a virtual console to a pushpull channel (a VLLP client +# channel, typically). Needs pbufs, so it lives here rather than next to +# vcon.c in util/; boards decide whether to actually use it. +SRCS-${ENABLE_VCON}-${ENABLE_NET_STACK} += \ + ${SRC}/net/vcon_pushpull.c \ + SRCS-${ENABLE_NET_DSIG_UDP} += \ ${SRC}/net/dsig_udp.c \ diff --git a/src/net/pbuf.c b/src/net/pbuf.c index 17547d2e..d07020b8 100644 --- a/src/net/pbuf.c +++ b/src/net/pbuf.c @@ -28,6 +28,15 @@ typedef struct pbuf_pool { static struct pbuf_pool pbuf_datas = { . pp_wait = WAITABLE_INITIALIZER("pbufdata")}; static struct pbuf_pool pbufs = { . pp_wait = WAITABLE_INITIALIZER("pbuf")}; +#ifdef PBUF_ORIGIN_TRACE +// Provenance for every buffer in the data pool, indexed by arena slot. +// The pool is one contiguous arena of fixed-size slots, so an address +// maps straight to an index. +static void *pbuf_origin_arena_start; +static const char **pbuf_origin_tab; +static int pbuf_origin_slots; +#endif + int pbuf_buffer_avail(void) { @@ -219,20 +228,64 @@ pbuf_data_add(void *start, void *end) end = start + size; } size_t count = pbuf_pool_add(&pbuf_datas, start, end, PBUF_DATA_SIZE); +#ifdef PBUF_ORIGIN_TRACE + pbuf_origin_arena_start = start; + pbuf_origin_slots = count; + pbuf_origin_tab = xalloc(count * sizeof(const char *), 0, MEM_CLEAR); +#endif printf("pbuf: size:%d arena:%zd count:%zd\n", PBUF_DATA_SIZE, end - start, count); pbuf_alloc(count); } +#ifdef PBUF_ORIGIN_TRACE + +static int +pbuf_origin_slot(void *buf) +{ + if(pbuf_origin_tab == NULL || buf < pbuf_origin_arena_start) + return -1; + const size_t off = (uint8_t *)buf - (uint8_t *)pbuf_origin_arena_start; + const int slot = off / PBUF_DATA_SIZE; + return slot < pbuf_origin_slots ? slot : -1; +} + +void +pbuf_origin_dump(struct stream *st) +{ + if(pbuf_origin_tab == NULL) { + stprintf(st, "pbuf origin tracking not initialised\n"); + return; + } + stprintf(st, "pbuf data buffers currently held:\n"); + for(int i = 0; i < pbuf_origin_slots; i++) { + if(pbuf_origin_tab[i] != NULL) + stprintf(st, " slot %d: %s\n", i, pbuf_origin_tab[i]); + } +} + +#endif + void * pbuf_data_get0(int wait PBUF_ORIGIN_ARG_DECL) { - return pbuf_pool_get(&pbuf_datas, wait PBUF_ORIGIN_ARG_CALL); + void *buf = pbuf_pool_get(&pbuf_datas, wait PBUF_ORIGIN_ARG_CALL); +#ifdef PBUF_ORIGIN_TRACE + const int slot = pbuf_origin_slot(buf); + if(slot >= 0) + pbuf_origin_tab[slot] = origin; +#endif + return buf; } void pbuf_data_put(void *buf) { +#ifdef PBUF_ORIGIN_TRACE + const int slot = pbuf_origin_slot(buf); + if(slot >= 0) + pbuf_origin_tab[slot] = NULL; +#endif pbuf_pool_put(&pbuf_datas, buf); } diff --git a/src/net/pbuf.h b/src/net/pbuf.h index 1174fc80..a5374db2 100644 --- a/src/net/pbuf.h +++ b/src/net/pbuf.h @@ -176,6 +176,13 @@ void pbuf_fault_inject(unsigned int pct, uint32_t seed); int pbuf_buffer_total(void); +#ifdef PBUF_ORIGIN_TRACE +struct stream; +// List every data buffer currently checked out, with the function that +// took it. For chasing pool exhaustion. +void pbuf_origin_dump(struct stream *st); +#endif + // ========================================================= // Debug helpers diff --git a/src/net/vcon_pushpull.c b/src/net/vcon_pushpull.c new file mode 100644 index 00000000..07a951e0 --- /dev/null +++ b/src/net/vcon_pushpull.c @@ -0,0 +1,165 @@ +#include +#include +#include +#include + +#include + +#include +#include +#include +#include + +#include "net/pbuf.h" + +// Binds a virtual console to a pushpull channel: console output arrives as +// messages and is appended to the scrollback, keystrokes typed by attached +// clients go back out as messages. +// +// The usual consumer is a VLLP client channel opened against a remote +// unit's "shell" service, so that `attach ` on a gateway gives you a +// console on the unit (see vllp_client_bind). Nothing here knows about +// VLLP though -- it is a pushpull app like the ones in net/service, and +// works over any transport that speaks pushpull. +// +// No thread. push()/pull() run in net context, and keystrokes reach us +// through vcon_set_backend_notify() from whichever thread the attached +// client runs on. That matters when a gateway carries a console for each +// of a dozen units: a pump thread per unit would be a dozen stacks. + +typedef struct vcon_pp { + pushpull_t *vp_pp; + vcon_t *vp_vc; + uint8_t vp_dead; // the channel is gone; stop touching vp_pp +} vcon_pp_t; + + +// Console output from the far end. vcon_backend()'s write side only +// appends to the scrollback ring (dropping oldest when full) and never +// blocks, so this is safe to do on the net thread. +static uint32_t +vcon_pp_push(void *opaque, pbuf_t *pb) +{ + vcon_pp_t *vp = opaque; + stream_t *backend = vcon_backend(vp->vp_vc); + + for(pbuf_t *p = pb; p != NULL; p = p->pb_next) { + if(p->pb_buflen) + stream_write(backend, pbuf_cdata(p, 0), p->pb_buflen, 0); + } + + pbuf_free(pb); + return 0; +} + + +// Always ready. The scrollback ring drops the oldest bytes rather than +// refusing new ones, so there is no state in which we want the peer to +// stop sending -- and clearing the rx-flow-bit for a channel that never +// rejects anything confuses the peer's flow bookkeeping (see the note in +// vllp_refresh_local_flow_status). +static int +vcon_pp_may_push(void *opaque) +{ + return 1; +} + + +// Keystrokes toward the far end. Takes as much as the fifo has in one +// message: the per-message CRC and the link's one-fragment-at-a-time +// window make a burst of one-byte messages far more expensive than a +// single larger one, and a fast typist or a pasted line arrives as a +// burst. +static pbuf_t * +vcon_pp_pull(void *opaque) +{ + vcon_pp_t *vp = opaque; + pushpull_t *pp = vp->vp_pp; + + const size_t offset = pp->preferred_offset; + size_t room = MIN(pp->max_fragment_size, PBUF_DATA_SIZE - offset); + if(room == 0) + return NULL; + + // Peek before allocating. pull() is polled for every established + // channel on every transmit opportunity, so allocating first would have + // a gateway with a dozen idle consoles take and release a dozen buffers + // each time round -- and on a tight pool that can be the buffer the ACK + // path needed. Peeking also means a failed allocation leaves the + // keystrokes queued instead of eating them. + uint8_t buf[64]; + const size_t n = vcon_input_peek(vp->vp_vc, buf, MIN(room, sizeof(buf))); + if(n == 0) + return NULL; + + pbuf_t *pb = pbuf_make(offset, 0); + if(pb == NULL) + return NULL; // Nothing consumed; the engine re-polls us later + + memcpy(pbuf_append(pb, n), buf, n); + vcon_input_consume(vp->vp_vc, n); + return pb; +} + + +// The channel is gone -- link down, peer closed, or the open was refused. +// Leave a mark in the scrollback: an operator watching an attached console +// should be able to tell "the unit went away" from "the unit went quiet". +static void +vcon_pp_close(void *opaque, const char *reason) +{ + vcon_pp_t *vp = opaque; + char msg[96]; + + vp->vp_dead = 1; + vcon_set_backend_notify(vp->vp_vc, NULL, NULL); + + const int len = snprintf(msg, sizeof(msg), "\n[disconnected: %s]\n", + reason ? reason : "unknown"); + stream_write(vcon_backend(vp->vp_vc), msg, MIN((size_t)len, sizeof(msg) - 1), + 0); + + free(vp); +} + + +static const pushpull_app_fn_t vcon_pp_fn = { + .push = vcon_pp_push, + .may_push = vcon_pp_may_push, + .pull = vcon_pp_pull, + .close = vcon_pp_close, +}; + + +// A keystroke arrived on some other thread; ask the engine to pull. +static void +vcon_pp_notify(void *opaque) +{ + vcon_pp_t *vp = opaque; + if(!vp->vp_dead) + pushpull_wakeup(vp->vp_pp, PUSHPULL_EVENT_PULL); +} + + +error_t +vcon_pushpull_open(vcon_t *vc, pushpull_t *pp) +{ + vcon_pp_t *vp = xalloc(sizeof(vcon_pp_t), 0, MEM_MAY_FAIL | MEM_CLEAR); + if(vp == NULL) + return ERR_NO_MEMORY; + + vp->vp_pp = pp; + vp->vp_vc = vc; + + pp->app = &vcon_pp_fn; + pp->app_opaque = vp; + + // Anything an attached client typed before now is stale -- it was aimed + // at a session that no longer exists. + vcon_input_flush(vc); + vcon_set_backend_notify(vc, vcon_pp_notify, vp); + + static const char banner[] = "\n[connected]\n"; + stream_write(vcon_backend(vc), banner, sizeof(banner) - 1, 0); + return 0; +} diff --git a/src/net/vllp.c b/src/net/vllp.c index 6b93939e..e60d5d14 100644 --- a/src/net/vllp.c +++ b/src/net/vllp.c @@ -27,6 +27,27 @@ TAILQ_HEAD(vllp_channel_queue, vllp_channel); static struct vllp_list vllps; +// Everything below #ifdef ENABLE_VLLP_CLIENT is the client role. It is a +// compile-time option rather than dead code the linker drops, because it +// cannot be dropped: the client paths hang off vllp_rx(), the keepalive +// timer and vllp_maybe_tx(), all of which a server needs. See the flag's +// comment in the top-level Makefile. +#ifdef ENABLE_VLLP_CLIENT + +LIST_HEAD(vllp_bind_list, vllp_bind); + +// A persistent client-side channel: re-opened on every new session and, +// after a refused open, on a backoff. See vllp_client_bind(). +typedef struct vllp_bind { + LIST_ENTRY(vllp_bind) link; + const char *service; + error_t (*open)(void *opaque, pushpull_t *pp); + void *opaque; + struct vllp_channel *channel; // NULL while not open +} vllp_bind_t; + +#endif + typedef struct vllp { LIST_ENTRY(vllp) link; @@ -34,6 +55,26 @@ typedef struct vllp { struct vllp_channel_list channels; struct vllp_channel_queue established_channels; +#ifdef ENABLE_VLLP_CLIENT + // Channels the app has asked for but whose OPEN has not gone out yet, + // and the persistent binds that re-create them. + struct vllp_channel_queue pending_open; + struct vllp_bind_list binds; + + timer_t bind_timer; // backoff before retrying a refused open + + // Arms the first SYN. vllp_client_create() runs from board init, and + // net_timer_arm() touches an unprotected list (see net_core.c), so the + // timer has to be armed from net context, not from the caller. + net_task_t bootstrap; + + // Channel ids we may hand out. Channels 14 (management) and 15 (not a + // channel) are never available, so this starts at 0x3fff. The client + // allocates ids; the server takes whatever it is told. + uint16_t available_channel_ids; + uint8_t is_client; +#endif + timer_t ack_timer; timer_t rtx_timer; timer_t timeout_timer; @@ -70,11 +111,28 @@ typedef struct vllp { +// Client channels walk PENDING -> OPEN_SENT -> ESTABLISHED. Server +// channels are born ESTABLISHED, so only a client sees the first two. +// Which tx queue a channel sits on is tracked explicitly in vc->queue, +// not inferred from the state. #define VLLP_CHANNEL_STATE_PENDING 0 #define VLLP_CHANNEL_STATE_OPEN_SENT 1 #define VLLP_CHANNEL_STATE_ESTABLISHED 2 #define VLLP_CHANNEL_STATE_CLOSED_SENT 3 +#ifdef ENABLE_VLLP_CLIENT +// How long to wait before re-opening a bind the server refused. Without a +// backoff a bind whose service does not exist would re-ask as fast as the +// link allows, which on a shared CAN bus is a flood. +#define VLLP_BIND_RETRY_US 5000000 + +#define VLLP_IS_CLIENT(v) ((v)->is_client) +#else +// Folds to a constant so the compiler drops the client branches that do +// not need their own #ifdef. +#define VLLP_IS_CLIENT(v) 0 +#endif + struct vllp_channel { net_task_t task; @@ -93,6 +151,22 @@ struct vllp_channel { // for the CRC). Retried before pulling anything new. pbuf_t *stalled_tx; +#ifdef ENABLE_VLLP_CLIENT + // The remote service this channel asks for. Referenced, not copied -- + // callers pass a string literal or other long-lived storage, since it + // is needed again on every reconnect. + const char *service; + + // The bind that owns this channel, if any. + vllp_bind_t *bind; +#endif + + // The tx queue this channel is currently on, or NULL. Explicit rather + // than derived from the state: a channel exists briefly before it is + // queued at all, and a TAILQ_REMOVE on an unqueued entry corrupts the + // queue silently. + struct vllp_channel_queue *queue; + uint32_t tx_crc_IV; uint32_t rx_crc_IV; @@ -145,6 +219,7 @@ struct vllp_channel { #define VLLP_CMC_OPCODE_CLOSE 3 static void vllp_channel_task_cb(net_task_t *nt, uint32_t signals); +static pbuf_t *vllp_maybe_tx(vllp_t *v, pbuf_t *reuse); static void __attribute__((unused)) logpkt(const pbuf_t *pb, const char *prefix) @@ -200,11 +275,60 @@ vllp_channel_destroy(vllp_t *v, vllp_channel_t *vc) if(vc->stalled_tx) pbuf_free_irq_blocked(vc->stalled_tx); irq_permit(q); + +#ifdef ENABLE_VLLP_CLIENT + if(v->is_client && vc != v->cmc) { + // The id is ours again. Only once the peer can no longer refer to it, + // which is why this lives here rather than at close time. + v->available_channel_ids |= 1 << vc->id; + + if(vc->bind != NULL) { + vc->bind->channel = NULL; + // Re-open after a backoff if the link is still up. On a dead link + // there is nothing to re-open onto; the next session drives every + // idle bind from scratch (see vllp_drive_binds). + if(v->connected) + net_timer_arm(&v->bind_timer, clock_get() + VLLP_BIND_RETRY_US); + } + } +#endif + evlog(LOG_DEBUG, "VLLP: channel %d closed", vc->id); free(vc); } +static void +vllp_channel_enqueue(vllp_channel_t *vc, struct vllp_channel_queue *q) +{ + assert(vc->queue == NULL); + vc->queue = q; + TAILQ_INSERT_TAIL(q, vc, qlink); +} + +// Take the channel off its tx queue, if it is on one. +static void +vllp_channel_dequeue(vllp_channel_t *vc) +{ + if(vc->queue == NULL) + return; + TAILQ_REMOVE(vc->queue, vc, qlink); + vc->queue = NULL; +} + + +static void +vllp_net_event_cb(void *opaque, uint32_t events) +{ + vllp_channel_t *vc = opaque; + net_task_raise(&vc->task, events); +} + +static const pushpull_net_fn_t vllp_net_fn = { + .event = vllp_net_event_cb, +}; + + static vllp_channel_t * vllp_channel_make(vllp_t *v, int id) { @@ -334,6 +458,14 @@ vllp_disconnect(vllp_t *v, const char *reason) timer_disarm(&v->ack_timer); timer_disarm(&v->rtx_timer); timer_disarm(&v->timeout_timer); +#ifdef ENABLE_VLLP_CLIENT + timer_disarm(&v->bind_timer); +#endif + + // Before tearing the channels down, so vllp_channel_destroy() does not + // arm the bind backoff: there is no session left to re-open onto, and + // the next one drives every idle bind anyway (vllp_drive_binds). + v->connected = 0; evlog(LOG_DEBUG, "VLLP: 0x%x:0x%x Disconnected -- %s", v->txid, v->rxid, reason); @@ -344,18 +476,20 @@ vllp_disconnect(vllp_t *v, const char *reason) if(vllp_channel_net_close(v, vc, reason)) continue; // No session to send a CLOSE on; the channel is simply gone - TAILQ_REMOVE(&v->established_channels, vc, qlink); + vllp_channel_dequeue(vc); vc->app_closed = 2; vllp_channel_destroy(v, vc); } // Anything on the CMC belongs to the old session. The CMC outlives the - // session (it is never destroyed), so unlike every other channel it is - // not cleaned up by the loop above and has to be reset here -- + // session (it is never destroyed), so unlike every other channel it + // does not get cleaned up by the loop above and has to be reset here -- // including a half-reassembled *inbound* message. Leaving that in place - // does not just hold the buffers: the next session's first fragments + // does not just leak the buffers: the next session's first fragments // are appended to the stale ones, so the message fails its CRC, which - // resets the link, which strands another partial message. + // resets the link, which strands another partial message. A link that + // resets under load then bleeds the pbuf pool a buffer at a time until + // nothing works. { int q = irq_forbid(IRQ_LEVEL_NET); pbuf_free_queue_irq_blocked(&v->cmc->txq); @@ -375,9 +509,209 @@ vllp_disconnect(vllp_t *v, const char *reason) v->current_tx_len = 0; v->current_tx_channel = 0; - v->connected = 0; +#ifdef ENABLE_VLLP_CLIENT + if(v->is_client) { + // Every id is free again -- the peer has forgotten the session, so + // reusing them cannot be confused with the old channels. + v->available_channel_ids = 0x3fff; + // Start knocking again straight away. + net_timer_arm(&v->ack_timer, clock_get()); + } +#endif +} + + +#ifdef ENABLE_VLLP_CLIENT + +// Client: open a new session. Every attempt uses a fresh cookie, which +// becomes the CRC IV for the whole session, so a SYN-ACK answering an +// earlier attempt fails its CRC and is ignored. That costs a retry +// interval when a SYN-ACK and a SYN retransmit cross, but it matches the +// reference implementation (host/dsig/vllp.c) exactly, and diverging from +// it on handshake details is a worse trade than a slower reconnect. +static void +vllp_send_syn(vllp_t *v) +{ + net_timer_arm(&v->ack_timer, clock_get() + 1000000); + + pbuf_t *pb = pbuf_make(v->tx_headroom, 0); + if(pb == NULL) + return; // No buffer; the timer above brings us back + + // rand() is the same source the DHCP xid and the TCP ISN use. The + // cookie only has to differ between successive sessions (it exists to + // stop a peer that reset from being desynchronised against our old + // state), not to be unpredictable. Two calls because RAND_MAX is 31 + // bits. + v->crc_IV = ((uint32_t)rand() << 16) ^ (uint32_t)rand(); + + v->channel_iv_cnt = 0; + const uint32_t cmc_iv = vllp_gen_channel_crc(v); + // Mirrored relative to the server: our tx IV is its rx IV. + v->cmc->tx_crc_IV = ~cmc_iv; + v->cmc->rx_crc_IV = cmc_iv; + + v->SE = VLLP_HDR_E; + + uint8_t *pkt = pbuf_append(pb, 7); + pkt[0] = VLLP_SYN; + pkt[1] = VLLP_VERSION; + // v->mtu is already the adapted value (see vllp_server_create); the + // server compares the SYN byte against its own adapted mtu, so send + // that and not what the caller passed in. + pkt[2] = v->mtu; + memcpy(pkt + 3, &v->crc_IV, sizeof(v->crc_IV)); + + dsig_emit_pbuf(v->txid, pb); +} + + +// Client: allocate a channel and queue it for an OPEN. The caller must +// bind an app to vc->pp before the OPEN goes out, since a reply can +// arrive as soon as we return to the net loop. +static vllp_channel_t * +vllp_client_channel_new(vllp_t *v, const char *service) +{ + if(!v->connected) + return NULL; + + if(v->available_channel_ids == 0) + return NULL; + + const int id = __builtin_ffs(v->available_channel_ids) - 1; + + vllp_channel_t *vc = vllp_channel_make(v, id); + if(vc == NULL) + return NULL; + + v->available_channel_ids &= ~(1 << id); + + vc->service = service; + vc->state = VLLP_CHANNEL_STATE_PENDING; + vc->pp.max_fragment_size = PBUF_DATA_SIZE - 4; // Make place for CRC32 + vc->pp.preferred_offset = 0; + vc->pp.net = &vllp_net_fn; + vc->pp.net_opaque = vc; + + vllp_channel_enqueue(vc, &v->pending_open); + return vc; +} + + +// The peer never opened this channel (it refused, or the link died before +// answering), so no CLOSE is owed on the wire -- as far as the peer is +// concerned the channel does not exist. Tell the app and drop it. +static void +vllp_channel_abandon(vllp_t *v, vllp_channel_t *vc, const char *reason) +{ + if(vc->pp.app != NULL) + vc->pp.app->close(vc->pp.app_opaque, reason); + vllp_channel_dequeue(vc); + vc->net_closed = 1; + vc->app_closed = 2; + LIST_REMOVE(vc, link); + vllp_channel_destroy(v, vc); +} + + +// Give every bind that is not currently open a channel. Called when a +// session comes up and from the retry backoff. +static void +vllp_drive_binds(vllp_t *v) +{ + vllp_bind_t *b; + + LIST_FOREACH(b, &v->binds, link) { + if(b->channel != NULL) + continue; + + vllp_channel_t *vc = vllp_client_channel_new(v, b->service); + if(vc == NULL) { + // Out of ids or out of memory. Come back later. + net_timer_arm(&v->bind_timer, clock_get() + VLLP_BIND_RETRY_US); + return; + } + + const error_t err = b->open(b->opaque, &vc->pp); + if(err) { + evlog(LOG_WARNING, "VLLP: 0x%x:0x%x could not bind a local app for " + "'%s' -- %s", v->txid, v->rxid, b->service, + error_to_string(err)); + // vc->bind is still NULL, so destroy() will not touch b->channel + // and the backoff below is the only thing that retries. + vllp_channel_abandon(v, vc, "local open failed"); + net_timer_arm(&v->bind_timer, clock_get() + VLLP_BIND_RETRY_US); + continue; + } + + vc->bind = b; + b->channel = vc; + } +} + + +static void +vllp_bind_timer(void *opaque, uint64_t expire) +{ + vllp_t *v = opaque; + if(!v->connected) + return; + vllp_drive_binds(v); + vllp_maybe_tx(v, NULL); +} + + +// Client: turn queued channels into OPEN requests on the management +// channel. Runs from the tx path, so a message that cannot be built for +// want of a buffer just stays queued. +static void +handle_pending_channels(vllp_t *v) +{ + vllp_channel_t *vc; + + while((vc = TAILQ_FIRST(&v->pending_open)) != NULL) { + + const size_t namelen = strlen(vc->service); + + // 1 opcode byte + the name, and vllp_channel_tx() appends a 4 byte + // CRC. A name that cannot fit is a programming error, not something + // to retry forever. + if(1 + namelen + 4 > PBUF_DATA_SIZE) { + evlog(LOG_ERR, "VLLP: service name '%s' too long", vc->service); + vllp_channel_abandon(v, vc, "service name too long"); + continue; + } + + pbuf_t *pb = pbuf_make(0, 0); + if(pb == NULL) { + // Keep the queue order and try again once buffers are back. + net_timer_arm(&v->rtx_timer, clock_get() + 25000); + return; + } + + vllp_channel_dequeue(vc); + vc->state = VLLP_CHANNEL_STATE_OPEN_SENT; + + // One IV per OPEN we send. The server generates one per OPEN it + // receives -- including ones it goes on to refuse -- so the two + // counters only stay in step if we advance here unconditionally. + // Get this wrong and every later channel on the link fails its CRC. + const uint32_t iv = vllp_gen_channel_crc(v); + vc->tx_crc_IV = ~iv; + vc->rx_crc_IV = iv; + + uint8_t *u8 = pbuf_append(pb, 1 + namelen); + u8[0] = (VLLP_CMC_OPCODE_OPEN << 4) | vc->id; + memcpy(u8 + 1, vc->service, namelen); + STAILQ_INSERT_TAIL(&v->cmc->txq, pb, pb_link); + + evlog(LOG_DEBUG, "VLLP: requesting '%s' on channel %d", vc->service, + vc->id); + } } +#endif // ENABLE_VLLP_CLIENT + static pbuf_t * vllp_accept_syn(vllp_t *v, const uint8_t *data, size_t len, @@ -385,6 +719,14 @@ vllp_accept_syn(vllp_t *v, const uint8_t *data, size_t len, { evlog(LOG_DEBUG, "VLLP syn, len=%zd", len); + if(VLLP_IS_CLIENT(v)) { + // Only a client may open a link. Two clients on the same id pair is a + // configuration error; say so rather than silently half-working. + evlog(LOG_WARNING, "VLLP: 0x%x:0x%x client got a SYN -- is the peer " + "also configured as a client?", v->txid, v->rxid); + return pb; + } + if(len != 7) return pb; @@ -430,18 +772,6 @@ send_cmc_message(vllp_t *v, vllp_channel_t *cmc, pbuf_t *pb, } -static void -vllp_net_event_cb(void *opaque, uint32_t events) -{ - vllp_channel_t *vc = opaque; - net_task_raise(&vc->task, events); -} - -static const pushpull_net_fn_t vllp_net_fn = { - .event = vllp_net_event_cb, -}; - - static error_t handle_cmc_open(vllp_t *v, vllp_channel_t *cmc, int target_channel, @@ -483,7 +813,7 @@ handle_cmc_open(vllp_t *v, vllp_channel_t *cmc, return err; } - TAILQ_INSERT_TAIL(&v->established_channels, vc, qlink); + vllp_channel_enqueue(vc, &v->established_channels); evlog(LOG_DEBUG, "VLLP: service open %s on channel %d", s->name, vc->id); return 0; @@ -512,6 +842,55 @@ handle_cmc_close(vllp_t *v, vllp_channel_t *cmc, pbuf_t *pb, return err; } +#ifdef ENABLE_VLLP_CLIENT + +// Client: the server has answered one of our OPEN requests. +static error_t +handle_cmc_open_response(vllp_t *v, int target_channel, + const uint8_t *data, size_t len) +{ + vllp_channel_t *vc = vllp_channel_find(v, target_channel); + + // A stray response -- one still in flight from a session that has since + // been reset, say -- is not a protocol violation. Ignore it. Tearing + // the link down here would cascade into another reset, which is the bug + // the host client had until it was made to ignore these too. + if(vc == NULL || vc == v->cmc) { + evlog(LOG_DEBUG, "VLLP: open response for unknown channel %d, ignoring", + target_channel); + return 0; + } + + if(vc->state != VLLP_CHANNEL_STATE_OPEN_SENT) { + evlog(LOG_DEBUG, "VLLP: open response for channel %d in state %d, " + "ignoring", target_channel, vc->state); + return 0; + } + + if(len != 2) + return ERR_MALFORMED; + + const int16_t err = data[0] | (data[1] << 8); + + if(err) { + evlog(LOG_WARNING, "VLLP: 0x%x:0x%x peer refused '%s' -- %s", + v->txid, v->rxid, vc->service, error_to_string(err)); + vllp_channel_abandon(v, vc, "peer refused the open"); + return 0; + } + + vc->state = VLLP_CHANNEL_STATE_ESTABLISHED; + vllp_channel_enqueue(vc, &v->established_channels); + evlog(LOG_DEBUG, "VLLP: '%s' open on channel %d", vc->service, vc->id); + + // The app may have queued output while the open was in flight. + net_task_raise(&vc->task, PUSHPULL_EVENT_PULL); + return 0; +} + +#endif // ENABLE_VLLP_CLIENT + + static error_t handle_cmc(vllp_t *v, vllp_channel_t *cmc, pbuf_t *pb) { @@ -527,6 +906,25 @@ handle_cmc(vllp_t *v, vllp_channel_t *cmc, pbuf_t *pb) uint8_t opcode = u8[0] >> 4; uint8_t target_channel = u8[0] & 0xf; + // OPEN is client-to-server and OPEN_RESPONSE is server-to-client; the + // spec is explicit that neither travels the other way. CLOSE is the + // only opcode both roles both send and receive. +#ifdef ENABLE_VLLP_CLIENT + if(v->is_client) { + switch(opcode) { + case VLLP_CMC_OPCODE_OPEN_RESPONSE: + err = handle_cmc_open_response(v, target_channel, u8 + 1, len - 1); + pbuf_free(pb); + return err; + case VLLP_CMC_OPCODE_CLOSE: + return handle_cmc_close(v, cmc, pb, target_channel, u8 + 1, len - 1); + default: + pbuf_free(pb); + return ERR_BAD_STATE; + } + } +#endif + switch(opcode) { case VLLP_CMC_OPCODE_OPEN: err = handle_cmc_open(v, cmc, target_channel, u8 + 1, len - 1); @@ -538,7 +936,7 @@ handle_cmc(vllp_t *v, vllp_channel_t *cmc, pbuf_t *pb) default: // Ownership of pb is ours (see vllp_channel_receive), and the - // ERR_BAD_STATE below drops the link, not the buffer. + // ERR_BAD_STATE below drops the link rather than the buffer. pbuf_free(pb); return ERR_BAD_STATE; } @@ -784,8 +1182,8 @@ vllp_channel_tx(vllp_t *v, vllp_channel_t *vc, pbuf_t *pb, pbuf_t *reuse) reuse = vllp_fragment(v, reuse); // Move to tail for round-robin scheduling - TAILQ_REMOVE(&v->established_channels, vc, qlink); - TAILQ_INSERT_TAIL(&v->established_channels, vc, qlink); + vllp_channel_dequeue(vc); + vllp_channel_enqueue(vc, &v->established_channels); return reuse; } @@ -793,7 +1191,7 @@ vllp_channel_tx(vllp_t *v, vllp_channel_t *vc, pbuf_t *pb, pbuf_t *reuse) static error_t vllp_tx_close(vllp_t *v, vllp_channel_t *vc) { - TAILQ_REMOVE(&v->established_channels, vc, qlink); + vllp_channel_dequeue(vc); vc->state = VLLP_CHANNEL_STATE_CLOSED_SENT; if(!v->connected) @@ -817,6 +1215,11 @@ vllp_maybe_tx(vllp_t *v, pbuf_t *reuse) if(!v->connected) return reuse; +#ifdef ENABLE_VLLP_CLIENT + if(v->is_client) + handle_pending_channels(v); +#endif + if(v->current_tx_buf) { // Waiting for the peer to accept our outstanding frame. An ACK we // owe can not ride on anything, so send it by itself. @@ -942,6 +1345,29 @@ vllp_rx(vllp_t *v, pbuf_t *pb) !(u8[0] & VLLP_HDR_S) == !(v->SE & VLLP_HDR_E); if(!v->connected) { + +#ifdef ENABLE_VLLP_CLIENT + if(v->is_client) { + // The only thing we accept before a session exists is the ACK + // answering our SYN. Its CRC was validated above against the cookie + // we just generated, so a reply to an earlier attempt (different + // cookie) has already been dropped. + if(u8[0] != (VLLP_HDR_E | 0x1f)) { + evlog(LOG_DEBUG, "VLLP: 0x%x:0x%x expected a SYN response, got 0x%02x", + v->txid, v->rxid, u8[0]); + return pb; + } + + v->connected = 1; + evlog(LOG_DEBUG, "VLLP: 0x%x:0x%x Connected", v->txid, v->rxid); + + net_timer_arm(&v->ack_timer, clock_get() + 1000000); + + // Re-open everything that wants to be open on this fresh session. + vllp_drive_binds(v); + return vllp_maybe_tx(v, pb); + } +#endif return pb; } @@ -1000,6 +1426,17 @@ static void vllp_ack_timer(void *opaque, uint64_t expire) { vllp_t *v = opaque; + + if(!v->connected) { +#ifdef ENABLE_VLLP_CLIENT + // Client only -- a server arms this timer only once a session exists, + // and disarms it on disconnect. + if(v->is_client) + vllp_send_syn(v); +#endif + return; + } + vllp_refresh_local_flow_status(v); vllp_tx_ack(v, NULL); } @@ -1073,9 +1510,8 @@ vllp_max_message_size(void) } -vllp_t * -vllp_server_create(uint32_t txid, uint32_t rxid, uint8_t mtu, - uint8_t timeout) +static vllp_t * +vllp_create(uint32_t txid, uint32_t rxid, uint8_t mtu, uint8_t timeout) { // Reserve headroom for the headers prepended below us, but never so // much that a full frame no longer fits the buffer. VLLP_TX_HEADROOM @@ -1112,7 +1548,7 @@ vllp_server_create(uint32_t txid, uint32_t rxid, uint8_t mtu, v->cmc->state = VLLP_CHANNEL_STATE_ESTABLISHED; TAILQ_INIT(&v->established_channels); - TAILQ_INSERT_TAIL(&v->established_channels, v->cmc, qlink); + vllp_channel_enqueue(v->cmc, &v->established_channels); v->rxid = rxid; v->txid = txid; @@ -1128,7 +1564,7 @@ vllp_server_create(uint32_t txid, uint32_t rxid, uint8_t mtu, v->ack_timer.t_cb = vllp_ack_timer; v->ack_timer.t_opaque = v; - v->ack_timer.t_name = "vllprtx"; + v->ack_timer.t_name = "vllpack"; v->timeout_timer.t_cb = vllp_timeout_timer; v->timeout_timer.t_opaque = v; @@ -1138,6 +1574,116 @@ vllp_server_create(uint32_t txid, uint32_t rxid, uint8_t mtu, return v; } + +vllp_t * +vllp_server_create(uint32_t txid, uint32_t rxid, uint8_t mtu, + uint8_t timeout) +{ + return vllp_create(txid, rxid, mtu, timeout); +} + + +#ifdef ENABLE_VLLP_CLIENT + +// Deferred work for a client, so callers can create links and binds from +// board init without touching net-context-only state. See v->bootstrap. +static void +vllp_bootstrap_cb(net_task_t *nt, uint32_t signals) +{ + vllp_t *v = ((void *)nt) - offsetof(vllp_t, bootstrap); + + if(!v->connected) { + vllp_send_syn(v); + return; + } + + // A bind added while the link was already up. + vllp_drive_binds(v); + vllp_maybe_tx(v, NULL); +} + + +vllp_t * +vllp_client_create(uint32_t txid, uint32_t rxid, uint8_t mtu, + uint8_t timeout) +{ + vllp_t *v = vllp_create(txid, rxid, mtu, timeout); + if(v == NULL) + return NULL; + + v->is_client = 1; + v->available_channel_ids = 0x3fff; // Channels 0-13; 14 and 15 are ours + v->SE = VLLP_HDR_E; + + TAILQ_INIT(&v->pending_open); + LIST_INIT(&v->binds); + + // Set up here rather than in vllp_create(): the assignment is what + // makes vllp_bind_timer (and through it the whole bind machinery) + // reachable, so doing it on the shared path linked it into images that + // only ever instantiate servers. + v->bind_timer.t_cb = vllp_bind_timer; + v->bind_timer.t_opaque = v; + v->bind_timer.t_name = "vllpbind"; + + v->bootstrap.nt_cb = vllp_bootstrap_cb; + net_task_raise(&v->bootstrap, 1); + return v; +} + + +error_t +vllp_client_channel_open(vllp_t *v, const char *service, pushpull_t *pp) +{ + if(!v->is_client) + return ERR_NOT_IMPLEMENTED; + + if(!v->connected) + return ERR_NOT_CONNECTED; + + vllp_channel_t *vc = vllp_client_channel_new(v, service); + if(vc == NULL) + return v->available_channel_ids ? ERR_NO_MEMORY : ERR_QUEUE_FULL; + + // The caller's app functions, transplanted onto the channel's own + // pushpull -- the app talks to vc->pp, not to the pp it passed in. + vc->pp.app = pp->app; + vc->pp.app_opaque = pp->app_opaque; + pp->net = vc->pp.net; + pp->net_opaque = vc->pp.net_opaque; + pp->max_fragment_size = vc->pp.max_fragment_size; + pp->preferred_offset = vc->pp.preferred_offset; + + vllp_maybe_tx(v, NULL); + return 0; +} + + +vllp_bind_t * +vllp_client_bind(vllp_t *v, const char *service, + error_t (*open)(void *opaque, pushpull_t *pp), + void *opaque) +{ + if(!v->is_client) + return NULL; + + vllp_bind_t *b = xalloc(sizeof(vllp_bind_t), 0, MEM_MAY_FAIL | MEM_CLEAR); + if(b == NULL) + return NULL; + + b->service = service; + b->open = open; + b->opaque = opaque; + LIST_INSERT_HEAD(&v->binds, b, link); + + // If the link is already up, open it now; otherwise the next SYN-ACK + // picks it up. Either way this runs on the net thread only. + net_task_raise(&v->bootstrap, 1); + return b; +} + +#endif // ENABLE_VLLP_CLIENT + static const char vllp_channel_state_strtbl[] = { "PENDING\0" "OPEN_SENT\0" @@ -1160,26 +1706,70 @@ static const char vllp_channel_net_closed_strtbl[] = { }; static error_t -cmd_tcp(cli_t *cli, int argc, char **argv) +cmd_show_vllp(cli_t *cli, int argc, char **argv) { vllp_t *v; vllp_channel_t *vc; LIST_FOREACH(v, &vllps, link) { +#ifdef ENABLE_VLLP_CLIENT + // Only worth saying when there is more than one role to be in. + cli_printf(cli, "%s ", v->is_client ? "client" : "server"); +#endif cli_printf(cli, "TX:0x%x RX:0x%x %sonnected", v->txid, v->rxid, v->connected ? "C" : "Disc"); cli_printf(cli, " Flow status Local:0x%04x Remote:0x%04x\n", v->local_flow_status, v->remote_flow_status); +#ifdef ENABLE_VLLP_CLIENT + if(v->is_client) + cli_printf(cli, " Free channel ids:0x%04x\n", + v->available_channel_ids); + if(v->current_tx_buf != NULL) + cli_printf(cli, " In-flight fragment on channel %d\n", + v->current_tx_channel); +#endif cli_printf(cli, " Channels:\n"); LIST_FOREACH(vc, &v->channels, link) { cli_printf(cli, " %2d : state:%s app:%s net:%s\n", vc->id, strtbl(vllp_channel_state_strtbl, vc->state), strtbl(vllp_channel_app_closed_strtbl, vc->app_closed), strtbl(vllp_channel_net_closed_strtbl, vc->net_closed)); +#ifdef ENABLE_VLLP_CLIENT + // Which service a channel is for, and what it is sitting on. The + // buffer counts are what localise a draining pool to a channel and + // a queue, which is otherwise a guessing game -- they are useful on + // a server too, but a server-only build should not pay for a + // command it never grew, so they live here with the rest. + int rx = 0, tx = 0; + pbuf_t *pb; + STAILQ_FOREACH(pb, &vc->rxq, pb_link) + rx++; + STAILQ_FOREACH(pb, &vc->txq, pb_link) + tx++; + if(vc->service != NULL || rx || tx || vc->stalled_tx) { + cli_printf(cli, " "); + if(vc->service != NULL) + cli_printf(cli, " service:%s", vc->service); + if(rx || tx || vc->stalled_tx) + cli_printf(cli, " pbufs:rxq=%d,txq=%d%s", rx, tx, + vc->stalled_tx ? ",stalled" : ""); + cli_printf(cli, "\n"); + } +#endif } +#ifdef ENABLE_VLLP_CLIENT + if(v->is_client) { + vllp_bind_t *b; + LIST_FOREACH(b, &v->binds, link) { + cli_printf(cli, " bind '%s' : %s\n", b->service, + b->channel ? "open" : "waiting"); + } + } +#endif + cli_printf(cli, "\n"); } return 0; } -CLI_CMD_DEF_EXT("show_vllp", cmd_tcp, NULL, "Show VLLP connections"); +CLI_CMD_DEF_EXT("show_vllp", cmd_show_vllp, NULL, "Show VLLP connections"); diff --git a/src/platform/host/host.mk b/src/platform/host/host.mk index 96adc4a7..39a69c9a 100644 --- a/src/platform/host/host.mk +++ b/src/platform/host/host.mk @@ -14,6 +14,14 @@ ENABLE_NET_IPV4 := yes ENABLE_NET_DSIG_UDP := yes ENABLE_NET_CAN := yes +# Virtual consoles. No in-tree target enabled these before, so the host +# suites are their only coverage. +ENABLE_VCON := yes + +# The VLLP client role, exercised by suite_vllp_client.c and +# suite_vcon_vllp.c. +ENABLE_VLLP_CLIENT := yes + P := ${SRC}/platform/host GLOBALDEPS += ${P}/host.mk @@ -41,10 +49,15 @@ SRCS += ${P}/host.c \ ${P}/suite_snprintf.c \ ${P}/vnet.c \ ${P}/vcan.c \ + ${P}/vcan_loop.c \ + ${P}/testterm.c \ ${P}/sim_dhcpd.c \ ${P}/suite_dhcp.c \ + ${P}/suite_vcon.c \ + ${P}/suite_vcon_vllp.c \ ${P}/suite_vllp.c \ ${P}/suite_vllp_frames.c \ + ${P}/suite_vllp_client.c \ ${P}/suite_ota.c \ ${P}/vspiflash.c \ ${P}/host_ota.c \ diff --git a/src/platform/host/suite_vcon.c b/src/platform/host/suite_vcon.c new file mode 100644 index 00000000..ffb02107 --- /dev/null +++ b/src/platform/host/suite_vcon.c @@ -0,0 +1,676 @@ +/* + * vcon: the virtual console (src/util/vcon.c, src/util/vcon_ring.h) and its + * shell commands (src/shell/cmd_vcon.c), in virtual time. + * + * ENABLE_VCON is off by default and no in-tree target switched it on, so + * until now none of this code had ever been executed by CI. It is about to + * become the far end of a VLLP client link, so pin the behaviour down + * first -- every claim vcon.h makes about scrollback replay, cursor + * resync, multi-client mirroring and input merging. + * + * A test terminal (testterm.h) stands in for whatever a user attaches + * with: it buffers both directions and never blocks the writer, so the + * suite can inject keystrokes and inspect console output without running + * in lockstep with the thread under test. + */ + +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +#include "util/vcon_ring.h" + +#include "hosttest.h" +#include "testterm.h" + +#define SEC 1000000ull + +static int fails; + +#define VCHECK(cond, ...) \ + do { if(!(cond)) { fails++; \ + hosttest_check(0, __FILE__, __LINE__, __VA_ARGS__); } } while(0) + + +/* ---------------- helpers ---------------- */ + +/* Drain everything currently pending for a client into buf. Loops because + one call only yields what the ring can hand over contiguously. */ +static size_t +drain_client(vcon_client_t *vcc, void *buf, size_t size) +{ + size_t off = 0; + while(off < size) { + size_t n = vcon_client_output(vcc, buf + off, size - off); + if(n == 0) + break; + off += n; + } + return off; +} + + +/* Read from the backend with a deadline. Never use a blocking + stream_read(..., required) here: if the thing under test stops pumping, + an unbounded read turns a failed assertion into a hung run, which in CI + means a wedged job instead of a message naming the broken check. Found + the hard way -- a mutant that swallowed keystrokes hung this suite + instead of failing it. */ +static size_t +backend_read(vcon_t *vc, void *buf, size_t want, uint64_t timeout) +{ + const uint64_t deadline = clock_get() + timeout; + size_t off = 0; + while(off < want) { + ssize_t n = stream_read(vcon_backend(vc), buf + off, want - off, 0); + if(n > 0) { + off += n; + continue; + } + if(clock_get() >= deadline) + break; + usleep(10000); + } + return off; +} + + +static void +backend_puts(vcon_t *vc, const char *str) +{ + stream_write(vcon_backend(vc), str, strlen(str), 0); +} + + +/* ---------------- phases ---------------- */ + +/* vcon_ring.h advertises itself as dependency-free and host-testable, so + test it directly: the drop-oldest and wraparound arithmetic is where an + off-by-one silently corrupts scrollback rather than crashing. */ +static void +phase_ring(void) +{ + hosttest_log("-- ring"); + + /* Allocated, not a local union: vcon_ring_t ends in a flexible array and + GCC rightly flags writes past a stack object whose real size it can + see. vcon.c folds the storage into a calloc'd block, so do the same. */ + vcon_ring_t *r = calloc(1, sizeof(vcon_ring_t) + 8); + uint8_t out[32]; + + r->size = 8; + + vcon_ring_append(r, (const uint8_t *)"abc", 3); + VCHECK(r->used == 3, "ring: used %zu after 3 bytes", r->used); + vcon_ring_copy(r, 3, out, 3); + VCHECK(!memcmp(out, "abc", 3), "ring: copy mismatch"); + + /* Overflow by 2: oldest two bytes ("ab") must be the ones lost. */ + vcon_ring_append(r, (const uint8_t *)"defghij", 7); + VCHECK(r->used == 8, "ring: used %zu after overflow", r->used); + vcon_ring_copy(r, 8, out, 8); + VCHECK(!memcmp(out, "cdefghij", 8), "ring: overflow kept '%.8s'", out); + + /* Partial read from the middle of the window. */ + vcon_ring_copy(r, 5, out, 3); + VCHECK(!memcmp(out, "fgh", 3), "ring: mid-window copy got '%.3s'", out); + + /* Two segments, oldest first, must reassemble to the same thing. */ + const uint8_t *p0, *p1; + size_t l0, l1; + vcon_ring_segments(r, &p0, &l0, &p1, &l1); + VCHECK(l0 + l1 == 8, "ring: segments total %zu", l0 + l1); + memcpy(out, p0, l0); + memcpy(out + l0, p1, l1); + VCHECK(!memcmp(out, "cdefghij", 8), "ring: segments gave '%.8s'", out); + + /* A single append larger than the ring: only the tail survives, and the + implementation takes a different code path to lay it out. */ + vcon_ring_append(r, (const uint8_t *)"0123456789ABCDEFGHIJ", 20); + VCHECK(r->used == 8, "ring: used %zu after oversize", r->used); + vcon_ring_copy(r, 8, out, 8); + VCHECK(!memcmp(out, "CDEFGHIJ", 8), "ring: oversize kept '%.8s'", out); + + /* Exactly-full append, which is the boundary between the two paths. */ + memset(r, 0, sizeof(vcon_ring_t) + 8); + r->size = 8; + vcon_ring_append(r, (const uint8_t *)"01234567", 8); + VCHECK(r->used == 8, "ring: used %zu after exact fill", r->used); + vcon_ring_copy(r, 8, out, 8); + VCHECK(!memcmp(out, "01234567", 8), "ring: exact fill gave '%.8s'", out); + + free(r); + /* The size==0 ring is covered through the real API in + phase_no_scrollback(): poking it directly from here means handing + vcon_ring_append() an object with nothing behind buf[], which GCC + diagnoses at every allocation size we could pick. */ +} + + +/* A console with no scrollback at all. Legal (vcon_create takes the size + from the caller) and it drives the ring's size==0 guards, which are + otherwise dead code. Nothing is buffered, so a client only ever sees + what is written while it is attached. */ +static void +phase_no_scrollback(void) +{ + hosttest_log("-- no scrollback"); + + vcon_t *vc = vcon_create("c-nosb", 0, 64); + VCHECK(vc != NULL, "nosb: create failed"); + if(vc == NULL) + return; + + backend_puts(vc, "dropped-on-the-floor"); + VCHECK(vcon_scrollback_used(vc) == 0, "nosb: scrollback used %zu, want 0", + vcon_scrollback_used(vc)); + + testterm_t *tt = testterm_create(); + vcon_client_t *vcc = vcon_attach(vc, testterm_stream(tt)); + char buf[64]; + VCHECK(drain_client(vcc, buf, sizeof(buf)) == 0, + "nosb: replayed history that was never stored"); + + /* Live output is still accounted for, it just is not retained. */ + backend_puts(vc, "live"); + VCHECK(drain_client(vcc, buf, sizeof(buf)) == 0, + "nosb: returned bytes from a zero-size ring"); + + /* Input is unaffected -- that fifo is a separate buffer. */ + VCHECK(vcon_input(vc, "k", 1) == 1, "nosb: input rejected"); + size_t n = backend_read(vc, buf, 1, 2 * SEC); + VCHECK(n == 1 && buf[0] == 'k', "nosb: backend read %zu", n); + + vcon_detach(vcc); + free(tt); +} + + +static void +phase_basic(void) +{ + hosttest_log("-- basic"); + + vcon_t *vc = vcon_create("c-basic", 256, 64); + VCHECK(vc != NULL, "basic: create failed"); + if(vc == NULL) + return; + + backend_puts(vc, "hello"); + VCHECK(vcon_scrollback_used(vc) == 5, "basic: scrollback %zu, want 5", + vcon_scrollback_used(vc)); + VCHECK(vcon_client_count(vc) == 0, "basic: %d clients before attach", + vcon_client_count(vc)); + + testterm_t *tt = testterm_create(); + vcon_client_t *vcc = vcon_attach(vc, testterm_stream(tt)); + VCHECK(vcc != NULL, "basic: attach failed"); + if(vcc == NULL) + return; + + VCHECK(vcon_client_count(vc) == 1, "basic: %d clients after attach", + vcon_client_count(vc)); + + /* Attaching starts the cursor at the oldest buffered byte, so output + written before the attach is replayed. */ + char buf[64]; + size_t n = drain_client(vcc, buf, sizeof(buf)); + VCHECK(n == 5 && !memcmp(buf, "hello", 5), + "basic: replay got %zu bytes '%.*s'", n, (int)n, buf); + + /* Nothing pending now. */ + VCHECK(vcon_client_output(vcc, buf, sizeof(buf)) == 0, + "basic: output pending after drain"); + + backend_puts(vc, "world"); + n = drain_client(vcc, buf, sizeof(buf)); + VCHECK(n == 5 && !memcmp(buf, "world", 5), + "basic: live got %zu bytes '%.*s'", n, (int)n, buf); + + /* Keystrokes go the other way, out of the backend's read side. */ + VCHECK(vcon_input(vc, "ping", 4) == 4, "basic: input short"); + n = backend_read(vc, buf, 4, 2 * SEC); + VCHECK(n == 4 && !memcmp(buf, "ping", 4), + "basic: backend read got %zu '%.*s'", n, (int)n, buf); + + vcon_detach(vcc); + VCHECK(vcon_client_count(vc) == 0, "basic: %d clients after detach", + vcon_client_count(vc)); + free(tt); +} + + +/* A client that attaches after more output than the scrollback can hold + sees the most recent window, not the beginning. */ +static void +phase_scrollback(void) +{ + hosttest_log("-- scrollback"); + + vcon_t *vc = vcon_create("c-sb", 16, 64); + char src[40]; + for(size_t i = 0; i < sizeof(src); i++) + src[i] = '0' + (i % 10); + stream_write(vcon_backend(vc), src, sizeof(src), 0); + + VCHECK(vcon_scrollback_used(vc) == 16, "scrollback: used %zu, want 16", + vcon_scrollback_used(vc)); + + testterm_t *tt = testterm_create(); + vcon_client_t *vcc = vcon_attach(vc, testterm_stream(tt)); + char buf[64]; + size_t n = drain_client(vcc, buf, sizeof(buf)); + VCHECK(n == 16, "scrollback: replayed %zu bytes, want 16", n); + VCHECK(!memcmp(buf, src + sizeof(src) - 16, 16), + "scrollback: replayed '%.*s', want '%.16s'", (int)n, buf, + src + sizeof(src) - 16); + + vcon_detach(vcc); + free(tt); +} + + +/* A client that stops draining while the producer runs ahead must skip the + gap it lost and resume cleanly -- not read stale bytes, not read the same + byte twice, not walk off the ring. */ +static void +phase_lag(void) +{ + hosttest_log("-- lagging client"); + + vcon_t *vc = vcon_create("c-lag", 16, 64); + testterm_t *tt = testterm_create(); + vcon_client_t *vcc = vcon_attach(vc, testterm_stream(tt)); + + char src[40]; + for(size_t i = 0; i < sizeof(src); i++) + src[i] = 'A' + (i % 26); + + /* Write 10, consume 4. Cursor is at 4, well inside the window. */ + stream_write(vcon_backend(vc), src, 10, 0); + char buf[64]; + size_t n = vcon_client_output(vcc, buf, 4); + VCHECK(n == 4 && !memcmp(buf, src, 4), "lag: first read '%.*s'", (int)n, buf); + + /* Now run the producer 30 bytes ahead without draining. 40 bytes total + written, a 16 byte window, so the oldest surviving byte is #24 and the + cursor (at 4) has fallen 20 bytes behind the window. */ + stream_write(vcon_backend(vc), src + 10, 30, 0); + + n = drain_client(vcc, buf, sizeof(buf)); + VCHECK(n == 16, "lag: after resync got %zu bytes, want 16", n); + VCHECK(!memcmp(buf, src + 24, 16), + "lag: after resync got '%.*s', want '%.16s'", (int)n, buf, src + 24); + + /* And the stream continues correctly from there. */ + stream_write(vcon_backend(vc), "tail", 4, 0); + n = drain_client(vcc, buf, sizeof(buf)); + VCHECK(n == 4 && !memcmp(buf, "tail", 4), + "lag: continuation got %zu '%.*s'", n, (int)n, buf); + + vcon_detach(vcc); + free(tt); +} + + +/* vcon.h promises shared sessions: output mirrored to every client, input + from all of them merged into one backend stream. */ +static void +phase_multi(void) +{ + hosttest_log("-- multi-client"); + + vcon_t *vc = vcon_create("c-multi", 256, 64); + testterm_t *ta = testterm_create(); + testterm_t *tb = testterm_create(); + + vcon_client_t *a = vcon_attach(vc, testterm_stream(ta)); + vcon_client_t *b = vcon_attach(vc, testterm_stream(tb)); + VCHECK(vcon_client_count(vc) == 2, "multi: %d clients", vcon_client_count(vc)); + + backend_puts(vc, "mirrored"); + + char bufa[64], bufb[64]; + size_t na = drain_client(a, bufa, sizeof(bufa)); + size_t nb = drain_client(b, bufb, sizeof(bufb)); + VCHECK(na == 8 && !memcmp(bufa, "mirrored", 8), + "multi: client A got %zu '%.*s'", na, (int)na, bufa); + VCHECK(nb == 8 && !memcmp(bufb, "mirrored", 8), + "multi: client B got %zu '%.*s'", nb, (int)nb, bufb); + + /* Each client's cursor is independent: a second client attaching later + replays the same history the first one already consumed. */ + testterm_t *tc = testterm_create(); + vcon_client_t *c = vcon_attach(vc, testterm_stream(tc)); + char bufc[64]; + size_t nc = drain_client(c, bufc, sizeof(bufc)); + VCHECK(nc == 8 && !memcmp(bufc, "mirrored", 8), + "multi: late client got %zu '%.*s'", nc, (int)nc, bufc); + + /* Merged input. Interleaving between the two is not specified, but every + byte must arrive exactly once. */ + vcon_input(vc, "aaa", 3); + vcon_input(vc, "bbb", 3); + char in[8]; + size_t n = backend_read(vc, in, 6, 2 * SEC); + VCHECK(n == 6, "multi: merged read %zu, want 6", n); + int na_cnt = 0, nb_cnt = 0; + for(size_t i = 0; i < n; i++) { + if(in[i] == 'a') na_cnt++; + else if(in[i] == 'b') nb_cnt++; + } + VCHECK(na_cnt == 3 && nb_cnt == 3, + "multi: merged got %d a's and %d b's from '%.*s'", na_cnt, nb_cnt, + (int)n, in); + + vcon_detach(a); + vcon_detach(b); + vcon_detach(c); + VCHECK(vcon_client_count(vc) == 0, "multi: %d clients after detach", + vcon_client_count(vc)); + free(ta); + free(tb); + free(tc); +} + + +/* The input fifo is small and bounded; a flood must be truncated, not + overrun the buffer or wrap silently. */ +static void +phase_input_limits(void) +{ + hosttest_log("-- input limits"); + + vcon_t *vc = vcon_create("c-in", 64, 8); + + VCHECK(vcon_input(vc, "0123456789ABCDEF", 16) == 8, + "input: fifo accepted more than 8 bytes"); + VCHECK(vcon_input(vc, "x", 1) == 0, "input: accepted a byte while full"); + + char buf[16]; + size_t n = backend_read(vc, buf, 8, 2 * SEC); + VCHECK(n == 8 && !memcmp(buf, "01234567", 8), + "input: drained %zu '%.*s'", n, (int)n, buf); + + /* Room again, and the ring indices wrapped in between. */ + VCHECK(vcon_input(vc, "abcdefgh", 8) == 8, "input: refill short"); + n = backend_read(vc, buf, 8, 2 * SEC); + VCHECK(n == 8 && !memcmp(buf, "abcdefgh", 8), + "input: after wrap got %zu '%.*s'", n, (int)n, buf); + + /* Non-blocking backend read on an empty fifo returns 0 rather than + hanging -- this is the path the VLLP pull() callback will use. */ + n = stream_read(vcon_backend(vc), buf, sizeof(buf), 0); + VCHECK(n == 0, "input: non-blocking read returned %zu on empty fifo", n); +} + + +static void +phase_registry(void) +{ + hosttest_log("-- registry"); + + VCHECK(vcon_find("c-basic") != NULL, "registry: c-basic not found"); + VCHECK(vcon_find("c-lag") != NULL, "registry: c-lag not found"); + VCHECK(vcon_find("no-such-console") == NULL, + "registry: found a console that was never created"); + + vcon_t *found = vcon_find("c-multi"); + VCHECK(found != NULL && !strcmp(vcon_name(found), "c-multi"), + "registry: name mismatch"); + + int n = 0; + int seen_basic = 0; + for(vcon_t *vc = vcon_first(); vc != NULL; vc = vcon_next(vc)) { + n++; + if(!strcmp(vcon_name(vc), "c-basic")) + seen_basic++; + VCHECK(n < 100, "registry: iteration does not terminate"); + if(n >= 100) + break; + } + VCHECK(seen_basic == 1, "registry: c-basic appears %d times in the list", + seen_basic); + hosttest_log(" %d consoles registered", n); +} + + +/* vcon_bind() is the "dedicated port" path: a thread of its own pumping + both directions for the life of the system. */ +static void +phase_bind(void) +{ + hosttest_log("-- bind"); + + vcon_t *vc = vcon_create("c-bind", 256, 64); + testterm_t *tt = testterm_create(); + + vcon_bind(vc, testterm_stream(tt)); + + backend_puts(vc, "to-the-port"); + VCHECK(testterm_out_wait(tt, "to-the-port", 2 * SEC), + "bind: console output never reached the bound terminal"); + + /* And terminal input reaches the backend. */ + testterm_types(tt, "typed"); + char buf[16]; + size_t n = backend_read(vc, buf, 5, 2 * SEC); + VCHECK(n == 5 && !memcmp(buf, "typed", 5), + "bind: backend got %zu '%.*s'", n, (int)n, buf); + + /* A bound terminal is a normal client, so others may attach alongside. */ + testterm_t *t2 = testterm_create(); + vcon_client_t *vcc = vcon_attach(vc, testterm_stream(t2)); + VCHECK(vcon_client_count(vc) == 2, "bind: %d clients with one attached", + vcon_client_count(vc)); + backend_puts(vc, "both"); + VCHECK(testterm_out_wait(tt, "both", 2 * SEC), "bind: bound terminal missed it"); + char b2[64]; + size_t n2 = drain_client(vcc, b2, sizeof(b2)); + b2[MIN(n2, sizeof(b2) - 1)] = 0; + VCHECK(n2 > 0 && testterm_contains(b2, "both"), + "bind: attached client missed it (%zu bytes)", n2); + vcon_detach(vcc); + free(t2); + /* tt stays alive: vcon_bind's thread owns it forever by design. */ +} + + +/* vcon_create_shell(): a real MIOS shell on the backend. Runs `consoles` + through it, which is a nice closed loop -- the shell reached over the + vcon lists the vcon it is running on. */ +static void +phase_shell(void) +{ + hosttest_log("-- shell on a vcon"); + + vcon_t *vc = vcon_create_shell("c-shell", 4096, 64); + VCHECK(vc != NULL, "shell: create failed"); + if(vc == NULL) + return; + + testterm_t *tt = testterm_create(); + vcon_client_t *vcc = vcon_attach(vc, testterm_stream(tt)); + + /* Pump the client in the background the way cmd_attach does, so the + shell's output lands in the terminal as it is produced. */ + char buf[256]; + const uint64_t deadline = clock_get() + 5 * SEC; + int saw_prompt = 0; + while(clock_get() < deadline) { + size_t n = vcon_client_output(vcc, buf, sizeof(buf)); + if(n) { + stream_write(testterm_stream(tt), buf, n, 0); + continue; + } + if(testterm_out_has(tt, ">")) { + saw_prompt = 1; + break; + } + usleep(10000); + } + VCHECK(saw_prompt, "shell: no prompt appeared"); + + /* Ask the shell to list consoles. Its own name must come back. */ + testterm_out_clear(tt); + vcon_input(vc, "consoles\n", 9); + + const uint64_t d2 = clock_get() + 5 * SEC; + int saw_self = 0; + while(clock_get() < d2) { + size_t n = vcon_client_output(vcc, buf, sizeof(buf)); + if(n) { + stream_write(testterm_stream(tt), buf, n, 0); + continue; + } + if(testterm_out_has(tt, "c-shell")) { + saw_self = 1; + break; + } + usleep(10000); + } + VCHECK(saw_self, "shell: `consoles` output did not mention c-shell"); + + vcon_detach(vcc); + free(tt); +} + + +/* ---------------- the attach CLI command ---------------- */ + +typedef struct attach_ctx { + testterm_t *tt; + const char *console; + volatile int running; + volatile int done; + error_t err; +} attach_ctx_t; + +__attribute__((noreturn)) +static void * +attach_thread(void *arg) +{ + attach_ctx_t *ac = arg; + char line[64]; + snprintf(line, sizeof(line), "attach %s", ac->console); + + cli_t cli = { testterm_stream(ac->tt) }; + ac->running = 1; + ac->err = cli_dispatch(&cli, line); + ac->done = 1; + thread_exit(NULL); +} + +static int +pred_done(void *arg) +{ + attach_ctx_t *ac = arg; + return ac->done; +} + + +/* cmd_vcon.c: `consoles` and `attach`, including the ^A escape handling + that decides whether a byte reaches the console or detaches the user. */ +static void +phase_cli(void) +{ + hosttest_log("-- consoles / attach commands"); + + vcon_t *vc = vcon_create("c-cli", 256, 64); + + /* `consoles` lists it. */ + testterm_t *tl = testterm_create(); + cli_t cli = { testterm_stream(tl) }; + char cmd[] = "consoles"; + cli_dispatch(&cli, cmd); + VCHECK(testterm_out_has(tl, "c-cli"), "cli: `consoles` did not list c-cli"); + free(tl); + + /* `attach c-cli` in a thread of its own; it does not return until the + user detaches. */ + attach_ctx_t ac = { .tt = testterm_create(), .console = "c-cli" }; + thread_create(attach_thread, &ac, 4096, "attach", TASK_DETACHED, 4); + + VCHECK(testterm_out_wait(ac.tt, "[attached to c-cli", 2 * SEC), + "cli: attach banner never printed"); + + /* Console output reaches the attached terminal. */ + backend_puts(vc, "REMOTE-OUTPUT"); + VCHECK(testterm_out_wait(ac.tt, "REMOTE-OUTPUT", 2 * SEC), + "cli: console output did not reach the attached terminal"); + + /* Ordinary keystrokes reach the console. */ + testterm_types(ac.tt, "hello"); + char buf[16]; + size_t n = backend_read(vc, buf, 5, 2 * SEC); + VCHECK(n == 5 && !memcmp(buf, "hello", 5), + "cli: console got %zu '%.*s'", n, (int)n, buf); + + /* ^A ^A is an escaped literal ^A -- one byte through, no detach. */ + testterm_types(ac.tt, "\x01\x01"); + n = backend_read(vc, buf, 1, 2 * SEC); + VCHECK(n == 1 && buf[0] == 0x01, + "cli: ^A^A delivered %zu bytes%s", n, + n ? (buf[0] == 0x01 ? "" : " (wrong byte)") : " (nothing arrived)"); + VCHECK(!ac.done, "cli: ^A^A detached instead of sending a literal"); + + /* ^A followed by an unrelated key delivers that key as-is. */ + testterm_types(ac.tt, "\x01z"); + n = backend_read(vc, buf, 1, 2 * SEC); + VCHECK(n == 1 && buf[0] == 'z', + "cli: ^A z delivered %zu bytes%s", n, + n ? (buf[0] == 'z' ? "" : " (wrong byte)") : " (nothing arrived)"); + VCHECK(!ac.done, "cli: ^A z detached"); + + /* ^A d detaches. */ + testterm_types(ac.tt, "\x01" "d"); + VCHECK(hosttest_wait(pred_done, &ac, 2 * SEC), "cli: ^A d did not detach"); + VCHECK(testterm_out_has(ac.tt, "[detached from c-cli]"), + "cli: no detach message"); + VCHECK(ac.err == 0, "cli: attach returned %d", ac.err); + VCHECK(vcon_client_count(vc) == 0, + "cli: %d clients still attached after detach", + vcon_client_count(vc)); + + /* Attaching to something that does not exist fails cleanly. */ + testterm_t *tn = testterm_create(); + cli_t cli2 = { testterm_stream(tn) }; + char bad[] = "attach no-such-console"; + error_t err = cli_dispatch(&cli2, bad); + VCHECK(err == ERR_NOT_FOUND, "cli: attach to a missing console returned %d", + err); + VCHECK(testterm_out_has(tn, "No such console"), "cli: no diagnostic printed"); + free(tn); + + free(ac.tt); +} + + +static int +test_vcon(void) +{ + hosttest_log("---- vcon ----"); + + phase_ring(); + phase_no_scrollback(); + phase_basic(); + phase_scrollback(); + phase_lag(); + phase_multi(); + phase_input_limits(); + phase_registry(); + phase_bind(); + phase_shell(); + phase_cli(); + + return fails; +} + +HOSTTEST_SUITE("vcon", test_vcon, 0); diff --git a/src/platform/host/suite_vcon_vllp.c b/src/platform/host/suite_vcon_vllp.c new file mode 100644 index 00000000..1bb5d42c --- /dev/null +++ b/src/platform/host/suite_vcon_vllp.c @@ -0,0 +1,864 @@ +/* + * vcon-vllp: the actual usecase, end to end, in virtual time. + * + * A gateway sits on a CAN bus with nine units. Each unit runs a mios VLLP + * server with the ordinary "shell" service. The gateway is a VLLP client + * on nine links; each link has a bind that keeps a channel open to the + * remote shell, wired to a local virtual console. An operator telnets to + * the gateway and types `attach unit3`. + * + * Everything in this suite is shipping code: src/net/vllp.c on both ends, + * src/net/service/svc_shell.c on the unit side, src/net/vcon_pushpull.c + * and src/util/vcon.c on the gateway side, src/shell/cmd_vcon.c for the + * attach itself. The only test scaffolding is the wire (vcan_loop.h) and + * the terminal (testterm.h). + * + * Both ends live in this process, so the "units" are the same mios + * instance the gateway runs in. That is invisible to the protocol -- the + * frames really do go out over the vcan and come back -- but it does mean + * the process has nine shell threads that a real gateway would not. + * + * Gateway-side wiring is deliberately written the way a board init file + * would write it, because that wiring is itself part of what is under + * test: mios ships vllp_client_bind() and vcon_pushpull_open() as + * separate primitives and leaves the joining to the application. + */ + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "net/pbuf.h" + +#include "hosttest.h" +#include "sim.h" +#include "vcan.h" +#include "vcan_loop.h" +#include "testterm.h" + +#define SEC 1000000ull + +#define MTU 64 +#define UNITS 9 + +/* Link i: 0x200+2i gateway->unit, 0x201+2i unit->gateway. Opposite ids at + the two ends, so the loopback delivers each frame to exactly one. */ +#define GW_TX(i) (0x200 + (i) * 2) +#define GW_RX(i) (0x201 + (i) * 2) + +static int fails; + +#define GCHECK(cond, ...) \ + do { if(!(cond)) { fails++; \ + hosttest_check(0, __FILE__, __LINE__, __VA_ARGS__); } } while(0) + + +typedef struct unit { + char name[16]; + vcon_t *vcon; /* gateway side */ + vllp_t *client; /* gateway side */ + vllp_t *server; /* the unit */ + vllp_bind_t *bind; +} unit_t; + +static unit_t units[UNITS]; +static vcan_loop_t *g_lb; + +/* Set when this registration forces allocation failures (see the bottom + of the file). A few phases can only assert what they assert when + buffers are actually available. */ +static int g_starved; + + +/* ---------------- gateway wiring, board-init style ---------------- */ + +/* vllp_client_bind() calls this for every new session and expects a fresh + app bound to the channel. Binding a vcon is one call. */ +static error_t +unit_console_open(void *opaque, pushpull_t *pp) +{ + return vcon_pushpull_open(opaque, pp); +} + + +static void +gateway_setup(void) +{ + for(int i = 0; i < UNITS; i++) { + unit_t *u = &units[i]; + snprintf(u->name, sizeof(u->name), "unit%d", i + 1); + + /* The unit: a plain VLLP server offering whatever services it has + (here that includes the real "shell"). */ + u->server = vllp_server_create(GW_RX(i), GW_TX(i), MTU, 3); + + /* The gateway: a console, a client link, and a bind joining them. */ + u->vcon = vcon_create(u->name, 4096, 128); + u->client = vllp_client_create(GW_TX(i), GW_RX(i), MTU, 3); + u->bind = vllp_client_bind(u->client, "shell", unit_console_open, + u->vcon); + + GCHECK(u->server && u->vcon && u->client && u->bind, + "%s: setup failed", u->name); + } +} + + +/* ---------------- an attached operator ---------------- */ + +/* Attaches to a vcon and pumps it the way cmd_attach() does, in a thread, + so console output reaches the terminal as it is produced. Phases that + test cmd_attach() itself use the real command instead. */ +typedef struct viewer { + testterm_t *tt; + vcon_t *vc; + vcon_client_t *vcc; + volatile int stop; + volatile int stopped; +} viewer_t; + + +__attribute__((noreturn)) +static void * +viewer_thread(void *arg) +{ + viewer_t *vw = arg; + uint8_t buf[128]; + + while(!vw->stop) { + size_t n = vcon_client_output(vw->vcc, buf, sizeof(buf)); + if(n) { + stream_write(testterm_stream(vw->tt), buf, n, 0); + continue; + } + vcon_client_wait(vw->vcc); + } + vw->stopped = 1; + thread_exit(NULL); +} + + +static viewer_t * +viewer_attach(vcon_t *vc) +{ + viewer_t *vw = calloc(1, sizeof(viewer_t)); + vw->tt = testterm_create(); + vw->vc = vc; + vw->vcc = vcon_attach(vc, testterm_stream(vw->tt)); + thread_create(viewer_thread, vw, 4096, "viewer", TASK_DETACHED, 4); + return vw; +} + + +/* Stop the pump and detach. Both, and in that order: stopping the thread + alone leaves the client attached, which then shows up as a bogus + vcon_client_count() several phases later. */ +static void +viewer_detach(viewer_t *vw) +{ + vw->stop = 1; + + /* The pump is asleep in vcon_client_wait(), which polls the terminal's + read side. One byte of terminal input wakes it; nothing ever reads + the viewer's terminal, so the byte is inert. */ + testterm_type(vw->tt, "\0", 1); + + const uint64_t deadline = clock_get() + 5 * SEC; + while(!vw->stopped && clock_get() < deadline) + usleep(10000); + + vcon_detach(vw->vcc); +} + + +/* Type at the console the way an attached client does. */ +static void +viewer_type(vcon_t *vc, const char *str) +{ + vcon_input(vc, str, strlen(str)); +} + + +/* Run a command on a console and wait for it to come back, retrying. + * + * Two things make the naive version unreliable, and both are by design + * rather than bugs: + * + * - A viewer's cursor starts at the oldest buffered byte, so attaching + * replays the whole scrollback. Waiting for "[connected]" or a prompt + * therefore matches history, not the session that is up now. Every tag + * here is unique across the run, so a match can only be a real round + * trip. + * + * - vcon_pushpull_open() flushes queued input for each new session, + * because what is queued was typed at a shell that no longer exists. + * A command typed during a reconnect window is discarded on purpose, + * so the test has to retry rather than race it -- which is exactly + * what an operator does. + */ +static uint32_t g_tag_seq; + +static int +console_command(viewer_t *vw, vcon_t *vc, const char *what, char *tagout, + size_t tagsize, uint64_t timeout) +{ + const uint64_t deadline = clock_get() + timeout; + + while(clock_get() < deadline) { + char tag[40]; + snprintf(tag, sizeof(tag), "zz-%s-%u", what, ++g_tag_seq); + if(tagout != NULL) + snprintf(tagout, tagsize, "%s", tag); + + char line[48]; + snprintf(line, sizeof(line), "%s\n", tag); + vcon_input(vc, line, strlen(line)); + + /* A round trip is fast in virtual time; if it has not come back in a + few seconds the session was not there and we try again. */ + const uint64_t attempt = clock_get() + 4 * SEC; + while(clock_get() < attempt) { + if(testterm_out_has(vw->tt, tag)) + return 1; + usleep(10000); + } + } + return 0; +} + + +/* ---------------- phases ---------------- */ + +/* Every link comes up and every bind opens a channel to the remote shell. + vcon_pushpull_open() writes "[connected]" into the scrollback, which is + the gateway-visible signal that the far end answered. */ +static void +phase_connect(void) +{ + hosttest_log("-- connect (%d units)", UNITS); + + const uint64_t deadline = clock_get() + 30 * SEC; + int up = 0; + + while(clock_get() < deadline) { + up = 0; + for(int i = 0; i < UNITS; i++) { + if(vcon_scrollback_used(units[i].vcon) > 0) + up++; + } + if(up == UNITS) + break; + usleep(10000); + } + + GCHECK(up == UNITS, "connect: only %d of %d units came up", up, UNITS); +} + + +/* Attach, and see the remote shell's prompt. This is the whole path: + cmd_attach -> vcon scrollback -> vcon_pushpull push -> VLLP client + channel -> loopback -> VLLP server -> svc_shell -> cli_on_stream. */ +static void +phase_prompt(void) +{ + hosttest_log("-- prompt from each remote shell"); + + for(int i = 0; i < UNITS; i++) { + viewer_t *vw = viewer_attach(units[i].vcon); + + GCHECK(testterm_out_wait(vw->tt, "[connected]", 10 * SEC), + "%s: no connect marker in the scrollback", units[i].name); + GCHECK(testterm_out_wait(vw->tt, ">", 20 * SEC), + "%s: the remote shell never sent a prompt", units[i].name); + + viewer_detach(vw); + } +} + + +/* Run a command on each unit and read the answer back. `consoles` is a + convenient one: its output is distinctive, and it proves a real shell + is parsing and dispatching on the far end rather than something merely + echoing bytes. */ +static void +phase_command(void) +{ + hosttest_log("-- run a command on each unit"); + + viewer_t *vw[UNITS]; + + for(int i = 0; i < UNITS; i++) { + vw[i] = viewer_attach(units[i].vcon); + GCHECK(testterm_out_wait(vw[i]->tt, ">", 20 * SEC), + "%s: no prompt before the command", units[i].name); + testterm_out_clear(vw[i]->tt); + viewer_type(units[i].vcon, "consoles\n"); + } + + for(int i = 0; i < UNITS; i++) { + /* Every unit is the same mios instance, so its console list contains + all nine names. Look for this unit's own, which must be there. */ + GCHECK(testterm_out_wait(vw[i]->tt, units[i].name, 25 * SEC), + "%s: `consoles` produced no usable output", units[i].name); + viewer_detach(vw[i]); + } +} + + +/* All nine driven at once with per-unit payloads. Catches a channel id, + CRC IV or vcon mixed up between links -- the failure mode where unit 3 + answers on unit 7's console, which a one-link test cannot see. */ +static void +phase_crosstalk(void) +{ + hosttest_log("-- crosstalk (%d units at once)", UNITS); + + viewer_t *vw[UNITS]; + char tag[UNITS][32]; + + for(int i = 0; i < UNITS; i++) { + vw[i] = viewer_attach(units[i].vcon); + GCHECK(testterm_out_wait(vw[i]->tt, ">", 20 * SEC), + "%s: no prompt", units[i].name); + testterm_out_clear(vw[i]->tt); + } + + /* A distinct, wrong-on-purpose command per unit. The shell echoes the + line as it is typed and then complains about it, so the tag comes + back either way -- what matters is *which* console it comes back on. */ + for(int i = 0; i < UNITS; i++) { + snprintf(tag[i], sizeof(tag[i]), "zz-tag-%d-%d", i, i * 7 + 3); + char line[48]; + snprintf(line, sizeof(line), "%s\n", tag[i]); + viewer_type(units[i].vcon, line); + } + + for(int i = 0; i < UNITS; i++) { + GCHECK(testterm_out_wait(vw[i]->tt, tag[i], 30 * SEC), + "%s: never saw its own tag '%s'", units[i].name, tag[i]); + } + + /* Nothing may carry another unit's tag. */ + for(int i = 0; i < UNITS; i++) { + for(int j = 0; j < UNITS; j++) { + if(i == j) + continue; + GCHECK(!testterm_out_has(vw[i]->tt, tag[j]), + "%s: saw %s's tag '%s' -- traffic crossed between links", + units[i].name, units[j].name, tag[j]); + } + } + + for(int i = 0; i < UNITS; i++) + viewer_detach(vw[i]); +} + + +/* Both directions busy at once on the same link. + * + * This is the phase that matters most for whether a console over VLLP is + * usable at all. VLLP's sequence numbers are a single bit shared by the + * whole link, and test/vllp/FINDINGS.md records an open finding that + * concurrent bidirectional origination could desynchronise it until the + * 3 s timeout. A console is exactly that: the far end is writing output + * while the operator types. If this wedges, the feature does not work. + */ +static void +phase_bidir(void) +{ + hosttest_log("-- bidirectional: remote output while the operator types"); + + unit_t *u = &units[0]; + viewer_t *vw = viewer_attach(u->vcon); + GCHECK(testterm_out_wait(vw->tt, ">", 20 * SEC), "bidir: no prompt"); + testterm_out_clear(vw->tt); + + /* `help` on the far end produces a screenful, so output is streaming + down while we push keystrokes up. */ + viewer_type(u->vcon, "help\n"); + + int typed = 0; + const uint64_t deadline = clock_get() + 20 * SEC; + while(clock_get() < deadline && typed < 40) { + viewer_type(u->vcon, "x"); + typed++; + usleep(50000); + } + + /* The link must still be up and still be carrying data in both + directions: a fresh command has to complete after all that. */ + viewer_type(u->vcon, "\n"); + GCHECK(console_command(vw, u->vcon, "after-bidir", NULL, 0, 30 * SEC), + "bidir: the link stopped carrying data after both ends " + "originated at once -- this is the shared 1-bit sequence " + "desynchronising (see test/vllp/FINDINGS.md)"); + + GCHECK(!testterm_out_has(vw->tt, "[disconnected"), + "bidir: the link dropped while both ends were busy"); + + hosttest_log(" %d keystrokes while output was streaming", typed); + viewer_detach(vw); +} + + +/* Keystroke latency on an idle link. + * + * A console that works but lags by a second per keypress is not usable, + * and "works" is all the other phases check. The engine only pulls from + * an app when something makes it run, and on an idle link that something + * is the once-a-second keepalive -- so without + * vcon_set_backend_notify() wiring a keystroke to PUSHPULL_EVENT_PULL, a + * character waits up to a full keepalive interval before it is even + * fragmented. This phase is what makes that hook load-bearing; it is + * measured in virtual time, so it is a count of protocol round trips and + * not of host speed. + */ +static void +phase_latency(void) +{ + if(g_starved) { + // Not measurable here, and not a defect either: with allocations + // being forced to fail, pull() legitimately returns NULL because it + // could not get a buffer, and the keystroke then waits for the next + // thing that runs the engine -- the keepalive. Asserting sub-200ms + // latency under injected allocation failure would be asserting that + // the pool is never empty. + hosttest_log("-- keystroke latency (skipped: allocations are being " + "forced to fail)"); + return; + } + + hosttest_log("-- keystroke latency on an idle link"); + + unit_t *u = &units[6]; + viewer_t *vw = viewer_attach(u->vcon); + GCHECK(console_command(vw, u->vcon, "warmup", NULL, 0, 30 * SEC), + "latency: console not working before the measurement"); + + /* Let the link go quiet, so the next thing to happen is our keystroke + rather than traffic that was already in flight. */ + usleep(2 * SEC); + + uint64_t worst = 0; + for(int i = 0; i < 5; i++) { + char tag[40]; + snprintf(tag, sizeof(tag), "zz-lat-%u", ++g_tag_seq); + char line[48]; + snprintf(line, sizeof(line), "%s\n", tag); + + const uint64_t t0 = clock_get(); + vcon_input(u->vcon, line, strlen(line)); + + const uint64_t deadline = t0 + 5 * SEC; + while(!testterm_out_has(vw->tt, tag) && clock_get() < deadline) + usleep(1000); + + const uint64_t took = clock_get() - t0; + if(!testterm_out_has(vw->tt, tag)) { + GCHECK(0, "latency: round trip %d never completed", i); + break; + } + if(took > worst) + worst = took; + usleep(2 * SEC); /* back to idle for the next one */ + } + + hosttest_log(" worst round trip %u us", (unsigned)worst); + + /* The keepalive is 1 s. Anything approaching that means the keystroke + waited for a timer instead of waking the engine. 200 ms leaves plenty + of room for a slow multi-fragment round trip while still being far + below the interval it must not be gated on. */ + GCHECK(worst < 200000, + "latency: worst keystroke round trip was %u us. A keystroke is " + "waiting for the 1 s keepalive to move it, which means nothing " + "is waking the engine when a client types -- check that " + "vcon_set_backend_notify() is wired to PUSHPULL_EVENT_PULL", + (unsigned)worst); + + viewer_detach(vw); +} + + +/* Detach, let the far end talk, re-attach: the point of a vcon is that + output produced while nobody was looking is still there. */ +static void +phase_scrollback(void) +{ + hosttest_log("-- scrollback across a detach"); + + unit_t *u = &units[1]; + + /* The whole cycle is retried, not just the command: the command is + issued while nobody is attached, so there is no way to see that it + was lost (which under injected allocation failure it can be) other + than by not finding its output afterwards. */ + for(int attempt = 0; attempt < 5; attempt++) { + + viewer_t *vw = viewer_attach(u->vcon); + if(!console_command(vw, u->vcon, "sb-warmup", NULL, 0, 40 * SEC)) { + viewer_detach(vw); + continue; /* console not responsive yet; try the cycle again */ + } + viewer_detach(vw); + + /* Nobody attached. Make the far end produce something identifiable. */ + char tag[40]; + snprintf(tag, sizeof(tag), "zz-while-detached-%u", ++g_tag_seq); + char line[48]; + snprintf(line, sizeof(line), "%s\n", tag); + vcon_input(u->vcon, line, strlen(line)); + usleep(3 * SEC); + + viewer_t *vw2 = viewer_attach(u->vcon); + const int replayed = testterm_out_wait(vw2->tt, tag, 20 * SEC); + viewer_detach(vw2); + + if(replayed) + return; /* pass */ + } + + GCHECK(0, "scrollback: output produced while detached was never replayed " + "to a client that attached afterwards"); +} + + +/* Two operators on one console: output to both, keystrokes from both. */ +static void +phase_multi_attach(void) +{ + hosttest_log("-- two operators on one console"); + + unit_t *u = &units[2]; + viewer_t *a = viewer_attach(u->vcon); + viewer_t *b = viewer_attach(u->vcon); + + GCHECK(vcon_client_count(u->vcon) == 2, "multi: %d clients", + vcon_client_count(u->vcon)); + + GCHECK(testterm_out_wait(a->tt, ">", 20 * SEC), "multi: A saw no prompt"); + GCHECK(testterm_out_wait(b->tt, ">", 20 * SEC), "multi: B saw no prompt"); + + char tag[40]; + GCHECK(console_command(a, u->vcon, "shared", tag, sizeof(tag), 25 * SEC), + "multi: A missed the output"); + GCHECK(testterm_out_wait(b->tt, tag, 25 * SEC), + "multi: B missed the output -- output is not mirrored"); + + viewer_detach(a); + viewer_detach(b); +} + + +/* A unit reboots. The gateway must notice, mark the console, re-SYN, + re-open the channel and give the operator a working shell again -- + without the operator having to detach. This is the thing that will + actually happen in the field, repeatedly. + */ +static void +phase_reconnect(int round) +{ + hosttest_log("-- unit reboot / reconnect (round %d)", round); + + unit_t *u = &units[3]; + viewer_t *vw = viewer_attach(u->vcon); + GCHECK(testterm_out_wait(vw->tt, ">", 20 * SEC), "reconnect: no prompt"); + testterm_out_clear(vw->tt); + + /* 3 s timeouts at both ends; 7 s of silence takes the session down. */ + vcan_loop_outage(g_lb, 7 * SEC); + + GCHECK(testterm_out_wait(vw->tt, "[disconnected", 10 * SEC), + "reconnect: the console was not marked disconnected, so an " + "operator cannot tell a dead unit from a quiet one"); + + /* And it comes back by itself. */ + GCHECK(testterm_out_wait(vw->tt, "[connected]", 30 * SEC), + "reconnect: the console never reconnected"); + + /* A fresh shell on the far end, reachable without re-attaching. */ + GCHECK(console_command(vw, u->vcon, "after-reboot", NULL, 0, 40 * SEC), + "reconnect: no working shell on the console after reconnecting"); + + /* The other units were on the same bus and must have recovered too. + Wait for each to show a prompt on the new session before typing: + vcon_pushpull_open() flushes pending input for every new session (it + was aimed at a shell that no longer exists), so a command injected + during the reconnect window is discarded by design. Racing that would + make this phase flaky rather than wrong. */ + for(int i = 0; i < UNITS; i++) { + if(i == 3) + continue; + viewer_t *o = viewer_attach(units[i].vcon); + GCHECK(console_command(o, units[i].vcon, "still-there", NULL, 0, + 40 * SEC), + "%s: console not usable after the bus outage", units[i].name); + viewer_detach(o); + } + + viewer_detach(vw); +} + + +/* The same thing through the real `attach` command, over a stream, the way + a telnet session reaches it. Covers the bit the viewer_thread stands in + for everywhere else: cmd_attach()'s own pump and its ^A escape. */ +typedef struct attach_ctx { + testterm_t *tt; + const char *console; + volatile int done; + error_t err; +} attach_ctx_t; + +__attribute__((noreturn)) +static void * +attach_thread(void *arg) +{ + attach_ctx_t *ac = arg; + char line[64]; + snprintf(line, sizeof(line), "attach %s", ac->console); + cli_t cli = { testterm_stream(ac->tt) }; + ac->err = cli_dispatch(&cli, line); + ac->done = 1; + thread_exit(NULL); +} + +static int +pred_attach_done(void *arg) +{ + attach_ctx_t *ac = arg; + return ac->done; +} + + +static void +phase_attach_command(void) +{ + hosttest_log("-- the real `attach` command end to end"); + + unit_t *u = &units[4]; + + attach_ctx_t ac = { .tt = testterm_create(), .console = u->name }; + thread_create(attach_thread, &ac, 4096, "attach", TASK_DETACHED, 4); + + GCHECK(testterm_out_wait(ac.tt, "[attached to", 5 * SEC), + "attach: no banner"); + GCHECK(testterm_out_wait(ac.tt, ">", 25 * SEC), + "attach: no prompt from the remote shell through `attach`"); + + /* Type at the terminal, not at the vcon: this goes through + cmd_attach()'s escape handling on the way in. */ + testterm_out_clear(ac.tt); + testterm_types(ac.tt, "zz-via-attach\n"); + GCHECK(testterm_out_wait(ac.tt, "zz-via-attach", 30 * SEC), + "attach: keystrokes did not reach the remote shell"); + + /* ^A d detaches, and leaves the console running for the next operator. */ + testterm_types(ac.tt, "\x01" "d"); + GCHECK(hosttest_wait(pred_attach_done, &ac, 5 * SEC), + "attach: ^A d did not detach"); + GCHECK(testterm_out_has(ac.tt, "[detached from"), + "attach: no detach message"); + GCHECK(vcon_client_count(u->vcon) == 0, "attach: %d clients left attached", + vcon_client_count(u->vcon)); + + /* Re-attaching gets a working console, and the scrollback is still + there -- the far-end shell was never torn down by the detach. */ + viewer_t *vw = viewer_attach(u->vcon); + GCHECK(testterm_out_wait(vw->tt, "zz-via-attach", 20 * SEC), + "attach: scrollback lost after detach"); + GCHECK(console_command(vw, u->vcon, "after-detach", NULL, 0, 30 * SEC), + "attach: console dead after a detach/re-attach cycle"); + viewer_detach(vw); +} + + +/* Consoles over a lossy bus. Loss and duplication must be ridden through + by the link layer; corruption resets the session, so the console must + come back rather than wedge. Either way an operator must never see + corrupted output presented as real. */ +static void +phase_faults(const char *name, int drop, int dup, int corrupt) +{ + hosttest_log("-- %s (drop=%d%% dup=%d%% corrupt=%d%%)", name, drop, dup, + corrupt); + + unit_t *u = &units[5]; + viewer_t *vw = viewer_attach(u->vcon); + GCHECK(testterm_out_wait(vw->tt, ">", 25 * SEC), "%s: no prompt to start", + name); + + const uint32_t frames0 = g_lb->frames; + const uint32_t dropped0 = g_lb->dropped; + const uint32_t dup0 = g_lb->duplicated; + const uint32_t corrupt0 = g_lb->corrupted; + + g_lb->drop_pct = drop; + g_lb->dup_pct = dup; + g_lb->corrupt_pct = corrupt; + + int ok = 0; + for(int i = 0; i < 20; i++) { + char tag[32]; + snprintf(tag, sizeof(tag), "zz-f%d-%d", corrupt, i); + char line[48]; + snprintf(line, sizeof(line), "%s\n", tag); + testterm_out_clear(vw->tt); + viewer_type(u->vcon, line); + if(testterm_out_wait(vw->tt, tag, 30 * SEC)) + ok++; + } + + g_lb->drop_pct = 0; + g_lb->dup_pct = 0; + g_lb->corrupt_pct = 0; + + const uint32_t frames = g_lb->frames - frames0; + const uint32_t dropped = g_lb->dropped - dropped0; + const uint32_t duped = g_lb->duplicated - dup0; + const uint32_t corrupted = g_lb->corrupted - corrupt0; + + /* Enough traffic for the configured rate to have bitten. Without this + the phase passes just as happily when nothing was injected at all, + which is indistinguishable from real coverage. */ + GCHECK(frames > 100, "%s: only %u frames crossed the bus -- too few for " + "a %d%%/%d%%/%d%% fault rate to have done anything, so this phase " + "proved nothing", name, frames, drop, dup, corrupt); + if(drop) + GCHECK(dropped > 0, "%s: %u frames at %d%% loss and not one was " + "dropped -- faults are not reaching the wire", name, frames, drop); + if(dup) + GCHECK(duped > 0, "%s: %u frames at %d%% duplication and not one was " + "duplicated -- faults are not reaching the wire", name, frames, + dup); + if(corrupt) + GCHECK(corrupted > 0, "%s: %u frames at %d%% corruption and not one was " + "corrupted -- faults are not reaching the wire", name, frames, + corrupt); + + if(corrupt) { + /* A CRC failure resets the link, so some commands are expected to be + lost. What must hold is that it recovers. */ + GCHECK(ok > 0, "%s: nothing got through at all", name); + } else if(g_starved) { + // With allocations being forced to fail, a command can be lost + // because a buffer was refused rather than because a frame was. The + // claim that survives is progress, not perfection. + GCHECK(ok >= 15, "%s: only %d of 20 commands got through even allowing " + "for injected allocation failures", name, ok); + } else { + GCHECK(ok == 20, "%s: only %d of 20 commands got through -- loss and " + "duplication should be invisible above the link layer", name, ok); + } + + GCHECK(console_command(vw, u->vcon, "recovered", NULL, 0, 120 * SEC), + "%s: console did not recover once the faults stopped", name); + + hosttest_log(" %d of 20 commands through; %u frames, %u dropped, " + "%u duplicated, %u corrupted", ok, frames, dropped, duped, + corrupted); + + viewer_detach(vw); +} + + +/* ---------------- driver ---------------- */ + +static int +test_vcon_vllp(void) +{ + const int pool_total = pbuf_buffer_avail(); + const unsigned int fails_at_start = pbuf_alloc_fail_count(); + const int pool_at_start = pool_total; + + // The starved registration is the one that wants injected failures: a + // small pool alone does not reliably produce them, which is checked at + // the end. + const int starved = pool_total <= 48; + g_starved = starved; +#ifdef ENABLE_PBUF_FAULT_INJECT + if(starved) + pbuf_fault_inject(5, 1); +#endif + + hosttest_log("---- %d remote consoles over VLLP (pool %d buffers) ----%s", + UNITS, pool_total, + starved ? " [5% of allocations forced to fail]" : ""); + + vcan_t *vcan = vcan_create("vcan0", MTU); + gateway_setup(); + vcan_set_link(vcan, 1); + g_lb = vcan_loop_create(vcan, 0x0badc0de); + + phase_connect(); + phase_prompt(); + phase_command(); + phase_crosstalk(); + phase_bidir(); + phase_latency(); + phase_scrollback(); + phase_multi_attach(); + phase_attach_command(); + phase_reconnect(1); + phase_reconnect(2); + phase_faults("loss-5", 5, 0, 0); + phase_faults("dup-10", 0, 10, 0); + /* 5%, not 1%: at 1% only one or two frames in a phase are hit, + which is too thin to be sure the reset-and-recover path ran. */ + phase_faults("corrupt-5", 0, 0, 5); + + hosttest_log(" %u frames crossed the bus, %u dropped", g_lb->frames, + g_lb->dropped); + g_lb->stop = 1; + + const unsigned int allocfails = pbuf_alloc_fail_count() - fails_at_start; + hosttest_log(" %u buffer allocation failures", allocfails); + + // Everything has been closed and the bus has been quiet; the pool must + // be back where it started. Buffers still held here are leaked, and a + // leak on a link-reset path is invisible until a device that has been + // up for a month stops working. + const int pool_now = pbuf_buffer_avail(); + hosttest_log(" pool %d of %d buffers free", pool_now, pool_at_start); + GCHECK(pool_now >= pool_at_start - 2, + "%d of %d buffers are still held after everything went idle -- " + "something on the link-reset path is leaking them", + pool_at_start - pool_now, pool_at_start); + + // A starved run that never actually starved is a duplicate of the + // others: passing, meaningless, and impossible to tell from real + // coverage. + if(starved) + GCHECK(allocfails > 0, "no allocation ever failed -- this registration " + "is meant to run under pressure, so it tested nothing"); + + return fails; +} + +// Registered three times, same scenarios, different pbuf pools. Nine +// concurrent links is a much harder allocation test than the single-link +// suites, and the paths that matter here are the ones that must cope with +// pbuf_make() returning NULL: vcon_pushpull.c's pull() (which peeks +// before allocating precisely so a failure loses no keystrokes) and the +// client's OPEN request, which has to stay queued rather than be dropped. +// +// vcon-vllp platform default (512 byte buffers, 256 of them) +// vcon-vllp-tight 72 byte buffers, what a CAN-only target ends up +// with when sized for one max CAN-FD payload plus +// dsig's 4-byte signal id. Every frame nearly fills +// a buffer. +// vcon-vllp-starved 72 byte buffers and only 40 of them, for nine +// links, with allocation failures injected on top. +HOSTTEST_SUITE_EX("vcon-vllp", test_vcon_vllp, 0, 0); +HOSTTEST_SUITE_EX("vcon-vllp-tight", test_vcon_vllp, 0, 72); +HOSTTEST_SUITE_POOL("vcon-vllp-starved", test_vcon_vllp, 0, 72, 40); diff --git a/src/platform/host/suite_vllp_client.c b/src/platform/host/suite_vllp_client.c new file mode 100644 index 00000000..1df73b4d --- /dev/null +++ b/src/platform/host/suite_vllp_client.c @@ -0,0 +1,639 @@ +/* + * vllp-client: the mios VLLP *client* (src/net/vllp.c, vllp_client_create) + * driven against the mios VLLP *server* (the same file, the other role), + * both in one host-mios binary, over a looped-back virtual CAN bus, in + * virtual time. + * + * The client is the new code; the server has been in service for a while + * and is the reference here. That asymmetry is what makes this test + * useful: every per-session convention the two ends have to agree on -- + * the cookie that seeds the CRC, the mirrored per-channel CRC IVs, the + * channel-management opcodes, who allocates channel ids -- is checked + * against an implementation that was not written from the same notes. + * Only one side can be wrong. + * + * Topology. All endpoints live in this process and share one vcan. A + * simulation thread (loopback_fn) moves each frame mios transmits back + * into mios's receive ring unchanged, so a frame sent on signal id N is + * delivered to whichever endpoint has rxid N -- exactly one, since the + * two ends of a link use opposite ids. mios never delivers locally + * emitted dsig to itself, so nothing short-circuits. + * + * The loopback (and the fault injection that rides on it) is shared with + * the other suites -- see vcan_loop.h. + */ + +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +#include "net/pbuf.h" + +#include "hosttest.h" +#include "sim.h" +#include "vcan.h" +#include "vcan_loop.h" + +#define SEC 1000000ull + +#define MTU 64 + +/* Frame header bits, from docs/vllp.txt. Private to vllp.c, so restated. */ +#define VLLP_HDR_S_TEST 0x80 + +/* Signal ids. Link i uses (0x200 + 2i) client->server and (0x201 + 2i) + the other way, so a single loopback delivers each frame to exactly one + endpoint. */ +#define LINK_C2S(i) (0x200 + (i) * 2) +#define LINK_S2C(i) (0x201 + (i) * 2) + +static int fails; + +#define LCHECK(cond, ...) \ + do { if(!(cond)) { fails++; \ + hosttest_check(0, __FILE__, __LINE__, __VA_ARGS__); } } while(0) + + +/* ---------------- a pushpull app that records what it gets ------------- */ + +/* Stands in for whatever an application binds to a client channel. Keeps + the received bytes in one flat buffer and hands out queued messages on + pull(), which is enough to check ordering, framing and reconnects. */ + +#define APP_RX_SIZE 8192 +#define APP_TX_SIZE 2048 + +typedef struct app { + pushpull_t *pp; + + mutex_t mutex; + cond_t cond; + + uint8_t rx[APP_RX_SIZE]; + size_t rx_used; + int rx_msgs; + + uint8_t tx[APP_TX_SIZE]; + size_t tx_used; + + int opens; /* how many times a channel was bound */ + int closes; /* ...and unbound */ + int live; /* a channel is currently bound */ + char last_close[64]; +} app_t; + + +static uint32_t +app_push(void *opaque, pbuf_t *pb) +{ + app_t *a = opaque; + + mutex_lock(&a->mutex); + for(pbuf_t *p = pb; p != NULL; p = p->pb_next) { + const size_t n = MIN(p->pb_buflen, APP_RX_SIZE - a->rx_used); + memcpy(a->rx + a->rx_used, pbuf_cdata(p, 0), n); + a->rx_used += n; + } + a->rx_msgs++; + cond_broadcast(&a->cond); + mutex_unlock(&a->mutex); + + pbuf_free(pb); + return 0; +} + + +static int +app_may_push(void *opaque) +{ + app_t *a = opaque; + return a->rx_used < APP_RX_SIZE; +} + + +static pbuf_t * +app_pull(void *opaque) +{ + app_t *a = opaque; + pbuf_t *pb = NULL; + + mutex_lock(&a->mutex); + if(a->tx_used) { + pb = pbuf_make(a->pp->preferred_offset, 0); + if(pb != NULL) { + const size_t n = MIN(a->tx_used, MIN(a->pp->max_fragment_size, + PBUF_DATA_SIZE - + a->pp->preferred_offset)); + memcpy(pbuf_append(pb, n), a->tx, n); + memmove(a->tx, a->tx + n, a->tx_used - n); + a->tx_used -= n; + cond_broadcast(&a->cond); + } + } + mutex_unlock(&a->mutex); + return pb; +} + + +static void +app_close(void *opaque, const char *reason) +{ + app_t *a = opaque; + + mutex_lock(&a->mutex); + a->closes++; + a->live = 0; + snprintf(a->last_close, sizeof(a->last_close), "%s", + reason ? reason : "(none)"); + /* Anything still queued belonged to the session that just went away. */ + a->tx_used = 0; + cond_broadcast(&a->cond); + mutex_unlock(&a->mutex); +} + + +static const pushpull_app_fn_t app_fn = { + .push = app_push, + .may_push = app_may_push, + .pull = app_pull, + .close = app_close, +}; + + +/* The bind callback: mios calls this from net context for every new + session, and expects a fresh app bound to the channel each time. */ +static error_t +app_open(void *opaque, pushpull_t *pp) +{ + app_t *a = opaque; + + mutex_lock(&a->mutex); + a->opens++; + a->live = 1; + mutex_unlock(&a->mutex); + + a->pp = pp; + pp->app = &app_fn; + pp->app_opaque = a; + return 0; +} + + +static app_t * +app_create(void) +{ + app_t *a = calloc(1, sizeof(app_t)); + mutex_init(&a->mutex, "appmtx"); + cond_init(&a->cond, "appcond"); + return a; +} + + +static void +app_send(app_t *a, const void *data, size_t len) +{ + mutex_lock(&a->mutex); + const size_t n = MIN(len, APP_TX_SIZE - a->tx_used); + memcpy(a->tx + a->tx_used, data, n); + a->tx_used += n; + mutex_unlock(&a->mutex); + + /* Tell the engine there is something to pull. */ + if(a->pp != NULL) + pushpull_wakeup(a->pp, PUSHPULL_EVENT_PULL); +} + + +static size_t +app_rx_used(app_t *a) +{ + mutex_lock(&a->mutex); + size_t n = a->rx_used; + mutex_unlock(&a->mutex); + return n; +} + + +static void +app_rx_clear(app_t *a) +{ + mutex_lock(&a->mutex); + a->rx_used = 0; + a->rx_msgs = 0; + mutex_unlock(&a->mutex); +} + + +/* Wait until the app has received at least `want` bytes. */ +static int +app_wait_rx(app_t *a, size_t want, uint64_t timeout) +{ + const uint64_t deadline = clock_get() + timeout; + while(app_rx_used(a) < want) { + if(clock_get() >= deadline) + return 0; + usleep(10000); + } + return 1; +} + + +static int +app_wait_live(app_t *a, int live, uint64_t timeout) +{ + const uint64_t deadline = clock_get() + timeout; + while(a->live != live) { + if(clock_get() >= deadline) + return 0; + usleep(10000); + } + return 1; +} + + +/* ---------------- phases ---------------- */ + +typedef struct link { + vllp_t *client; + vllp_t *server; + app_t *app; + vllp_bind_t *bind; + vcan_loop_t *lb; +} link_t; + + +static void +fill_pattern(uint8_t *buf, size_t len, uint32_t seq) +{ + for(size_t i = 0; i < len; i++) + buf[i] = (uint8_t)(seq * 31 + i * 7); +} + + +/* The handshake, and that the client really did drive it: a client SYNs, + a server answers, and the bind opens a channel to a real service. */ +static void +phase_connect(link_t *l) +{ + hosttest_log("-- connect"); + + LCHECK(app_wait_live(l->app, 1, 10 * SEC), + "connect: bind never opened (opens=%d closes=%d)", + l->app->opens, l->app->closes); + LCHECK(l->app->opens == 1, "connect: %d opens, want 1", l->app->opens); +} + + +/* Echo, both directions, over a client-opened channel. The remote is the + real svc_echo.c, so this is the whole path: app -> pull -> fragment -> + loopback -> server reassembly -> service -> back again. */ +static void +phase_echo(link_t *l) +{ + hosttest_log("-- echo"); + + static const size_t sizes[] = { 1, 7, 8, 9, 63, 64, 200, 500 }; + uint8_t tx[512]; + + for(size_t i = 0; i < sizeof(sizes) / sizeof(sizes[0]); i++) { + const size_t len = sizes[i]; + app_rx_clear(l->app); + fill_pattern(tx, len, i + 1); + app_send(l->app, tx, len); + + if(!app_wait_rx(l->app, len, 10 * SEC)) { + LCHECK(0, "echo: %zu bytes never came back (got %zu)", len, + app_rx_used(l->app)); + continue; + } + LCHECK(app_rx_used(l->app) == len, + "echo: %zu bytes out, %zu back", len, app_rx_used(l->app)); + LCHECK(!memcmp(l->app->rx, tx, len), "echo: %zu byte payload differs", + len); + } +} + + +/* A bind whose service the peer does not have. The open must be refused + cleanly, retried on a backoff rather than as fast as the bus allows, + and -- the part that matters -- must not poison the link for anyone + else. The server bumps its per-channel CRC IV counter for every OPEN it + receives, including ones it refuses, so if the client does not do the + same every later channel on that link fails its CRC. */ +static void +phase_open_refused(link_t *l) +{ + hosttest_log("-- refused open"); + + app_t *bad = app_create(); + vllp_bind_t *b = vllp_client_bind(l->client, "no-such-service", + app_open, bad); + LCHECK(b != NULL, "refused: bind failed"); + + /* It should be refused, i.e. opened locally then closed again. */ + const uint64_t deadline = clock_get() + 10 * SEC; + while(bad->closes == 0 && clock_get() < deadline) + usleep(10000); + LCHECK(bad->closes > 0, "refused: the open was never refused"); + LCHECK(bad->live == 0, "refused: channel still live after a refusal"); + hosttest_log(" refused with reason '%s'", bad->last_close); + + /* The backoff must be a backoff at both ends of the scale: slow enough + not to flood a shared bus, but it must actually keep trying. A bind + that gives up for good is a real failure mode -- a unit that boots + after the gateway, or one whose service only appears after a firmware + update, would never get a console. */ + const int retries_at_start = bad->opens; + uint64_t t0 = clock_get(); + while(clock_get() < t0 + 2 * SEC) + usleep(100000); + const int fast = bad->opens - retries_at_start; + LCHECK(fast <= 2, "refused: retried %d times in 2 s -- the backoff is not " + "working, this would flood a shared bus", fast); + + t0 = clock_get(); + while(bad->opens <= retries_at_start && clock_get() < t0 + 30 * SEC) + usleep(100000); + LCHECK(bad->opens > retries_at_start, + "refused: the bind never retried -- a service that appears later " + "would never be picked up"); + hosttest_log(" %d retries in the first 2 s, %d after 30 s", fast, + bad->opens - retries_at_start); + + /* And the link still works for the channel that was already up. This is + the CRC-IV check: a desynchronised counter shows up here, not above. */ + app_rx_clear(l->app); + uint8_t tx[64]; + fill_pattern(tx, sizeof(tx), 0xabc); + app_send(l->app, tx, sizeof(tx)); + LCHECK(app_wait_rx(l->app, sizeof(tx), 10 * SEC), + "refused: echo broken after a refused open -- per-channel CRC IVs " + "have probably drifted out of step with the server"); + LCHECK(!memcmp(l->app->rx, tx, sizeof(tx)), + "refused: echo payload differs after a refused open"); +} + + +/* A new channel opened after a refusal must also work -- the IV counters + have to be in step for channels created later, not just for the one + that predates the refusal. */ +static void +phase_open_after_refusal(link_t *l) +{ + hosttest_log("-- open after refusal"); + + app_t *a = app_create(); + vllp_bind_t *b = vllp_client_bind(l->client, "echo", app_open, a); + LCHECK(b != NULL, "after-refusal: bind failed"); + + if(!app_wait_live(a, 1, 10 * SEC)) { + LCHECK(0, "after-refusal: channel never opened"); + return; + } + + uint8_t tx[100]; + fill_pattern(tx, sizeof(tx), 0xdef); + app_send(a, tx, sizeof(tx)); + LCHECK(app_wait_rx(a, sizeof(tx), 10 * SEC), + "after-refusal: echo on a channel opened after a refusal never " + "came back -- CRC IV counters are out of step"); + LCHECK(!memcmp(a->rx, tx, sizeof(tx)), + "after-refusal: payload differs"); +} + + +/* Pull the link down long enough for the server to time out, then let it + back. The client must notice, re-SYN, and re-open its binds onto the + fresh session; the app must be told the old channel died and be handed + a new one. This is what happens every time one of the units reboots. */ +static void +phase_reconnect(link_t *l, int round) +{ + hosttest_log("-- reconnect (round %d)", round); + + const int opens_before = l->app->opens; + const int closes_before = l->app->closes; + + /* The link timeout is 3 s at both ends; 6 s of silence takes both down. */ + vcan_loop_outage(l->lb, 6 * SEC); + + LCHECK(l->app->closes > closes_before, + "reconnect: app was never told the channel died"); + + if(!app_wait_live(l->app, 1, 20 * SEC)) { + LCHECK(0, "reconnect: bind never re-opened (opens=%d closes=%d)", + l->app->opens, l->app->closes); + return; + } + LCHECK(l->app->opens > opens_before, + "reconnect: the app was reused instead of re-opened"); + + /* And the fresh session actually carries data. */ + app_rx_clear(l->app); + uint8_t tx[64]; + fill_pattern(tx, sizeof(tx), 0x5150 + round); + app_send(l->app, tx, sizeof(tx)); + LCHECK(app_wait_rx(l->app, sizeof(tx), 15 * SEC), + "reconnect: no echo on the new session"); + LCHECK(!memcmp(l->app->rx, tx, sizeof(tx)), + "reconnect: payload differs on the new session"); +} + + +/* Channel ids are a 14-entry space the client allocates. Exhaust it and + check the 15th bind is refused rather than reusing an id or scribbling + past the bitmap. */ +static void +phase_exhaust(link_t *l) +{ + hosttest_log("-- channel exhaustion"); + + enum { N = 20 }; + app_t *apps[N]; + int live = 0; + + for(int i = 0; i < N; i++) { + apps[i] = app_create(); + vllp_client_bind(l->client, "echo", app_open, apps[i]); + } + + /* Give them all a chance to settle. */ + const uint64_t t0 = clock_get(); + while(clock_get() < t0 + 10 * SEC) + usleep(100000); + + for(int i = 0; i < N; i++) + live += apps[i]->live; + + /* 14 ids total; one is already taken by the echo bind from + phase_connect and one by phase_open_after_refusal, and the refused + bind is idle in backoff. So we cannot say exactly how many of these + came up -- only that it stopped at the ceiling instead of going past + it, and that nothing was handed a duplicate id. */ + hosttest_log(" %d of %d extra binds came up", live, N); + LCHECK(live < N, "exhaust: all %d binds opened -- the id space is only 14 " + "channels wide, so at least one should have been refused", N); + LCHECK(live > 0, "exhaust: no bind opened at all"); + + /* The link is still healthy. */ + app_rx_clear(l->app); + uint8_t tx[32]; + fill_pattern(tx, sizeof(tx), 0x99); + app_send(l->app, tx, sizeof(tx)); + LCHECK(app_wait_rx(l->app, sizeof(tx), 15 * SEC), + "exhaust: link broken after exhausting the id space"); +} + + +/* A bind that keeps being refused must not consume the link as it + retries. Channel ids are a 14-entry space and every attempt allocates + one, so an id that is not returned when the refused channel is torn + down means that after a dozen or so retries the link has no ids left + and *every* console on it dies -- while the bind that caused it looks + perfectly healthy. The mios client releases the id in + vllp_channel_destroy(); this is what proves it. */ +static void +phase_refusal_leak(link_t *l) +{ + hosttest_log("-- refused opens do not consume the link"); + + app_t *bad = app_create(); + LCHECK(vllp_client_bind(l->client, "still-no-such-service", app_open, + bad) != NULL, "leak: bind failed"); + + /* More retry cycles than there are channel ids, so a leak has to show. + Virtual time, so this is free. */ + const uint64_t t0 = clock_get(); + while(clock_get() < t0 + 80 * SEC) + usleep(100000); + hosttest_log(" %d refusals in 80 s", bad->closes); + LCHECK(bad->closes >= 4, "leak: only %d refusals in 80 s -- the retry " + "loop stopped, so this phase proved nothing", bad->closes); + + /* The link must still have ids to give out. */ + app_t *fresh = app_create(); + LCHECK(vllp_client_bind(l->client, "echo", app_open, fresh) != NULL, + "leak: bind failed"); + LCHECK(app_wait_live(fresh, 1, 20 * SEC), + "leak: a new channel could not be opened after %d refused ones -- " + "channel ids are not being released when a refused channel is " + "torn down", bad->closes); + + /* ...and the channel that was already up is untouched. */ + app_rx_clear(l->app); + uint8_t tx[48]; + fill_pattern(tx, sizeof(tx), 0x77); + app_send(l->app, tx, sizeof(tx)); + LCHECK(app_wait_rx(l->app, sizeof(tx), 15 * SEC), + "leak: the established channel stopped working"); +} + + +/* A client must not take any old frame on its rx id for the ACK that + answers its SYN. On a shared CAN bus there is other traffic, and a + client that comes "up" on a frame the server never sent goes on to use + a CRC IV the server does not have -- a link that is established at one + end only, which then just times out. */ +static void +phase_noise(link_t *l) +{ + hosttest_log("-- bus noise is not a SYN response"); + + const int opens_before = l->app->opens; + + /* Take both ends down, and hold them down for the injection below. */ + l->lb->blackhole = 1; + uint64_t t0 = clock_get(); + while(clock_get() < t0 + 6 * SEC) + usleep(100000); + LCHECK(l->app->live == 0, "noise: channel still live after an outage"); + + /* Now play a data frame at the client: header S=1 on channel 0, which + is neither a SYN (0x0f) nor ACK-shaped (low five bits 0x1f), so it + reaches the not-connected path instead of being dropped as a bad + CRC. */ + l->lb->inject[0] = VLLP_HDR_S_TEST | 0x00; + l->lb->inject[1] = 'x'; + l->lb->inject[2] = 'y'; + l->lb->inject[3] = 'z'; + l->lb->inject_id = LINK_S2C(0); + l->lb->inject_len = 4; + + t0 = clock_get(); + while(clock_get() < t0 + 3 * SEC) + usleep(100000); + LCHECK(l->lb->inject_len == 0, "noise: the frame was never injected, so " + "this phase proved nothing"); + LCHECK(l->app->opens == opens_before, + "noise: the client brought a channel up while the bus was black-" + "holed -- it accepted a frame that was not the SYN response"); + + /* Let the real link back and confirm it recovers properly. */ + l->lb->blackhole = 0; + LCHECK(app_wait_live(l->app, 1, 25 * SEC), + "noise: link did not recover after the outage"); + + app_rx_clear(l->app); + uint8_t tx[32]; + fill_pattern(tx, sizeof(tx), 0x33); + app_send(l->app, tx, sizeof(tx)); + LCHECK(app_wait_rx(l->app, sizeof(tx), 15 * SEC), + "noise: no echo after recovery"); +} + + +/* ---------------- driver ---------------- */ + +static int +test_vllp_client(void) +{ + hosttest_log("---- mios VLLP client vs mios VLLP server over vcan ----"); + + vcan_t *vcan = vcan_create("vcan0", MTU); + + vcan_loop_t *lb = NULL; + + link_t l = {}; + + /* The unit: a server serving whatever services this build has. */ + l.server = vllp_server_create(LINK_S2C(0), LINK_C2S(0), MTU, 3); + LCHECK(l.server != NULL, "server create failed"); + + /* The gateway: a client, plus one bind that should simply stay up. */ + l.client = vllp_client_create(LINK_C2S(0), LINK_S2C(0), MTU, 3); + LCHECK(l.client != NULL, "client create failed"); + if(l.client == NULL || l.server == NULL) + return fails + 1; + + l.app = app_create(); + l.bind = vllp_client_bind(l.client, "echo", app_open, l.app); + LCHECK(l.bind != NULL, "bind failed"); + + vcan_set_link(vcan, 1); + lb = vcan_loop_create(vcan, 0x13579bdf); + l.lb = lb; + + phase_connect(&l); + phase_echo(&l); + phase_open_refused(&l); + phase_open_after_refusal(&l); + phase_reconnect(&l, 1); + phase_reconnect(&l, 2); + phase_noise(&l); + phase_echo(&l); + phase_refusal_leak(&l); + phase_exhaust(&l); + + hosttest_log(" %u frames crossed the bus", lb->frames); + lb->stop = 1; + return fails; +} + +HOSTTEST_SUITE("vllp-client", test_vllp_client, 0); diff --git a/src/platform/host/testterm.c b/src/platform/host/testterm.c new file mode 100644 index 00000000..ceabae87 --- /dev/null +++ b/src/platform/host/testterm.c @@ -0,0 +1,203 @@ +#include +#include +#include +#include + +#include +#include + +#include "testterm.h" + +#define TT_IN_SIZE 512 +#define TT_OUT_SIZE 32768 + +struct testterm { + stream_t st; + + mutex_t mutex; + cond_t in_cond; /* keystrokes available for the console side to read */ + cond_t out_cond; /* the console wrote something the suite can inspect */ + + uint8_t in[TT_IN_SIZE]; + size_t in_used; + + uint8_t out[TT_OUT_SIZE]; + size_t out_used; + size_t out_dropped; +}; + + +int +testterm_contains(const char *hay, const char *needle) +{ + const size_t n = strlen(needle); + const size_t h = strlen(hay); + if(n > h) + return 0; + for(size_t i = 0; i + n <= h; i++) { + if(!memcmp(hay + i, needle, n)) + return 1; + } + return 0; +} + + +static ssize_t +tt_read(stream_t *s, void *buf, size_t size, size_t required) +{ + testterm_t *tt = (testterm_t *)s; + uint8_t *u8 = buf; + + mutex_lock(&tt->mutex); + size_t i = 0; + while(i < size) { + while(tt->in_used == 0) { + if(i >= required) { + mutex_unlock(&tt->mutex); + return i; + } + cond_wait(&tt->in_cond, &tt->mutex); + } + size_t n = MIN(size - i, tt->in_used); + memcpy(u8 + i, tt->in, n); + memmove(tt->in, tt->in + n, tt->in_used - n); + tt->in_used -= n; + i += n; + } + mutex_unlock(&tt->mutex); + return i; +} + + +static ssize_t +tt_write(stream_t *s, const void *buf, size_t size, int flags) +{ + testterm_t *tt = (testterm_t *)s; + + if(buf == NULL) + return 0; /* flush */ + + mutex_lock(&tt->mutex); + size_t n = MIN(size, TT_OUT_SIZE - tt->out_used); + memcpy(tt->out + tt->out_used, buf, n); + tt->out_used += n; + tt->out_dropped += size - n; + cond_broadcast(&tt->out_cond); + mutex_unlock(&tt->mutex); + return size; +} + + +static task_waitable_t * +tt_poll(stream_t *s, poll_type_t type) +{ + testterm_t *tt = (testterm_t *)s; + + if(type == POLL_STREAM_WRITE) + return NULL; /* never blocks */ + + if(tt->in_used) + return NULL; + return &tt->in_cond; +} + + +static const stream_vtable_t tt_vtable = { + .read = tt_read, + .write = tt_write, + .poll = tt_poll, +}; + + +testterm_t * +testterm_create(void) +{ + testterm_t *tt = calloc(1, sizeof(testterm_t)); + tt->st.vtable = &tt_vtable; + mutex_init(&tt->mutex, "ttmtx"); + cond_init(&tt->in_cond, "ttin"); + cond_init(&tt->out_cond, "ttout"); + return tt; +} + + +stream_t * +testterm_stream(testterm_t *tt) +{ + return &tt->st; +} + + +size_t +testterm_type(testterm_t *tt, const void *buf, size_t len) +{ + mutex_lock(&tt->mutex); + size_t n = MIN(len, TT_IN_SIZE - tt->in_used); + memcpy(tt->in + tt->in_used, buf, n); + tt->in_used += n; + if(n) + cond_broadcast(&tt->in_cond); + mutex_unlock(&tt->mutex); + return n; +} + + +size_t +testterm_types(testterm_t *tt, const char *str) +{ + return testterm_type(tt, str, strlen(str)); +} + + +void +testterm_out_clear(testterm_t *tt) +{ + mutex_lock(&tt->mutex); + tt->out_used = 0; + tt->out_dropped = 0; + mutex_unlock(&tt->mutex); +} + + +size_t +testterm_out_len(testterm_t *tt) +{ + mutex_lock(&tt->mutex); + size_t n = tt->out_used; + mutex_unlock(&tt->mutex); + return n; +} + + +size_t +testterm_out_get(testterm_t *tt, char *dst, size_t dstsize) +{ + mutex_lock(&tt->mutex); + size_t n = MIN(tt->out_used, dstsize - 1); + memcpy(dst, tt->out, n); + dst[n] = 0; + mutex_unlock(&tt->mutex); + return n; +} + + +int +testterm_out_has(testterm_t *tt, const char *needle) +{ + static char snap[TT_OUT_SIZE + 1]; /* suites are single-threaded here */ + testterm_out_get(tt, snap, sizeof(snap)); + return testterm_contains(snap, needle); +} + + +int +testterm_out_wait(testterm_t *tt, const char *needle, uint64_t timeout) +{ + const uint64_t deadline = clock_get() + timeout; + while(!testterm_out_has(tt, needle)) { + if(clock_get() >= deadline) + return 0; + usleep(10000); + } + return 1; +} diff --git a/src/platform/host/testterm.h b/src/platform/host/testterm.h new file mode 100644 index 00000000..4ce51e8b --- /dev/null +++ b/src/platform/host/testterm.h @@ -0,0 +1,39 @@ +#pragma once + +/* + * A stream_t standing in for the terminal a user attaches with. + * + * Unlike pipe() it buffers both directions and never blocks the writer, so + * a suite can inject keystrokes and inspect console output without running + * in lockstep with the thread under test. Supports poll() on the read side, + * which is what vcon_client_wait() and cmd_attach() need. + */ + +#include +#include + +struct stream; + +typedef struct testterm testterm_t; + +testterm_t *testterm_create(void); + +/* The stream to hand to vcon_attach(), cli_t.cl_stream, etc. */ +struct stream *testterm_stream(testterm_t *tt); + +/* Type at it. Returns bytes accepted (short if the input buffer is full). */ +size_t testterm_type(testterm_t *tt, const void *buf, size_t len); +size_t testterm_types(testterm_t *tt, const char *str); + +/* What the console has written to it. */ +size_t testterm_out_get(testterm_t *tt, char *dst, size_t dstsize); +int testterm_out_has(testterm_t *tt, const char *needle); +size_t testterm_out_len(testterm_t *tt); +void testterm_out_clear(testterm_t *tt); + +/* Wait until the console has written `needle`. Virtual time advances while + we sleep, so this costs no real time. Returns 1 if it appeared. */ +int testterm_out_wait(testterm_t *tt, const char *needle, uint64_t timeout); + +/* The MIOS libc has no strstr(). */ +int testterm_contains(const char *hay, const char *needle); diff --git a/src/platform/host/vcan.c b/src/platform/host/vcan.c index 56189f5f..e1f4a14e 100644 --- a/src/platform/host/vcan.c +++ b/src/platform/host/vcan.c @@ -76,8 +76,12 @@ static void vcan_print_info(struct device *dev, struct stream *st) { vcan_t *v = (vcan_t *)dev; - stprintf(st, "vcan mtu %d ring drops: tx %u rx %u\n", - v->v_cni.cni_ni.ni_mtu, v->v_tx.drops, v->v_rx.drops); + int q = 0; + pbuf_t *pb; + STAILQ_FOREACH(pb, &v->v_cni.cni_ni.ni_rx_queue, pb_link) + q++; + stprintf(st, "vcan mtu %d ring drops: tx %u rx %u rx_queue %d pbufs\n", + v->v_cni.cni_ni.ni_mtu, v->v_tx.drops, v->v_rx.drops, q); } static const device_class_t vcan_device_class = { diff --git a/src/platform/host/vcan_loop.c b/src/platform/host/vcan_loop.c new file mode 100644 index 00000000..12b06fcf --- /dev/null +++ b/src/platform/host/vcan_loop.c @@ -0,0 +1,93 @@ +#include +#include + +#include + +#include "sim.h" +#include "vcan.h" +#include "vcan_loop.h" + +#define VCAN_LOOP_MAX_FRAME 80 + + +static uint32_t +rng32(vcan_loop_t *lb) +{ + uint32_t x = lb->rng ? lb->rng : 0x2545f491; + x ^= x << 13; x ^= x >> 17; x ^= x << 5; + lb->rng = x; + return x; +} + + +static int +roll(vcan_loop_t *lb, int pct) +{ + return pct > 0 && (rng32(lb) % 100) < (uint32_t)pct; +} + + +static void +vcan_loop_fn(void *arg) +{ + vcan_loop_t *lb = arg; + uint8_t frame[VCAN_LOOP_MAX_FRAME]; + uint32_t id; + + while(!lb->stop) { + + if(lb->inject_len > 0) { + vcan_peer_send(lb->vcan, lb->inject_id, (const void *)lb->inject, + lb->inject_len); + lb->inject_len = 0; + } + + // A long deadline rather than SIM_NEVER: the loop has to come back + // round now and then to notice `stop` and pending injections. + long n = vcan_peer_recv(lb->vcan, &id, frame, sizeof(frame), + clock_get() + 10000000); + if(n < 0) + continue; + + lb->frames++; + + if(lb->blackhole || roll(lb, lb->drop_pct)) { + lb->dropped++; + continue; + } + + if(roll(lb, lb->corrupt_pct) && n > 0) { + frame[rng32(lb) % n] ^= 1 << (rng32(lb) & 7); + lb->corrupted++; + } + + vcan_peer_send(lb->vcan, id, frame, n); + + if(roll(lb, lb->dup_pct)) { + vcan_peer_send(lb->vcan, id, frame, n); + lb->duplicated++; + } + } +} + + +vcan_loop_t * +vcan_loop_create(vcan_t *vcan, uint32_t seed) +{ + vcan_loop_t *lb = calloc(1, sizeof(vcan_loop_t)); + lb->vcan = vcan; + lb->rng = seed ? seed : 0x2545f491; + sim_thread_create("vcan-loop", vcan_loop_fn, lb, 1 << 18); + return lb; +} + + +void +vcan_loop_outage(vcan_loop_t *lb, uint64_t us) +{ + lb->blackhole = 1; + const uint64_t deadline = clock_get() + us; + while(clock_get() < deadline) + usleep(100000); + lb->blackhole = 0; +} diff --git a/src/platform/host/vcan_loop.h b/src/platform/host/vcan_loop.h new file mode 100644 index 00000000..679e95cc --- /dev/null +++ b/src/platform/host/vcan_loop.h @@ -0,0 +1,59 @@ +#pragma once + +/* + * Loop a vcan back on itself, with optional faults. + * + * Lets several VLLP (or other dsig) endpoints in one host-mios binary + * talk to each other over a virtual CAN bus: every frame mios transmits + * is put back into mios's own receive ring unchanged, so a frame sent on + * signal id N is delivered to whichever endpoint has rxid N. mios never + * delivers locally emitted dsig to itself, so nothing short-circuits -- + * the frame really does go out and come back. + * + * Pair the two ends of a link on opposite ids and each frame lands at + * exactly one endpoint. + * + * Runs as a simulation thread (see cpu/host/sim.h), which is also why the + * fault injection lives here: it is the one place every frame passes + * through. + */ + +#include + +typedef struct vcan vcan_t; + +typedef struct vcan_loop { + vcan_t *vcan; + + volatile int stop; + volatile int drop_pct; /* % of frames discarded */ + volatile int dup_pct; /* % of frames delivered twice */ + volatile int corrupt_pct; /* % of frames with one bit flipped */ + volatile int blackhole; /* discard everything: a link outage */ + + /* Counters, so a suite can prove the faults it configured actually + happened. A fault phase that injected nothing looks exactly like one + that proved resilience. */ + volatile uint32_t frames; /* frames seen */ + volatile uint32_t dropped; + volatile uint32_t duplicated; + volatile uint32_t corrupted; + + /* One-shot: inject this frame into mios's receive ring next time round, + i.e. play traffic mios never sent. The only way to reach the code + that has to reject it. */ + volatile int inject_len; + uint32_t inject_id; + uint8_t inject[16]; + + uint32_t rng; +} vcan_loop_t; + +/* Create the loopback and start its simulation thread. `seed` seeds the + fault RNG; runs are deterministic for a given seed. */ +vcan_loop_t *vcan_loop_create(vcan_t *vcan, uint32_t seed); + +/* Discard everything for `us` of virtual time, then let traffic back. A + link outage long enough to trip the peers' timeouts. Must be called + from the suite (mios) thread, not the loopback. */ +void vcan_loop_outage(vcan_loop_t *lb, uint64_t us); diff --git a/src/util/vcon.c b/src/util/vcon.c index 28d8c2b4..2b1c3e5d 100644 --- a/src/util/vcon.c +++ b/src/util/vcon.c @@ -41,6 +41,12 @@ struct vcon { cond_t vc_out_cond; // Signalled on new output or client stop struct vcon_client_list vc_clients; + // Optional notification for a backend that is not a thread blocked in + // read(). Guarded by vc_mutex, and invoked with it held -- see + // vcon_set_backend_notify(). + void (*vc_notify)(void *opaque); + void *vc_notify_opaque; + // Input fifo: keystrokes from clients toward the backend uint8_t *vc_in; size_t vc_in_size; @@ -331,11 +337,67 @@ vcon_input(vcon_t *vc, const void *buf, size_t len) if(n) cond_signal(&vc->vc_in_cond); + // Under the lock, deliberately. A backend clears its callback and then + // frees itself; firing outside the lock means we can load the pointer, + // lose the race, and call into freed memory. The contract is therefore + // "must not block, must not re-enter vcon" rather than "may take its + // own locks" -- raising an event is all this is for. + if(n && vc->vc_notify != NULL) + vc->vc_notify(vc->vc_notify_opaque); + + mutex_unlock(&vc->vc_mutex); + return n; +} + + +void +vcon_set_backend_notify(vcon_t *vc, void (*cb)(void *opaque), void *opaque) +{ + mutex_lock(&vc->vc_mutex); + vc->vc_notify_opaque = opaque; + vc->vc_notify = cb; + mutex_unlock(&vc->vc_mutex); +} + + +size_t +vcon_input_peek(vcon_t *vc, void *buf, size_t size) +{ + uint8_t *b = buf; + + mutex_lock(&vc->vc_mutex); + size_t n = MIN(size, vc->vc_in_used); + size_t tail = vc->vc_in_tail; + for(size_t i = 0; i < n; i++) { + b[i] = vc->vc_in[tail]; + tail = (tail + 1) % vc->vc_in_size; + } mutex_unlock(&vc->vc_mutex); return n; } +void +vcon_input_consume(vcon_t *vc, size_t len) +{ + mutex_lock(&vc->vc_mutex); + size_t n = MIN(len, vc->vc_in_used); + vc->vc_in_tail = (vc->vc_in_tail + n) % vc->vc_in_size; + vc->vc_in_used -= n; + mutex_unlock(&vc->vc_mutex); +} + + +void +vcon_input_flush(vcon_t *vc) +{ + mutex_lock(&vc->vc_mutex); + vc->vc_in_tail = vc->vc_in_head; + vc->vc_in_used = 0; + mutex_unlock(&vc->vc_mutex); +} + + vcon_t * vcon_find(const char *name) { From 36ac4745ee458d8d1fb44b7b82258523463a5446 Mon Sep 17 00:00:00 2001 From: Andreas Smas Date: Tue, 8 Sep 2026 05:43:44 +0000 Subject: [PATCH 03/10] vcon: make attach work from a shell reached over a pushpull transport Running "attach" on a shell reached over VLLP brought the device down, and once it stopped doing that, showed nothing. The crash: cmd_attach() polls its terminal through vcon_client_wait(), that terminal is the pushpull-to-stream bridge, and pushpull streams had no poll method -- so poll() hit its assert. Nothing had ever polled one before; the bridge is normally read and written straight from a shell thread. The same applies to any pushpull transport, so BLE L2CAP and MBUS shells were equally affected, and splice_bidir() on such a stream would have gone the same way. pushpull_stream_poll() reports readable when a message is queued and also when the stream has shut down, so a poller wakes up to see the shutdown rather than sleeping through it, and it gives the network side the same "I have room" nudge that read() does before sleeping -- without which a peer that was told to stop sending is never told it may resume. The silence: pushpull_stream_write() only hands data to the network once a fragment fills or someone flushes, and neither cmd_attach() nor vcon_bind()'s pump ever flushed their terminal. Against a plain byte stream that goes unnoticed; against a pushpull stream a nested session's console output sat in the buffer indefinitely. Keystrokes arrived and the far shell answered -- its scrollback grew -- but nothing came back to the operator, so the console looked dead. Both pumps now flush when they run out of things to do, which still lets a burst coalesce into full fragments. cmd_attach() also ignored a negative read from its terminal. With the poll method in place a shut-down terminal is always ready, so the loop would have made no progress, slept on a stream that never blocks, and spun at priority 2 forever. It now leaves the loop, which is also what should happen when the link carrying an attached session drops. Found by asking what happens with nested sessions -- attach to a unit, then attach to something else from that unit's shell. That is a reasonable thing to do on a bus of units and none of it worked. The escape handling itself needed no change and behaves the way screen does: the outermost cmd_attach() is the first filter a byte meets, so ^A d always detaches the outermost session from any depth, and reaching an inner level means doubling the prefix for every level above it. Left that way deliberately -- preferring the innermost cannot be done without the outer loop knowing whether an inner one exists, and the only channel it could learn that on is the console output itself. Written down in cmd_vcon.c, including that the count is a power of two and that detaching an outer session leaves the inner ones attached. The vcon-vllp suite grows a nested-attach phase that attaches to a unit, attaches again from that unit's shell, checks that traffic actually flows through both levels, and checks that each detach sequence unwinds exactly one level. Co-Authored-By: Claude --- src/net/pushpull.c | 36 ++++++++++++++ src/platform/host/suite_vcon_vllp.c | 76 +++++++++++++++++++++++++++++ src/shell/cmd_vcon.c | 38 ++++++++++++++- src/util/vcon.c | 7 ++- 4 files changed, 154 insertions(+), 3 deletions(-) diff --git a/src/net/pushpull.c b/src/net/pushpull.c index 13f44c36..236ed6c8 100644 --- a/src/net/pushpull.c +++ b/src/net/pushpull.c @@ -13,6 +13,8 @@ #include "net/pbuf.h" +#include "irq.h" + typedef struct pushpull_stream { stream_t pps_stream; @@ -234,10 +236,44 @@ static const pushpull_app_fn_t pps_pushpull_vtable = { .close = pushpull_stream_close_pp, }; +// Without this a pushpull stream cannot be handed to poll() at all -- +// poll() asserts on a stream with no poll method. That is reachable from +// the shell: `attach` on a console whose terminal is a pushpull stream +// (any shell reached over VLLP, BLE L2CAP or MBUS) polls its terminal +// through vcon_client_wait(), and used to bring the device down on the +// spot. +// +// Called from poll() with the scheduler already held off, so the check +// and the caller's enlisting on the returned waitable cannot be split by +// the net thread setting pps_rxbuf in between. +static task_waitable_t * +pushpull_stream_poll(struct stream *s, poll_type_t type) +{ + pushpull_stream_t *pps = (pushpull_stream_t *)s; + + if(type != POLL_STREAM_READ) + return NULL; // The write side blocks internally, not through poll + + irq_forbid(IRQ_LEVEL_SWITCH); + + // A shut-down stream is "readable": read() returns an error straight + // away, so a poller must wake and see that rather than sleep forever. + if(pps->pps_rxbuf != NULL || pps->pps_shutdown) + return NULL; + + // The same nudge read() gives the network side before it sleeps. A peer + // that was told to stop sending is only told it may resume when we ask, + // so without this we could sleep waiting for data nobody will send. + pps->pps_pp->net->event(pps->pps_pp->net_opaque, PUSHPULL_EVENT_PUSH); + return &pps->pps_cond; +} + + static const stream_vtable_t pps_stream_vtable = { .read = pushpull_stream_read, .write = pushpull_stream_write, .close = pushpull_stream_destroy, + .poll = pushpull_stream_poll, }; diff --git a/src/platform/host/suite_vcon_vllp.c b/src/platform/host/suite_vcon_vllp.c index 1bb5d42c..e4b83f9d 100644 --- a/src/platform/host/suite_vcon_vllp.c +++ b/src/platform/host/suite_vcon_vllp.c @@ -772,6 +772,81 @@ phase_faults(const char *name, int drop, int dup, int corrupt) } +/* Nested attach: attach to a unit, and from that unit's shell attach to + another. Two cmd_attach() loops are then in series on the same byte + stream, and both filter the same escape prefix. */ +static void +phase_nested(void) +{ + if(g_starved) { + // What this phase checks is where an escape byte gets consumed, which + // has nothing to do with buffer pressure. Establishing the nesting + // does though: it takes a typed command through two hops, and with + // allocations being forced to fail that command can simply be + // dropped. The other two registrations cover the routing. + hosttest_log("-- nested attach (skipped: allocations are being forced " + "to fail)"); + return; + } + + hosttest_log("-- nested attach"); + + attach_ctx_t ac = { .tt = testterm_create(), .console = units[7].name }; + thread_create(attach_thread, &ac, 4096, "attach", TASK_DETACHED, 4); + + GCHECK(testterm_out_wait(ac.tt, "[attached to unit8", 10 * SEC), + "nested: outer attach did not start"); + GCHECK(testterm_out_wait(ac.tt, ">", 25 * SEC), "nested: no outer prompt"); + + /* From the remote shell, attach to a second console. Everything we type + now passes through the outer cmd_attach() on its way there. */ + testterm_out_clear(ac.tt); + testterm_types(ac.tt, "attach unit9\n"); + GCHECK(testterm_out_wait(ac.tt, "[attached to unit9", 25 * SEC), + "nested: inner attach did not start"); + + /* The nesting has to actually carry traffic, in both directions and + through both levels. Checking only that the inner attach *started* + misses the case that mattered: the inner cmd_attach's terminal is a + pushpull stream, which holds written data until a fragment fills or + someone flushes, so console output used to sit in that buffer + indefinitely -- keystrokes reached the inner console and its shell + answered, but nothing ever came back up to the operator. */ + testterm_out_clear(ac.tt); + testterm_types(ac.tt, "zz-through-both-levels\n"); + GCHECK(testterm_out_wait(ac.tt, "zz-through-both-levels", 30 * SEC), + "nested: nothing came back through the nested session -- the " + "inner terminal is buffering output that never gets flushed"); + + /* A bare ^A is eaten by the OUTER loop -- it is the first filter the + byte meets -- so the inner one never sees it. Doubling it makes the + outer loop pass one through, which the inner loop then takes as its + own prefix. So the inner detach is ^A ^A d, and the outer is ^A d. */ + testterm_out_clear(ac.tt); + testterm_types(ac.tt, "\x01\x01" "d"); + GCHECK(testterm_out_wait(ac.tt, "[detached from unit9]", 15 * SEC), + "nested: ^A^Ad did not detach the inner session"); + GCHECK(!ac.done, "nested: ^A^Ad detached the outer session too"); + GCHECK(vcon_client_count(units[8].vcon) == 0, + "nested: %d clients left on unit9", + vcon_client_count(units[8].vcon)); + + /* Still attached to the outer one, and it still works. */ + viewer_t probe = { .tt = ac.tt }; + (void)probe; + GCHECK(vcon_client_count(units[7].vcon) == 1, + "nested: outer session lost, %d clients on unit8", + vcon_client_count(units[7].vcon)); + + /* And a single ^A d now detaches the outer one. */ + testterm_types(ac.tt, "\x01" "d"); + GCHECK(hosttest_wait(pred_attach_done, &ac, 10 * SEC), + "nested: ^Ad did not detach the outer session"); + GCHECK(testterm_out_has(ac.tt, "[detached from unit8]"), + "nested: no outer detach message"); +} + + /* ---------------- driver ---------------- */ static int @@ -809,6 +884,7 @@ test_vcon_vllp(void) phase_scrollback(); phase_multi_attach(); phase_attach_command(); + phase_nested(); phase_reconnect(1); phase_reconnect(2); phase_faults("loss-5", 5, 0, 0); diff --git a/src/shell/cmd_vcon.c b/src/shell/cmd_vcon.c index 44cdf75d..7855ac59 100644 --- a/src/shell/cmd_vcon.c +++ b/src/shell/cmd_vcon.c @@ -12,6 +12,28 @@ // ^A d detach and return to the shell // ^A ^A send a literal ^A to the console // (^A rather than tmux's ^B, which many host terminal emulators bind to detach.) +// +// Nesting: attach to a unit, then attach to something else from that +// unit's shell, and two of these loops are in series on one byte stream. +// The outer one is the first filter a byte meets, so it always wins -- +// ^A d detaches the outermost session from any depth, which is the +// property you want from the key you reach for when something is wrong. +// Addressing an inner level means doubling the prefix for every level +// above it, so it takes 2^(N-1) of them to reach level N: +// +// ^A d detach the outermost +// ^A ^A d detach one level in +// ^A ^A ^A ^A d two levels in +// +// Note that the count is a power of two, not a running total: an odd +// number above one just arms a prefix on some inner level and then +// detaches an outer one. Deliberately left as-is rather than made to +// prefer the innermost, which cannot be done without the outer loop +// knowing whether an inner one exists -- and the only channel it could +// learn that on is the console output itself. +// +// Detaching an outer session does not tear down the inner ones; they stay +// attached, and re-attaching puts you back inside them. #define VCON_PREFIX 0x01 // Ctrl-A @@ -74,6 +96,10 @@ cmd_attach(cli_t *cli, int argc, char **argv) // console is flooding output. Non-blocking; buf is reused for output below // only after these bytes are consumed. ssize_t r = stream_read(term, buf, sizeof(buf), 0); + if(r < 0) + break; // The terminal went away (a dropped link, say). Without + // this the loop makes no progress, sleeps on a stream that + // is permanently ready, and spins forever. for(ssize_t i = 0; i < r && !detach; i++) { uint8_t c = buf[i]; @@ -108,9 +134,17 @@ cmd_attach(cli_t *cli, int argc, char **argv) progress = 1; } - // Idle: sleep until there is output or the user types something. - if(!progress) + // Idle: flush, then sleep until there is output or the user types + // something. The flush matters when the terminal buffers -- a pushpull + // stream (any shell reached over VLLP, BLE or MBUS) only hands data to + // the network once a fragment fills or someone flushes, so without + // this a nested session's output sits in the buffer indefinitely and + // the console looks dead. Doing it here rather than after every write + // means a burst still coalesces into full fragments. + if(!progress) { + stream_flush(term); vcon_client_wait(vcc); + } } vcon_detach(vcc); diff --git a/src/util/vcon.c b/src/util/vcon.c index 2b1c3e5d..605333ee 100644 --- a/src/util/vcon.c +++ b/src/util/vcon.c @@ -283,8 +283,13 @@ vcon_bind_thread(void *arg) progress = 1; } - if(!progress) + if(!progress) { + // Nothing more to pump; make sure what we wrote is actually on its + // way before sleeping. A terminal that buffers until a fragment + // fills (a pushpull stream) would otherwise hold it indefinitely. + stream_flush(term); vcon_client_wait(vcc); + } } } From 69d89bbf0e8f874366b4c68a46f56fdba2e267de Mon Sep 17 00:00:00 2001 From: Andreas Smas Date: Tue, 8 Sep 2026 06:31:31 +0000 Subject: [PATCH 04/10] vllp: cross-check the client against the host reference implementation The other client suite runs mios against mios. That catches a great deal, because only one of the two roles is new and the server is a reference that was not touched -- but it cannot catch a convention both ends read the same wrong way out of the same source file, and the VLLP wire has a fair number of those: what the MTU byte in the SYN holds, the cookie becoming the link CRC IV, the per-channel IV derivation and which end inverts it, the counter deriving those IVs stepping in lockstep, the OPEN/OPEN_RESPONSE encoding. This suite drives the mios client against the host reference stack (host/dsig/vllp.c, what the `dsig` tool ships) playing the server, in sim mode, over the virtual CAN bus. The roles are the mirror of suite_vllp.c, where the host client drives the mios server. Two symmetric mutations show what it buys, both self-consistent so mios-vs-mios stays green: - putting the unadapted MTU in the SYN and expecting it there: vllp-client PASS, vllp-xcheck FAIL - seeding the per-channel IV counter at a different value: vllp-client PASS, vllp-xcheck FAIL hvllp_create_server() was already renamed by vllp_sim.h but never declared, so this adds it to vllp_sim_api.h along with the result struct the open-channel callback returns. Two notes there worth keeping: sim mode has no rx-dispatch thread, so an accepted channel must return NULL callbacks and be drained with hvllp_channel_read(); and a read that times out marks the channel closed for good, so the peer blocks with a generous deadline rather than polling on short ones. The production pthread build of host/dsig is unaffected and still builds. Co-Authored-By: Claude --- host/dsig/vllp_sim_api.h | 26 ++ src/platform/host/host.mk | 1 + src/platform/host/suite_vllp_xcheck.c | 429 ++++++++++++++++++++++++++ 3 files changed, 456 insertions(+) create mode 100644 src/platform/host/suite_vllp_xcheck.c diff --git a/host/dsig/vllp_sim_api.h b/host/dsig/vllp_sim_api.h index ddc5d1a4..0a10e17e 100644 --- a/host/dsig/vllp_sim_api.h +++ b/host/dsig/vllp_sim_api.h @@ -21,6 +21,32 @@ hvllp_t *hvllp_create_client(int mtu, int timeout, uint32_t flags, void *opaque, size_t len), void (*log)(void *opaque, int level, const char *msg)); + +// The server role, for driving the *guest* client against an independent +// implementation. Layout-identical to vllp.h's open_channel_result_t; the +// two headers deliberately describe the same structs under different +// names so a suite can include both (see the note on hvllp_t above). +// +// In sim mode there is no rx-dispatch thread, so an accepted channel must +// return rx = eof = NULL and be drained with hvllp_channel_read(). Note +// that a read which times out marks the channel closed for good, so a +// peer polls by blocking with a generous deadline rather than spinning on +// short ones. +typedef struct { + int error; + void (*rx)(void *opaque, const void *data, size_t length); + void (*eof)(void *opaque, int error_code); + void *opaque; +} hvllp_open_channel_result_t; + +hvllp_t *hvllp_create_server(int mtu, int timeout, uint32_t flags, void *opaque, + void (*tx)(void *opaque, const void *data, + size_t len), + void (*log)(void *opaque, int level, + const char *msg), + hvllp_open_channel_result_t (*open_channel)( + void *opaque, const char *name, + hvllp_channel_t *vc)); void hvllp_start(hvllp_t *v); void hvllp_destroy(hvllp_t *v); int hvllp_is_connected(hvllp_t *v); diff --git a/src/platform/host/host.mk b/src/platform/host/host.mk index 39a69c9a..6aa0aaaa 100644 --- a/src/platform/host/host.mk +++ b/src/platform/host/host.mk @@ -58,6 +58,7 @@ SRCS += ${P}/host.c \ ${P}/suite_vllp.c \ ${P}/suite_vllp_frames.c \ ${P}/suite_vllp_client.c \ + ${P}/suite_vllp_xcheck.c \ ${P}/suite_ota.c \ ${P}/vspiflash.c \ ${P}/host_ota.c \ diff --git a/src/platform/host/suite_vllp_xcheck.c b/src/platform/host/suite_vllp_xcheck.c new file mode 100644 index 00000000..16de5b64 --- /dev/null +++ b/src/platform/host/suite_vllp_xcheck.c @@ -0,0 +1,429 @@ +/* + * vllp-xcheck: the mios VLLP client against an independent implementation + * of the server -- the host reference stack (host/dsig/vllp.c, the same + * code the `dsig` tool ships) compiled into this binary in sim mode. + * + * The other client suite runs mios against mios. That catches a great + * deal, because only one of the two roles is new, but it cannot catch a + * convention both ends read the same wrong way out of the same source + * file. The wire has a fair number of those: + * + * - what the MTU byte in the SYN actually holds (the adapted value, not + * what the caller passed in) + * - the cookie becoming the link CRC IV + * - the per-channel IV derivation, and which end inverts it + * - the counter that derives those IVs stepping in lockstep + * - OPEN / OPEN_RESPONSE encoding on the management channel + * - S/E sequencing, the flow bit, and the FDCAN length adaptation + * + * Get any of them wrong in a way that is symmetric and mios-vs-mios still + * passes. This suite is the one that would not. + * + * The host stack plays the server, so the roles are the mirror image of + * suite_vllp.c: there the host client drove the guest server. No loopback + * here -- the peer is a real second implementation on the other side of + * the virtual CAN bus. + */ + +#include +#include +#include +#include +#include + +#include +#include +#include + +#include "net/pbuf.h" + +#include "hosttest.h" +#include "sim.h" +#include "vcan.h" +#include "../../../host/dsig/vllp_sim_api.h" + +#define SEC 1000000ull + +#define MTU 64 + +/* mios is the client: it transmits on GUEST_TX and listens on GUEST_RX. */ +#define GUEST_TX 0x400 +#define GUEST_RX 0x401 + +/* The service the host server offers, and how many round trips to make. */ +#define XSERVICE "xecho" +#define ROUNDS 24 + +static int fails; + +#define XCHECK(cond, ...) \ + do { if(!(cond)) { fails++; \ + hosttest_check(0, __FILE__, __LINE__, __VA_ARGS__); } } while(0) + + +/* ---------------- the mios side: a small pushpull app ---------------- */ + +#define APP_BUF 4096 + +typedef struct app { + pushpull_t *pp; + mutex_t mutex; + + uint8_t rx[APP_BUF]; + size_t rx_used; + + uint8_t tx[APP_BUF]; + size_t tx_used; + + volatile int opens; + volatile int live; +} app_t; + +static app_t g_app; + + +static uint32_t +app_push(void *opaque, pbuf_t *pb) +{ + app_t *a = opaque; + + mutex_lock(&a->mutex); + for(pbuf_t *p = pb; p != NULL; p = p->pb_next) { + const size_t n = MIN(p->pb_buflen, APP_BUF - a->rx_used); + memcpy(a->rx + a->rx_used, pbuf_cdata(p, 0), n); + a->rx_used += n; + } + mutex_unlock(&a->mutex); + + pbuf_free(pb); + return 0; +} + +static int +app_may_push(void *opaque) +{ + app_t *a = opaque; + return a->rx_used < APP_BUF; +} + +static pbuf_t * +app_pull(void *opaque) +{ + app_t *a = opaque; + pbuf_t *pb = NULL; + + mutex_lock(&a->mutex); + if(a->tx_used) { + pb = pbuf_make(a->pp->preferred_offset, 0); + if(pb != NULL) { + const size_t n = MIN(a->tx_used, + MIN(a->pp->max_fragment_size, + PBUF_DATA_SIZE - a->pp->preferred_offset)); + memcpy(pbuf_append(pb, n), a->tx, n); + memmove(a->tx, a->tx + n, a->tx_used - n); + a->tx_used -= n; + } + } + mutex_unlock(&a->mutex); + return pb; +} + +static void +app_close(void *opaque, const char *reason) +{ + app_t *a = opaque; + mutex_lock(&a->mutex); + a->live = 0; + a->tx_used = 0; + mutex_unlock(&a->mutex); +} + +static const pushpull_app_fn_t app_fn = { + .push = app_push, + .may_push = app_may_push, + .pull = app_pull, + .close = app_close, +}; + +static error_t +app_open(void *opaque, pushpull_t *pp) +{ + app_t *a = opaque; + a->pp = pp; + a->opens++; + a->live = 1; + pp->app = &app_fn; + pp->app_opaque = a; + return 0; +} + +static void +app_send(app_t *a, const void *data, size_t len) +{ + mutex_lock(&a->mutex); + const size_t n = MIN(len, APP_BUF - a->tx_used); + memcpy(a->tx + a->tx_used, data, n); + a->tx_used += n; + mutex_unlock(&a->mutex); + if(a->pp != NULL) + pushpull_wakeup(a->pp, PUSHPULL_EVENT_PULL); +} + +static size_t +app_rx_used(app_t *a) +{ + mutex_lock(&a->mutex); + size_t n = a->rx_used; + mutex_unlock(&a->mutex); + return n; +} + +static int +app_wait_rx(app_t *a, size_t want, uint64_t timeout) +{ + const uint64_t deadline = clock_get() + timeout; + while(app_rx_used(a) < want) { + if(clock_get() >= deadline) + return 0; + usleep(10000); + } + return 1; +} + + +/* ---------------- the peer: the host reference stack, as server -------- */ + +typedef struct peer { + vcan_t *vcan; + hvllp_t *v; + hvllp_channel_t *ch; /* the channel the guest opened */ + int opens; + int bad_name; + int echoed; + int warnings; + volatile int ready; + volatile int done; +} peer_t; + +static peer_t g_peer; + + +static void +peer_tx(void *opaque, const void *data, size_t len) +{ + peer_t *p = opaque; + vcan_peer_send(p->vcan, GUEST_RX, data, len); +} + +static long +peer_recv(void *tr, uint32_t *id, void *buf, size_t buflen, int64_t deadline) +{ + peer_t *p = tr; + while(1) { + long n = vcan_peer_recv(p->vcan, id, buf, buflen, (uint64_t)deadline); + if(n < 0) + return -1; + if(*id == GUEST_TX) + return n; + } +} + +static void +peer_log(void *opaque, int level, const char *msg) +{ + peer_t *p = opaque; + if(level <= 4 /* LOG_WARNING */) { + p->warnings++; + hosttest_log(" peer WARN: %s", msg); + } +} + + +/* The guest is asking for a service. Accept exactly one name, so a + mangled name shows up as a refusal rather than silently working. */ +static hvllp_open_channel_result_t +peer_open_channel(void *opaque, const char *name, hvllp_channel_t *vc) +{ + peer_t *p = opaque; + hvllp_open_channel_result_t r = {}; + + p->opens++; + + if(strcmp(name, XSERVICE)) { + hosttest_log(" peer: refusing unknown service '%s'", name); + p->bad_name++; + r.error = -17; /* VLLP_ERR_NOT_FOUND */ + return r; + } + + /* Sim mode has no rx-dispatch thread, so take the handle and drain it + from the peer loop instead of asking for callbacks. */ + p->ch = vc; + return r; +} + + +static void +peer_fn(void *arg) +{ + peer_t *p = arg; + + p->v = hvllp_create_server(MTU, 3, HVLLP_FDCAN_ADAPTATION, p, + peer_tx, peer_log, peer_open_channel); + hvllp_sim_setup(p->v, 0x5eed1234, p, peer_recv, GUEST_TX); + hvllp_start(p->v); + p->ready = 1; + + /* Wait for the guest to establish a link and open its channel. */ + const int64_t dl = clock_get() + 30 * SEC; + while(p->ch == NULL && clock_get() < dl) + hvllp_sim_poll(p->v, dl); + + if(p->ch == NULL) { + hosttest_log(" peer: guest never opened a channel"); + p->done = 1; + return; + } + + /* Echo whatever arrives. A blocking read is what pumps the protocol in + sim mode; a read that times out would mark the channel dead, so give + it a deadline far longer than the test needs. */ + for(int i = 0; i < ROUNDS; i++) { + void *data = NULL; + size_t len = 0; + if(hvllp_channel_read(p->ch, &data, &len, 60 * SEC) || data == NULL) { + hosttest_log(" peer: read %d failed", i); + break; + } + hvllp_channel_send(p->ch, data, len); + hvllp_sim_free(data); + p->echoed++; + } + + /* Keep the link alive while the guest checks up on it. */ + hvllp_sim_run(p->v, clock_get() + 10 * SEC); + p->done = 1; +} + + +static int +pred_peer_done(void *arg) +{ + peer_t *p = arg; + return p->done; +} + +static int +pred_peer_ready(void *arg) +{ + peer_t *p = arg; + return p->ready; +} + + +/* ---------------- the test ---------------- */ + +static void +fill_pattern(uint8_t *buf, size_t len, uint32_t seq) +{ + for(size_t i = 0; i < len; i++) + buf[i] = (uint8_t)(seq * 31 + i * 7); +} + + +static int +test_vllp_xcheck(void) +{ + hosttest_log("---- mios VLLP client vs the host reference server ----"); + + vcan_t *vcan = vcan_create("vcan0", MTU); + g_peer.vcan = vcan; + mutex_init(&g_app.mutex, "xappmtx"); + + sim_thread_create("host-server", peer_fn, &g_peer, 1 << 20); + CHECK(hosttest_wait(pred_peer_ready, &g_peer, 10 * SEC), + "peer did not start"); + + vllp_t *client = vllp_client_create(GUEST_TX, GUEST_RX, MTU, 3); + XCHECK(client != NULL, "client create failed"); + if(client == NULL) + return fails + 1; + + XCHECK(vllp_client_bind(client, XSERVICE, app_open, &g_app) != NULL, + "bind failed"); + vcan_set_link(vcan, 1); + + /* The handshake and the channel open, against a stack that was written + from the spec rather than from src/net/vllp.c. */ + const uint64_t dl = clock_get() + 30 * SEC; + while(!g_app.live && clock_get() < dl) + usleep(10000); + XCHECK(g_app.live, "no channel to the reference server (opens=%d, peer " + "saw %d opens, %d with a name it did not recognise)", + g_app.opens, g_peer.opens, g_peer.bad_name); + XCHECK(g_peer.bad_name == 0, + "the reference server did not recognise the service name -- the " + "OPEN request is not encoded the way it expects"); + + if(!g_app.live) { + g_peer.done = 1; + return fails; + } + + /* Round trips across the interesting size boundaries: under a fragment, + exactly a fragment, and spanning several. Every one of these exercises + the message CRC and steps the per-message IV on both ends, so a + divergence shows up as a mismatch rather than as a hang. */ + static const size_t sizes[] = { + 1, 2, 7, 8, 9, 61, 62, 63, 64, 65, 100, 127, 128, 200, 255, 256, + 300, 400, 500, 501, 502, 503, 504, 505, + }; + const size_t nsizes = sizeof(sizes) / sizeof(sizes[0]); + _Static_assert(sizeof(sizes) / sizeof(sizes[0]) == ROUNDS, + "the peer echoes exactly ROUNDS messages"); + + static uint8_t tx[1024]; + int ok = 0; + for(size_t i = 0; i < nsizes; i++) { + const size_t len = sizes[i]; + + mutex_lock(&g_app.mutex); + g_app.rx_used = 0; + mutex_unlock(&g_app.mutex); + + fill_pattern(tx, len, i + 1); + app_send(&g_app, tx, len); + + if(!app_wait_rx(&g_app, len, 20 * SEC)) { + XCHECK(0, "no echo for %zu bytes (got %zu)", len, + app_rx_used(&g_app)); + break; + } + XCHECK(app_rx_used(&g_app) == len, + "%zu bytes out, %zu back", len, app_rx_used(&g_app)); + XCHECK(!memcmp(g_app.rx, tx, len), + "%zu byte payload differs -- the two stacks disagree about the " + "message framing or its CRC", len); + ok++; + } + + hosttest_log(" %d/%zu round trips against the reference stack", ok, + nsizes); + XCHECK(ok == (int)nsizes, "only %d of %zu round trips completed", ok, + nsizes); + + CHECK(hosttest_wait(pred_peer_done, &g_peer, 60 * SEC), + "peer did not finish"); + XCHECK(g_peer.echoed == ROUNDS, "the peer echoed %d of %d messages", + g_peer.echoed, ROUNDS); + XCHECK(g_peer.warnings == 0, + "the reference stack logged %d warnings about our traffic", + g_peer.warnings); + XCHECK(g_app.opens == 1, + "the channel was re-opened %d times, so the link was resetting", + g_app.opens); + + return fails; +} + +HOSTTEST_SUITE("vllp-xcheck", test_vllp_xcheck, 0); From a38558623bf0458a3b827d9c7223d980f6a3b8ed Mon Sep 17 00:00:00 2001 From: Andreas Smas Date: Tue, 8 Sep 2026 14:23:56 +0200 Subject: [PATCH 05/10] stm32 bxcan: free the pbuf after loading a queued frame into the mailbox stm32_bxcan_send_pb() copies into the transmit mailbox and leaves the pbuf alone. The direct path in stm32_bxcan_output() gets away with that because it returns the pbuf and the stack frees it, but the queued path in the transmit interrupt spliced a pbuf off our own queue and then dropped the pointer. Every frame that went through the software queue leaked one pbuf, permanently. Frames only queue when the mailbox is still busy, so this needed a burst to show up, and dsig_send_cb() drains its whole queue back to back. On a board with ten VLLP links the pool -- 32 buffers -- was gone in seconds, which starves the whole stack, not just CAN. It went unnoticed this long because a board has to emit dsig to reach the queued path at all: the low-latency path drops rather than queues, and until now the boards using bxcan only ever received. Also guard the pbuf_drop() result, which was passed to send_pb() unchecked, and release the mailbox if it does come back NULL so a failure there cannot wedge transmission. Co-Authored-By: Claude --- src/platform/stm32/stm32_bxcan.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/platform/stm32/stm32_bxcan.c b/src/platform/stm32/stm32_bxcan.c index cce214f4..ecd4cb3a 100644 --- a/src/platform/stm32/stm32_bxcan.c +++ b/src/platform/stm32/stm32_bxcan.c @@ -84,6 +84,8 @@ stm32_bxcan_send(bxcan_t *bx, const void *data, size_t len, } +// Copies out of the pbuf; ownership stays with the caller, which either +// frees it or hands it back for the network stack to free. static void stm32_bxcan_send_pb(bxcan_t *bx, pbuf_t *pb, uint32_t id, int mailbox) { @@ -165,7 +167,15 @@ stm32_bxcan_tx_irq(void *arg) bx->qlen--; uint32_t group = rd32_le(pbuf_data(pb, 0)); pb = pbuf_drop(pb, 4, 0); - stm32_bxcan_send_pb(bx, pb, group, i); + if(pb != NULL) { + stm32_bxcan_send_pb(bx, pb, group, i); + // Nothing downstream owns a queued pbuf: unlike the direct path in + // stm32_bxcan_output(), which hands it back to the stack, this one + // took it off our own queue and must release it. + pbuf_free_irq_blocked(pb); + } else { + bx->tx_status[i] = 0; + } } } } From b3905d4a17114459122e5b29733c697f5521c1d3 Mon Sep 17 00:00:00 2001 From: Andreas Smas Date: Tue, 8 Sep 2026 14:25:20 +0200 Subject: [PATCH 06/10] stm32 bxcan: bound the software transmit queue The queue behind the transmit mailbox had no depth limit. A bus with no other node on it never acknowledges a frame, so the mailbox stays busy indefinitely and the queue grows until the pbuf pool is empty -- which takes down every other protocol on the board, not just CAN. Cap it and count the drops. FDCAN already behaves this way when its hardware FIFO fills. Dropping is the right answer here: protocols that care about a frame retransmit, and the ones that do not are periodic, where the next sample supersedes whatever was dropped. Co-Authored-By: Claude --- src/platform/stm32/stm32_bxcan.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/platform/stm32/stm32_bxcan.c b/src/platform/stm32/stm32_bxcan.c index ecd4cb3a..e2eb7715 100644 --- a/src/platform/stm32/stm32_bxcan.c +++ b/src/platform/stm32/stm32_bxcan.c @@ -41,6 +41,15 @@ #define TX_MAILBOXES 3 +// Depth of the software queue behind the transmit mailbox. Anything +// beyond this is dropped rather than queued: a bus with no other node on +// it never acknowledges a frame, so the mailbox stays busy indefinitely +// and an unbounded queue drains the pbuf pool -- taking the rest of the +// stack down with it. Protocols that care retransmit; those that do not +// are periodic and the next sample supersedes what was dropped. FDCAN +// does the same when its hardware FIFO fills. +#define TX_QUEUE_DEPTH 8 + typedef struct bxcan { can_netif_t cni; @@ -51,6 +60,7 @@ typedef struct bxcan { uint32_t reg_base; size_t qlen; + uint32_t tx_drop; char name[5]; @@ -121,6 +131,13 @@ stm32_bxcan_output(can_netif_t *cni, pbuf_t *pb, uint32_t id) int q = irq_forbid(IRQ_LEVEL_NET); if(bx->tx_status[mailbox]) { + + if(bx->qlen >= TX_QUEUE_DEPTH) { + bx->tx_drop++; + irq_permit(q); + return pb; // Caller frees it + } + pb = pbuf_prepend(pb, 4, 0, 0); if(pb != NULL) { wr32_le(pbuf_data(pb, 0), id); From 28f8d6ca0959a13a72da77d6eeaeec3b5677497c Mon Sep 17 00:00:00 2001 From: Andreas Smas Date: Tue, 8 Sep 2026 14:25:20 +0200 Subject: [PATCH 07/10] stm32 bxcan: packet counters and a decoded last error code print_info reported only the error counters, so there was no way to tell from the shell whether the interface was moving packets at all, whether it was dropping them, or whether receives were failing for want of a pbuf -- the last of which was entirely silent. Diagnosing a draining pbuf pool on a bxcan board meant guessing. Report the same shape FDCAN already does: per-FIFO receive counts, NoPbufs, transmitted, drops and the current queue depth. Decode the last error code instead of printing the raw field, and put the three bus-state flags on one line. Co-Authored-By: Claude --- src/platform/stm32/stm32_bxcan.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/platform/stm32/stm32_bxcan.c b/src/platform/stm32/stm32_bxcan.c index e2eb7715..7e19a5db 100644 --- a/src/platform/stm32/stm32_bxcan.c +++ b/src/platform/stm32/stm32_bxcan.c @@ -60,12 +60,16 @@ typedef struct bxcan { uint32_t reg_base; size_t qlen; + uint32_t rx[2]; + uint32_t nobufs; + uint32_t tx; uint32_t tx_drop; char name[5]; } bxcan_t; +// Every transmitted frame goes through here static void stm32_bxcan_send(bxcan_t *bx, const void *data, size_t len, uint32_t id, int mailbox) @@ -91,6 +95,7 @@ stm32_bxcan_send(bxcan_t *bx, const void *data, size_t len, reg_wr(reg_base + CAN_TI(mailbox), (id << 3) | 0x5); } bx->tx_status[mailbox] = 1; + bx->tx++; } @@ -230,6 +235,9 @@ stm32_bxcan_rx(bxcan_t *bx, int mailbox) } STAILQ_INSERT_TAIL(&bx->cni.cni_ni.ni_rx_queue, pb, pb_link); netif_wakeup(&bx->cni.cni_ni); + bx->rx[mailbox]++; + } else { + bx->nobufs++; } reg_wr(bx->reg_base + CAN_RF(mailbox), 1 << 5); } @@ -265,12 +273,19 @@ stm32_bxcan_print_info(struct device *dev, struct stream *st) uint32_t tec = (esr >> 16) & 0xff; uint32_t lec = (esr >> 4) & 7; + stprintf(st, "Received packets, Fifo0:%u Fifo1:%u NoPbufs:%u\n", + bx->rx[0], bx->rx[1], bx->nobufs); + stprintf(st, "Transmitted packets:%u Drops:%u Queued:%d/%d\n", + bx->tx, bx->tx_drop, (int)bx->qlen, TX_QUEUE_DEPTH); + stprintf(st, "Receive error counter: %d\n", rec); stprintf(st, "Transmit error counter: %d\n", tec); - stprintf(st, "Last error code: %d\n", lec); - stprintf(st, "Bus Off: %s\n", esr & 0x4 ? "Yes" : "No"); - stprintf(st, "Error Passive: %s\n", esr & 0x2 ? "Yes" : "No"); - stprintf(st, "Error Warning: %s\n", esr & 0x1 ? "Yes" : "No"); + stprintf(st, "Bus state: O%s, ", esr & 0x4 ? "ff" : "n"); + stprintf(st, "Error passive: %s, ", esr & 0x2 ? "Yes" : "No"); + stprintf(st, "Error warning: %s\n", esr & 0x1 ? "Yes" : "No"); + stprintf(st, "Last error code: %s\n", + strtbl("None\0Stuffing\0Form\0AckErr\0Bit1Err\0Bit0Err\0CRC\0" + "SoftwareSet\0\0", lec)); } static const device_class_t stm32_bxcan_device_class = { From 5dc6572191ff66919018b3561c90003c5c9421c4 Mon Sep 17 00:00:00 2001 From: Andreas Smas Date: Tue, 8 Sep 2026 14:25:30 +0200 Subject: [PATCH 08/10] pbuf: report held-buffer origins from pbuf_status PBUF_ORIGIN_TRACE records which allocation site owns each data buffer and pbuf_origin_dump() lists them, but nothing ever called it, so turning the option on produced no output and the tracking was unreachable in practice. Call it from pbuf_status(), so a build with the option on answers "who is holding the pool" through the same command that already reports how little of it is left. Still off by default. Co-Authored-By: Claude --- src/net/pbuf.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/net/pbuf.c b/src/net/pbuf.c index d07020b8..466e2140 100644 --- a/src/net/pbuf.c +++ b/src/net/pbuf.c @@ -800,6 +800,9 @@ pbuf_status(stream_t *st) stprintf(st, "pbuf: %d avail, %u alloc failures\n", pbufs.pp_avail, pbuf_alloc_fails); stprintf(st, "pbuf_data: %d avail\n", pbuf_datas.pp_avail); +#ifdef PBUF_ORIGIN_TRACE + pbuf_origin_dump(st); +#endif } From 15fbf70972b53f34d2d9b3f5b3a6da8ee2636290 Mon Sep 17 00:00:00 2001 From: Andreas Smas Date: Tue, 8 Sep 2026 14:25:30 +0200 Subject: [PATCH 09/10] vllp: report nothing per-channel for a link with no session show_vllp listed the channel management channel as ESTABLISHED under a Disconnected link, which reads as though something is up when nothing is. Channel 14 is ESTABLISHED from the moment the link is created: it is a sentinel for "needs no setup", not a state it ever reached. The flow bits and the free-channel-id mask are just as misleading in that state -- both print their reset values, and 0x3fff in particular looks like information. So skip the whole per-channel block when there is no session and print only the header and the binds, where "waiting" is the fact that actually matters. Co-Authored-By: Claude --- src/net/vllp.c | 79 ++++++++++++++++++++++++++++---------------------- 1 file changed, 45 insertions(+), 34 deletions(-) diff --git a/src/net/vllp.c b/src/net/vllp.c index e60d5d14..a5c849d3 100644 --- a/src/net/vllp.c +++ b/src/net/vllp.c @@ -1717,44 +1717,55 @@ cmd_show_vllp(cli_t *cli, int argc, char **argv) #endif cli_printf(cli, "TX:0x%x RX:0x%x %sonnected", v->txid, v->rxid, v->connected ? "C" : "Disc"); - cli_printf(cli, " Flow status Local:0x%04x Remote:0x%04x\n", - v->local_flow_status, v->remote_flow_status); + + // Nothing below means anything without a session: the flow bits are + // stale, every channel id is still free, and the only channel that + // exists is the CMC -- which is ESTABLISHED from the moment the link + // is created, as a sentinel for "needs no setup" rather than a state + // it ever reached. Printing it against a disconnected link just + // invites the reader to believe something is up when nothing is. + if(!v->connected) { + cli_printf(cli, "\n"); + } else { + cli_printf(cli, " Flow status Local:0x%04x Remote:0x%04x\n", + v->local_flow_status, v->remote_flow_status); #ifdef ENABLE_VLLP_CLIENT - if(v->is_client) - cli_printf(cli, " Free channel ids:0x%04x\n", - v->available_channel_ids); - if(v->current_tx_buf != NULL) - cli_printf(cli, " In-flight fragment on channel %d\n", - v->current_tx_channel); + if(v->is_client) + cli_printf(cli, " Free channel ids:0x%04x\n", + v->available_channel_ids); + if(v->current_tx_buf != NULL) + cli_printf(cli, " In-flight fragment on channel %d\n", + v->current_tx_channel); #endif - cli_printf(cli, " Channels:\n"); - LIST_FOREACH(vc, &v->channels, link) { - cli_printf(cli, " %2d : state:%s app:%s net:%s\n", vc->id, - strtbl(vllp_channel_state_strtbl, vc->state), - strtbl(vllp_channel_app_closed_strtbl, vc->app_closed), - strtbl(vllp_channel_net_closed_strtbl, vc->net_closed)); + cli_printf(cli, " Channels:\n"); + LIST_FOREACH(vc, &v->channels, link) { + cli_printf(cli, " %2d : state:%s app:%s net:%s\n", vc->id, + strtbl(vllp_channel_state_strtbl, vc->state), + strtbl(vllp_channel_app_closed_strtbl, vc->app_closed), + strtbl(vllp_channel_net_closed_strtbl, vc->net_closed)); #ifdef ENABLE_VLLP_CLIENT - // Which service a channel is for, and what it is sitting on. The - // buffer counts are what localise a draining pool to a channel and - // a queue, which is otherwise a guessing game -- they are useful on - // a server too, but a server-only build should not pay for a - // command it never grew, so they live here with the rest. - int rx = 0, tx = 0; - pbuf_t *pb; - STAILQ_FOREACH(pb, &vc->rxq, pb_link) - rx++; - STAILQ_FOREACH(pb, &vc->txq, pb_link) - tx++; - if(vc->service != NULL || rx || tx || vc->stalled_tx) { - cli_printf(cli, " "); - if(vc->service != NULL) - cli_printf(cli, " service:%s", vc->service); - if(rx || tx || vc->stalled_tx) - cli_printf(cli, " pbufs:rxq=%d,txq=%d%s", rx, tx, - vc->stalled_tx ? ",stalled" : ""); - cli_printf(cli, "\n"); - } + // Which service a channel is for, and what it is sitting on. The + // buffer counts are what localise a draining pool to a channel + // and a queue, which is otherwise a guessing game -- they are + // useful on a server too, but a server-only build should not pay + // for a command it never grew, so they live here with the rest. + int rx = 0, tx = 0; + pbuf_t *pb; + STAILQ_FOREACH(pb, &vc->rxq, pb_link) + rx++; + STAILQ_FOREACH(pb, &vc->txq, pb_link) + tx++; + if(vc->service != NULL || rx || tx || vc->stalled_tx) { + cli_printf(cli, " "); + if(vc->service != NULL) + cli_printf(cli, " service:%s", vc->service); + if(rx || tx || vc->stalled_tx) + cli_printf(cli, " pbufs:rxq=%d,txq=%d%s", rx, tx, + vc->stalled_tx ? ",stalled" : ""); + cli_printf(cli, "\n"); + } #endif + } } #ifdef ENABLE_VLLP_CLIENT From d1906e6835135d535559604e2ee2619e682997ec Mon Sep 17 00:00:00 2001 From: Andreas Smas Date: Tue, 8 Sep 2026 14:25:42 +0200 Subject: [PATCH 10/10] dsig: DSIG_FLAG_DENY, so an output filter can be a deny-list An output filter was an allow-list only: first prefix match wins and no match means the signal does not go out. That is the wrong shape when all a board wants is to keep one id range off an interface, because expressing it means enumerating every id the board legitimately emits -- an inventory that silently drops the next signal someone adds. An entry with DSIG_FLAG_DENY, followed by a catch-all, states the constraint instead. Backward compatible: no match still means deny, so existing allow-lists and a NULL filter behave exactly as before, and dsig_output() consumes the bit so no driver sees it. Bit 0 is already DSIG_FLAG_EXTENDED, which the bxcan output path reads. mask_from_prefixlen(0) computed 1 << 32, which is undefined, so the catch-all had no well-defined spelling until now. Nothing used a zero prefix length before. Co-Authored-By: Claude --- include/mios/dsig.h | 8 ++++++++ src/net/dsig.c | 2 +- src/net/net.h | 2 ++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/include/mios/dsig.h b/include/mios/dsig.h index 5ee62a0d..20214680 100644 --- a/include/mios/dsig.h +++ b/include/mios/dsig.h @@ -16,6 +16,11 @@ dsig_sub_t *dsig_sub(uint32_t signal, uint32_t mask, uint16_t ttl_ms, uint32_t signal), void *opaque); +// Filters are matched in order and the first match decides. No match at +// all means deny, so a filter list is an allow-list by default. An entry +// with DSIG_FLAG_DENY inverts that for the IDs it covers, which together +// with a catch-all entry ({0, 0}) at the end turns the list into a +// deny-list instead. struct dsig_filter { uint32_t prefix; uint8_t prefixlen; @@ -26,6 +31,9 @@ struct dsig_filter { #define DSIG_FLAG_EXTENDED 0x1 +// Output filters only. Matching IDs are not passed to the interface. +#define DSIG_FLAG_DENY 0x2 + #define DSIG_FILTER_END { .prefixlen = 0xff } diff --git a/src/net/dsig.c b/src/net/dsig.c index 3a37970f..48875f23 100644 --- a/src/net/dsig.c +++ b/src/net/dsig.c @@ -96,7 +96,7 @@ dsig_output(uint32_t id, struct pbuf *pb, struct netif *exclude) const struct dsig_filter *dof = ni->ni_dsig_output_filter; if(dof != NULL) { dof = dsig_filter_match(dof, id); - if(dof == NULL) + if(dof == NULL || (dof->flags & DSIG_FLAG_DENY)) continue; flags = dof->flags; } diff --git a/src/net/net.h b/src/net/net.h index 1a20655d..e74a632f 100644 --- a/src/net/net.h +++ b/src/net/net.h @@ -13,5 +13,7 @@ uint32_t inet_addr(const char *s); static inline uint32_t mask_from_prefixlen(int prefixlen) { + if(prefixlen == 0) + return 0; // 1 << 32 is undefined return ~((1 << (32 - prefixlen)) - 1); }