storm: meters from 24 up, plainer parser, documentation from the review

Storm control used meters 0 to 39, the same meters the ACL hands out,
so a storm limit and an acl meter could overwrite each other. It now
uses 24 + port * 4 + type, meters 24 to 63.

parse_storm() is no longer reentrant and uses plain locals, looks the
type up in a while loop and returns straight from the one-line cases.
storm_show() uses plain locals too.

The documentation says how IGMP decides between mcast and umcast, how
to get from a meter index to its limit, where the IFG bit of a meter
is, and what an ingress bandwidth limit on the same port does to the
storm limit.

cmd_parser.c

@@ -1576,31 +1576,26 @@ err:
 
 static __code char * __code storm_types[STORM_TYPES] = { "bcast", "mcast", "ucast", "umcast" };
 
-void parse_storm(void) __reentrant
+void parse_storm(void)
 {
-	static __xdata uint8_t port, type, d;
+	uint8_t port, type, d;
-	static __xdata uint32_t rate;
+	__xdata uint32_t rate;
-	static __xdata uint8_t * __xdata p;
+	__xdata uint8_t *p;
 
-	if (cmd_words_len == 1) {
+	if (cmd_words_len == 1)
-		storm_show();
+		return storm_show();
-		return;
+	if (cmd_words_len < 4 || !cmd_parse_port_separator(cmd_words_b[1]))
-	}
-	if (cmd_words_len < 4 || cmd_parse_port_separator(cmd_words_b[1]) == 0)
 		goto err;
 	port = atoi_results_u8;
 
-	for (type = 0; type < STORM_TYPES; type++) {
+	type = 0;
-		if (cmd_compare(2, storm_types[type]))
+	while (!cmd_compare(2, storm_types[type])) {
-			break;
+		if (++type == STORM_TYPES)
+			goto err;
 	}
-	if (type == STORM_TYPES)
-		goto err;
 
-	if (cmd_words_len == 4 && cmd_compare(3, "off")) {
+	if (cmd_words_len == 4 && cmd_compare(3, "off"))
-		storm_off(port, type);
+		return storm_off(port, type);
-		return;
-	}
 	if (cmd_words_len != 5)
 		goto err;
 

doc/storm_control.md

@@ -12,7 +12,10 @@ storm <port> bcast|mcast|ucast|umcast off
 storm
 ```
 `ucast` is unicast to a MAC address the switch has not learned yet, and
-`umcast` is multicast without an entry in the address table. A limit in
+`umcast` is multicast without an entry in the address table. Multicast groups
+get their entries from IGMP snooping, so without `igmp on` all multicast is
+unknown and only the `umcast` limit applies to it; `mcast` covers the groups
+IGMP has learned. A limit in
 packets per second goes from 1 to 1048575, a limit in kbit/s from 1 to
 10000000. `storm` without arguments prints the limits of every port. The
 limits are kept in the configuration like any other setting, and the Bandwidth
@@ -21,8 +24,11 @@ page of the web interface has a Storm control card to set them.
 ## Registers
 Each port and type has an enable bit and a meter index, and each limit uses
 one of the 64 shared meters of the switch. RTLPlayground gives every port and
-type its own meter, index `port * 4 + type`, so one port's storm does not eat
+type its own meter, index `24 + port * 4 + type`, so one port's storm does not
-into the limit of another.
+eat into the limit of another. Storm control uses meters 24 to 63 and leaves
+0 to 23 to the ACL. The limit of a meter is in `RTL837X_METER_RATE + 4 *
+meter`, and bit `meter % 32` of `RTL837X_METER_MODE + 4 * (meter / 32)`
+selects pps instead of kbit/s.
 ```
 #define RTL837X_STORM_CTRL		0x54e4	/* + 4 * type, bit = port */
 #define RTL837X_STORM_MIDX		0x54f4	/* + 8 * type + 4 * (port / 5), 6 bits per port */
@@ -31,7 +37,15 @@ into the limit of another.
 ```
 In kbit/s mode one step of the rate is 1 kbit/s. Measured with 100 and 1000 byte
 frames, the meter counts a frame with its FCS, but without preamble and
-interframe gap.
+interframe gap. Every meter has a bit that adds them, in `0x5f08 + 4 * (meter
+/ 32)`, which storm control leaves cleared. `IGBW_INC_IFG` and `EGBW_INC_IFG`
+are the matching bits of the bandwidth control.
+
+An ingress bandwidth limit on the same port works on the same arriving frames
+as the storm limit rather than after it, and the two together pass far less
+than either one: a 1 Mbit/s `umcast` limit with a 10 Mbit/s ingress limit let
+0.12 to 0.16 Mbit/s of a 1.4 Gbit/s stream through, where each alone passed
+1.17 and 10.2 Mbit/s.
 
 The type is 0 for broadcast, 1 for multicast, 2 for unknown unicast and 3 for
 unknown multicast. The burst size of a meter (0x5df0 + 4 * meter) counts packets

httpd/page_impl.c

@@ -843,12 +843,12 @@ void send_storm(void)
 			char_to_html(reg_bit_test(RTL837X_STORM_CTRL + (t << 2), i) ? '1' : '0');
 		slen += strtox(outbuf + slen, "\",\"pps\":\"");
 		for (t = 0; t < STORM_TYPES; t++) {
-			idx = (i << 2) | t;
+			idx = STORM_METER(i, t);
 			char_to_html(reg_bit_test(RTL837X_METER_MODE + ((idx >> 5) << 2), idx & 0x1f) ? '1' : '0');
 		}
 		slen += strtox(outbuf + slen, "\",\"rate\":\"");
 		for (t = 0; t < STORM_TYPES; t++) {
-			reg_read_m(RTL837X_METER_RATE + ((((i << 2) | t)) << 2));
+			reg_read_m(RTL837X_METER_RATE + (STORM_METER(i, t) << 2));
 			byte_to_html(sfr_data[1]);
 			byte_to_html(sfr_data[2]);
 			byte_to_html(sfr_data[3]);

rtl837x_storm.c

@@ -11,12 +11,13 @@
 extern __code struct machine machine;
 extern __xdata uint8_t sfr_data[4];
 
-/* Meter index fields of RTL837X_STORM_MIDX, [type << 1 | ports 5-9] */
+/* RTL837X_STORM_MIDX words, row type * 2 + half: half 0 holds ports 0-4, half 1 ports 5-9,
+ * 6 bits per port, each set to STORM_METER(port, type) */
 static const __code uint8_t storm_midx[8][4] = {
-	{ 0x10, 0x30, 0x81, 0x00 }, { 0x24, 0x81, 0xc6, 0x14 },
+	{ 0x28, 0x92, 0x07, 0x18 }, { 0x3c, 0xe3, 0x4c, 0x2c },
-	{ 0x11, 0x34, 0x91, 0x41 }, { 0x25, 0x85, 0xd6, 0x55 },
+	{ 0x29, 0x96, 0x17, 0x59 }, { 0x3d, 0xe7, 0x5c, 0x6d },
-	{ 0x12, 0x38, 0xa1, 0x82 }, { 0x26, 0x89, 0xe6, 0x96 },
+	{ 0x2a, 0x9a, 0x27, 0x9a }, { 0x3e, 0xeb, 0x6c, 0xae },
-	{ 0x13, 0x3c, 0xb1, 0xc3 }, { 0x27, 0x8d, 0xf6, 0xd7 },
+	{ 0x2b, 0x9e, 0x37, 0xdb }, { 0x3f, 0xef, 0x7c, 0xef },
 };
 
 static __code char * __code storm_names[STORM_TYPES] = { " bcast ", " mcast ", " ucast ", " umcast " };
@@ -24,7 +25,7 @@ static __code char * __code storm_names[STORM_TYPES] = { " bcast ", " mcast ", "
 
 void storm_set(uint8_t port, __xdata uint8_t type, __xdata uint32_t rate, __xdata uint8_t pps) __banked
 {
-	__xdata uint8_t idx = (port << 2) | type;
+	__xdata uint8_t idx = STORM_METER(port, type);
 	__xdata uint8_t row = (type << 1) | (port >= 5 ? 1 : 0);
 	__xdata uint8_t * __xdata r = (uint8_t *)&rate;
 
@@ -64,7 +65,7 @@ void storm_off(uint8_t port, __xdata uint8_t type) __banked
 
 void storm_show(void) __banked
 {
-	__xdata uint8_t i, t, idx;
+	uint8_t i, t, idx;
 
 	for (i = machine.min_port; i <= machine.max_port; i++) {
 		print_string("port ");
@@ -75,7 +76,7 @@ void storm_show(void) __banked
 				print_string("off");
 				continue;
 			}
-			idx = (i << 2) | t;
+			idx = STORM_METER(i, t);
 			reg_read_m(RTL837X_METER_RATE + (idx << 2));
 			print_byte(sfr_data[1]); print_byte(sfr_data[2]); print_byte(sfr_data[3]);
 			if (reg_bit_test(RTL837X_METER_MODE + ((idx >> 5) << 2), idx & 0x1f))

rtl837x_storm.h

@@ -4,6 +4,8 @@
 #include <stdint.h>
 
 #define STORM_TYPES	4
+#define STORM_METER_BASE	24
+#define STORM_METER(port, type)	(STORM_METER_BASE + (((port) << 2) | (type)))
 
 void storm_set(uint8_t port, __xdata uint8_t type, __xdata uint32_t rate, __xdata uint8_t pps) __banked;
 void storm_off(uint8_t port, __xdata uint8_t type) __banked;