handle_sfp() called sfp_module_read() as soon as it saw a module appear, and
that path opened with delay(100). delay() spins on PCON until the tick counter
runs down, so the main loop stood still for 100 ticks - half a second at
SYS_TICK_HZ 200. Nothing else ran in that window: no STP, no LACP, no packet
handling. A diagnostic build measured the sfp phase at up to 107 ticks, the
longest stall of any phase in the loop.
Remember when the module showed up and let the loop continue. The I2C read
happens on a later pass, once the wake-up period has elapsed. Pulling a module
cancels a pending read, and a failed read still restores the detect bit, so the
existing retry-on-the-next-poll behaviour is unchanged.
The comparison is written out rather than using a helper so that this does not
depend on anything outside the current tree. The pending flag is an array
indexed by slot, like the other per-slot state in this file.
Cost on SWTGW218AS, measured against the branch point: BANK3 11173 -> 11264,
CSEG, BANK1 and BANK2 unchanged, plus 4 bytes of xdata.
Some generic 1.25gbps DAC cables were seen to have a NUL *after* the
space padding in the model field. This caused the normalization to miss
the space padding in the string. The vendor field did not have the
issue.
Note that this seems to be against spec per SFF-8472, which mentions
only space padding or *all* NUL, not a combination of both. In the case
of all NUL, we'll waste a few cycles with no-op writes.
Building with SDCC 4.6.0 emits tons of warnings e.g.:
cmd_parser.c:31: warning 356: object in read-only code space should be const
cmd_parser.c:36: warning 357: pointer to object in read-only code space should be pointer to const
These of course become fatal when building with --Werror.
Comparing resultant binaries, it seems that a single string is shifted
(deduplication?) but the numbers emitted at the end of the build are
otherwise identical.
idle() went to sleep before it looked at the receive buffer, so a packet that
arrived just after the loop dozed off waited for the next tick before anything
touched it. Every TCP round-trip paid that toll, which is why the web UI felt
sluggish even though no phase of the loop was anywhere near busy.
Check the receive buffer first and only sleep when it is empty. The read takes
its 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 this change alone:
web UI, median of 15 requests for the main page 88 ms -> 76 ms
CPU port under a one-way UDP stream 159 -> 230 packets/s
The loop still sleeps normally when there is nothing to do, so this does not
turn the core into a busy-wait: loops/ticks stays at 0.99 on an idle switch,
and rises above 1.0 only while packets are actually queued.
Costs 12 bytes of code and no extra XDATA.
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.
All the possibilities are handled, or otherwise a compile-time macro
I2CBUS_VALID ensures invalid options are rejected with a compiler error.
Also fix a compiler warning.
Use 0x7 as error value for an invalid I2C SDA/SCL-pin value.
The macro `I2CBUS_VALID` checks for this value.
When a invalid pin is ues, the compiler errors out with:
"machine.c:<line>: error 176: sizeof applied to an incomplete type",
Error message is not ideal but at-least points to the right location in
the code.
We can reduce the i2c-settings to just one byte, and this new const value
can directly be used by the I2C-peripheral without additional computation.
This saves 1-xdata load and a lot of bit-shifting.
I see a codesize savings of +200 bytes.
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 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.
René van Dorst
DrDoof
Matt Merhar
Andreas Erhard
Fredrik Gustafsson
gnacho
SkullyDev
SkullyDev
bloqaudio