On Sun, May 21, 2017 at 11:01:34PM +0200, Dean Luga wrote:
From: dean <dluga93@gmail.com>
This patch adds a new network of type NET_SADR_IP6, including new structures, constants, and switch cases for the new network type. Some existing functions are duplicates to handle the new network type, and netlink can now handle SADR routes.
The net_route_sadr_ip6 function is a bit of a hack. Routing in SADR is supposed to match the destination address first, then out of the entries with that dst, it should choose the most specific matching src address.
Well, net_route() in integrated branch is a bit of a hack generally. It was carried over from plain-IP branch, it needs some redesign to better handle net types different than NET_IP.
For the destination-only part, I use a net_addr_ip6 as the destination prefix. Its type is changed to NET_SADR_IP6 to satisfy the assertion in fib_find. The fib_find function checks the length of the prefix to distinguish between destination-only or full SADR searches. --- lib/net.c | 34 ++++++++++++++++++++++++++ lib/net.h | 66 +++++++++++++++++++++++++++++++++++++++++++++++--- nest/rt-fib.c | 16 ++++++++++++ nest/rt-table.c | 35 +++++++++++++++++++++++++- sysdep/linux/netlink.c | 26 ++++++++++++++++++-- sysdep/unix/krt.c | 4 ++- 6 files changed, 174 insertions(+), 7 deletions(-)
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); +} + +int net_compare(const net_addr *a, const net_addr *b) { if (a->type != b->type) @@ -158,6 +177,8 @@ net_compare(const net_addr *a, const net_addr *b) return net_compare_flow6((const net_addr_flow6 *) a, (const net_addr_flow6 *) b); case NET_MPLS: return net_compare_mpls((const net_addr_mpls *) a, (const net_addr_mpls *) b); + case NET_SADR_IP6: + return net_compare_sadr_ip6((const net_addr_sadr_ip6 *) a, (const net_addr_sadr_ip6 *) b); } return 0; } @@ -178,6 +199,7 @@ net_hash(const net_addr *n) case NET_FLOW4: return NET_HASH(n, flow4); case NET_FLOW6: return NET_HASH(n, flow6); case NET_MPLS: return NET_HASH(n, mpls); + case NET_SADR_IP6: return NET_HASH(n, ip6); default: bug("invalid type"); } }
This should be NET_HASH(n, sadr_ip6) even if it is the same function.
diff --git a/lib/net.h b/lib/net.h index 332f4c9..22eee60 100644 --- a/lib/net.h +++ b/lib/net.h @@ -12,7 +12,6 @@
#include "lib/ip.h"
- #define NET_IP4 1 #define NET_IP6 2 #define NET_VPN4 3 @@ -22,7 +21,8 @@ #define NET_FLOW4 7 #define NET_FLOW6 8 #define NET_MPLS 9 -#define NET_MAX 10 +#define NET_SADR_IP6 10 +#define NET_MAX 11
I would prefer that NET_SADR_IP6 would be before NET_MPLS, as it is still IP-family net type,
#define NB_IP4 (1 << NET_IP4) #define NB_IP6 (1 << NET_IP6) @@ -33,8 +33,9 @@ #define NB_FLOW4 (1 << NET_FLOW4) #define NB_FLOW6 (1 << NET_FLOW6) #define NB_MPLS (1 << NET_MPLS) +#define NB_SADR (1 << NET_SADR_IP6)
-#define NB_IP (NB_IP4 | NB_IP6) +#define NB_IP (NB_IP4 | NB_IP6 | NB_SADR)
No, NB_IP should stay as is. It is used by protocols to specify what network types they support and they do not get SADR support automatically.
#define NB_VPN (NB_VPN4 | NB_VPN6) #define NB_FLOW (NB_FLOW4 | NB_FLOW6) #define NB_DEST (NB_IP | NB_VPN | NB_MPLS)
But you should add it to NB_DEST, so destinations are allowed.
@@ -120,6 +121,15 @@ typedef struct net_addr_mpls { u32 label; } net_addr_mpls;
+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 + ip6_addr src_prefix; +} net_addr_sadr_ip6; + typedef union net_addr_union { net_addr n; net_addr_ip4 ip4; @@ -131,6 +141,7 @@ typedef union net_addr_union { net_addr_flow4 flow4; net_addr_flow6 flow6; net_addr_mpls mpls; + net_addr_sadr_ip6 sadr_ip6; } net_addr_union;
@@ -169,6 +180,9 @@ extern const u16 net_max_text_length[]; #define NET_ADDR_MPLS(label) \ ((net_addr_mpls) { NET_MPLS, 20, sizeof(net_addr_mpls), label })
static inline ip4_addr net4_prefix(const net_addr *a) { return ((net_addr_ip4 *) a)->prefix; } @@ -243,6 +263,12 @@ static inline ip4_addr net4_prefix(const net_addr *a) static inline ip6_addr net6_prefix(const net_addr *a) { return ((net_addr_ip6 *) a)->prefix; }
+static inline ip6_addr net6_sadr_dst_prefix(const net_addr *a) +{ return ((net_addr_sadr_ip6 *) a)->dst_prefix; } + +static inline ip6_addr net6_sadr_src_prefix(const net_addr *a) +{ return ((net_addr_sadr_ip6 *) a)->src_prefix; } +
These accessors seems pointless. dst_prefix is the same as net6_prefix() (assuming net_addr_sadr_ip6 extends net_addr_ip6 like other IPv6-based nettypes) and src_prefix is not used outside specialized code, which would have net typed as net_addr_sadr_ip6 and could access src_prefix directly.
static inline ip_addr net_prefix(const net_addr *a) { switch (a->type) @@ -259,6 +285,9 @@ static inline ip_addr net_prefix(const net_addr *a) case NET_FLOW6: return ipa_from_ip6(net6_prefix(a));
+ case NET_SADR_IP6: + return ipa_from_ip6(net6_sadr_dst_prefix(a)); +
You could handle that by net6_prefix() together with other IPv6-based nettypes.
case NET_MPLS: default: return IPA_NONE; @@ -279,6 +308,12 @@ static inline uint net4_pxlen(const net_addr *a) static inline uint net6_pxlen(const net_addr *a) { return a->pxlen; }
+static inline uint net6_sadr_dst_pxlen(const net_addr *a) +{ return ((net_addr_sadr_ip6 *) a)->dst_pxlen; } + +static inline uint net6_sadr_src_pxlen(const net_addr *a) +{ return ((net_addr_sadr_ip6 *) a)->src_pxlen; } +
Like net6_sadr_*_prefix
@@ -327,6 +362,13 @@ static inline int net_equal_flow6(const net_addr_flow6 *a, const net_addr_flow6 static inline int net_equal_mpls(const net_addr_mpls *a, const net_addr_mpls *b) { return !memcmp(a, b, sizeof(net_addr_mpls)); }
+typedef net_addr_ip6 net_addr_sadr_dst; +static inline int net_equal_sadr_dst(const net_addr_ip6 *a, const net_addr_ip6 *b) +{ return net_equal_ip6(a, b); } + +static inline int net_equal_sadr_ip6(const net_addr_sadr_ip6 *a, const net_addr_sadr_ip6 *b) +{ return !memcmp(a, b, sizeof(net_addr_sadr_ip6)); } +
@@ -455,6 +502,12 @@ static inline u32 net_hash_flow6(const net_addr_flow6 *n) static inline u32 net_hash_mpls(const net_addr_mpls *n) { return n->label; }
+static inline u32 net_hash_sadr_dst(const net_addr_sadr_dst* n) +{ return net_hash_ip6(n); } + +static inline u32 net_hash_sadr_ip6(const net_addr_sadr_ip6 *n) +{ return ip6_hash(n->dst_prefix) ^ ((u32) n->dst_pxlen << 26); } +
Perhaps unnecessary, see my comment to fib_find() below
@@ -231,6 +232,20 @@ fib_find(struct fib *f, const net_addr *a) case NET_ROA6: return FIB_FIND(f, a, roa6); case NET_FLOW4: return FIB_FIND(f, a, flow4); case NET_FLOW6: return FIB_FIND(f, a, flow6); + case NET_SADR_IP6: + { + if (a->length != sizeof(net_addr_sadr_ip6)) + { + // dst only search + net_addr_ip6 a0; + net_copy((net_addr *)&a0, a); + a0.length = sizeof(net_addr_sadr_ip6); + + return FIB_FIND(f, &a0, sadr_dst); + } + else + return FIB_FIND(f, a, sadr_ip6); + }
No. Generic fib_find() should implement exact-match search with matching net_addr type. If you need/want partial match, it should be done as a separate function. BTW, where do you need that? Perhaps we should add some generic function partial-match IP-based search for network types that support it (e.g., ROA, SADR). But it would need a different interface, so caller could enumerate all valid matches.
+static inline void * +net_route_sadr_ip6(rtable *t, net_addr_sadr_ip6 *n) +{ + net *r; + + net_addr_ip6 dst = NET_ADDR_IP6(n->dst_prefix, n->dst_pxlen); + dst.type = NET_SADR_IP6; + + // search for matching dst + while (r = net_route_ip6(t, &dst)) + { + // found a matching dst, start search for entries that match both prefixes + net_addr_sadr_ip6 full_addr = + NET_ADDR_SADR_IP6(dst.prefix, dst.pxlen, n->src_prefix, n->src_pxlen); + + while (r = net_find_valid(t, (net_addr *) &full_addr), (!r) && (full_addr.src_pxlen > 0)) + { + full_addr.src_pxlen--; + ip6_clrbit(&full_addr.src_prefix, full_addr.src_pxlen); + } + + if (r) + return r; + dst.pxlen--;
You should set dst.pxlen based on r found by net_route_ip6(), to avoid repeated iteration over the same prefix lengths that did not match in the inner cycle. Also be sure that pxlen is not already zero.
@@ -1747,6 +1779,7 @@ rt_preconfig(struct config *c)
rt_new_table(cf_get_symbol("master4"), NET_IP4); rt_new_table(cf_get_symbol("master6"), NET_IP6); + rt_new_table(cf_get_symbol("master_sadr6"), NET_SADR_IP6); }
No, we have only IPv4 and IPv6 master table defined by default. Other tables are defined in config file if necessary.
@@ -1980,7 +2013,7 @@ rt_new_table(struct symbol *s, uint addr_type) /* Hack that allows to 'redefine' the master table */ if ((s->class == SYM_TABLE) && (s->def == new_config->def_tables[addr_type]) && - ((addr_type == NET_IP4) || (addr_type == NET_IP6))) + ((addr_type == NET_IP4) || (addr_type == NET_IP6) || (addr_type == NET_SADR_IP6))) return s->def;
ditto
struct rtable_config *c = cfg_allocz(sizeof(struct rtable_config)); diff --git a/sysdep/linux/netlink.c b/sysdep/linux/netlink.c
I will check KRT/Netlink code later. -- 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."