Merge pull request #333 from DrDoof/feat/syslog-hostname-port

syslog: send a hostname, allow a port other than 514, and stop the save from dropping the address

cmd_parser.c

@@ -67,6 +67,10 @@ __xdata	char save_cmd;
 
 __xdata uint8_t ip[4];
 
+/* Scratch for parse_syslog(), in xdata: a local here would take internal RAM
+ * the linker has none of. */
+__xdata uint8_t  syslog_port_was_on;
+
 // These variables combined create a Fixed-capacity vector/bounded buffer.
 // `N_WORDS`: The total number of command arguments that can be tracked.
 // `cmd_words_len` stores the number of arguments found inside `cmd_buffer`
@@ -1415,6 +1419,7 @@ void parse_syslog(void)
 		if (syslog_state.enabled) {
 			print_string("enabled, sending to ");
 			print_ip(syslog_state.server_ip);
+			write_char(':'); itoa_short(syslog_state.server_port);
 			write_char('\n');
 		} else {
 			print_string("disabled\n");
@@ -1443,11 +1448,34 @@ void parse_syslog(void)
 		} else {
 			print_string("Invalid IP address\n");
 		}
+	} else if (cmd_compare(1, "port")) {
+		if (cmd_words_len < 3) { // no additional argument -> print current port
+			print_string("Current syslog port: ");
+			itoa_short(syslog_state.server_port); write_char('\n');
+			return;
+		}
+		/* atoi_short() returns zero for no digits and for a value past
+		 * 65535; port zero is not a port either, so both tests are needed. */
+		if (!atoi_short(cmd_words_b[2]) || !atoi_results_short) {
+			print_string("Invalid port\n");
+			return;
+		}
+		/* The port is frozen into the connection by uip_udp_new(), so
+		 * changing it has to tear the connection down and build a new one -
+		 * same dance as setting the address above. */
+		syslog_port_was_on = syslog_state.enabled;
+		if (syslog_port_was_on)
+			syslog_stop();
+		print_string("Setting new syslog port.\n");
+		syslog_state.server_port = atoi_results_short;
+		if (syslog_port_was_on)
+			syslog_start();
 	}
 	else
 	{
-		print_string("Error: syslog [on|off|ip [ip-address]]\n");
+		print_string("Error: syslog [on|off|ip [ip-address]|port [number]]\n");
-		print_string("  on/off enables or disables syslog, ip sets the syslog server IP address\n");
+		print_string("  on/off enables or disables syslog, ip sets the syslog server IP address,\n");
+		print_string("  port sets the destination UDP port (default 514)\n");
 	}
 }
 

cmd_parser.h

@@ -13,6 +13,7 @@ void cmd_tokenize(void) __banked;
 void cmd_parser(void) __banked;
 void execute_config(void) __banked;
 void execute_commands(__xdata uint8_t *p) __banked;
+void print_ip(__xdata uint8_t *ptr);
 void print_sw_version(void) __banked;
 void clear_command_history(void) __banked;
 

html/config.js

