parse_isolate() accepted a two digit port and mapped it to logical port 9,
the CPU port, while port_isolate() and port_isolation_get() both refuse
anything above machine.max_port. Setting the isolation of the CPU port was
therefore declined without a word and reading it always answered no
members, whatever the hardware held.
Bound the port to the front panel, so the command says what it does. The
digit is checked before it indexes phys_to_log_port[], which a non numeric
argument used to read past.
handle_rx() dispatched to igmp_packet_handler() on the destination
address alone, so an ordinary IGMPv3 report off the wire reached the
handler and could write a table entry whether or not anyone had asked
for snooping. The STP branch right above it is gated on stpEnabled;
this brings the IGMP branch in line.
Snooping state lived only in the per-port registers, and the receive
path cannot afford to read one per packet, so the flag shadows it:
igmp_enable() sets it, igmp_setup() clears it, and igmp_setup() runs
from both the boot path and "igmp off".
While here, igmp off becomes an explicit subcommand instead of the
fall-through, and an unrecognised igmp subcommand prints the usage
line rather than silently turning snooping off.
Six bytes of BANK1 and one of xdata, no internal RAM.
The two flags added with the JSON fix sit in data, where internal RAM is
full enough that this branch stopped linking for some toolchains. __bit
puts them in the bit area instead and hands three bytes back to the
stack: SSEG goes from 131 to 134 on SWTGW218AS.
Patch by vDorst on the pull request.
lag show has always printed the groups as 1 to 4 while lag <n> took the
number literally, so typing what you saw configured the group beside it.
Both lag and lag hash count from one now, matching how ports are numbered
everywhere else, and reject anything outside 1 to 4. Subtracting '1' makes
0 wrap well past three, so one test covers both ends.
The port argument indexed machine.phys_to_log_port, which holds nine
entries, before it was checked, and a two digit argument reaches 109. It is
bounded before the table is touched rather than after.
port_lag_members_set() and port_lag_hash_set() complained about a group out
of range and then wrote the registers anyway, past the four the groups
occupy. They return instead.
lag hash also read cmd_words_b[1] without checking a word was there, and
now shares the error path parse_lag() already had.
Both the L2 page and the ports page in #335 need to page through /l2.json
and decode the same fields, and the second copy arrived carrying the two
bugs the first one had only just been fixed for. Rather than keep two
copies in step by hand, the transport and the decoding move to main.js,
which every page already loads, and each page keeps only what is its own.
walkL2(onDone) pages through the table once, parses idx and vlan out of
hex, maps the port to a physical number or to 'CPU', and calls
onDone(entries, ok). It stops on a wrapped index, an empty page or 4096
entries, all of which set ok. A page that comes back as anything other than
200, or with a body JSON.parse rejects, is asked for again at the same
index up to three times; only once those run out does the walk end with ok
clear, so a caller can tell a finished table from a partial one. l2.js
keeps the s and l to label mapping, since that needs the page's own
translations, redraws only when ok is set, and restarts the walk from its
callback either way.
Two things change while moving:
The next request goes out from the previous reply rather than from a
setInterval that fires whether or not the last one came back. The httpd
serves one connection at a time, so a timer that outruns the responses only
queues work it cannot use.
A walk that reaches 4096 entries hands over what it collected. Before it
threw the entries away and cleared its own interval, which left the page
unable to refresh again until it was reloaded.
The retry is not a new idea, it is the old behaviour written down. The
previous code ignored anything that was not a 200 and let the interval ask
for the same index again, so a blip never disturbed the table on screen.
Dropping that on the way to a chained walk would have made every timeout
redraw the page with a truncated table, which at one connection at a time
is not a rare event.
Driven with a scripted server in node, running the helper itself rather
than a copy of it: an empty table gives 0 entries in 1 request; three pages
ending in a repeated index give 61 entries in 3 requests, asking for 0, 30
and 60; an empty page ends the walk after 2; a 500 and a malformed body are
each retried at the same index and then complete normally, asking 0, 30, 30
and 31; three failures in a row end the walk with ok clear and the 30
entries already collected; 4096 entries in one page end it with ok set; the
CPU port decodes to 'CPU'; vlan and idx come back as numbers.
main.js grows by 1331 bytes and l2.js loses 1039, so 292 bytes of flash.
Worth stating where they land: main.js is loaded by every page, so pages
that never walk the table now carry the helper too. That is the cost of
having the decoding exist exactly once, which is the point of the move.
l2.js reads the last index of a reply to know where the next page starts.
With the firmware side of this branch an empty table answers [], and
s[s.length-1] then throws on undefined. It used to answer commas with
nothing between them and throw in JSON.parse instead, so this is the same
case reaching a different line rather than a new one. An empty reply now renders what
has been collected and starts the next pass from zero.
The 4096 entry cap compared the array against the number instead of its
length, so it never fired: an empty array and a 5000 element one both
compare false. Comparing the length restores what the check was for.
The MAC table listing wrote its separator once per iteration rather than
once per object. An entry the table engine reports as invalid produces no
object, so it contributed a bare comma, and two in a row give ",," which
JSON.parse rejects. The whole table then fails to load, not just the row
that was missing. The separator now goes before each object and the
closing bracket after the loop, which is the shape send_vlanlist already
uses further down the file.
The next index for an invalid entry was computed as h | low + 1, and the
addition binds tighter than the or. That agrees with (h | low) + 1 except
when the low byte reads 0xff and bit 8 of the index is already set, eight
of the 4096 combinations. There the result is the start of the current
block of 256 rather than the start of the next one, so the walk repeats a
block it has already covered. Reading the index once after the branch
rather than once in each arm removes the second copy of that expression
along with the bug.
The VLAN now comes first in each object. It is taken from the same
L2_DATA_OUT_B read that decides whether the entry is valid, which saves
reading that register a second time. The page addresses the fields by
name, so the order they arrive in does not matter to it.
A bound check on the output buffer goes in for consistency with
send_vlanlist. Thirty entries of at most 74 bytes plus the brackets fit in
the 2500 byte buffer with 179 to spare, so nothing changes today, but the
margin was nowhere stated and L2_MAX_TRANSFER is a tunable.
5 bytes of BANK1, nothing in BANK2, xdata or internal RAM. Built for SWTGW218AS
and KP_9000_6XHML_X2 on sdcc 4.5.0.
__sdcc_banked_call and __sdcc_banked_ret were assembled into GSFINAL, which
sits in the startup path: GSINIT ends exactly where GSFINAL begins, so the
processor falls into it rather than being sent there. It works today only
because this object comes after every C object on the link line, so the
LJMP to __sdcc_program_startup is laid down first and the helpers land
behind it. Reordering that line, or moving main() into another module,
would put the helper body at the fallthrough address instead, and the board
would not come up out of a build that reports nothing wrong.
SDCC's own crtbank.asm declares the area order and then puts both symbols
in HOME, so the file takes that name and that preamble as well.
GSFINAL now holds the three byte jump and nothing else. Of 401 symbols 17
change address, every one in the startup region, and between the reset
vector and 0x0094 not a byte differs, so no interrupt vector is disturbed.
Run on a SWTGW218AS: it came back after about 42 seconds reporting the new
build, with its stored configuration byte identical and every link at the
speed it had before.
Wrapping REG_SET restored the guard in front of the hash default, and
that exposed three things about the line it guards.
The test was against zero. The register does not read zero: it comes out
of reset holding source port number plus both MAC fields, both IP fields
and the L4 source port, which the header now names LAG_HASH_RESET.
Measured on an SWTGW218AS, where all four groups read 0x3f after a cold
boot and a value written before a power cycle is gone afterwards. With
the guard working and the test unreachable, the default would never be
installed, where before it was installed on every call. Testing against
the reset value restores the intent, and zero is still accepted in case
another device does reset that way.
The write went to the base address while the read that decides it used
the group offset, so a group other than zero was tested and group zero
was written. Both ends use the offset now.
The range check printed a complaint and carried on. It returns, which
matters more now that the hash write also uses the group number to build
an address.
Thirty bytes of BANK1.
The member mask of an aggregation group is decoded by hand in two places,
the lag command and the JSON behind the aggregation page, and every branch
that touches trunks adds another copy.
port_lag_members_get() sits next to port_lag_members_set() and both readers
call it. It answers from the hardware, so it covers a group configured with
lag and one a protocol brought up, without either having to say so.
It reads through reg_read() rather than reg_read_m(), so sfr_data is left
alone. Neither caller looked at it afterwards; both read the hash register
next.
The handler took one raw character of the request line and passed it to
send_counters(), which uses it to index machine.phys_to_log_port. That
array has nine entries and the character is whatever the client sent, so
the read ran up to 246 entries past the end and the result went on to
STAT_GET as a port number. is_word() accepts any request whose name is
followed by a question mark, so nothing constrained the byte to a digit.
Bounding it where it is read keeps the check beside the assumption it
protects and needs nothing from the machine description. sdcc leaves
plain char unsigned and the subtraction wraps in eight bits, so a byte
below '0' comes out above 200 and one upper test covers both ends:
exactly '0' to '8' now reach send_counters. The compiled test is
add a,#0xf7 followed by jnc, which I read back out of the assembly rather
than assuming.
Out of range answers 400 by the path the other malformed requests already
take, rather than an empty array. An empty array would have been worse
than useless here, since the statistics page calls BigInt on the first
element before it looks at the length. The page asks only for index zero
to the port count minus one, so nothing that answered before stops
answering, and a non-200 reply makes its handler do nothing at all.
11 bytes of BANK1, nothing in the common segment, BANK2, xdata or
internal RAM. Built for SWTGW218AS and KP_9000_6XHML_X2 on sdcc 4.5.0.
Mark create_build_dir as PHONY so directory creation is never skipped.
Also add html_data.h as order-only prerequisite to the .c pattern rule,
fixing another the race where httpd/httpd.c and httpd/page_impl.c are
compiled before the generated header exists.
Replace file-path prerequisite tools/output/fileadder with order-only
dependency on the tools PHONY target.
Also add tools as order-only dependency to the final .bin target which
invokes all of the tools.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
René van Dorst
d00f
logicog
d00f
Priit Laes