From cca9b6e1ba4865bbc6c13c3ab60fbcab52bacb45 Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp@nicira.com>
Date: Mon, 5 Jan 2009 16:04:05 -0800
Subject: [PATCH] Improve STP library.

- Don't expose the funny STP time units; instead, convert to and from
  milliseconds at the module boundary.

- Allow the client to configure the max age, hello time, and forward delay
  parameters.

- Do more of the construction of BPDUs internally, instead of making the
  client do it.
---
 lib/stp.c                              | 230 +++++++++++++++++++++----
 lib/stp.h                              |  20 ++-
 secchan/stp-secchan.c                  |  59 ++-----
 tests/test-stp-ieee802.1d-1998         |   2 +-
 tests/test-stp-ieee802.1d-2004-fig17.4 |   4 +-
 tests/test-stp-ieee802.1d-2004-fig17.6 |   2 +-
 tests/test-stp-ieee802.1d-2004-fig17.7 |   4 +-
 tests/test-stp-iol-io-1.1              |  10 +-
 tests/test-stp-iol-io-1.2              |   4 +-
 tests/test-stp-iol-io-1.4              |   2 +-
 tests/test-stp-iol-io-1.5              |  10 +-
 tests/test-stp-iol-op-1.4              |   2 +-
 tests/test-stp-iol-op-3.1              |   2 +-
 tests/test-stp-iol-op-3.3              |   2 +-
 tests/test-stp-iol-op-3.4              |   2 +-
 tests/test-stp.c                       |  34 ++--
 16 files changed, 272 insertions(+), 117 deletions(-)

diff --git a/lib/stp.c b/lib/stp.c
index 0847f57a..fa9dd010 100644
--- a/lib/stp.c
+++ b/lib/stp.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2008 The Board of Trustees of The Leland Stanford
+/* Copyright (c) 2008, 2009 The Board of Trustees of The Leland Stanford
  * Junior University
  *
  * We are making the OpenFlow specification and associated documentation
@@ -39,6 +39,7 @@
 #include <assert.h>
 #include <inttypes.h>
 #include <stdlib.h>
+#include "ofpbuf.h"
 #include "packets.h"
 #include "util.h"
 #include "xtoxll.h"
@@ -121,6 +122,10 @@ struct stp {
     int bridge_max_age;             /* 8.5.3.8: max_age when we're root. */
     int bridge_hello_time;          /* 8.5.3.9: hello_time as root. */
     int bridge_forward_delay;       /* 8.5.3.10: forward_delay as root. */
+    int rq_max_age;                 /* User-requested max age, in ms. */
+    int rq_hello_time;              /* User-requested hello time, in ms. */
+    int rq_forward_delay;           /* User-requested forward delay, in ms. */
+    int elapsed_remainder;          /* Left-over msecs from last stp_tick(). */
 
     /* Dynamic bridge data. */
     stp_identifier designated_root; /* 8.5.3.1: Bridge believed to be root. */
@@ -139,8 +144,7 @@ struct stp {
 
     /* Interface to client. */
     struct stp_port *first_changed_port;
-    void (*send_bpdu)(const void *bpdu, size_t bpdu_size,
-                      int port_no, void *aux);
+    void (*send_bpdu)(struct ofpbuf *bpdu, int port_no, void *aux);
     void *aux;
 };
 
@@ -159,8 +163,6 @@ stp_next_enabled_port(const struct stp *stp, const struct stp_port *port)
     return NULL;
 }
 
-#define SECONDS_TO_TIMER(SECS) ((SECS) * 0x100)
-
 #define MESSAGE_AGE_INCREMENT 1
 
 static void stp_transmit_config(struct stp_port *);
@@ -198,27 +200,33 @@ static void stp_tcn_timer_expiry(struct stp *);
 static void stp_topology_change_timer_expiry(struct stp *);
 static void stp_hold_timer_expiry(struct stp_port *);
 static void stp_initialize_port(struct stp_port *, enum stp_state);
-static void stp_become_root_bridge(struct stp *stp);
+static void stp_become_root_bridge(struct stp *);
+static void stp_update_bridge_timers(struct stp *);
 
+static int clamp(int x, int min, int max);
+static int ms_to_timer(int ms);
+static int ms_to_timer_remainder(int ms);
+static int timer_to_ms(int timer);
 static void stp_start_timer(struct stp_timer *, int value);
 static void stp_stop_timer(struct stp_timer *);
 static bool stp_timer_expired(struct stp_timer *, int elapsed, int timeout);
 
