With UIP_CONF_MAX_CONNECTIONS set to one, a verified upload can leave
reset intent set when its response is never acknowledged. A subsequent
TCP connection reuses that slot, and ACKing its plain GET response can
otherwise trigger reset_chip(). This is sequential slot reuse, not
concurrent HTTP connections.
Store reset intent in the connection appstate and initialize both tstate
and fw_reset_pending on every uip_connected() event, regardless of stale
appstate contents. Reset only after the verified upload response is fully
acknowledged.
Exercise the real HTTP callback for all stale tstate bytes, sequential
slot reuse and a later GET, partial/final ACKs, abort, timeout and CRC
failure. Keep the regression tests in the existing tab-indented style.
Move port_bit() to rtl837x_common.h as PORT_BIT(), a uint16_t shift,
and MAC_ADDR_LEN from rtl837x_lldp.h next to it, and use MAC_ADDR_LEN
for every address in the LACP code instead of a literal 6. Define
lacp_mux_update() before its first caller and drop the forward
declaration, declare variables at the top of their blocks, put the
operator at the end of a continued line, drop the field value comments
of the LACPDU struct, and leave lacp_port_lag to lacp_init() instead of
initialising it in the test environment.
The comments repeated the documentation, described tests or explained
the code to its author. What stays describes the LACPDU structures. The
timer constants are decimal, in units of the LACP timer.
Listing by next-address also returned the reserved-multicast entries STP
writes, shown as port 1 with a delete button, while l2_delete() still
looks up by next-unicast and so removed the unicast entry after that
index instead. The change was there to show the LACP steering entries;
delivery by trap writes none, so it goes.
Dynamic link aggregation on top of the static LAGG, for RTL8372/8373:
receive, selection and mux state machines (coupled mux, one aggregator
per group), with "lacp show" to look inside. Each of the four trunk
groups runs its own aggregator on the candidate ports assigned with
"lag <n> lacp <ports>"; the engine starts with the first such group and
stops with the last.
LACPDUs reach the CPU through the reserved-multicast trap, so delivery
needs no L2 entry and nothing written per VLAN or per PVID. The trap
acts on the address before the VLAN lookup and before the ingress
filters, so a LACPDU also arrives on a port that admits tagged frames
only and on a port STP holds in blocking, neither of which a forwarded
one reaches. A trapped frame carries no VLAN tag,
so lacp_in() takes the offset of the PDU body from TRAP_RX_BODY(). The
action is set with trap_rma_set(), which leaves the flags sharing that
register alone, and "lacp off" puts the slow-protocols drop default
back. doc/lacp.md and doc/link_aggregation.md record what was measured,
including why the trap looked like a dead end at first.
The web UI gets a Static/LACP mode per group on the LAG page with live
status, and the startup-config validator knows the per-group syntax. A
host-side sandbox for the state machines runs in the test harness
together with the LAG hash check. It drives both forms of an incoming
LACPDU, so the body offset of a trapped one is covered as well.
The L2 table page iterated with the "next unicast" read method, so
static multicast entries such as an IGMP group were invisible. It now
iterates with "next address", skips L3 entries (their layout carries IP
addresses where a MAC is expected), and reports the lowest member of a
multicast entry's port mask, so such an entry shows up as a static
entry on that port.
A port taken out of LACP forgets its partner, so "lacp show" after
"lacp off" no longer reports one. Applying the same candidate set to a
group again, as the LAG page does on every Apply, leaves the trunk
alone instead of tearing it down for a second or two. lacp_in() checks
the frame length and ethertype before reading a LACPDU, so a runt or a
foreign frame on the slow-protocols address cannot leave stale buffer
bytes as a partner. The sandbox covers these three in T20-T22. On the
LAG page, LACP mode with no ports checked sends "lag N lacp off", and
the startup-config validator wants at least one port after "lacp".
Needed to increase buffer size for this to send app.js
The logical port status is what is returned in the array, for
non-sequential systems we need to map through physToLog rather
than just subtracting 1
physToLog is 1 based! not 0; also fixing up typecasts
Reimplementing using an array of objects to represent ports.
The previous approach of a bitmask of logical ports, was just too
simple on devices such as the SWGT024_V2_0_MANAGED which has
gaps in the port assignments.
timing with ticks
Fixing up the log_to_phys_port assignment
2000 is about every 10 seconds, so scaling up to once every 30ish
Removing the attempt to block lldp frames,
This does not work however the hard work has been done for us in
the #514 so we can borrow that work later on
boot_to_html outputs 1 and 0 not true and false, making the sim match
the real implementation.
lldp.json has a hanging , in the array; so overwriting it
Moving the global on/off onto the LLDP page
Building a table of LLDP status from the S ports. Later on, if
we wish to implement an lldp receiver we can display the remote
details here
Adding support to the cmd parser for the lldp commands
The switch keeps a count of the dynamic L2 entries learned on every
port in L2_LRN_PORT_CONSTRT_CNT (0x53ac + 4 * port, 13 bits). `stat`
prints it in a new MACs column and /status.json carries it as "macs".
On a SWTGW218AS the count matched the L2 table port by port, static
entries are not included, and taking a port down drops it to 0.
A matrix of the ports, one row per ingress port, showing which ports a
frame entering there may leave through. The port number of a row turns
red when the port is not fully open, and the cell for the port itself is
marked since it is always allowed. Apply sends one "isolate" command per
changed row, and Allow all ticks the whole matrix.
The switch side is a new /isolation.json, which reports each port's
allowance mask read back from the hardware. The page is an Isolation
section of the single-page UI.
Storm control used meters 0 to 39, the same meters the ACL hands out,
so a storm limit and an acl meter could overwrite each other. It now
uses 24 + port * 4 + type, meters 24 to 63.
parse_storm() is no longer reentrant and uses plain locals, looks the
type up in a while loop and returns straight from the one-line cases.
storm_show() uses plain locals too.
The documentation says how IGMP decides between mcast and umcast, how
to get from a meter index to its limit, where the IFG bit of a meter
is, and what an ingress bandwidth limit on the same port does to the
storm limit.
The server timeout is an inactivity timeout: scan_header() refreshes
last_session_use on every authenticated request. Max-Age counts from
login instead, and the cookie is only set there, so the browser deletes
it session_timeout seconds after logging in however busy the session
is. Leave expiry to the server, which already handles it.
The five-digit itoa16_html() stays: information.json must print
timeouts from 10000 to 65535 correctly.
edc469f made max_upload count, which bounds how many bytes a request may
carry. Nothing bounds where those bytes land. The two are not the same
check: the first watches the request, the second watches the flash address.
uptr walks forward one page at a time from FIRMWARE_UPLOAD_START and has no
ceiling. A body whose closing boundary never arrives keeps writing page after
page past the staging region, and on a flash exactly twice FIRMWARE_UPLOAD_START
the address wraps onto the running image at zero, so a bad upload takes the
working firmware with it. That commit's own message notes the related hazard:
flash_write_bytes() carries the address in three bytes and drops what does not
fit.
Bound it where uptr actually moves: refuse the next page once it would leave
the area check_and_flash_update_image() reads back, and answer 400. A partial
image left behind is harmless - boot rejects it on CRC and erases it.
This rebase drops the part of the original patch that removed max_upload;
the length limit and the address limit now sit side by side.
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.
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.
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.
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.
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.
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.
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 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.
Save to flash ran the stored config through the same filter as the
command log, so stored lines the page does not know were dropped. The
stored lines are now carried over as they are and only the log is
filtered.
ingress only changes the ports it lists, but the merge replaced every
stored ingress line. It now merges per port; "ingress t|u|a" still
replaces everything, and per-port settings an all-ports line already
overrode are not revived.
Write from the config editor no longer clears the command log or the
unsaved marker, since the edited text does not hold the changes made on
other pages; only Save, which merges them, clears the log.
The command log grows from 1 kB to 2 kB, which still fits the 2500 byte
output buffer, and send_cmd_log() stops at the end of that buffer.
handle_post() takes the request path from byte 6 of the segment it is
given. For the first segment of a POST that is the path, but for a later
segment of a multipart upload it is whatever part of the file happens to
sit there. The cmd and login handlers were matched before the multipart
state was checked, so a segment of a config or firmware upload with
"cmd " at that offset ran the rest of the segment as CLI commands, and
one with "login " there aborted the upload with 400.
Check for an upload in progress, and for new upload and config requests,
before looking at cmd and login. A new POST cannot match both sets of
words, so nothing changes for requests that are not uploads.
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.
avicarmeli
bloq
João Silva
HL Yi
René van Dorst
d00f
Rob Emery
gnacho
feelfree69
Matt Merhar
bloqaudio