On Mon, Sep 8, 2014 at 6:10 AM, Baptiste Jonglez <[email protected]> wrote: > Hi, > > Note that babeld is maintained in https://github.com/openwrt-routing/packages. > Nevertheless, comments inline. > > On Sun, Sep 07, 2014 at 05:56:32PM -0400, Tristan Plumb wrote: >> Changes the babeld init script to utilize procd and > > Nice, it was a requested feature: > > https://github.com/openwrt-routing/packages/issues/17 > > Could you separate your patch into three patches? (procd support, hotplug > script, pidfile hack) > >> adds a hotplug.d file to restart babeld when interfaces go up or down. > > Somebody provided a patch for this just a few days ago, see: > > https://github.com/openwrt-routing/packages/pull/52 > > However, see the comments: this should not be needed, as babeld detects > when an interface is brought up and then starts using it. What is your > use-case? > >> Additionally adds a patch to the babeld source is to prevent the >> attempted recreation of a pidfile when babeld restarts uncleanly. This is >> also possible to do by passing -I '' on the command line, however procd >> interprets an empty argument terminating the list, and thus cannot supply >> empty arguments to programs it manages.
> Patching babeld itself is a workaround. Why not tackle the root issue? > It seems that you encountered crashes in babeld, so that should be fixed, > instead of working around the crashes. I did experience a few babeld > crashes myself, but never managed to find where they came from. There lies the rub. If you have a deployed machine on a rooftop somewhere, it is better to restart the daemon automagically and log the action rather than attempt to debug it. Using procd to manage this stuff reduces the probability of a babel failure taking out a node by several orders of magnitude, and I strongly favor managing it this way. Certainly finding and fixing bugs so that it doesn't ever crash is a good option, but in the real world, nothing is perfect, and defense in depth is needed. > >> Signed-off-by: Tristan Plumb <[email protected]> >> --- >> diff --git a/babeld/Makefile b/babeld/Makefile >> index e939310..70fcd64 100644 >> --- a/babeld/Makefile >> +++ b/babeld/Makefile >> @@ -54,6 +54,8 @@ define Package/babeld/install >> $(INSTALL_CONF) ./files/babeld.config $(1)/etc/config/babeld >> $(INSTALL_DIR) $(1)/etc/init.d >> $(INSTALL_BIN) ./files/babeld.init $(1)/etc/init.d/babeld >> + $(INSTALL_DIR) $(1)/etc/hotplug.d/iface >> + $(INSTALL_DATA) ./files/babeld.hotplug >> $(1)/etc/hotplug.d/iface/25-babeld >> endef >> >> $(eval $(call BuildPackage,babeld)) >> diff --git a/babeld/files/babeld.hotplug b/babeld/files/babeld.hotplug >> new file mode 100644 >> index 0000000..3fe674a >> --- /dev/null >> +++ b/babeld/files/babeld.hotplug >> @@ -0,0 +1,6 @@ >> +#!/bin/sh >> + >> +[ "$ACTION" = ifup ] || exit 0 >> + >> +/etc/init.d/babeld enabled && /etc/init.d/babeld restart >> + >> diff --git a/babeld/files/babeld.init b/babeld/files/babeld.init >> index 180fc7e..de8d73b 100755 >> --- a/babeld/files/babeld.init >> +++ b/babeld/files/babeld.init >> @@ -2,9 +2,9 @@ >> >> . /lib/functions/network.sh >> >> +USE_PROCD=1 >> START=70 >> >> -pidfile='/var/run/babeld.pid' >> CONFIGFILE='/var/etc/babeld.conf' >> OTHERCONFIGFILE="/etc/babeld.conf" >> EXTRA_COMMANDS="status" >> @@ -197,7 +197,7 @@ babel_config_cb() { >> esac >> } >> >> -start() { >> +start_service() { >> mkdir -p /var/lib >> # Start by emptying the generated config file >> >"$CONFIGFILE" >> @@ -213,26 +213,17 @@ start() { >> config_foreach parse_old_global_options general >> # Parse filters separately, since we know which options we expect >> config_foreach babel_filter filter >> + procd_open_instance >> # Using multiple config files is supported since babeld 1.5.1 >> - /usr/sbin/babeld -D -I "$pidfile" -c "$OTHERCONFIGFILE" -c >> "$CONFIGFILE" >> - # Wait for the pidfile to appear >> - for i in 1 2 >> - do >> - [ -f "$pidfile" ] || sleep 1 >> - done >> - [ -f "$pidfile" ] || (echo "Failed to start babeld"; exit 42) >> + procd_set_param command /usr/sbin/babeld -c "$OTHERCONFIGFILE" -c >> "$CONFIGFILE" >> + procd_set_param respawn >> + procd_close_instance >> } >> >> -stop() { >> - [ -f "$pidfile" ] && kill $(cat $pidfile) >> - # avoid race-condition on restart: wait for >> - # babeld to die for real. >> - [ -f "$pidfile" ] && sleep 1 >> - [ -f "$pidfile" ] && sleep 1 >> - [ -f "$pidfile" ] && sleep 1 >> - [ -f "$pidfile" ] && exit 42 >> +service_triggers() { >> + procd_add_reload_trigger babeld >> } >> >> status() { >> - [ -f "$pidfile" ] && kill -USR1 $(cat $pidfile) >> + kill -USR1 $(pidof babeld) >> } >> diff --git a/babeld/patches/001-no-default-pidfile.patch >> b/babeld/patches/001-no-default-pidfile.patch >> new file mode 100644 >> index 0000000..74ac6b4 >> --- /dev/null >> +++ b/babeld/patches/001-no-default-pidfile.patch >> @@ -0,0 +1,21 @@ >> +diff --git a/babeld.c b/babeld.c >> +index 656c9da..88e4aec 100644 >> +--- a/babeld.c >> ++++ b/babeld.c >> +@@ -66,7 +66,7 @@ int resend_delay = -1; >> + int random_id = 0; >> + int do_daemonise = 0; >> + const char *logfile = NULL, >> +- *pidfile = "/var/run/babeld.pid", >> ++ *pidfile = NULL, >> + *state_file = "/var/lib/babel-state"; >> + >> + unsigned char *receive_buffer = NULL; >> +@@ -251,6 +251,7 @@ main(int argc, char **argv) >> + break; >> + case 'D': >> + do_daemonise = 1; >> ++ pidfile = "/var/run/babeld.pid"; >> + break; >> + case 'L': >> + logfile = optarg; > > _______________________________________________ > Babel-users mailing list > [email protected] > http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/babel-users -- Dave Täht https://www.bufferbloat.net/projects/make-wifi-fast _______________________________________________ Babel-users mailing list [email protected] http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/babel-users
