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.
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 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.
The port was frozen at 514 in the call that opens the connection, so a
collector listening anywhere else was simply out of reach, and plenty do,
because 514 is privileged and containerised collectors usually get a high
port instead. Working around it meant putting a proxy in front, which then
rewrites the source address and hides which device actually sent the line.
Add `syslog port <n>`. The default stays 514, so nothing changes for an
existing configuration. Setting it tears the connection down and builds a
new one, the same dance the address already does, because uip_udp_new()
freezes the port into the connection.
The value is parsed here rather than through atoi_short(), which reports
"no digits" but not overflow: 65540 would quietly wrap to 4 and we would
open a connection to a port nobody asked for. Checking before each multiply
keeps it in 16-bit arithmetic. Zero then covers all three ways to be wrong -
no digits, past 65535, or an explicit "0", which is not a port either.
Whitelist the new command in config.js as well, next to `syslog ip`, or the
line is dropped without a word when the configuration is saved and the
setting quietly fails to survive a reboot. The dedup entry has to precede
the general `^syslog\b` one, otherwise it would deduplicate against
`syslog on`.
itoa_short() is new: itoa() only handles a byte and print_short() prints
hex, so there was no way to show a port as a decimal number. Kept separate
from itoa() rather than widening it, because every existing caller passes a
byte and would start paying for 16-bit divisions it does not need.
The scratch variables for the parser live in xdata. Locals there would take
internal RAM, and this image has none left: data ends at 0x77, the overlay
occupies 0x78-0x7f and the stack starts at 0x80. Measured: no module's DSEG
grew.
RFC 3164 has a hostname between the priority and the message text, and we
were sending nothing there. A receiver still has to fill that field, so it
takes the first word of the text instead, so every line arrives attributed to
"STP:", or "REGGET:", or whatever the log happens to start with. The sender
is then impossible to select on: the name changes with the content, and
anything in front of us on the path replaces the source address with its
own.
Observed on a collector behind a UDP proxy: the same switch showed up as two
different hosts depending on which subsystem had logged last.
Send the configured hostname. Skipped when it is empty, so we never emit a
lone separator.
The header length lives in xdata rather than on the stack. syslog_callback()
is __banked, so a local there cannot be overlaid, and while the variable
itself is only two bytes it costs more than that: it takes a register, which
pushes log_start out of one and into internal RAM. Those two bytes are
enough to stop an image carrying LACP and STP from linking at all.
bloqaudio
d00f
René van Dorst
bennydiamond
feelfree69