> On 26.3.2015, at 11.36, Juliusz Chroboczek <[email protected]> > wrote: > After a first read, it looks like a pretty complete implementation of > RFC 6126. I have't checked in detail, but it looks like you got both the > loop avoidance and the blackhole avoidance mechanisms right. You're also > doing bidirectional reachability detection and fairly reasonable route > selection.
Thanks for the comments ;) >> - I implemented it like ‘zeroconf IS-IS’ works, that is, without useful >> cost function. It fully interoperates with more comprehensive >> implemenations though. > > You're doing a little better than that -- you're taking txcost into > account, so if the neighbour is performing link quality estimation, you're > going to use his value. > > For consistency with Appendix A.2.2 of RFC 6126, please change your cost > function to: > > cost = rxcost * MAX(txcost, 256) / 256 > > where rxcost is some value no smaller than 256, instead of the current > > cost = rxcost + txcost > > which will cause pybabels to be too strongly preferred. Good point. Changed. >> - IPv6 only (who wants legacy IP?) > Well, you already parse and honour the Next-Hop TLV, so adding IPv4 > support would be trivial. (NH is needed to advertise an IPv4 route when > speaking over IPv6.) Yeah, the main cost is just adding logic to deal with e.g. getting local IPv4 address too etc. Obviously not many lines (and e.g. imported route addition would already works with IPv4 too, if I did not intentionally disable it). So essentially given 2 removed lines it would be IPv4 stub router too. >> - No source [specific] routing > You should be using a multiplier of the interval as a hold time, rather > than a fixed value (is Section 3.4.1 unclear?). Interestingly, you > already do the right thing with IHUs (is Section 3.4.2 any clearer than > 3.4.1?). See also Appendix B. As noted in the comments txt I generated when coding it, the whole ‘recently heard from’ was bit vague to me. That’s why I stuck in fixed # there for now, although I guess use of multiplier on hello interval is correct way to do it. > I think you should always be preferring redistributed routes to learned > ones. I am :) local_routes are handled after the learned traversal, and they override them. > Oh, one last thing -- I don't understand the comment on line 367. How can > the source table entry be a historic copy? Admittedly that is not really valid in terms of RFC6126 (strictly interpreted), but I am intentionally maintaining separate set of selected and full routes. Due to that, I can allow for e.g. route selection to be done with some delay, and I still have consistent ’snapshot’ of selected routes at some point in time available even if I update the routes based on TLVs in the interim. (This is mostly a potential performance optimization I left on the table, and it is virtually free; one pointer essentially per selected route. Not sure if it is really needed as such.) Cheers, -Markus _______________________________________________ homenet mailing list [email protected] https://www.ietf.org/mailman/listinfo/homenet
