handle_rx() picked up exactly one packet per iteration of the main loop. Since
the loop sleeps until the next tick, that capped the CPU port at roughly one
packet per tick no matter how little work it had to do. A burst of broadcast,
ARP or IGMP traffic aimed at the CPU queued up behind that and pushed out the
STP and LACP timers.
Drain up to RX_BUDGET packets in one pass instead. The budget matters: without
one a sustained flood would spin here and starve everything else in the loop.
The two register reads in the loop take their result straight from
SFR_DATA_U16 rather than going through reg_read_m() and the sfr_data[] copy,
which is what the rest of the tree already does.
Measured on SWTGW218AS with a one-way UDP stream, counting the packets the CPU
actually processed, with this change alone:
before 159 packets/s
RX_BUDGET 4 245 packets/s, loop iteration ~16 ms
RX_BUDGET 8 298 packets/s, loop iteration ~26 ms
Throughput is already most of the way there at 4, so the larger budget mostly
buys a longer worst-case delay for the other phases. STP stayed responsive in
both cases.
Costs 71 bytes of code and no extra XDATA.
The TODO next to the disabled TCP checksum code asks which bit of the RX tag
carries the result. It is bit 2 of byte 1: the hardware sets it when the L4
checksum is bad.
That answers the question for UDP as well, which was not being checked either
because UIP_CONF_UDP_CHECKSUMS is 0. Both paths now read the flag instead of
running a checksum over the payload on the 8051.
A zero UDP checksum, which is optional in IPv4, is not flagged by the hardware,
so DHCP keeps working and no extra condition is needed.
rx_headers lives in rtlplayground.c, which the host tests do not link, so
test_httpd_tx carries the storage itself the way it already carries uip_buf.
A zeroed descriptor reads as a good checksum, so the existing scenarios are
unaffected.
A new scenario sets the flag on a request and expects it counted as a checksum
error and left unanswered, then sends the same request with the flag clear and
expects it served. Built against uip.c without this change, those first two
checks fail and the two control checks still pass, so the scenario tells the
two versions apart rather than merely exercising the code.
Costs 156 bytes of code on SWTGW218AS and no extra XDATA.
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 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.
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.
d00f
René van Dorst
Fredrik Gustafsson
gnacho
SkullyDev
SkullyDev
bloqaudio
Sylwester Petela
Rob Emery
Rob Emery