-/* Creates and returns a new STP instance that initially has no port enabled.
+static void stp_send_bpdu(struct stp_port *, const void *, size_t);
+
+/* Creates and returns a new STP instance that initially has no ports enabled.
  *
  * 'bridge_id' should be a 48-bit MAC address as returned by
  * eth_addr_to_uint64().  'bridge_id' may also have a priority value in its top
- * 16 bits; if those bits are set to 0, the default bridge priority of 32768 is
- * used.  (This priority may be changed with stp_set_bridge_priority().)
+ * 16 bits; if those bits are set to 0, STP_DEFAULT_BRIDGE_PRIORITY is used.
+ * (This priority may be changed with stp_set_bridge_priority().)
  *
- * When the bridge needs to send out a BPDU, it calls 'send_bpdu', passing
- * 'aux' as auxiliary data.  This callback may be called from stp_tick() or
- * stp_received_bpdu().
+ * When the bridge needs to send out a BPDU, it calls 'send_bpdu'.  This
+ * callback may be called from stp_tick() or stp_received_bpdu().  The
+ * arguments to 'send_bpdu' are an STP BPDU encapsulated in 
  */
 struct stp *
 stp_create(const char *name, stp_identifier bridge_id,
-           void (*send_bpdu)(const void *bpdu, size_t bpdu_size,
-                             int port_no, void *aux),
+           void (*send_bpdu)(struct ofpbuf *bpdu, int port_no, void *aux),
            void *aux)
 {
     struct stp *stp;
@@ -228,18 +236,16 @@ stp_create(const char *name, stp_identifier bridge_id,
     stp->name = xstrdup(name);
     stp->bridge_id = bridge_id;
     if (!(stp->bridge_id >> 48)) {
-        stp->bridge_id |= UINT64_C(32768) << 48;
+        stp->bridge_id |= (uint64_t) STP_DEFAULT_BRIDGE_PRIORITY << 48;
     }
-    stp->max_age = SECONDS_TO_TIMER(6);
-    stp->hello_time = SECONDS_TO_TIMER(2);
-    stp->forward_delay = SECONDS_TO_TIMER(4);
-    stp->bridge_max_age = stp->max_age;
-    stp->bridge_hello_time = stp->hello_time;
-    stp->bridge_forward_delay = stp->forward_delay;
 
-    /* Verify constraints stated by 802.1D. */
-    assert(2 * (stp->forward_delay - SECONDS_TO_TIMER(1)) >= stp->max_age);
-    assert(stp->max_age >= 2 * (stp->hello_time + SECONDS_TO_TIMER(1)));
+    stp->rq_max_age = 6000;
+    stp->rq_hello_time = 2000;
+    stp->rq_forward_delay = 4000;
+    stp_update_bridge_timers(stp);
+    stp->max_age = stp->bridge_max_age;
+    stp->hello_time = stp->bridge_hello_time;
+    stp->forward_delay = stp->bridge_forward_delay;
 
     stp->designated_root = stp->bridge_id;
     stp->root_path_cost = 0;
@@ -257,7 +263,7 @@ stp_create(const char *name, stp_identifier bridge_id,
     stp->first_changed_port = &stp->ports[ARRAY_SIZE(stp->ports)];
     for (p = stp->ports; p < &stp->ports[ARRAY_SIZE(stp->ports)]; p++) {
         p->stp = stp;
-        p->port_id = (stp_port_no(p) + 1) | (128 << 8);
+        p->port_id = (stp_port_no(p) + 1) | (STP_DEFAULT_PORT_PRIORITY << 8);
         p->path_cost = 19;      /* Recommended default for 100 Mb/s link. */
         stp_initialize_port(p, STP_DISABLED);
     }
@@ -271,10 +277,22 @@ stp_destroy(struct stp *stp)
     free(stp);
 }
 
+/* Runs 'stp' given that 'ms' milliseconds have passed. */
 void
-stp_tick(struct stp *stp, int elapsed)
+stp_tick(struct stp *stp, int ms)
 {
     struct stp_port *p;
+    int elapsed;
+
+    /* Convert 'ms' to STP timer ticks.  Preserve any leftover milliseconds
+     * from previous stp_tick() calls so that we don't lose STP ticks when we
+     * are called too frequently. */
+    ms = clamp(ms, 0, INT_MAX - 1000) + stp->elapsed_remainder;
+    elapsed = ms_to_timer(ms);
+    stp->elapsed_remainder = ms_to_timer_remainder(ms);
+    if (!elapsed) {
+        return;
+    }
 
     if (stp_timer_expired(&stp->hello_timer, elapsed, stp->hello_time)) {
         stp_hello_timer_expiry(stp);
@@ -296,7 +314,7 @@ stp_tick(struct stp *stp, int elapsed)
                               stp->forward_delay)) {
             stp_forward_delay_timer_expiry(p);
         }
-        if (stp_timer_expired(&p->hold_timer, elapsed, SECONDS_TO_TIMER(1))) {
+        if (stp_timer_expired(&p->hold_timer, elapsed, ms_to_timer(1000))) {
             stp_hold_timer_expiry(p);
         }
     }
@@ -340,6 +358,40 @@ stp_set_bridge_priority(struct stp *stp, uint16_t new_priority)
                         | ((uint64_t) new_priority << 48)));
 }
 
