Hi Julien, hi Cédric, Thanks for the code submission. But you already know what I'm going to say, right?
> re6stnet limits the number of OpenVPN tunnels so we don't have > a full-mesh and from time to time, it deletes one to create a new one to > a random node. Good design. > However, since the beginning (mid-2012), we have an issue with the > destruction of tunnels because we couldn't make sure there were no route > through the tunnel being destroyed. There's a very simple way to do what you want with no changes to babeld -- send a bunch of spoofed wildcard retractions. It will cause babeld to switch to routes through a different interface, and the hysteresis mechanism will give you enough time to tear down the tunnel before routes are selected again. But I agree that having an official mechanism in babeld itself might be desirable. > The first one (d37e373) implements a new way to communicate with babeld. It would appear that you've rebased your tree -- I assume you mean aa16227. This patch is wrong on so many levels that I just don't know where to start. 1. it uses a new binary protocol. If you need a binary protocol, extend the Babel protocol. 2. it defines a new control protocol. There's already a control protocol in Babel, and it uses plain text. Extending the control protocol is a good idea, but it must be a plain text protocol usable over telnet. 3. as binary protocols go, it's not very well designed -- it's just a dump of babeld's internal data structures on the wire, difficult to extend. 4. as binary protocols go, it's really implemented badly -- just a bunch of packed structures with no explicit protocol parsing and no input validation. I'm not going to trawl through that mess searching for security issues. > - It is done via a unix socket for security, but uses network byte > - ordering so it can easily be transformed into TCP with socat. I have no objection to a new flag that puts the existing control interface on a Unix socket. > - Binary protocol for reliability and simplicity. I didn't want to > format and parse strings with regex. Babeld already contains a recursive descent parser for a carefully designed LL(1) configuration language (configuration.c). Please extend this parser. You won't manage to implement a meaningful language with regular expressions. > In fact, I even think this new interface should replace local.c Julien I'm really trying very hard to be friendly and constructive. Please don't provoke me. > The second commit (bd1cf65) implements a second packet to alter the > result of neighbour_cost > 1. Node C (as client) decides to destroy a openvpn client tunnel that is > connected to node S > 2. C sends a packet to its babeld to increase cost to S > 3. C requests dumps until no route go via S > [...half a dozen other steps omitted for the sake of brevity...] What makes you so sure that step (3) will terminate? There's absolutely no need for this complexity. Your new command should simply flush all routes going through the interface (this will cause the route selection code to consider other routes), and send a wildcard retraction. This is reliable, simple to implement, and simple to understand. Julien, if you wish to discuss this viva voce, please let me know. -- Juliusz _______________________________________________ Babel-users mailing list [email protected] http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/babel-users
