Option 58 is optional, and dhcp_state.renewal is only ever written by
parse_opts(), so a server that leaves it out keeps it at zero. The ACK
handler then sets dhcp_timer to zero, and the callback treats a zero timer
in LEASING as time to renew, so it sends a REQUEST on the next pass.
That REQUEST does set a 30 second timeout, but the ACK that answers it
comes back through the same path and zeroes the timer again, so the two
sides trade REQUEST and ACK as fast as the round trip allows, with the
console printing the whole lease each time round.
RFC 2131 has the client renew at half the lease when the server does not
say, so use that, and fall back to a fixed value if there is no lease
either. Both only fill in a value that is missing: a server that asks for a
short renewal still gets exactly what it asked for, which a blanket lower
bound would have overridden.
Not reproduced here, that needs a server configured without the option.
Leaving SFP ports out of the table is deliberate for now, and being able to
force a module's speed belongs in a feature of its own rather than arriving
inside a fix, so that half goes again.
What remains is the off-by-one: createPortTable() read pState[i] where the
array is filled by portNum - 1, so every row started out showing the next
port's link speed until the first refresh corrected it. The debug line above
it goes too; it printed a third index, pState[i-2].
The id was built and checked with HTONS(), which casts to 16 bits, so the
top half of the 32-bit transaction id was thrown away and the rest byte
swapped. The id is opaque: the client only has to receive its own value
back, so there is no byte order to honour and no reason to narrow it.
Assign and compare the field directly, so the full 32-bit id round-trips.
Fold the "group with no ports" case into the argument-count guard
(cmd_words_len < 3) instead of a separate check, and accept only "d" for
delete, dropping the longer "delete" spelling. Update the serial-console
docs to match.
The EX0..EX3 handlers each emitted a single debug character through
write_char(), which busy-waits on the serial TX-empty flag that is set by
the serial interrupt. If one of these handlers runs while a byte is being
transmitted, it waits for a flag that the equal-or-lower-priority serial
interrupt cannot deliver, hanging the handler with the system tick stopped.
The characters were only ever debug markers, so drop them; the handlers now
just acknowledge the interrupt. No interrupt service routine should call the
blocking console path.
uip_arp_timer() was defined but never called, so ARP entries were only
refreshed on new learning and only removed under table pressure. A cached
entry for a peer whose MAC later changes (a gateway failover, a replaced
device) is then used until it is evicted, and traffic to anything reached
through it is sent to the old MAC and lost, while on-link traffic is
unaffected.
uIP expects the timer roughly every ten seconds; drive it from the
once-per-second housekeeping in idle().
The guard read `if (!DHCP_P->tid == HTONS(...))`. Unary `!` binds tighter
than `==`, so this evaluated `(!tid) == HTONS(id)`, i.e. `(tid ? 0 : 1) ==
HTONS(id)`, which is false for any non-zero id and never returns. The reply
was therefore accepted regardless of its transaction id, so a broadcast
reply meant for another client on the segment was parsed as ours.
Compare the field directly. The id is stored network-order in the packet
and host-order in the state, matching the HTONS() used when the request is
built, so `DHCP_P->tid != HTONS(dhcp_state.transaction_id)` is the intended
test.
The port table skipped every SFP port outright, so on a SWTGW218AS the
uplink simply was not there. Of the seven columns only Set Speed is
meaningless for a module: the name, the link speed and the devices behind
the port all are worth showing, and the name in particular was accepted on
another page and then never displayed anywhere. The MTU table further down
the same page has always drawn the port, so the page disagreed with itself.
Give an SFP port its row and leave the three control cells empty. Whether
speed and disable should work for a module is a separate question and
untested, so this does not offer a button for it.
The link state was read one port too far. main.js fills pState by
portNum - 1, updatePortTable() reads pState[i-1], and createPortTable()
read pState[i], so every row started out showing the next port's link
speed until the first refresh corrected it. On the last copper port that
meant showing the uplink's speed. Feeding the real status.json of a nine
port switch through both versions:
expected 7 7 7 1 7 1 7 7 6
before 7 7 1 7 1 7 7 6
after 7 7 7 1 7 1 7 7 6
Dropping the skip also removes a trap rather than papering over it.
updatePortTable() addresses rows as tbl.rows[i], which only lined up with
port i because the SFP port happens to be last on this board. With a
module in the middle every row after it would have been off by one.
A device on an aggregate is learned against whichever member the frame came in
on, and the hash moves that around: the same address changes port every time
the flow mix changes. The table was telling the truth and there was no way to
read it, since nothing on the page or the console says those two ports are one
link.
port_lag_of() already exists and already knows the answer, so both views ask
it. The console prints LAG1 in place of the member, and /l2.json carries a
"lag" field beside the port, 0 when the port stands alone, which is the shape
/stp.json already uses.
Costs 151 bytes of BANK1.
Two small things about the same command.
The usage line offered "lag <1-4> [port]...", which leaves out "lag show" even
though that is the only way to see the groups, so the one subcommand a reader
most needs is the one the error does not mention.
And a group number with no ports after it quietly wiped the group. That is a
reasonable thing to want and an unreasonable thing to do by accident, as it is
one fat-fingered return away from a bare "lag 1". It now takes "lag 1 delete"
or "lag 1 d", and a bare group number prints the usage instead.
Nothing changes for a saved configuration: "lag 1 5 6" is what gets written and
what gets replayed, and neither the old bare form nor the new delete matches
the pattern that decides what is persisted.
The LAG page draws four sections headed LAG 1 to LAG 4 and wires their buttons
to lagSub(0) through lagSub(3), which then sends the index straight through as
the group number. So the first button asks for "lag 0", which the parser
rejects, and the page shows nothing at all because the error only goes to the
console. Every other button is off by one: the section headed LAG 2 creates
group 1.
The index stays 0-based for the element ids and only the command gets the
1-based number the CLI documents.
* uip: time out an ESTABLISHED connection whose peer went silent
uIP has no keepalive and no idle timeout for an ESTABLISHED connection, so
a peer that vanishes without FIN or RST holds it open forever. A crashed
client does it, and so does a SYN caught while the switch is still
booting. With UIP_CONF_MAX_CONNECTIONS = 1 that wedges the single slot:
the web UI stops answering, port 80 no longer even SYN-ACKs, and only a
power cycle recovers it. Every other TCP state already has an exit path
(UIP_MAXRTX, UIP_TIME_WAIT_TIMEOUT); idle ESTABLISHED was the one hole.
Close it inside uIP rather than in httpd, so any future TCP service is
covered too. The retransmission timer is unused while nothing is in
flight, so the idle count lives in it: a 1 Hz strobe derived from the
uip_periodic() rate ages it, any arriving segment clears it, and past the
limit the connection is aborted through the same path as UIP_MAXRTX. Two
new options in uipopt.h, default off; this project enables 30 s.
httpd also joins uip_timedout() to its cleanup: it only handled closed
and aborted, so both this timeout and the existing UIP_MAXRTX one would
have left the transfer state stuck on a dead connection.
* uip: keep the tuning values out of the prose
The comment named the timeout and the tick rate, which go stale the
moment either constant is edited. It now says why an idle connection
is aged out and leaves the numbers to the defines under it.
The /cmd path carried its own copy of the plain-text response header,
which was the same string page_impl.c already keeps in
HTTP_RESPONCE_TXT, minus one difference: the shared one never said
Connection: close.
This httpd closes after every response, so a reply that does not
advertise it invites the browser to reuse the socket and lose the next
request. HTTP_RESPONCE_JSON has always said so; the text one was the
odd one out, and send_config() and send_cmd_log() were answering
without it.
The copy is gone and the shared string carries the header. The check
for a command that printed nothing now compares against the length
strtox() returned instead of sizeof() on the macro.
The firmware now refuses to delete the switch's own entry, so the
button on the CPU row can only fail; leave it out. When a delete
succeeds, remove the entry from the cached table and repaint instead
of leaving the stale row until the next refresh.
Test the busy flag and compare the entry's MAC against the SFR data
registers directly instead of going through the sfr_data copy, and
rewrite TBL_DATA_0 in place rather than reassembling all four bytes.
Smaller and faster on the deepest httpd path.
Suggested-by: vDorst
The status line built the dotted quad by hand, which is what
print_ip() from cmd_parser.c already does. Reusing it drops seven
calls to one and takes 48 bytes off BANK2.
The prototype is not __banked because cmd_parser.c and syslog.c are
both in BANK2; a plain call is what the compiler needs here. Should
either file move to another bank, this has to become __banked with it.
The L2 page's Delete button removes any table entry by index, including
the entry for the switch's own MAC on the CPU port. Without that entry
unicast to the management address is no longer delivered to the CPU, so
management goes dark until the CPU happens to transmit again and the
entry is relearned. Compare the entry's MAC against uip_ethaddr before
rewriting it and answer result 0 for the switch's own address, as for a
missing entry. The compare is inline: this handler sits at the bottom of
the uIP call chain, where every extra call level costs internal RAM.
The CPU's own MAC exists in the L2 table only as a learned entry,
created whenever the CPU transmits, and it ages out like any other
dynamic entry. Unknown unicast is not flooded to the CPU port, so on an
idle switch (no web session, no management traffic) the entry expires
after the aging time and frames for the management IP no longer reach
the CPU: pings die until the next CPU transmission (an ARP reply)
re-teaches the entry, then die again. Real management sessions keep the
entry alive continuously, which is why this is only visible on idle
switches.
Install a static entry for the management MAC on the CPU port after the
startup config has been replayed, so it lands in the final management
VLAN, and move it when the management VLAN is changed at runtime.
Entry mechanics, established on hardware: a table write with read-method
0 hashes the key in TBL_DATA_IN and places the entry itself (a search
returns index 0 on a miss, there is no software-visible insertion slot).
The age field in the third data word must be non-zero for the entry to
be visible to lookup at all, and the static flag (bit 0 of byte 1 of
that word) stops the aging engine from decrementing it: with the flag a
full-age entry survives untouched where an identical unflagged entry
ages to invisible within the aging period. With no management VLAN
configured the entry lives in VLAN 1.
fetch() does not throw on 4xx, so a request that came back 401 after the
session had timed out reached the same line as a successful one: the
window showed the prompt and nothing under it, and the input was cleared
as if the command had run. The session lasts 200 seconds, so leaving the
tab open is enough to hit it.
A 401 now goes to the login page instead. The transcript also gets a
trailing newline when the answer lacks one, so the next prompt does not
run into the last line of output, and it is capped so a long session
does not keep growing the node.
Three ways the answer could mislead the caller, all in the paths that
matter most when something is wrong.
The capture sat in write_char(), so anything printed through
print_string_no_syslog() escaped it. That is exactly the replies of the
syslog commands, which deliberately avoid the syslog path: "syslog on"
answered with the bare address and no sentence around it, "syslog off"
answered with nothing at all. Moving the capture one level down, into
write_char_no_syslog(), catches both paths and costs nothing, since
write_char() reaches it anyway.
On a parse error send_bad_request() rewrote the buffer from the start,
throwing away the very text that explains the error and replacing it
with the 400 HTML page. Keeping the captured text and restating only the
status keeps the parser's own message; "400 NO" is as long as "200 OK",
so the header does not have to be rebuilt.
An overlong listing was cut at the buffer end with nothing to show for
it, so a clipped answer read as a short one. It now ends in a marker,
and the reserved room is accounted for in the guard.
While here: a command that configures something prints nothing, and an
empty body reads the same as "nothing ran", so a successful silent
command now answers "OK".
No internal RAM and no xdata; 174 bytes of BANK1, which has 7356 left.
The console tab posts to /cmd and throws the response away, so the page stayed
silent whether the command worked, was refused or was just misspelt. Now that
/cmd answers with the command's output, the page keeps a running transcript:
each command echoed after a prompt, then whatever it printed.
The box is a pre element filled through textContent, so nothing the device
prints is taken for markup. It scrolls to the newest line and the input clears
after a send.
POST /cmd runs the command and answers with an empty 200, so a caller cannot
tell "done" from "nothing happened", and whatever the command prints only
reaches the serial console. Reading a register over the network needs a second
channel just to see the answer.
write_char() is the one funnel every console helper goes through, and it already
tees into the syslog buffer. A third sink, live only while the httpd runs a
command that arrived over the network, puts the same bytes into the response
behind the header. Serial and syslog keep getting their copy.
The capture stops at the end of the response buffer, so a long listing is
truncated rather than overrunning it.
BANK1 grows 39 bytes and the common area 52. One byte of xdata, BANK2 unchanged.
login.html asks for i18n.js, but the file is not on the short list a
client may have before it authenticates, so the request is answered with
a redirect back to the login page and the strings stay as the English
that is baked into the markup.
The file carries only the interface text, the same text the page already
ships untranslated, so serving it before login gives nothing away.
The enabled flag is a flag, so say bool. The header length no longer needs
to live in xdata: it was put there when a local would not link, and there
is room for it here.
The overview names the row after the JSON key, so the syslog row said
"syslog_server_ip" while carrying only an address. It now says
"syslog_server" and carries the port with it.
A POST /cmd whose body arrives in a separate TCP segment from the headers
executed nothing: handle_post() read the body from the segment that
carried the request line, found it empty, and answered 200 OK. Python's
urllib and requests both write headers and body separately, so every
scripted command from those clients became a silent no-op. POST /login had
the same hole. The multipart endpoints were fixed earlier; this covers the
plain ones.
When the first segment holds fewer body bytes than Content-Length
announces, the bytes accumulate in config_buf (idle outside multipart
uploads) and the connection waits in TSTATE_POSTBODY; once the announced
length is in, the endpoint runs against the buffer. The /cmd and /login
executions move into run_cmd_body() and run_login_body() so both paths
share them; the 200 OK builder they duplicated becomes send_ok(), and
run_login_body() checks the pwd= prefix instead of skipping four bytes
blindly.
The announced length is the contract on both paths. A body that arrives
with the headers is terminated at Content-Length before it runs, so
pipelined bytes after it are not executed as commands the way the old
code did. A request without a usable Content-Length answers 411: without
one there is no way to know when the body has arrived, and the old 200
OK for an empty body is the bug this fixes. Bodies announced at or above
the buffer size answer 400 before anything is buffered.
The wait state has a deadline. uIP is built with a single connection,
and an ESTABLISHED connection with nothing in flight never times out on
its own, so headers followed by silence (a script interrupted, a link
dropped mid-request, or someone holding the socket on purpose) would
otherwise keep the slot until reboot; POST /login reaches the wait
before authentication. The poll handler aborts the connection when the
body has made no progress for five seconds, measured on the 200 Hz
system tick rather than on poll count, which runs faster under interrupt
load.
Record the announced body length alongside Content-Type and the session
cookie. The value goes through parse_short(), so an announcement that does
not fit sixteen bits saturates at 0xffff rather than wrapping to something
the size checks would accept, and a missing or unparseable header reads as 0.
Nothing consumes it yet; the plain POST endpoints take it up next.
d00f
René van Dorst
logicog
feelfree69
bloqaudio
Pietro AmeruosoandPietro Ameruoso
eeeple