+/* Sets the desired hello time for 'stp' to 'ms', in milliseconds.  The actual
+ * hello time is clamped to the range of 1 to 10 seconds and subject to the
+ * relationship (bridge_max_age >= 2 * (bridge_hello_time + 1 s)).  The bridge
+ * hello time is only used when 'stp' is the root bridge. */
+void
+stp_set_hello_time(struct stp *stp, int ms)
+{
+    stp->rq_hello_time = ms;
+    stp_update_bridge_timers(stp);
+}
+
+/* Sets the desired max age for 'stp' to 'ms', in milliseconds.  The actual max
+ * age is clamped to the range of 6 to 40 seconds and subject to the
+ * relationships (2 * (bridge_forward_delay - 1 s) >= bridge_max_age) and
+ * (bridge_max_age >= 2 * (bridge_hello_time + 1 s)).  The bridge max age is
+ * only used when 'stp' is the root bridge. */
+void
+stp_set_max_age(struct stp *stp, int ms)
+{
+    stp->rq_max_age = ms;
+    stp_update_bridge_timers(stp);
+}
+
+/* Sets the desired forward delay for 'stp' to 'ms', in milliseconds.  The
+ * actual forward delay is clamped to the range of 4 to 30 seconds and subject
+ * to the relationship (2 * (bridge_forward_delay - 1 s) >= bridge_max_age).
+ * The bridge forward delay is only used when 'stp' is the root bridge. */
+void
+stp_set_forward_delay(struct stp *stp, int ms)
+{
+    stp->rq_forward_delay = ms;
+    stp_update_bridge_timers(stp);
+}
+
 /* Returns the name given to 'stp' in the call to stp_create(). */
 const char *
 stp_get_name(const struct stp *stp)
@@ -376,6 +428,35 @@ stp_get_root_path_cost(const struct stp *stp)
     return stp->root_path_cost;
 }
 
+/* Returns the bridge hello time, in ms.  The returned value is not necessarily
+ * the value passed to stp_set_hello_time(): it is clamped to the valid range
+ * and quantized to the STP timer resolution.  */
+int
+stp_get_hello_time(const struct stp *stp)
+{
+    return timer_to_ms(stp->bridge_hello_time);
+}
+
+/* Returns the bridge max age, in ms.  The returned value is not necessarily
+ * the value passed to stp_set_max_age(): it is clamped to the valid range,
+ * quantized to the STP timer resolution, and adjusted to match the constraints
+ * due to the hello time.  */
+int
+stp_get_max_age(const struct stp *stp)
+{
+    return timer_to_ms(stp->bridge_max_age);
+}
+
+/* Returns the bridge forward delay, in ms.  The returned value is not
+ * necessarily the value passed to stp_set_forward_delay(): it is clamped to
+ * the valid range, quantized to the STP timer resolution, and adjusted to
+ * match the constraints due to the forward delay.  */
+int
+stp_get_forward_delay(const struct stp *stp)
+{
+    return timer_to_ms(stp->bridge_forward_delay);
+}
+
 /* Returns the port in 'stp' with index 'port_no', which must be between 0 and
  * STP_MAX_PORTS. */
 struct stp_port *
@@ -437,19 +518,25 @@ stp_state_name(enum stp_state state)
 }
 
 /* Returns true if 'state' is one in which packets received on a port should
- * be forwarded, false otherwise. */
+ * be forwarded, false otherwise.
+ *
+ * Returns true if 'state' is STP_DISABLED, since presumably in that case the
+ * port should still work, just not have STP applied to it. */
 bool
 stp_forward_in_state(enum stp_state state)
 {
-    return state == STP_FORWARDING;
+    return (state & (STP_DISABLED | STP_FORWARDING)) != 0;
 }
 
 /* Returns true if 'state' is one in which MAC learning should be done on
- * packets received on a port, false otherwise. */
+ * packets received on a port, false otherwise.
+ *
+ * Returns true if 'state' is STP_DISABLED, since presumably in that case the
+ * port should still work, just not have STP applied to it. */
 bool
 stp_learn_in_state(enum stp_state state)
 {
-    return state & (STP_LEARNING | STP_FORWARDING);
+    return (state & (STP_DISABLED | STP_LEARNING | STP_FORWARDING)) != 0;
 }
 
 /* Notifies the STP entity that bridge protocol data unit 'bpdu', which is
@@ -586,7 +673,7 @@ stp_port_set_priority(struct stp_port *p, uint8_t new_priority)
  * used to indicate faster links.  Use stp_port_set_speed() to automatically
  * generate a default path cost from a link speed. */
 void
