Hi, 2016-02-16 18:48 GMT+01:00 Ondrej Zajicek <santiago@crfreenet.org>:
On Wed, Feb 10, 2016 at 04:38:03PM +0100, Mikhail Sennikovskii wrote:
Hello Fellows,
as a follow-up for the recent discussion about BIRD support of IPv6 ECMP on Linux ( http://trubka.network.cz/pipermail/bird-users/2016-January/010163.html ), I've created a patch, which addresses this problem.
Hi
Thanks for the patch, i have several comments:
1) In the patch, the incoming direction (collection from kernel) is handled in unix/krt.c, while outgoing direction (krt_replace_rte()) is handled in linux/netlink.c . It would make more sense to handle both directions in the same place. As BIRD uses internally representation with one route and mpnh structure, i would prefer to move collection also to linux/netlink.c, so generic code in unix/krt.c would not care about OS-specific interface (and does not need any changes). Linux code would just collect next hops and then call krt_got_route() for the whole multipath route.
Agreed, will fix. 2) Function krt_replace_rte() should be smarter than just flushing the
whole old multipath rte. It should compare old and new next hops (we could suppose they are sorted) and either add missing
I'm not sure for now whether it's possible to make this work, as netlink returns EEXIST whenever one tries to add a route for the network, already present in the routing table. This is why removing and re-installing seemed the best solution for me. or remove
surplussing next hops by calling nl_send_route().
Just removing should work, yes.
3) When collecting received routes, you should avoid creating list of routes for the same net, and then merging such list by rt_merge_list(), you could simply collect next hops (struct mpnh *) for the current route.
I can do it this way.
4) Also rt_merge_list() uses mpnh_merge_rta() which uses rte_update_pool. That is probably not safe outside of nest update code. The linux netlink code should have its own linpool (static and shared, because netlink socket is also shared), so you could allocate mpnh structures from it. The linpool should be flushed when the route is collected and propagated to krt_got_route(). So you don't need to do rta_lookup() for temporary next hops.
Right, will do.
Note that such linpool could be also used for IPv4 multipath next hops in nl_parse_multipath() instead of nh_buffer.
I can add this fix as well, probably as a separate patch.
Note that the patch still does not support the learn mode for IPv6 ECMP.
The support for it can be added later.
Well, the learn mode is not really a problem. With the modifications mentioned above, learning would work automatically (it is handled by unix/krt.c, regardless of how route is generated).
What is problematic is handling asynchronous notifications (because you don't know inside netlink code what are other next hops and whether you will get another one related to the same net).
Sorry, I guess I just mixed up with bird terminology. I thought the learn mode is directly related to handling these asynchronous notifications and adjusting the bird routes set accordingly. Regards, Mikhail
-- Elen sila lumenn' omentielvo
Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."