The speed selector on the Ports tab was filled with its options and never
given a value, so it read "auto" on every load whatever the port was set
to. Forcing a port to 1G and reloading showed "auto" in the selector
beside a link badge reading 1G.
The advertisement bits ports.json already reports say what the port is
set to: phy_set_speed() writes the same three registers the JSON reads
back, and every forced speed leaves exactly one of the six bits standing
while auto leaves the lot. Map those to the selector, and leave it alone
while it has focus, as the MTU field does.
Values with no entry in the table, such as the 100M setting that
advertises both duplex modes, leave the selector untouched rather than
claim a setting the list cannot express.
port_eee_enable() and port_eee_disable() report the port with
print_byte(), which prints the internal index in hex, so turning EEE off
on port 5 answers "EEE off for 04". port_eee_status() in the same file
already uses print_phys_port(), and the status listing it produces counts
from one, so the two halves of the same command disagreed.
Use print_phys_port() in all three messages.
max_upload was armed once when the upload began and never reduced, so
`max_upload - uip_len > 0` was an unsigned expression that only went
false when a segment arrived whose length matched the limit exactly. The
write pointer climbed from the staging address with nothing to stop it,
while flash_write_bytes() carries the address in three bytes and drops
whatever does not fit.
Subtract each segment as it is taken and compare before subtracting, which
is what the comment there described. The limit was only ever armed for
firmware uploads; configuration uploads keep the bound their own buffer
already imposes.
The multipart branch for /config tests authenticated before it touches
anything; the branch for /upload does not, and the 401 for it is only
raised further down at the dispatch. By then the request has already set
config_upload, uptr, verify_crc, max_upload and pre_acc, and announced a
firmware upload on the console.
Those five are globals while the transfer state they steer is per
connection, and the stack is built for ten connections, so a request that
is about to be rejected can still retarget an upload another client is in
the middle of: a configuration upload continues as a firmware one and its
remaining parts are written to the staging area instead of the
configuration sector.
Apply the same test the neighbouring branch uses, before the assignments.
udp_callbacks() offers every UDP connection event to syslog_callback(),
which decides whether the event is its own by comparing the local port
against state.syslog_conn->lport. The pointer is null until syslog is
started: syslog_init() clears it at boot and syslog_stop() clears it
again, so with syslog off, which is the default, every poll dereferences
null and reads whatever lives at the bottom of xdata.
Should that word ever match the port being offered, the send path runs on
another application's connection, overwrites uip_appdata and drops the
log line it was holding.
Test the pointer first.
leds_setup() walks all four LED sets and writes the high-speed nibble of
each into one of two registers, LED3_0_SET1 for sets 0 and 1 and
LED3_0_SET3 for sets 2 and 3. The second branch reuses the index
expression of the first without rebasing it, so sets 2 and 3 address
sfr_data[-1], [-2], [-3] and [-4].
Two things follow. Sets 2 and 3 never receive their nibble, because the
read-modify-write pair writes back exactly what it read. And four bytes
below sfr_data are overwritten on every boot of every machine: the loop
runs over all four sets whatever the board declares. On this build the
linker places sfr_data directly after the serial receive ring, so the
writes land in the tail of that ring.
leds_dump() reads the same two registers back and already rebases with
(set - 2); use the same form here.
The EEE toggle posted `eee <port> <on|off>`, the reverse of the
`eee [on|off|status] [port] [100m|1g|2g5]` the parser and the
documentation describe, so the switch answered with its usage line and
the checkbox sprang back. The multi-page UI this replaced sent `eee on`
with no port at all, which toggled every port instead of one.
Moving the port to the end breaks the stored-configuration merge, which
builds its drop key as a line prefix: with the port trailing, a per-port
line matches the bare eee entry and replaces every eee line in the
configuration rather than the one for that port. Give the command its own
case beside the bandwidth one, so a line with a port replaces only that
port and a line without one clears them all, and accept the optional
speed word the command has always taken.
Reported by tofurky in issue #459.
parse_eee() sniffs word 2 for a trailing 'g' or 'm' to tell a speed from
a port number, and advances idx past the digits to do so. The port branch
then hands that advanced idx to cmd_parse_port_separator(), which expects
the first character of the number and finds the separator instead, so it
returns 0 and the command is rejected. Every other caller passes
cmd_words_b[].
The result is that no eee command carrying a port number works, in any
argument order, and the stored per-port lines a saved configuration
replays are silently dropped.
Reported by tofurky in issue #459, along with this fix.
The row sits between Hardware and Flash. The firmware sends the formatted string
rather than the raw register, since the System table prints its values as they
arrive.
Costs 186 bytes in BANK1.
The chip has a pair of thermal meters and nothing was reading them. This adds a
`temp` command that prints the current reading next to the one taken at power-on.
The result register at 0x340 splits a 19 bit value across two fields: bits 18:3
live in the low 16 bits, bits 2:0 in bits 18:16, with the sign at bit 18, degrees
in 17:10 and a fraction below. The nice part of that layout is that the degrees
fall out of a shift of the low half, so the conversion stays 16 bit.
The meter comes out of reset latched: the register keeps whatever was measured
when the board powered up and then never moves. Four samples 40 s apart came back
identical down to the raw ADC, and 0x340 matched the power-on register at 0x350.
Clearing bit 2 of the control register at 0x338 during init is what makes it live.
Measured on an SWTGW218AS: 58.7 to 59.5 C while running, with the power-on
register still showing the 49.2 C it started from. For what it is worth, the
scale lines up three ways: the threshold register reads -40 and +125, the slope
from warm-up works out at 4450 LSB/C, and the slope between the two meters at
4388 LSB/C.
We only read the meter that is up after boot. The other sits in reset with no
coefficients, so it would just repeat its power-on value.
Costs 231 bytes in BANK2.
The helper landed between that comment and the function it describes, so the
comment now reads as if it belonged to name_char(). Moving the helper up puts
the two back together. The generated code is unchanged.
gcc flags the else whose only statement is dbg_string(), because that macro
expands to nothing unless DEBUG is on, leaving the branch with an empty body.
The line is older than this branch; the bench is simply the first thing that
compiles httpd.c with a host compiler and -Wall -Wextra.
Turn warnings into errors in the test Makefile too, so the next one is caught
where it appears instead of scrolling past. It is the only warning the four
test binaries produce, so nothing else has to change.
The replay takes a line of at most 126 bytes, so a longer one is skipped
on boot. Neither the upload path nor the editor looked at line length,
so the file the user saved was not the configuration the switch would
come up with.
Reject such a body in config_take(), before the sector is erased, which
the upload path already maps to 400. The editor points at the offending
line and its length, and measures it in bytes rather than characters, so
a name written in a non-ASCII script is counted the way the switch
counts it.
execute_config() replays the saved configuration line by line into
cmd_buffer. A line that does not fit, or one cmd_tokenize() rejects,
ended the replay right there: everything below it was silently left
unapplied, while the file itself and the editor showing it stayed
complete. On a switch here, a 130-byte line placed after the last real
setting was enough to lose every line that followed it on the next boot.
Report the offending line and carry on with the next one. The skip flag
lives across the flash page refill, so a long line that crosses a page
boundary is not resumed halfway and run as a truncated command.
Drives the unmodified httpd.c and uip.c through a handshake and a GET
from a client that moves its receive window, and compares the bytes that
reach the wire with the file they are meant to carry. The served file
holds a position-dependent pattern, so a repeat or a gap is reported at
its offset rather than as content that differs somewhere.
The first scenario holds the window still and must pass: it is the check
that the bench itself is sound, before the two moving-window scenarios
are believed.
uip_mss() is the limit on the next segment, not a record of the last
one: uIP recomputes it from the window the peer advertises in every
incoming segment, before the application is called. The ACK path
subtracted it from slen and added it to o_idx, and the retransmit path
handed it to uip_send(), so whenever the window moved between a segment
going out and its ACK coming back, the file being served lost or
repeated the difference. A window shrinking from 1460 to 600 between the
two repeats 860 bytes under fresh sequence numbers, which a browser
writes into the file; a window opening the same way skips 860.
Remember the length in tx_send() and use it in both places. The
retransmit path then repeats exactly the segment uIP still counts as
outstanding, which is what uip_connr->len makes it put on the wire.
uip_appdata is void *, so p - uip_appdata subtracts pointers of
different types. SDCC accepts it, C does not, and no host compiler will
build the file for a unit test. Cast to the type the byte count is meant
in; the generated firmware code is unchanged.
The vendor, model and serial strings are filtered while the EEPROM read
fills the buffers, so the page copies them out as they are. NUL still
ends a string, so a field shorter than its slot reads the same as before.
The three loops in send_status() become one call each. strcpy() cannot
return the length: rtl837x_common.h hides that prototype under
RTLP_HOST_TEST so host builds take the C library version, and a call
that adds its result to slen does not compile there. A small static
helper does the copy instead, and bank 1 drops by 96 bytes.
The console, POST /cmd and the saved config all go through the command
parser, so one filter there covers every way a name can be set. The
hostname already did this inline; port and VLAN names now do it too.
The SFP strings still need escaping on the way out: they come from the
module's EEPROM, not from a command.
A port named NAS"1 made status.json unparsable, so the page stopped
loading, and the name survives a reboot in the saved config. VLAN names
and the SFP strings have the same problem.
They now go through a helper that replaces ", \ and non-printable bytes
with a dot, the way the hostname is already sanitized on ingest. The
length does not change, so the size checks still hold.
The helper also covers the 0xa0 byte that send_sfp_info() skipped as a
special case, so that check is gone.
Compiling these two files for the host turns up warnings that SDCC does
not print: four places mix & with | and two mix && with ||, and l2_delete
keeps a counter it never reads. The parentheses only say what the
compiler already does, so the generated code is unchanged.
counter_to_html() reads the SFRs directly for the zero test, which saves
14 bytes of bank 1.
page_impl.c and rtl837x_port.c are linked together over the simulated
engine, so /vlan.json, /vlanlist and /l2.json are checked against what
vlan_create, pvid and the L2 table hold, through the same bytes the switch
would. The L2 walk is driven the way the web page drives it, page after
page until an index repeats, which is also where the repeat of the first
entry is pinned down as the wrap marker the page relies on. The counter
test fails on the code before the previous commit.
send_status wrote the high and the low word of a packet counter one after
the other, each without its leading zeros. That reads fine until a port
passes four billion packets; from then on a low word of 5 prints as "0x15"
next to a high word of 1, and the rates the page derives from the
difference between two readings jump around. send_counters already padded
the low word, so both now share one helper that pads it only when the high
word is set, which also drops the leading "0" the counters page used to
print in front of every small value.
The switch is reached through four register functions and the SFR_DATA
registers, so a flat register file behind them is enough to run the table
code on the host. A few addresses are live: writing TBL_CTRL with EXECUTE
performs the VLAN or L2 operation at once and clears the bit, STAT_GET
fetches a counter, L2_TBL_FLUSH_CTRL drops entries. Nothing ever reports
busy, so a polling loop runs once and the tests say nothing about what
happens when hardware stalls; that question was settled elsewhere.
The header states the VLAN and L2 entry layouts on its own, so a test can
hold what rtl837x_port.c writes against a second reading of the same bits
rather than against itself. The first suite does that for the VLAN entry,
the PVID register two ports share, the static multicast and management
entries, the per-port flush and the trunk registers. Two stub headers stand
in for the ones a firmware build generates, so the harness needs no SDCC
build first.
Two things kept every register-driven module out of the host harness. The
SFR keywords were unknown, so rtl837x_sfr.h did not compile at all, and once
it did, each translation unit would have defined its own copy of the
registers. They map to extern declarations now; a mock emits the storage
once by re-including the header with the keywords redefined.
The other was memset. rtl837x_common.h hides the firmware's own prototypes
under RTLP_HOST_TEST so host builds use the C library, but nothing declared
the library versions, and the modules do call them. The shim pulls in
string.h ahead of everything, where nothing can clash with it.
The messages said Error: themselves, or usage:, or nothing at all.
cmd_error() prints the prefix now, so every rejected command answers the
same way and the strings only carry what is specific to them.
Two messages that started with Error or refusing are reworded so the
prefix does not read twice. The syslog usage text becomes one string
instead of three calls.
The session cookie carried neither Expires nor Max-Age, so browsers that
clean up session cookies aggressively (LibreWolf, for one) may drop it
while the session is still valid. Send Max-Age=session_timeout so the
declared lifetime matches the timeout already enforced by the firmware.
itoa16_html() only converted four digits, so extend it to five to print
timeouts up to 65535. That also fixes information.json, which used the
same helper and reported wrong values from 10000 s up.
Adds a numeric input next to the syslog settings to read and set the
session timeout without knowing the command. The current value is
reported in information.json, so the field also shows the default the
firmware was built with.
Costs 86 bytes of code (BANK1) and nothing in RAM.
Andreas Erhard
bloqaudio
René van Dorst
d00f
Sylwester Petela
Andreas Erhard
Rob Emery
Rob Emery
gnacho