-stp_port_set_path_cost(struct stp_port *p, unsigned int path_cost)
+stp_port_set_path_cost(struct stp_port *p, uint16_t path_cost)
 {
     if (p->path_cost != path_cost) {
         struct stp *stp = p->stp;
@@ -662,7 +749,7 @@ stp_transmit_config(struct stp_port *p)
         if (ntohs(config.message_age) < stp->max_age) {
             p->topology_change_ack = false;
             p->config_pending = false;
-            stp->send_bpdu(&config, sizeof config, stp_port_no(p), stp->aux);
+            stp_send_bpdu(p, &config, sizeof config);
             stp_start_timer(&p->hold_timer, 0);
         }
     }
@@ -735,7 +822,7 @@ stp_transmit_tcn(struct stp *stp)
     tcn_bpdu.header.protocol_id = htons(STP_PROTOCOL_ID);
     tcn_bpdu.header.protocol_version = STP_PROTOCOL_VERSION;
     tcn_bpdu.header.bpdu_type = STP_TYPE_TCN;
-    stp->send_bpdu(&tcn_bpdu, sizeof tcn_bpdu, stp_port_no(p), stp->aux);
+    stp_send_bpdu(p, &tcn_bpdu, sizeof tcn_bpdu);
 }
 
 static void
@@ -1081,3 +1168,78 @@ stp_timer_expired(struct stp_timer *timer, int elapsed, int timeout)
     return false;
 }
 
+/* Returns the number of whole STP timer ticks in 'ms' milliseconds.  There
+ * are 256 STP timer ticks per second. */
+static int
+ms_to_timer(int ms)
+{
+    return ms * 0x100 / 1000;
+}
+
+/* Returns the number of leftover milliseconds when 'ms' is converted to STP
+ * timer ticks. */
+static int
+ms_to_timer_remainder(int ms)
+{
+    return ms * 0x100 % 1000;
+}
+
+/* Returns the number of whole milliseconds in 'timer' STP timer ticks.  There
+ * are 256 STP timer ticks per second. */
+static int
+timer_to_ms(int timer)
+{
+    return timer * 1000 / 0x100;
+}
+
+static int
+clamp(int x, int min, int max)
+{
+    return x < min ? min : x > max ? max : x;
+}
+
+static void
+stp_update_bridge_timers(struct stp *stp)
+{
+    int ht, ma, fd;
+
+    ht = clamp(stp->rq_hello_time, 1000, 10000);
+    ma = clamp(stp->rq_max_age, MAX(2 * (ht + 1000), 6000), 40000);
+    fd = clamp(stp->rq_forward_delay, ma / 2 + 1000, 30000);
+
+    stp->bridge_hello_time = ms_to_timer(ht);
+    stp->bridge_max_age = ms_to_timer(ma);
+    stp->bridge_forward_delay = ms_to_timer(fd);
+
+    if (stp_is_root_bridge(stp)) {
+        stp->max_age = stp->bridge_max_age;
+        stp->hello_time = stp->bridge_hello_time;
+        stp->forward_delay = stp->bridge_forward_delay;
+    }
+}
+
+static void
+stp_send_bpdu(struct stp_port *p, const void *bpdu, size_t bpdu_size)
+{
+    struct eth_header *eth;
+    struct llc_header *llc;
+    struct ofpbuf *pkt;
+
+    /* Skeleton. */
+    pkt = ofpbuf_new(ETH_HEADER_LEN + LLC_HEADER_LEN + bpdu_size);
+    pkt->l2 = eth = ofpbuf_put_zeros(pkt, sizeof *eth);
+    llc = ofpbuf_put_zeros(pkt, sizeof *llc);
+    pkt->l3 = ofpbuf_put(pkt, bpdu, bpdu_size);
+
+    /* 802.2 header. */
+    memcpy(eth->eth_dst, stp_eth_addr, ETH_ADDR_LEN);
+    /* p->stp->send_bpdu() must fill in source address. */
+    eth->eth_type = htons(pkt->size - ETH_HEADER_LEN);
+
+    /* LLC header. */
+    llc->llc_dsap = STP_LLC_DSAP;
+    llc->llc_ssap = STP_LLC_SSAP;
+    llc->llc_cntl = STP_LLC_CNTL;
+
+    p->stp->send_bpdu(pkt, stp_port_no(p), p->stp->aux);
+}
diff --git a/lib/stp.h b/lib/stp.h
index 5e09c2f1..a51c92f1 100644
--- a/lib/stp.h
+++ b/lib/stp.h
@@ -42,6 +42,8 @@
 #include "compiler.h"
 #include "util.h"
 
+struct ofpbuf;
+
 /* Ethernet address used as the destination for STP frames. */
 extern const uint8_t stp_eth_addr[6];
 
