On Sat, Feb 03, 2018 at 09:40:56PM +0100, Toke Høiland-Jørgensen wrote:
This adds support for source-specific IPv6 routes to Bird core. This is based on Dean Luga's original patch, with the review comments addressed. Sadr support is added to network address parsing in confbase.Y and to the kernel protocol on Linux.
First, some general comments: Put sadr_ip6 variant consistently above mpls variant. There are several places where it is in reverse. If you do not object, i would much prefer *_ip6_sadr than *_sadr_ip6 (also in uppercase variants). It will also be consistent with user visible channel and table keywords for this type.
-%type <net> net_ip4_ net_ip6_ net_ip6 net_ip_ net_ip net_or_ipa +%type <net> net_ip4_ net_ip6_ net_sadr_ip6_ net_ip6 net_ip_ net_ip net_or_ipa %type <net_ptr> net_ net_any net_vpn4_ net_vpn6_ net_vpn_ net_roa4_ net_roa6_ net_roa_ net_mpls_
net_sadr_ip6_ should be in <net_ptr> instead of <net>, as net_addr_sadr_ip6 is longer than net_addr, so must be passed as pointer. That fixes the "Integer expression expected" issue.
%type <mls> label_stack_start label_stack
@@ -96,7 +96,7 @@ CF_DECLS %left '!' %nonassoc '.'
-CF_KEYWORDS(DEFINE, ON, OFF, YES, NO, S, MS, US, PORT, VPN, MPLS) +CF_KEYWORDS(DEFINE, ON, OFF, YES, NO, S, MS, US, PORT, VPN, MPLS, FROM)
CF_GRAMMAR
@@ -206,6 +206,22 @@ net_ip6_: IP6 '/' NUM n->prefix, n->pxlen, ip6_and(n->prefix, ip6_mkmask(n->pxlen)), n->pxlen); };
+net_sadr_ip6_: IP6 '/' NUM FROM IP6 '/' NUM +{ + if ($3 > IP6_MAX_PREFIX_LENGTH) + cf_error("Invalid prefix length %u", $3); + + if ($7 > IP6_MAX_PREFIX_LENGTH) + cf_error("Invalid prefix length %u", $7); + + net_fill_sadr_ip6(&($$), $1, $3, $5, $7); + + net_addr_sadr_ip6 *n = (void *) &($$);
W.r.t. the change from <net> to <net_ptr> it should be: $$ = cfg_alloc(sizeof(net_addr_sadr_ip6)); net_fill_sadr_ip6($$, $1, $3, $5, $7); net_addr_sadr_ip6 *n = (void *) $$;
-net_ip_: net_ip4_ | net_ip6_ ; +net_ip_: net_ip4_ | net_ip6_ | net_sadr_ip6_;
Here net_sadr_ip6_ should be separate
net_vpn_: net_vpn4_ | net_vpn6_ ; net_roa_: net_roa4_ | net_roa6_ ;
diff --git a/lib/net.c b/lib/net.c index 9335b78f..a2dad51d 100644 --- a/lib/net.c +++ b/lib/net.c @@ -14,6 +14,7 @@ const char * const net_label[] = { [NET_ROA6] = "roa6", [NET_FLOW4] = "flow4", [NET_FLOW6] = "flow6", + [NET_SADR_IP6] = "sadr6",
"ipv4 sadr" to be consistent with table keywords?
+typedef struct net_addr_sadr_ip6 { + u8 type; + u8 dst_pxlen; + u16 length; + ip6_addr dst_prefix; + u32 src_pxlen; // if u8, NET_ADDR_SADR_IP6 would leave non-zero padding
Perhaps we should use s32 so it is promoted to same type as u8?
+int +net_compare_sadr_ip6(const net_addr_sadr_ip6 *a, const net_addr_sadr_ip6 *b) +{ + int dst_cmp = ip6_compare(a->dst_prefix, b->dst_prefix) ?: uint_cmp(a->dst_pxlen, b->dst_pxlen); + if (dst_cmp) + return dst_cmp; + else + return ip6_compare(a->src_prefix, b->src_prefix) ?: uint_cmp(a->src_pxlen, b->src_pxlen); +}
Seems too complicated. Perhaps just: { return ip6_compare(a->dst_prefix, b->dst_prefix) ?: uint_cmp(a->dst_pxlen, b->dst_pxlen) ?: ip6_compare(a->src_prefix, b->src_prefix) ?: uint_cmp(a->src_pxlen, b->src_pxlen); }
+static inline void * +net_route_sadr_ip6(rtable *t, net_addr_sadr_ip6 *n)
IMHO proper way how to implement this is like net_roa_check_ip6() is done. As nets are hashed by dst, you can use fib_get_chain() to get hash chain, walk through it and find the best matching src from nodes with the exact dst of the current iteration, and decrease dst_pxlen in further iterations if no match. Also note that in order to net_route is used in 'show route for', you should add net_sadr_ip6_ to r_args_for grammar in nest/config.Y
--- a/sysdep/unix/krt.c +++ b/sysdep/unix/krt.c @@ -1103,7 +1103,9 @@ krt_start(struct proto *P) switch (p->p.net_type) { case NET_IP4: p->af = AF_INET; break; - case NET_IP6: p->af = AF_INET6; break; + case NET_IP6: + case NET_SADR_IP6: + p->af = AF_INET6; break; #ifdef AF_MPLS case NET_MPLS: p->af = AF_MPLS; break; #endif @@ -1219,9 +1221,9 @@ struct protocol proto_unix_kernel = { .attr_class = EAP_KRT, .preference = DEF_PREF_INHERITED, #ifdef HAVE_MPLS_KERNEL - .channel_mask = NB_IP | NB_MPLS, + .channel_mask = NB_IP | NB_SADR_IP6 | NB_MPLS, #else - .channel_mask = NB_IP, + .channel_mask = NB_IP | NB_SADR_IP6, #endif
SADR is not supported in BSD, channel_mask should reflect mask. There should be define for CONFIG_SADR_IP6_KERNEL in sysdef/cf/linux.h, and then in sysdep/unix/krt.c: #ifdef CONFIG_SADR_IP6_KERNEL #define MAYBE_SADR_IP6 NB_SADR_IP #else #define MAYBE_SADR_IP6 0 #endif #ifdef HAVE_MPLS_KERNEL #define MAYBE_MPLS NB_MPLS #else #define MAYBE_MPLS 0 #endif struct protocol proto_unix_kernel = { ... .channel_mask = NB_IP | MAYBE_SADR_IP6 | MAYBE_MPLS ... }; -- 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."