isr_ext2() ends with PCON |= 1, putting the core to sleep from inside an
interrupt handler.
The DW8051 leaves idle when any enabled interrupt is activated, so the core
does not stay down for long. It naps until the next enabled interrupt request
turns up, with the 200 Hz system tick putting a ceiling of 5 ms on that, and
then carries on.
The sleep buys nothing either way, because the instruction after it is the
return. While it lasts the handler holds the priority latch, so everything at
its level or below waits, and that includes the tick, the console and the NIC
receive interrupt.
The handler still clears its flag, which is what it has to do. The bit numbers
beside the two masks were wrong: 0xef clears bit 4 and 0xdf clears bit 5,
not 7 and 6.
isr_ext2() ends with PCON |= 1, putting the core to sleep from inside an
interrupt handler.
The DW8051 leaves idle when any enabled interrupt is activated, so the core
does not stay down for long. It naps until the next enabled interrupt request
turns up, with the 200 Hz system tick putting a ceiling of 5 ms on that, and
then carries on.
The sleep buys nothing either way, because the instruction after it is the
return. While it lasts the handler holds the priority latch, so everything at
its level or below waits, and that includes the tick, the console and the NIC
receive interrupt.
The handler still clears its flag, which is what it has to do. The two wrong
comments next to the masks go as well: 0xef clears bit 4 and 0xdf clears bit 5,
not 7 and 6, and the masks already say that without help.
Add a `mac <aa:bb:cc:dd:ee:ff>` console command (and a bare `mac` to show
the current address) that overrides the management MAC at runtime. The
address is validated (unicast, globally administered, non-zero OUI) and
the static L2 management entry is re-registered so the change applies
without a reboot. Because it is a normal command, a line in the startup
config persists it across boots: execute_config() runs it before the final
static L2 entry and the derived hostname are set up.
Also point the keepLink KP-9000-9XHML-X V3.1 machine definition at the
factory MAC area (0x1FC000, same location other machines use) so a valid
factory MAC is picked up at boot instead of falling back to a locally
administered address.
The MAC parsing/printing lives in parse_mac()/print_mac() helpers next to
parse_ip()/print_ip() so future commands can reuse them, and a memcmp()
helper joins the project's own memcpy()/memset() (no libc is linked).
isr_ext2() ends with PCON |= 1. On the DW8051 any enabled interrupt request
ends idle, so the handler sleeps until the next request arrives, the system
tick at the latest, and only then returns. Everything at the same priority
waits with it, and the instruction after the idle is RETI, so the sleep buys
nothing.
The handler still clears its flag, which is what it has to do. The other four
uses of PCON |= 1 are in task context and are left alone.
A file larger than the output buffer is sent in two phases: the first
buffer goes out in MSS-sized pieces, with o_idx walking through it, and
what is left is then streamed from flash a chunk at a time into outbuf
from offset 0.
The continuation branch never reset o_idx, so it kept the offset the first
phase had left it at, while uip_rexmit() retransmits from outbuf + o_idx.
A lost continuation segment was therefore retransmitted as the bytes 1460
further into the buffer, which the client accepts as the missing data, and
with o_idx + slen past the 2500-byte buffer it also read whatever xdata
follows outbuf.
Reproduced on a SWTGW218AS by dropping 15% of the switch's TCP segments:
seven of eight fetches of a 21 kB static came back the right length and
the wrong content, each first differing at a fixed phase of the 1460-byte
chunk. With o_idx reset the retransmission repeats the chunk it sent.
The o_idx += slen right after slen = 0 went too: it added nothing.
(cherry picked from commit 789d29423288f89411a41673e26ce1854b296c3d)
uip_arp_ipin() is an empty macro with its body under #if 0, both inherited
from the uIP import and untouched since. That leaves uip_arp_update()
reachable only from uip_arp_arpin(), so an entry is refreshed only by ARP
addressed to us and ages out after UIP_ARP_MAXAGE no matter how much traffic
passed over it.
When it goes, the next transmission to that host hits the miss path of
uip_arp_out(), which overwrites the packet in uip_buf with an ARP request and
returns. For TCP that costs a retransmission. For UDP nothing retransmits, and
handle_tx() sends the UDP connections through uip_arp_out() too, so the
datagram is lost.
The body could not be enabled as written: uip_len -= sizeof(struct uip_eth_hdr)
is a leftover from when UIP_LLH_LEN was 0. Here it is ETHER_HEADER_SIZE +
RTL_FRAME_DESC_SIZE and uip.c reads the IP header at uip_buf[UIP_LLH_LEN], so
uip_len has to keep the link layer in it. The rest of the function is sound.
replaceCalls() ended with strlen(&buffer[pos]), so the length recorded for a
file was the distance to its first 0x00 byte rather than the file size. The
byte just past the content happens to be zero today, which is the only reason
the numbers come out right.
Nothing in html/ contains a 0x00 today, so this is latent, but it is a trap
waiting for the first gzipped or binary asset: gzip puts a zero in the fourth
byte of its header. A gzipped style.css lands in the index as 3 bytes, and
since addr advances by that same value the next file is placed 3 bytes in and
overwrites what came before it. Two gzipped files in html/ are enough to see
it: both come out as 3 bytes and the second sits 1356 bytes inside the first.
Pass the length in and return the adjusted one. The substitution shortens
#{function} to #{NNN}, so the delta is known without measuring. The loops are
bounded by the length as well, which also covers -n, where nothing had put a
terminator in the buffer for strlen to find in the first place.
No output change: the image built from an unmodified html/ is byte for byte
the same before and after, and html_data.h is identical.
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.
dhcp_start() never looked at whether DHCP was already running, and "ip dhcp"
calls it without stopping first, so a second start took a fresh slot from
uip_udp_conns and left the previous one bound.
The leak is the smaller half of it. dhcp_callback() decides whether a call
is for it by comparing the local port, not the connection, and every
leftover socket still holds port 68, so each one runs the state machine on
every poll. The timers live in one shared dhcp_state, so ticks count down
once per socket: two starts and the retransmit and renewal timing runs at
twice the intended rate.
Stop the old one first. dhcp_stop() already does exactly that, and the
state test keeps it away from a connection that was never created.
uip_init() does not set uip_conn, and httpd_init() runs right after it, so
the assignment went through a pointer nothing had assigned yet.
It has stayed invisible because the startup code does clear xdata: uip_conn
is NULL rather than garbage, appstate sits 28 bytes into struct uip_conn, and
the byte therefore lands at xdata 0x1C. With the current map that is inside
the command line buffer, at an offset nothing reads while the line is empty,
and cmd_editor_init() runs later anyway.
Nothing is broken today, but it is a write to an address that has nothing to
do with the state being set, and it moves the moment the struct grows or the
map shifts. Address the connection directly.
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.
René van Dorst
d00f
gnacho
eraiza0816
logicog
feelfree69
bloqaudio