@@ -50,6 +52,10 @@ extern const uint8_t stp_eth_addr[6];
 #define STP_LLC_DSAP 0x42
 #define STP_LLC_CNTL 0x03
 
+/* Bridge and port priorities that should be used by default. */
+#define STP_DEFAULT_BRIDGE_PRIORITY 32768
+#define STP_DEFAULT_PORT_PRIORITY 128
+
 /* Bridge identifier.  Top 16 bits are a priority value (numerically lower
  * values are higher priorities).  Bottom 48 bits are MAC address of bridge. */
 typedef uint64_t stp_identifier;
@@ -57,13 +63,16 @@ typedef uint64_t stp_identifier;
 /* Basic STP functionality. */
 #define STP_MAX_PORTS 255
 struct stp *stp_create(const char *name, stp_identifier bridge_id,
-                       void (*send_bpdu)(const void *bpdu, size_t bpdu_size,
-                                         int port_no, void *aux),
+                       void (*send_bpdu)(struct ofpbuf *bpdu, int port_no,
+                                         void *aux),
                        void *aux);
 void stp_destroy(struct stp *);
-void stp_tick(struct stp *, int elapsed);
+void stp_tick(struct stp *, int ms);
 void stp_set_bridge_id(struct stp *, stp_identifier bridge_id);
 void stp_set_bridge_priority(struct stp *, uint16_t new_priority);
+void stp_set_hello_time(struct stp *, int ms);
+void stp_set_max_age(struct stp *, int ms);
+void stp_set_forward_delay(struct stp *, int ms);
 
 /* STP properties. */
 const char *stp_get_name(const struct stp *);
@@ -71,6 +80,9 @@ stp_identifier stp_get_bridge_id(const struct stp *);
 stp_identifier stp_get_designated_root(const struct stp *);
 bool stp_is_root_bridge(const struct stp *);
 int stp_get_root_path_cost(const struct stp *);
+int stp_get_hello_time(const struct stp *);
+int stp_get_max_age(const struct stp *);
+int stp_get_forward_delay(const struct stp *);
 
 /* Obtaining STP ports. */
 struct stp_port *stp_get_port(struct stp *, int port_no);
@@ -100,7 +112,7 @@ enum stp_state stp_port_get_state(const struct stp_port *);
 void stp_port_enable(struct stp_port *);
 void stp_port_disable(struct stp_port *);
 void stp_port_set_priority(struct stp_port *, uint8_t new_priority);
-void stp_port_set_path_cost(struct stp_port *, unsigned int path_cost);
+void stp_port_set_path_cost(struct stp_port *, uint16_t path_cost);
 void stp_port_set_speed(struct stp_port *, unsigned int speed);
 void stp_port_enable_change_detection(struct stp_port *);
 void stp_port_disable_change_detection(struct stp_port *);
diff --git a/secchan/stp-secchan.c b/secchan/stp-secchan.c
index 97ad02bb..9ded73b6 100644
--- a/secchan/stp-secchan.c
+++ b/secchan/stp-secchan.c
@@ -54,7 +54,7 @@ struct stp_data {
     struct port_watcher *pw;
     struct rconn *local_rconn;
     struct rconn *remote_rconn;
-    long long int last_tick_256ths;
+    long long int last_tick;
     int n_txq;
 };
 
@@ -126,18 +126,12 @@ stp_local_packet_cb(struct relay *r, void *stp_)
     return true;
 }
 
