Allow to independently monitor and dump events.

[MisterDA]

This code was written by @christf some time ago. I forgot about it, but now it’s here so we can keep track and see what to do.

[christf]

I like your changes. Thank you for bringing it up again.

[jech]

+    if(filter < FILTER_INVALID) {
+        *action_return += filter;

What is the addition supposed to achieve?

+#define CONFIG_ACTION_DUMP_NEIGHBOUR 3
+#define CONFIG_ACTION_DUMP_ROUTE 4
+#define CONFIG_ACTION_DUMP_XROUTE 5
+#define CONFIG_ACTION_DUMP_INTERFACE 6
+#define CONFIG_ACTION_NO 7
+#define CONFIG_ACTION_MONITOR 8
+#define CONFIG_ACTION_MONITOR_NEIGHBOUR 9
+#define CONFIG_ACTION_MONITOR_ROUTE 10
+#define CONFIG_ACTION_MONITOR_XROUTE 11
+#define CONFIG_ACTION_MONITOR_INTERFACE 12
+#define CONFIG_ACTION_UNMONITOR 13
+#define CONFIG_ACTION_UNMONITOR_NEIGHBOUR 14
+#define CONFIG_ACTION_UNMONITOR_ROUTE 15
+#define CONFIG_ACTION_UNMONITOR_XROUTE 16
+#define CONFIG_ACTION_UNMONITOR_INTERFACE 17

That's way to verbose. Please say something like

#define CONFIG_ACTION_MONITOR(thing) (8 + (thing))
#define CONFIG_ACTION_UNMONITOR(thing) (13 + (thing))

+        if(local_sockets[i].monitor & SHOW_INTERFACE)

Please say instead

       if((local_sockets[i].monitor & SHOW_INTERFACE) != 0)

There are many occurrences of this style.

[MisterDA]

+    if(filter < FILTER_INVALID) {
+        *action_return += filter;

What is the addition supposed to achieve?

If my understanding is correct, the "dump" (code 2), "monitor" (code
8), and "unmonitor" (code 13) actions are parametrized by objects in
the set of "neighbour" (code 1), "route" (code 2), "xroute" (code 3),
"interface" (code 4). The parameter describes which object is
filtered. The action_return code is set before that function, so the
addition sets the object type on which the filter is applied; for
instance code 10 means monitor + route. In local.c, that parameter is
extracted in show_flags_map, and is used to conditionally print the
object in local_notify_all().

[PolynomialDivision]

FYI: I'm maintaining the opposite OpenWrt IPC ubus Part. ;) I plan something like this, too.

openwrt/routing#633

[fcladera]

Hi!
I am currently working on a ROS interface for Babel, as we would like to use Babel's information to perform specific robot actions. I started doing a simple parser of the output, but using this interface may be a better solution. Could you please indicate how to use this interface?
Thanks!

[christf]

@fcladera this allows to monitor/dump certain events like exclusively route changes reducing the CPU usage compared to monitoring every event significantly on busy networks. The added manpage bits should already explain how to use it. If it doesn't, please provide feedback what might be missing.

@jech what is your opinion on the code? Can we get this merged? Using babeld in large networks on cheap plastic routers is CPU bound - not just because of babeld but also because of the processing load of surrounding daemons that parse events from babeld. It would be nice to be able to lighten the load a bit.

[christf]

any news on this one? I think @MisterDA addressed the concerns raised in the discussion. It would be nice to reduce CPU load by filtering messages before they are emitted.

[24367dfa]

@jech Have you found time to check out the latest changes? It seems the code is ready to merge.

[jech]

parse_monitor_dump_filter is wrong. The skip_whitespace is not needed (it's implied by getword). Also, getword cannot return success with token being NULL.

Also, I think the code around line 1163 is deeply suspicious: the way we set action_return to a temporary value and then tweak it in parse_monitor_dump_filter, relying on failure of the latter to handle the case of an end-of-line is fragile and probably wrong.

I suggest reworking this code to parse both keywords (e.g. monitor route) in a single function.

[jech]

+#define CONFIG_ACTION_DUMP(thing) (2 + (thing))
+#define CONFIG_ACTION_NO 7
+#define CONFIG_ACTION_MONITOR(thing) (8 + (thing))
+#define CONFIG_ACTION_UNMONITOR(thing) (13 + (thing))

That looks fragile to me, you need to count. What about

#define CONFIG_ACTION_DUMP(thing) (1 << 8 + (thing))
#define CONFIG_ACTION_MOITOR(thing) (2 << 8 + (thing))

and so on.