@@ -7,6 +7,7 @@ const conf_cmds = [
   /^netmask\s+(\d{1,3}\.){3}\d{1,3}$/,
   /^syslog\s+(on|off)$/,
   /^syslog\s+ip\s+(\d{1,3}\.){3}\d{1,3}$/,
+  /^syslog\s+port\s+\d{1,5}$/,
   /^passwd\s+\S+$/,
   /^vlan\s+\d{1,4}\s+d$/,
   /^vlan\s+\d{1,4}\s+mgmt$/,
@@ -36,12 +37,21 @@ const conf_cmds = [
   /^bw\s+(in|out)\s+\d{1,2}\s+\S+$/,
   /^hostname\s+.{1,23}$/,
 ];
+/* Commands that come in an on/off pair replace each other, which the list
+ * below cannot express: it drops lines starting with the text it matched, and
+ * "syslog off" does not start with "syslog on". Naming the stem separately
+ * keeps the pair collapsed without widening the match to the whole family. */
+const conf_toggle = [
+  /^(syslog)\s+(?:on|off)$/,
+];
+
 const conf_overwrite = [
   /^ip\b/,
   /^gw\b/,
   /^netmask\b/,
   /^syslog\s+ip\b/,
-  /^syslog\b/,
+  /^syslog\s+port\b/,
+  /^syslog\s+(on|off)$/,
   /^passwd\b/,
   /^vlan\s+\d{1,4}\s+mgmt$/,
   /^vlan\s+\d{1,4}(?!\s+mgmt\b)/,
@@ -79,6 +89,13 @@ function parseConf(s){
     for (const x of conf_cmds)
       if (x.test(line)) { ignore = false; break; }
     if (ignore) continue;
+    for (const x of conf_toggle) {
+      const t = line.match(x);
+      if (t) {
+        configuration = configuration.filter(item => item !== t[1] + " on" && item !== t[1] + " off");
+        break;
+      }
+    }
     for (const x of conf_overwrite) {
       if (x.test(line)) {
         let m = line.match(x);

httpd/page_impl.c

@@ -223,11 +223,12 @@ void send_basic_info(void)
 	itoa_html(uip_netmask[0] >> 8); char_to_html('.');
 	itoa_html(uip_netmask[1]); char_to_html('.');
 	itoa_html(uip_netmask[1] >> 8);
-	slen += strtox(outbuf + slen, "\",\"syslog_server_ip\":\"");
+	slen += strtox(outbuf + slen, "\",\"syslog_server\":\"");
 	itoa_html(syslog_state.server_ip[0]); char_to_html('.');
 	itoa_html(syslog_state.server_ip[1]); char_to_html('.');
 	itoa_html(syslog_state.server_ip[2]); char_to_html('.');
-	itoa_html(syslog_state.server_ip[3]);
+	itoa_html(syslog_state.server_ip[3]); char_to_html(':');
+	itoa_html(syslog_state.server_port);
 	slen += strtox(outbuf + slen, "\",\"mac_address\":\"");
 	byte_to_html(uip_ethaddr.addr[0]); char_to_html(':');
 	byte_to_html(uip_ethaddr.addr[1]); char_to_html(':');

rtl837x_common.h

@@ -138,6 +138,7 @@ void print_long(uint32_t a);
 void print_short(uint16_t a);
 void print_byte(uint8_t a);
 void itoa(uint8_t v);
+void itoa_short(uint16_t v);
 void print_sfr_data(void);
 void print_phy_data(void);
 void print_cmd_prompt(void);

rtlplayground.c

@@ -282,6 +282,32 @@ void itoa(uint8_t v)
 }
 
 
+/* Same as itoa(), one decade wider: enough for a port number. Kept separate
+ * rather than widening itoa() itself, because every existing caller passes a
+ * byte and would start paying for 16-bit divisions it does not need. */
+void itoa_short(uint16_t v)
+{
+	uint8_t t = v / 10000;
+	uint8_t print_zeros = t;
+
+	if (print_zeros)
+		write_char('0' + t);
+	t = (v / 1000) % 10;
+	print_zeros |= t;
+	if (print_zeros)
+		write_char('0' + t);
+	t = (v / 100) % 10;
+	print_zeros |= t;
+	if (print_zeros)
+		write_char('0' + t);
+	t = (v / 10) % 10;
+	print_zeros |= t;
+	if (print_zeros)
+		write_char('0' + t);
+	write_char('0' + (v % 10));
+}
+
+
 void print_string(__code char *p)
 {
 	while (*p)

syslog.c

@@ -1,5 +1,6 @@
 #include "machine.h"
 #include "syslog.h"
+#include "cmd_parser.h"
 #include "uip/uip.h"
 #include "rtl837x_common.h"
 
@@ -22,20 +23,21 @@ void syslog_init(void) __banked
 	state.readptr = 0;
 	state.line_available = 0;
 	state.server_ip[0] = 0; state.server_ip[1] = 0; state.server_ip[2] = 0; state.server_ip[3] = 0;// Default to 0.0.0.0
+	state.server_port = SYSLOG_PORT_DEFAULT;
 }
 
 void syslog_start(void) __banked
 {
 	if (state.syslog_conn == 0) {
 		uip_ipaddr(server_ip, state.server_ip[0], state.server_ip[1], state.server_ip[2], state.server_ip[3]);
-		state.syslog_conn = uip_udp_new(&server_ip, HTONS(514));
+		state.syslog_conn = uip_udp_new(&server_ip, HTONS(state.server_port));
 		if (state.syslog_conn == 0) {
 			print_string_newline_no_syslog("Failed to create a new UDP client");
 			return;
 		}
 		print_string_newline_no_syslog("Started syslog to IP ");
-		itoa(state.server_ip[0]); write_char('.'); itoa(state.server_ip[1]); write_char('.');
+		print_ip(state.server_ip);
-		itoa(state.server_ip[2]); write_char('.'); itoa(state.server_ip[3]); write_char('\n');
+		write_char(':'); itoa_short(state.server_port); write_char('\n');
 		state.enabled = 1;
 	}
 	else {
@@ -57,6 +59,7 @@ void syslog_stop(void) __banked
 
 void syslog_callback(uint16_t lport) __banked
 {
+	uint16_t syslog_hdr;
 	if (lport != state.syslog_conn->lport)
 		return;
 
@@ -91,14 +94,31 @@ void syslog_callback(uint16_t lport) __banked
 
 		memcpyc(SYSLOG_P, "<14>", 4); // Syslog priority prefix
 
-		if (log_end < log_start) {
+		/* RFC 3164 puts a hostname between the priority and the text, and
-			memcpy(SYSLOG_P + 4, logbuf + log_start, LOGBUF_SIZE - log_start);
+		 * we were leaving that slot empty. A receiver still has to fill
-			memcpy(SYSLOG_P + 4 + LOGBUF_SIZE - log_start, logbuf, log_end);
+		 * the field, so it takes the first word of the message instead:
+		 * every line arrives attributed to "STP:" or "REGGET:" or
+		 * whatever the log happens to start with, and the sender cannot
+		 * be selected on at all. Send our own name and the field means
+		 * something. Skipped when the name is empty, so we never emit a
+		 * lone separator. */
+		syslog_hdr = strlen_x(hostname);
+		if (syslog_hdr) {
+			memcpy(SYSLOG_P + 4, hostname, syslog_hdr);
+			SYSLOG_P[4 + syslog_hdr] = ' ';
+			syslog_hdr += 5;
 		} else {
-			memcpy(SYSLOG_P + 4, logbuf + log_start, log_end - log_start);
+			syslog_hdr = 4;
 		}
 
-		uip_udp_send(log_size+4);
+		if (log_end < log_start) {
+			memcpy(SYSLOG_P + syslog_hdr, logbuf + log_start, LOGBUF_SIZE - log_start);
+			memcpy(SYSLOG_P + syslog_hdr + LOGBUF_SIZE - log_start, logbuf, log_end);
+		} else {
+			 memcpy(SYSLOG_P + syslog_hdr, logbuf + log_start, log_end - log_start);
+		}
+
+		uip_udp_send(log_size + syslog_hdr);
 		state.readptr = state.writeptr;
 		state.line_available = 0;
 	}

syslog.h

@@ -1,16 +1,20 @@
 #ifndef _SYSLOG_H_
 #define _SYSLOG_H_
 
+#include <stdbool.h>
 #include <stdint.h>
 
 #define LOGBUF_SIZE 512
 
+#define SYSLOG_PORT_DEFAULT 514
+
 struct syslog_state {
-    uint8_t enabled;
+    bool enabled;
     uint8_t line_available;
     uint16_t writeptr ;
     uint16_t readptr;
     uint8_t server_ip[4];
+    uint16_t server_port;
 
     struct uip_udp_conn *syslog_conn;
 };