-static long long int
-time_256ths(void)
-{
-    return time_msec() * 256 / 1000;
-}
-
 static void
 stp_periodic_cb(void *stp_)
 {
     struct stp_data *stp = stp_;
-    long long int now_256ths = time_256ths();
-    long long int elapsed_256ths = now_256ths - stp->last_tick_256ths;
+    long long int now = time_msec();
+    long long int elapsed = now - stp->last_tick;
     struct stp_port *p;
 
     if (!port_watcher_is_ready(stp->pw)) {
@@ -145,12 +139,12 @@ stp_periodic_cb(void *stp_)
          * disable STP. */
         return;
     }
-    if (elapsed_256ths <= 0) {
+    if (elapsed <= 0) {
         return;
     }
 
-    stp_tick(stp->stp, MIN(INT_MAX, elapsed_256ths));
-    stp->last_tick_256ths = now_256ths;
+    stp_tick(stp->stp, MIN(INT_MAX, elapsed));
+    stp->last_tick = now;
 
     while (stp_get_changed_port(stp->stp, &p)) {
         int port_no = stp_port_no(p);
@@ -202,39 +196,22 @@ stp_wait_cb(void *stp_ UNUSED)
 }
 
 static void
-send_bpdu(const void *bpdu, size_t bpdu_size, int port_no, void *stp_)
+send_bpdu(struct ofpbuf *pkt, int port_no, void *stp_)
 {
     struct stp_data *stp = stp_;
-    const uint8_t *port_mac;
-    struct eth_header *eth;
-    struct llc_header *llc;
-    struct ofpbuf pkt, *opo;
+    const uint8_t *port_mac = port_watcher_get_hwaddr(stp->pw, port_no);
+    if (port_mac) {
+        struct eth_header *eth = pkt->l2;
+        struct ofpbuf *opo;
+
+        memcpy(eth->eth_src, port_mac, ETH_ADDR_LEN);
+        opo = make_unbuffered_packet_out(pkt, OFPP_NONE, port_no);
 
-    port_mac = port_watcher_get_hwaddr(stp->pw, port_no);
-    if (!port_mac) {
+        rconn_send_with_limit(stp->local_rconn, opo, &stp->n_txq, OFPP_MAX);
+    } else {
         VLOG_WARN_RL(&rl, "cannot send BPDU on missing port %d", port_no);
-        return;
     }
-
-    /* Packet skeleton. */
-    ofpbuf_init(&pkt, ETH_HEADER_LEN + LLC_HEADER_LEN + bpdu_size);
-    eth = ofpbuf_put_uninit(&pkt, sizeof *eth);
-    llc = ofpbuf_put_uninit(&pkt, sizeof *llc);
-    ofpbuf_put(&pkt, bpdu, bpdu_size);
-
-    /* 802.2 header. */
-    memcpy(eth->eth_dst, stp_eth_addr, ETH_ADDR_LEN);
-    memcpy(eth->eth_src, port_mac, ETH_ADDR_LEN);
-    eth->eth_type = htons(pkt.size - ETH_HEADER_LEN);
-
-    /* LLC header. */
-    llc->llc_dsap = STP_LLC_DSAP;
-    llc->llc_ssap = STP_LLC_SSAP;
-    llc->llc_cntl = STP_LLC_CNTL;
-
-    opo = make_unbuffered_packet_out(&pkt, OFPP_NONE, port_no);
-    ofpbuf_uninit(&pkt);
-    rconn_send_with_limit(stp->local_rconn, opo, &stp->n_txq, OFPP_MAX);
+    ofpbuf_delete(pkt);
 }
 
 static bool
@@ -308,7 +285,7 @@ stp_start(struct secchan *secchan, const struct settings *s,
     stp->pw = pw;
     stp->local_rconn = local;
     stp->remote_rconn = remote;
-    stp->last_tick_256ths = time_256ths();
+    stp->last_tick = time_msec();
 
     port_watcher_register_callback(pw, stp_port_changed_cb, stp);
     port_watcher_register_local_port_callback(pw, stp_local_port_changed_cb,
diff --git a/tests/test-stp-ieee802.1d-1998 b/tests/test-stp-ieee802.1d-1998
index 88aedb42..f1982a03 100644
--- a/tests/test-stp-ieee802.1d-1998
+++ b/tests/test-stp-ieee802.1d-1998
@@ -4,7 +4,7 @@ bridge 1 0x97 = c:5 a d:5
 bridge 2 0x45 = b e
 bridge 3 0x57 = b:5 e:5
 bridge 4 0x83 = a:5 e:5
-run 256
+run 1000
 check 0 = root
 check 1 = F F:10 F
 check 2 = F:10 B
diff --git a/tests/test-stp-ieee802.1d-2004-fig17.4 b/tests/test-stp-ieee802.1d-2004-fig17.4
index 29b13ff8..1f708630 100644
--- a/tests/test-stp-ieee802.1d-2004-fig17.4
+++ b/tests/test-stp-ieee802.1d-2004-fig17.4
@@ -7,7 +7,7 @@ bridge 4 0x555 = g i 0 0
 bridge 5 0x666 = h k 0 0
 bridge 6 0x777 = j m 0 0
 bridge 7 0x888 = l n 0 0
-run 256
+run 1000
 check 0 = root
 check 1 = F:10 B F F
 check 2 = F:10 B F F F F
@@ -20,7 +20,7 @@ check 7 = F:20 B F F
 # Now connect two ports of bridge 7 to the same LAN.
 bridge 7 = l n o o
 # Same results except for bridge 7:
-run 256
+run 1000
 check 0 = root
 check 1 = F:10 B F F
 check 2 = F:10 B F F F F
diff --git a/tests/test-stp-ieee802.1d-2004-fig17.6 b/tests/test-stp-ieee802.1d-2004-fig17.6
index 694d2383..6ed59177 100644
--- a/tests/test-stp-ieee802.1d-2004-fig17.6
+++ b/tests/test-stp-ieee802.1d-2004-fig17.6
@@ -5,7 +5,7 @@ bridge 2 0x333 = d e f
 bridge 3 0x444 = f g h
 bridge 4 0x555 = j h i
 bridge 5 0x666 = l j k
-run 256
+run 1000
 check 0 = root
 check 1 = F:10 F F
 check 2 = F:20 F F
diff --git a/tests/test-stp-ieee802.1d-2004-fig17.7 b/tests/test-stp-ieee802.1d-2004-fig17.7
index 278655e2..daa0cdf2 100644
--- a/tests/test-stp-ieee802.1d-2004-fig17.7
+++ b/tests/test-stp-ieee802.1d-2004-fig17.7
@@ -2,7 +2,7 @@
 bridge 0 0xaa = b
 bridge 1 0x111 = a b d f h g e c
 bridge 2 0x222 = g h j l n m k i
-run 256
+run 1000
 check 0 = root
 check 1 = F F:10 F F F F F F
 check 2 = B F:20 F F F F F F
@@ -11,7 +11,7 @@ check 2 = B F:20 F F F F F F
 # but I don't understand what port priority change would cause
 # that change.
 bridge 2 = g X j l n m k i
-run 256
+run 1000
 check 0 = root
 check 1 = F F:10 F F F F F F
 check 2 = F:20 D F F F F F F
diff --git a/tests/test-stp-iol-io-1.1 b/tests/test-stp-iol-io-1.1
index 1b53fedb..186d6c4c 100644
--- a/tests/test-stp-iol-io-1.1
+++ b/tests/test-stp-iol-io-1.1
@@ -4,22 +4,22 @@
 # STP.io.1.1: Link Failure
 bridge 0 0x111 = a b c
 bridge 1 0x222 = a b c
-run 256
+run 1000
 check 0 = root
 check 1 = F:10 B B
 bridge 1 = 0 _ _
-run 256
+run 1000
 check 0 = root
 check 1 = F F:10 B
 bridge 1 = X _ _
-run 256
+run 1000
 check 0 = root
 check 1 = D F:10 B
 bridge 1 = _ 0 _
-run 256
+run 1000
 check 0 = root
 check 1 = D F F:10
 bridge 1 = _ X _
-run 256
+run 1000
 check 0 = root
 check 1 = D D F:10
diff --git a/tests/test-stp-iol-io-1.2 b/tests/test-stp-iol-io-1.2
index a2ed48cb..285bbd88 100644
--- a/tests/test-stp-iol-io-1.2
+++ b/tests/test-stp-iol-io-1.2
@@ -4,11 +4,11 @@
 # STP.io.1.2: Repeated Network
 bridge 0 0x111 = a a
 bridge 1 0x222 = a a
-run 256
+run 1000
 check 0 = rootid:0x111 F B
 check 1 = rootid:0x111 F:10 B
 bridge 1 = a^0x90 _
-run 256
+run 1000
 check 0 = rootid:0x111 F B
 check 1 = rootid:0x111 B F:10
 
diff --git a/tests/test-stp-iol-io-1.4 b/tests/test-stp-iol-io-1.4
index f8b0b722..0065aaf5 100644
--- a/tests/test-stp-iol-io-1.4
+++ b/tests/test-stp-iol-io-1.4
@@ -6,7 +6,7 @@ bridge 0 0x111 = a b c
 bridge 1 0x222 = b d e
 bridge 2 0x333 = a d f
 bridge 3 0x444 = c e f
-run 256
+run 1000
 check 0 = root
 check 1 = F:10 F F
 check 2 = F:10 B F
diff --git a/tests/test-stp-iol-io-1.5 b/tests/test-stp-iol-io-1.5
index c5f08b8e..285d29de 100644
--- a/tests/test-stp-iol-io-1.5
+++ b/tests/test-stp-iol-io-1.5
@@ -6,25 +6,25 @@ bridge 0 0x111 = a b d c
 bridge 1 0x222 = a b f e
 bridge 2 0x333 = c d g h
 bridge 3 0x444 = e f g h
-run 256
+run 1000
 check 0 = root
 check 1 = F:10 B F F
 check 2 = B F:10 F F
 check 3 = B F:20 B B
 bridge 1^0x7000
-run 256
+run 1000
 check 0 = F:10 B F F
 check 1 = root
 check 2 = B F:20 B B
 check 3 = B F:10 F F
 bridge 2^0x6000
-run 256
+run 1000
 check 0 = F F B F:10
 check 1 = F:20 B B B
 check 2 = root
 check 3 = F F F:10 B
 bridge 3^0x5000
-run 256
+run 1000
 check 0 = B B B F:20
 check 1 = F F B F:10
 check 2 = F F F:10 B
@@ -33,7 +33,7 @@ bridge 0^0x4000
 bridge 1^0x4001
 bridge 2^0x4002
 bridge 3^0x4003
-run 256
+run 1000
 check 0 = root
 check 1 = F:10 B F F
 check 2 = B F:10 F F
diff --git a/tests/test-stp-iol-op-1.4 b/tests/test-stp-iol-op-1.4
index 18c09b89..6a121164 100644
--- a/tests/test-stp-iol-op-1.4
+++ b/tests/test-stp-iol-op-1.4
@@ -4,5 +4,5 @@
 # Test STP.op.1.4 ­ All Ports Initialized to Designated Ports
 bridge 0 0x123 = a b c d e f
 check 0 = Li Li Li Li Li Li
-run 256
+run 1000
 check 0 = F F F F F F
diff --git a/tests/test-stp-iol-op-3.1 b/tests/test-stp-iol-op-3.1
index 2e0a2f2c..3e1099cb 100644
--- a/tests/test-stp-iol-op-3.1
+++ b/tests/test-stp-iol-op-3.1
@@ -6,6 +6,6 @@ bridge 0 0x111 = a
 bridge 1 0x222 = a
 check 0 = rootid:0x111 Li
 check 1 = rootid:0x222 Li
-run 256
+run 1000
 check 0 = rootid:0x111 root
 check 1 = rootid:0x111 F:10
diff --git a/tests/test-stp-iol-op-3.3 b/tests/test-stp-iol-op-3.3
index 093c64ed..2bcd45e1 100644
--- a/tests/test-stp-iol-op-3.3
+++ b/tests/test-stp-iol-op-3.3
@@ -5,7 +5,7 @@
 bridge 0 0x333^0x6000 = a
 bridge 1 0x222^0x7000 = b
 bridge 2 0x111 = a b
-run 256
+run 1000
 check 0 = rootid:0x333^0x6000 root
 check 1 = rootid:0x333^0x6000 F:20
 check 2 = rootid:0x333^0x6000 F:10 F
diff --git a/tests/test-stp-iol-op-3.4 b/tests/test-stp-iol-op-3.4
index 093c64ed..2bcd45e1 100644
--- a/tests/test-stp-iol-op-3.4
+++ b/tests/test-stp-iol-op-3.4
@@ -5,7 +5,7 @@
 bridge 0 0x333^0x6000 = a
 bridge 1 0x222^0x7000 = b
 bridge 2 0x111 = a b
-run 256
+run 1000
 check 0 = rootid:0x333^0x6000 root
 check 1 = rootid:0x333^0x6000 F:20
 check 2 = rootid:0x333^0x6000 F:10 F
diff --git a/tests/test-stp.c b/tests/test-stp.c
index 9b87c0e6..ddb7db7e 100644
--- a/tests/test-stp.c
+++ b/tests/test-stp.c
@@ -38,6 +38,7 @@
 #include <inttypes.h>
 #include <stdarg.h>
 #include <stdlib.h>
+#include "ofpbuf.h"
 #include "packets.h"
 
 struct bpdu {
@@ -97,28 +98,31 @@ new_test_case(void)
 }
 
 static void
-send_bpdu(const void *data, size_t size, int port_no, void *b_)
+send_bpdu(struct ofpbuf *pkt, int port_no, void *b_)
 {
     struct bridge *b = b_;
     struct lan *lan;
-    int i;
 
     assert(port_no < b->n_ports);
     lan = b->ports[port_no];
-    if (!lan) {
-        return;
-    }
-    for (i = 0; i < lan->n_conns; i++) {
-        struct lan_conn *conn = &lan->conns[i];
-        if (conn->bridge != b || conn->port_no != port_no) {
-            struct bridge *dst = conn->bridge;
-            struct bpdu *bpdu = &dst->rxq[dst->rxq_head++ % RXQ_SIZE];
-            assert(dst->rxq_head - dst->rxq_tail <= RXQ_SIZE);
-            bpdu->data = xmemdup(data, size);
-            bpdu->size = size;
-            bpdu->port_no = conn->port_no;
+    if (lan) {
+        const void *data = pkt->l3;
+        size_t size = (char *) ofpbuf_tail(pkt) - (char *) data;
+        int i;
+
+        for (i = 0; i < lan->n_conns; i++) {
+            struct lan_conn *conn = &lan->conns[i];
+            if (conn->bridge != b || conn->port_no != port_no) {
+                struct bridge *dst = conn->bridge;
+                struct bpdu *bpdu = &dst->rxq[dst->rxq_head++ % RXQ_SIZE];
+                assert(dst->rxq_head - dst->rxq_tail <= RXQ_SIZE);
+                bpdu->data = xmemdup(data, size);
+                bpdu->size = size;
+                bpdu->port_no = conn->port_no;
+            }
         }
     }
+    ofpbuf_delete(pkt);
 }
 
 static struct bridge *
@@ -299,7 +303,7 @@ simulate(struct test_case *tc, int granularity)
 {
     int time;
 
-    for (time = 0; time < 256 * 180; time += granularity) {
+    for (time = 0; time < 1000 * 180; time += granularity) {
         int round_trips;
         int i;
 
-- 
2.30.2