Vllp client

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
 
 #

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);

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 }
 
 

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);

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 <unit>` 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 <mios/error.h>
+
+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);

include/mios/vllp.h

@@ -3,10 +3,17 @@
 #include <stdint.h>
 #include <stddef.h>
 
+#include <mios/error.h>
+#include <mios/pushpull.h>
+
 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

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;
     }

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);
 }

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 \
 

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);
 }
 
@@ -294,6 +347,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);
 }
 
 
@@ -737,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
 }
 
 

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

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,
 };