The workflow only ran on push, so a pull request opened from a fork
never got a build, and neither did the host-side unit tests that run in
the same job.
session_id is cleared at boot and only a successful login fills it in,
while the uptime counter and last_session_use both start at 0. For the
first SESSION_TIMEOUT seconds after a start, a request carrying an empty
"Cookie: session=" therefore passes is_word_x(session, session_id) and
is treated as logged in, which is enough for /cmd, /config and /upload.
Only look at the cookie once a session has been handed out.
flash_read_security() ends its loop the same way flash_read_bulk() did
before the previous commit: only through comparisons with 1, 2, 3 and 4. A length of
zero matches none of them, the following subtraction wraps on uint16_t
and the loop keeps printing until the counter comes back down to 4.
No caller passes zero today, since the only one is the CLI with a
constant length of 40, but the bound belongs in the function rather
than in whoever calls it.
flash_read_bulk() ends its copy loop only through the comparisons
len == 1|2|3|4, so a length of zero matches none of them. The
following len -= 4 wraps on uint16_t to 65532 and reaches 4 only
after 16384 iterations, writing 65536 bytes to the destination and
overwriting the whole XDATA space.
send_config() gets there with valid_len 0 whenever the stored
configuration begins with a NUL byte, which is the state left by
uploading an empty configuration file. Every later request for the
configuration page then clears the management VLAN and the STP
flag and replaces the host address, while the switch keeps running
and keeps forwarding traffic.
The harness builds with the gcc the workflow already installs and runs
in under a second, so it goes before the SDCC steps and fails the build
first when a host-visible regression lands.
Building the firmware headers on the host showed two warnings from
machine.h: `typedef struct machine { ... };` and the same for
machine_runtime declare no alias, so the keyword only draws "useless
storage class specifier in empty declaration". Every user says
`struct machine`, so drop the keyword.
-Wno-builtin-declaration-mismatch is a gcc switch and clang reports it
as an unknown option on every build; add it only when the compiler is
not clang.
Compile selected firmware translation units unmodified under host gcc with
AddressSanitizer and UBSan, force-including a shim header that erases the
SDCC 8051 keywords, and link them against a mocked serial ring and console.
A first suite covers the CLI line editor: line entry, backspace editing,
and both long-input overflow cases (length cap and ring-pointer stall).
The only firmware-source change is one #ifndef RTLP_HOST_TEST block in
rtl837x_common.h grouping the libc-named prototypes (sleep/memcpy/memset/
strlen/strcpy/strcmp) so host builds take them from the C library instead;
on-target builds are unaffected.
Run with: make -C test
BANK2 outgrew its 48 KB window on PCB_SWTG018AS_V2_1_0 (49182 bytes, 30
over) once the spanning tree code learnt about link aggregation, and the
image builder refuses to link. Only KP_9000_6XHML_X2 is built in CI, so
the overflow was not caught there.
dhcp.c is self-contained: its three exports are __banked, the rest of
the module is only reached from inside it, it holds no __code pointers,
and its uip calls already crossed into BANK1. Moving the pragma is the
whole change. BANK2 49182 -> 43691 on PCB_SWTG018AS_V2_1_0, BANK3 5709
-> 11206, BANK1 +6 for the trampolines; the generated assembly shows no
plain lcall leaving the module's bank.
The firmware lists a LAG as its own entity in /stp.json (`lag` and `mbr`,
`p` 0) and drops the member ports from the list; its settings take
`stp lag <n>`. Give the group its own row in both spanning tree tables,
named with its members, key the cells by entity rather than port number,
and rebuild the tables whenever the entity set or a membership changes so
a group created on the LAG tab appears without a reload. The `stp lag`
commands are recognised for the unsaved-changes badge and the startup
config merge alongside `stp port`.
The rename of the entity index turned `stp port <n>` into `stp ent <n>`
in the parser only. doc/stp.md, the web UI and every saved startup
config still spell it `stp port`, so per-port settings were rejected on
the console and silently dropped when the startup config was replayed at
boot. Restore the keyword; `stp lag <n>` is unchanged.
A dark receiver reports 0 mW, which printed as -Infinity dBm. The DOM
value has a resolution of 0.1 uW, so -40 dBm is the lowest reading the
module can express; clamp there.
The header carried the save button, the language selector and the theme
selector on every section, although the latter two are set once. They
now live in a Display card in the System section; the header keeps the
title, the unsaved-changes badge and Save to flash.
The forwarding database is read a page at a time; leaving the MAC table
or ports section let the walk carry on through the request queue. Give
the walk a generation number that the leave hooks bump, and stop the
walk at the next page when it is stale.
The VLAN table is one /vlanlist plus one /vlan.json per VLAN. A failed
per-VLAN request rejected the chain, so every VLAN after it was missing
from the table, indistinguishable from VLANs that do not exist. Keep the
row with the id and name from the list and a ? in the port columns.
Save to flash showed the merged startup config read-only, so a stray line
had to be fixed afterwards in the System section. The preview is now a
textarea: the size and unknown-line notes follow the edits, the write
button is disabled above 2048 bytes, and what is written is the edited
text.
The default hostname (RTLPlayground- plus six hex digits) overflows the
200 px sidebar; clip it with an ellipsis and keep the full name in the
title. The MTU and Mbit/s fields were 70 px, which leaves no room for five
digits next to a number spinner. The main column stops at 1400 px instead
of 1100 px so the dashboard cards sit side by side on a wide screen.
The old dashboard showed a hover tooltip per port with the link state and,
for SFP ports, the module and its DOM readings, and rewrote it on every
poll. The single-page app replaced it with a click-to-open dialog that was
a snapshot: the values did not change while it was open.
Build the rows once per port and use them for both: the dialog is
re-rendered on every status poll while it is open, and each port in the
strip carries the same rows as its title.
The firmware emits every per-port array in logical port order, which on a
board with a remapped port table (SWGT024: phys_to_log = {4,5,6,7,8,3,...})
is 6, 1, 2, 3, 4, 5. The old pages iterated over physical port numbers and
looked each entry up; the single-page app rendered the arrays as received,
so the ports, spanning tree, EEE and bandwidth tables and the dashboard
strip came out in logical order, and the strip put the SFP marker and the
detail view on the wrong port.
Sort every array by portNum on receipt so index i is port i+1 throughout.
The pages become one index.html holding every section, one app.js and a
self-contained login.html. The layout is a sidebar with light, dark and
Selenized themes (or follow the OS), a live dashboard with per-port
packet rates, and confirmation dialogs in front of the actions that can
lock the operator out.
Behaviour changes against the previous pages:
- every request goes through one queue that waits for the previous
response BODY rather than its headers: the firmware shares a single
output buffer across connections and delimits responses by closing,
so a second request in flight corrupts the first
- "Save to flash" merges the command log into the startup configuration
with the grammar the console accepts, shows the result before writing,
verifies the sector by reading it back and clears the command log only
after that; the header shows an "unsaved changes" badge while the
running configuration differs from flash
- the console section shows the output the firmware returns for a
command, including the parser's explanation on a rejected one
- the firmware update section checks size, bank header and CRC16 of the
image in the browser before uploading, shows upload progress and waits
for the switch to come back after the reset
- the ports table shows the learned MAC of a port when there is exactly
one and the count otherwise; the MAC table sorts per column, filters,
and names the LAG a learned address sits behind
- the spanning tree section sends only the settings that changed on
Apply instead of one command per control event
- link aggregation groups are numbered 1-4 like the command line, and
applying an empty group clears it
The translation table is inline in app.js with English, Japanese and
Chinese. The language selector sits in the page header and reloads the
page so that dynamically built tables are rebuilt in the new language.
Only login.html and the favicon are served before authentication; the
stylesheet, the port images and the old script bundle no longer exist.
The simulator's allowlist follows the firmware's.
The session timer was refreshed only when a page from the file table was
served, so a client that loads its pages once and then talks to the JSON
endpoints alone is logged out SESSION_TIMEOUT seconds after the load,
however busy it is. A firmware upload started after that point is answered
with 401 instead of a verdict. Refresh the timer wherever the session
cookie validates, which covers every authenticated request, and drop the
file-serving refresh that this makes redundant.
boot.c calls it from BANK3 and only rtlplayground.c had a prototype, so
the compiler saw an implicit declaration. The call itself was fine, the
function lives in the common area, but the warning was real. Move the
prototype into the shared header.
Reported by vDorst.
sfp.c takes handle_sfp, sfp_module_read, sfp_apply_quirks, sfp_get_info,
sfp_read_field, sfp_print_info, setup_sfp_gpio, the quirk table and the module
state that goes with them. Everything exported is __banked: handle_sfp() and
sfp_print_info() are reached from cmd_parser.c in BANK2, the rest from the
common area, and both are bank boundaries.
The quirk table needed checking rather than assuming. Its entries are __code
pointers, and a __code pointer carries no bank, so passing one to a function in
a different bank hands it an offset read through the wrong window. That is safe
here because strcmp() sits in the common segment at 0x51f and is reached with a
plain lcall, so the bank register still says 3 while it runs. Verified in the
built image: the table at 0x35580 holds 0x563c and 0x5644, which resolve to
"QSFPTEK" and "QT-SFP+-T" inside BANK3.
Common segment 13393 -> 8432 bytes, leaving 7492 free rather than 2531. BANK3
holds 5710, BANK1 is unchanged and BANK2 grows by 31 for the trampolines.
All 34 machines in machine.h build and link, and the same walk over the
generated assembly finds no plain lcall crossing a bank boundary.
The common segment is the scarce one: sixteen kilobytes shared by every bank,
and before this it had about 2.5 kB left. Most of what moves here runs once at
startup and does not have to sit in it. The two SerDes helpers are the
exception: the link-change handler and the SFP polling both reach sds_config()
while the switch is running, so they are not startup-only. None of it is
reached from an interrupt.
boot.c takes setup_i2c, init_smi, nic_setup, rtl8373_revision, rtl8224_enable,
early_boot_handle_button, set_hostname_default and the two SerDes helpers.
BANK3 is enabled with the linker flag; nothing else in the build needed
changing.
Everything moved is declared __banked, since every caller is outside BANK3:
main() and friends run from the common area and rtl837x_init.c calls
sds_config() and sds_config_mac() from BANK2. The calls the moved code makes
outward were already banked, so that side needed no work.
Common segment 13393 -> 10137 bytes, leaving 5787 free rather than 2531.
BANK3 holds 3732. All 34 machines in machine.h build and link. Walking
the generated assembly for plain lcalls that cross a bank boundary finds none
among 2808 calls; those build cleanly and only fail at run time, so it seemed
worth checking by machine rather than by eye.
IPBUF is laid out for frames we send, with the 12 byte TX descriptor in
front of the Ethernet header. A received frame starts with the Ethernet
header at uip_buf[0] and carries the RTL and VLAN tags after the
addresses, so IPBUF->ethhdr.src read the port mask and both VLAN fields
as a MAC. Every IP packet from the local network then replaced the
sender's ARP entry with that, and our replies went to an address nobody
has. The IP source address sits at the same offset in both layouts,
which is why the rest looked right.
Use BUF, the receive view uip_arp_arpin() already works from.
bennydiamond
René van Dorst
d00f
bloqaudio
logicog
David Warren